Return-Path: Delivered-To: apmail-geronimo-dev-archive@www.apache.org Received: (qmail 716 invoked from network); 22 Sep 2010 10:55:52 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 22 Sep 2010 10:55:52 -0000 Received: (qmail 27594 invoked by uid 500); 22 Sep 2010 10:55:52 -0000 Delivered-To: apmail-geronimo-dev-archive@geronimo.apache.org Received: (qmail 27433 invoked by uid 500); 22 Sep 2010 10:55:50 -0000 Mailing-List: contact dev-help@geronimo.apache.org; run by ezmlm Precedence: bulk list-help: list-unsubscribe: List-Post: Reply-To: dev@geronimo.apache.org List-Id: Delivered-To: mailing list dev@geronimo.apache.org Received: (qmail 27426 invoked by uid 99); 22 Sep 2010 10:55:50 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 22 Sep 2010 10:55:50 +0000 X-ASF-Spam-Status: No, hits=2.2 required=10.0 tests=FREEMAIL_FROM,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_PASS,T_TO_NO_BRKTS_FREEMAIL X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of xhhsld@gmail.com designates 209.85.214.182 as permitted sender) Received: from [209.85.214.182] (HELO mail-iw0-f182.google.com) (209.85.214.182) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 22 Sep 2010 10:55:43 +0000 Received: by iwn34 with SMTP id 34so543027iwn.13 for ; Wed, 22 Sep 2010 03:55:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:content-type; bh=9bw1F0SF/7uQLo32lg9E0L7y8u7rr4VpenTmQSjIo/c=; b=k4vc3q3/2039Wwo/ltQUV69Xox4EimIw1/AEAa5j7f+Yt58+jW70PQKkcNMbG198KY X4WyhK2YTlLp2r8EF0QpjuFUcTyXeLMSRjo4zmoYW5/hoVf0xzJnFW66yQ4AL6FCTvE5 vk71PcqFbuEezJ9A6hkY7osP3Pd1mJEyTWK1w= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; b=CouohOJHr5JhIIrvFbb639skA+Kqu8vqnsfCYddyZyL+V+pre/nENZDQG9SDEnEdRe 9LpzUaFfHj0G1taJekcytHg6AHlWaZvSmDEjyQ3eqKhgqHD6v5s8Xd5eHbWnnjE1vJ1J WDqTVgZfjnr9ACuME5Dx0YFkjAuDiIO4qAhXc= MIME-Version: 1.0 Received: by 10.231.30.76 with SMTP id t12mr13622377ibc.161.1285152922774; Wed, 22 Sep 2010 03:55:22 -0700 (PDT) Received: by 10.231.17.136 with HTTP; Wed, 22 Sep 2010 03:55:22 -0700 (PDT) In-Reply-To: <1D7FD7AD-8D4D-45BD-A90B-328BE11B01F4@yahoo.com> References: <74241933-EEE9-4430-83ED-EE06D8DF08E7@yahoo.com> <5B1B3F21-38F1-47F1-B521-4012ED8165BB@yahoo.com> <1D7FD7AD-8D4D-45BD-A90B-328BE11B01F4@yahoo.com> Date: Wed, 22 Sep 2010 18:55:22 +0800 Message-ID: Subject: Re: Enable Declartive Security in Jetty ? From: Ivan To: dev@geronimo.apache.org Content-Type: multipart/alternative; boundary=0003255746ce71ad940490d6fbe0 --0003255746ce71ad940490d6fbe0 Content-Type: text/plain; charset=ISO-8859-1 Finally, attach a patch file to *GERONIMO-5624:-) * 2010/9/19 David Jencks > > On Sep 18, 2010, at 1:14 AM, Ivan wrote: > > > > 2010/9/18 David Jencks > >> >> On Sep 18, 2010, at 12:24 AM, Ivan wrote: >> >> >> >> 2010/9/18 David Jencks >> >>> >>> On Sep 17, 2010, at 5:36 PM, Ivan wrote: >>> >>> >>> >>> 2010/9/18 David Jencks >>> >>>> >>>> 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 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 --0003255746ce71ad940490d6fbe0 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Finally, attach a patch file to GERONIMO-5624 = :-)

2010/9/19 David Jencks <david_jencks@yahoo.c= om>

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



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

On Sep 18, 2010, a= t 12:24 AM, Ivan wrote:



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

On Sep 17, 2010, a= t 5:36 PM, Ivan wrote:



2010/9/18 David Jencks <david_jencks@yahoo.com&= gt;

On Sep 17, 2010, at 12:44 AM, Ivan wrote:

> Hi,
> =A0 =A0 While looking at some Servlet Security JIRAs, I begun some cod= e refactors on the SpecSecurityBuilder, including :
> =A0 =A0 a. Add more Info class for the security configurations, and se= rialize those in the .ser file, with them, it would avoid the xml parsing o= n the startup time and make the codes look simple

excellent idea!

> =A0 =A0 b. Use ServletContext more in the SpecSecurityBuilder, as it i= s 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. =A0Could you be more specific?

=A0e.g. Use the method ServetContext.getServletRegistration().getMapp= ing could eaisly get all the url patterns of the target servlet.

I don't think this will b= e ideal. =A0It 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. =A0For jetty, I= think a more "native" solution will work better, see below.
=A0=A0=A0
=A0=A0=A0 I might double check this, = but from the Java Doc, it did not write explicitly that they are only for n= ew added servlets.

right= , but my point is that url patterns in a constraint in web.xml may not appe= ar 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.
=A0 =A0 Another thing that could be refractor is that to make specsecurityb= uilder is a pure specsecuritybuilder, currently, it is also used to handle = how to scan servletsecurity annotation, and=A0 detect which security config= uration has high priority. =A0=A0=A0




>
> =A0 =A0 To make these functions work, especially the option b. it requ= ires to enable declarative security in Jetty integration, generally speakin= g, will adopt the same way as Tomcat integration does,
> =A0 =A0 a. create a Wrapper class for ServletContextHandler.Context cl= ass, so that we could monitor those new added dynamic servlets. One thing m= ight be care is that the codes need to distinguish the servlets from web.xm= l, as they are also added by ServletContext now in Jetty.
> =A0 =A0 b. Add a EventListener to ServletContextHandler, it will be re= sposible for the security calculation and fill it into ApplicationPolicyCon= figurationManager.
>

I think you mean "declarative security for servlets added by the= addServlet methods on ServletContext"? =A0Jetty will want to deal wit= h that too, so I think putting something in the jetty code that calls out t= o a security builder of some kind (we can install our own) is the best plan= here. =A0Then we shouldn't need more wrapping. =A0Maybe I don't un= derstand exactly what you mean? =A0What would the event listener do?

=A0 In the Tomcat integration, a JACCListener is atta= ched to the web context, while it receives the initialize event, a specsecu= ritybuilder is created, while the started event is received, it will build = the permissions and fill the result to the policymanager.

=A0 We need wrapper here, especially in Jetty now, since all the servle= ts are registered dynamically, for jetty itself, it will not know what to r= eturn for some methods, e,g, ServletRegistration.Dynamic.setServletSecurity= method, it needs to return all the un-affected url patterns. Also, since m= ost of the security related issues are handled by Geronimo, there is no nee= d to call the initial codes.

Right now this is not impleme= nted in jetty at all. =A0I think we should define an interface
ConstraintStore {
=A0=A0 =A0=A0=A0 =A0 =A0 =A0public= Set<String> setServletSecurity(String servletName, ServletSecurityEl= ement securityElement);
}

that the ServletRegistration will delegate = to. =A0Our implementation can implement it based on the info tree from the = deployment descriptor. =A0When we start the context we can convert the Serv= letSecurityElements and url mappings to info, and proceed to generate the p= ermissions.


setServletSecurity definitely ne= eds to be delegated, but it is not=20 enought. Other methods like "createServlet/addServlet/.." might a= lso be=20 required.

We already prov= ide the implementation for creating servlet instances. =A0We can take the o= pportunity to scan for the security annotation as well.

Actually, th= e declaritive security is much complicated,=20 especially whether or not need to take care the ServletSecurity=20 annotation. I quoted some code comments from my working copy ;
=A0=A0 /*= *
=A0=A0=A0=A0=A0=A0=A0=A0 *=A0 a. The security constraints in the portable d= eployment descriptor are of the highest priority,
=A0=A0=A0=A0=A0=A0=A0=A0 *=A0 b .None security annotations will take effect= on the URL=20 patterns explicitly configured in the portable deployment desciptor,
=A0=A0=A0=A0=A0=A0=A0=A0 *=A0=A0=A0=A0=A0 but for those URL patterns are no= t configured, the=20 security annotations should take effect, except for META-COMPLETE is set with TRUE
=A0=A0=A0=A0=A0=A0=A0=A0 *=A0 c. All the dynamic added servlets should take= care the ServletSecurity annotation, and it seems to have
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 nothing to do with the META-C= OMPLETE flag, two exceptions are :
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 c1. Users create the servlet = by themselves
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 c2. ServletRegistration.Dynam= ic.setServletConstraint is called
=A0=A0=A0=A0=A0=A0=A0=A0 *=A0 d. For those URL patterns added by=20 ServletContext.getServletRegistration().addMappping, and those target=20 servlets are configured
=A0=A0=A0=A0=A0=A0=A0=A0 *=A0=A0=A0=A0=A0 in the portable deployment plan, = ServletSecurity=20 annotation should also be taken care, except for META-COMPLETE is set=20 with TRUE
=A0=A0=A0=A0=A0=A0=A0=A0 */

it's definitely even more complicated than it was in ee5 where i= t was bad enough :-)

As I understand the javadoc w= e can:

1. convert the web.xml constraints to info as soon as w= e know the complete web.xml.
2. if web.xml was not marked metadat= a 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 na= me too, merging the info by url pattern, where "last wins"
<= div>4. we merge the servlet name to security info map with the info from th= e web.xml, where the web.xml wins.

=A0 =A0
=A0=A0=A0 Generall= y speaking, that is what we do now, mightbe some differences on those detai= led handlering.
=A0 =A0 Another missing point, if the ServletContext.get= ServletRegistration.addMapping works for all the servlets, including the on= es 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 :-) =A0If all the security constraint info is in the web.xml,= calling ServletRegistration.addMapping has no effect on on security constr= aints. =A0Calling=A0ServletRegistration.addMapping has an effect on constra= ints only if there is a security annotation (not suppressed by metadataComp= lete =3D true) or if you call setServletSecurity. =A0Agreed?

=
=A0 =A0 Hope I could create a patch tomorrow,=A0 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

=A0 =A0=A0=
We can't mer= ge security constraint info from annotations into web.xml because it can be= overridden by this setServletSecurity method, whereas constraints original= ly in web.xml can't be.


Some= similar codes are in Tomcat integration, e.g.
=A0=A0=A0=A0=A0=A0=A0=20 https://svn= .apache.org/repos/asf/geronimo/server/trunk/plugins/tomcat/geronimo-tomcat7= /src/main/java/org/apache/geronimo/tomcat/core/GeronimoApplicationServletRe= gistrationAdapter.java
=A0=A0=A0=A0=A0=A0=A0=20 https://svn.apache.org/repos/a= sf/geronimo/server/trunk/plugins/tomcat/geronimo-tomcat7/src/main/java/org/= apache/geronimo/tomcat/core/GeronimoApplicationContext.java
=A0 =A0 =A0 =A0 I am not sure how to implement these without creating wrapp= er/adapter for the ServletContext/ServletRegistration, or I miss anything s= omewhere ?

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 s= et any servlet mappings, so you wont =A0find out about any conflicts with t= he xml dd.
=A0
=A0 =A0=A0 Even if it is of limited u= se, think that we need to obey the java doc :-) =A0=A0=A0=A0=A0=A0

of course :-)

thanks
david jencks

=A0
=A0 =A0=A0

thanks
= david jencks

=A0

=A0=A0

> =A0 =A0 Thoughts ?
> =A0 =A0 =A0To 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 starte= d anything. =A0I did look a little bit into configuring tomcat using the in= fo tree rather than letting tomcat read the web.xml. =A0I've found a bu= nch of tomcat problems and spec inconsistencies. =A0I haven't gotten to= security configuration yet.
=A0=A0

thanks
david jencks

> =A0 =A0 =A0Thanks !
> --
> Ivan




--
Ivan




--
Ivan




--
Ivan




--
Ivan
--0003255746ce71ad940490d6fbe0--