geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Jencks <david_jen...@yahoo.com>
Subject Re: Enable Declartive Security in Jetty ?
Date Sat, 18 Sep 2010 16:29:01 GMT

On Sep 18, 2010, at 1:14 AM, Ivan wrote:

> 
> 
> 2010/9/18 David Jencks <david_jencks@yahoo.com>
> 
> On Sep 18, 2010, at 12:24 AM, Ivan wrote:
> 
>> 
>> 
>> 2010/9/18 David Jencks <david_jencks@yahoo.com>
>> 
>> On Sep 17, 2010, at 5:36 PM, Ivan wrote:
>> 
>>> 
>>> 
>>> 2010/9/18 David Jencks <david_jencks@yahoo.com>
>>> 
>>> On Sep 17, 2010, at 12:44 AM, Ivan wrote:
>>> 
>>> > Hi,
>>> >     While looking at some Servlet Security JIRAs, I begun some code refactors
on the SpecSecurityBuilder, including :
>>> >     a. Add more Info class for the security configurations, and serialize
those in the .ser file, with them, it would avoid the xml parsing on the startup time and
make the codes look simple
>>> 
>>> excellent idea!
>>> 
>>> >     b. Use ServletContext more in the SpecSecurityBuilder, as it is more
helpful for some calculations, such as get the mapping urls for the target servlet.
>>> 
>>> I'm not sure what you mean here, but I haven't looked closely at SpecSecurityBuilder.
 Could you be more specific?
>>> 
>>>  e.g. Use the method ServetContext.getServletRegistration().getMapping could
eaisly get all the url patterns of the target servlet. 
>> 
>> I don't think this will be ideal.  It won't work except for servlets where the security
is set through the servletRegistration rather than through web.xml, since the url patterns
in web.xml don't have to be servlet mappings.  For jetty, I think a more "native" solution
will work better, see below.
>>     
>>     I might double check this, but from the Java Doc, it did not write explicitly
that they are only for new added servlets.
> 
> right, but my point is that url patterns in a constraint in web.xml may not appear explicitly
in any servlet mapping.
> 
>> Anyway, if it works, we could use it as an util method, if not, we could calculate
the mapping from the info classes.
>>     Another thing that could be refractor is that to make specsecuritybuilder is
a pure specsecuritybuilder, currently, it is also used to handle how to scan servletsecurity
annotation, and  detect which security configuration has high priority.     
>> 
>> 
>> 
>>> 
>>> >
>>> >     To make these functions work, especially the option b. it requires to
enable declarative security in Jetty integration, generally speaking, will adopt the same
way as Tomcat integration does,
>>> >     a. create a Wrapper class for ServletContextHandler.Context class, so
that we could monitor those new added dynamic servlets. One thing might be care is that the
codes need to distinguish the servlets from web.xml, as they are also added by ServletContext
now in Jetty.
>>> >     b. Add a EventListener to ServletContextHandler, it will be resposible
for the security calculation and fill it into ApplicationPolicyConfigurationManager.
>>> >
>>> 
>>> I think you mean "declarative security for servlets added by the addServlet methods
on ServletContext"?  Jetty will want to deal with that too, so I think putting something in
the jetty code that calls out to a security builder of some kind (we can install our own)
is the best plan here.  Then we shouldn't need more wrapping.  Maybe I don't understand exactly
what you mean?  What would the event listener do?
>>> 
>>>   In the Tomcat integration, a JACCListener is attached to the web context, while
it receives the initialize event, a specsecuritybuilder is created, while the started event
is received, it will build the permissions and fill the result to the policymanager.
>>> 
>>>   We need wrapper here, especially in Jetty now, since all the servlets are registered
dynamically, for jetty itself, it will not know what to return for some methods, e,g, ServletRegistration.Dynamic.setServletSecurity
method, it needs to return all the un-affected url patterns. Also, since most of the security
related issues are handled by Geronimo, there is no need to call the initial codes. 
>> 
>> Right now this is not implemented in jetty at all.  I think we should define an interface
>> 
>> ConstraintStore {
>>             public Set<String> setServletSecurity(String servletName, ServletSecurityElement
securityElement);
>> }
>> 
>> that the ServletRegistration will delegate to.  Our implementation can implement
it based on the info tree from the deployment descriptor.  When we start the context we can
convert the ServletSecurityElements and url mappings to info, and proceed to generate the
permissions.
>> 
>> 
>> setServletSecurity definitely needs to be delegated, but it is not enought. Other
methods like "createServlet/addServlet/.." might also be required.
> 
> We already provide the implementation for creating servlet instances.  We can take the
opportunity to scan for the security annotation as well.
> 
>> Actually, the declaritive security is much complicated, especially whether or not
need to take care the ServletSecurity annotation. I quoted some code comments from my working
copy ;
>>    /**
>>          *  a. The security constraints in the portable deployment descriptor are
of the highest priority,
>>          *  b .None security annotations will take effect on the URL patterns explicitly
configured in the portable deployment desciptor,
>>          *      but for those URL patterns are not configured, the security annotations
should take effect, except for META-COMPLETE is set with TRUE
>>          *  c. All the dynamic added servlets should take care the ServletSecurity
annotation, and it seems to have
>>                 nothing to do with the META-COMPLETE flag, two exceptions are :
>>                 c1. Users create the servlet by themselves
>>                 c2. ServletRegistration.Dynamic.setServletConstraint is called
>>          *  d. For those URL patterns added by ServletContext.getServletRegistration().addMappping,
and those target servlets are configured
>>          *      in the portable deployment plan, ServletSecurity annotation should
also be taken care, except for META-COMPLETE is set with TRUE
>>          */
> 
> it's definitely even more complicated than it was in ee5 where it was bad enough :-)
> 
> As I understand the javadoc we can:
> 
> 1. convert the web.xml constraints to info as soon as we know the complete web.xml.
> 2. if web.xml was not marked metadata complete, we scan all servlets for the security
annotation and track the results by servlet name.
> 3. we wait for people to call setServletSecurity and track those by name too, merging
the info by url pattern, where "last wins"
> 4. we merge the servlet name to security info map with the info from the web.xml, where
the web.xml wins.
> 
>     
>     Generally speaking, that is what we do now, mightbe some differences on those detailed
handlering.
>     Another missing point, if the ServletContext.getServletRegistration.addMapping works
for all the servlets, including the ones in the web.xml (It should be), we also need to apply
the servletsecurity annotation to those new added url mapping.

Maybe I'm not understanding :-)  If all the security constraint info is in the web.xml, calling
ServletRegistration.addMapping has no effect on on security constraints.  Calling ServletRegistration.addMapping
has an effect on constraints only if there is a security annotation (not suppressed by metadataComplete
= true) or if you call setServletSecurity.  Agreed?

>     Hope I could create a patch tomorrow,  anyway, the codes are the most useful tool
for expressing the ideas. And will apreciate your comments ;-)

looking forward to it.... definitely clearer that trying to talk about it :-)

thanks
david jencks

>      
> We can't merge security constraint info from annotations into web.xml because it can
be overridden by this setServletSecurity method, whereas constraints originally in web.xml
can't be.
> 
>> 
>> Some similar codes are in Tomcat integration, e.g.
>>         https://svn.apache.org/repos/asf/geronimo/server/trunk/plugins/tomcat/geronimo-tomcat7/src/main/java/org/apache/geronimo/tomcat/core/GeronimoApplicationServletRegistrationAdapter.java
>>         https://svn.apache.org/repos/asf/geronimo/server/trunk/plugins/tomcat/geronimo-tomcat7/src/main/java/org/apache/geronimo/tomcat/core/GeronimoApplicationContext.java
>>         I am not sure how to implement these without creating wrapper/adapter for
the ServletContext/ServletRegistration, or I miss anything somewhere ?
> 
> I'll try to take a look soon.
> 
>> 
>> It looks to me as if the return value from the spec setServletSecurity is of limited
use because you can set the security element on your servlet registration before you set any
servlet mappings, so you wont  find out about any conflicts with the xml dd.
>>   
>>      Even if it is of limited use, think that we need to obey the java doc :-)  
    
> 
> of course :-)
> 
> thanks
> david jencks
> 
>>   
>>      
>> 
>> thanks
>> david jencks
>> 
>>  
>> 
>>>   
>>> 
>>> >     Thoughts ?
>>> >      To David. I found you did some code changes for Jetty now, and wonder
whether you have bugun some simliar work ?
>>> 
>>> I was thinking about doing something like this but haven't started anything.
 I did look a little bit into configuring tomcat using the info tree rather than letting tomcat
read the web.xml.  I've found a bunch of tomcat problems and spec inconsistencies.  I haven't
gotten to security configuration yet.
>>>    
>>> 
>>> thanks
>>> david jencks
>>> 
>>> >      Thanks !
>>> > --
>>> > Ivan
>>> 
>>> 
>>> 
>>> 
>>> -- 
>>> Ivan
>> 
>> 
>> 
>> 
>> -- 
>> Ivan
> 
> 
> 
> 
> -- 
> Ivan


Mime
View raw message