geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan <xhh...@gmail.com>
Subject Re: Enable Declartive Security in Jetty ?
Date Wed, 22 Sep 2010 10:55:22 GMT
Finally, attach a patch file to
*GERONIMO-5624<https://issues.apache.org/jira/browse/GERONIMO-5624>:-)
*

2010/9/19 David Jencks <david_jencks@yahoo.com>

>
> 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
>
>
>


-- 
Ivan

Mime
View raw message