You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Mark Thomas <ma...@apache.org> on 2016/02/11 12:59:49 UTC

JASPIC requirements for request and response wrapping

JASPIC requires that an authentication module can wrap the
HttpServletRequest and/or HttpServletResponse passed to it and that the
wrapped instances are passed to the application. This creates a problem
since the authenticators are implemented as Valves which pass Tomcat's
o.a.c.connector.[Request|Response] objects. Tomcat's objects implement
the Servlet spec interfaces so, as long as no wrapping occurs, all is fine.

There is a long standing enhancement request [1] for being able to use
wrapping with Valves. Rémy is -1 on implementing it.

I'd still like to support JASPIC but I do understand where Rémy is
coming from w.r.t. the risks of wrapping. With all that in mind I have
been thinking through different implementation options and I have
reached the point where I think it makes sense to set out the options as
I see them for discussion.

1. Give up on JASPIC.
   Pros: All the design / integration issues go away
         No performance risk
   Cons: We lose the opportunity for a simple OAuth / SAML solution

2. Don't support wrapping with JASPIC.
   Pros: All the design / integration issues go away
   Cons: Not specification compliant
         We don't know how much implementations rely on this

3. Switch the Valve interface to use HttpServletRequest and
   HttpServletResponse.
   Pros: Enables the use of the associated Wrapper classes
   Cons: Valves need access to the internals. ValveBase could
         provide utility methods for accessing the wrapped
         o.a.c.connector.[Request|Response] implementations
         Breaks existing custom Valves

4. Make o.a.c.connector.[Request|Response] interfaces and provide
   separate concrete implementations.
   Pros: No change to Valve interface
   Cons: JASPIC would need additional code to bridge between these
         interfaces and the HttpServlet.*Wrapper classes

5. Move JASPIC processing to the start of the FilterChain since it
   uses HttpServlet[Request|Response]
   Pros: No / minimal API changes for Tomcat
   Cons: Would result in authentication happening in multiple places.
         Would need to be very careful to ensure requests couldn't
         bypass authentication, particularly during JASPIC provider
         (de)registratiob
         JASPIC would end up duplciating a lot of the authorization
         code in AuthenticatorBase


Of all of these, I think 4 holds the most promise. The first step for
that option would be reducing the current public interface of
o.a.c.connector.[Request|Response] to the minimum that is required. From
a design point of view that is a good thing to do anyway so I plan to
work on that while these options are discussed. Even if we don't go for
option 4, the work would be entirely pointless.

If we do follow option 4 then the Javadoc will need updating to make it
very clear what is supported and what isn't. Generally, I think the
message needs to be "If you wrap these you are messing with Tomcat's
internals. Tread very, very carefully. Correct operation of Tomcat
depends on the correct behaviour of these methods. Before providing an
alternative implementation of any method you should review the standard
implementation and how the method is used."

Mark


[1] https://bz.apache.org/bugzilla/show_bug.cgi?id=45014

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: JASPIC requirements for request and response wrapping

Posted by Mark Thomas <ma...@apache.org>.
On 11/02/2016 12:34, Rémy Maucherat wrote:
> 2016-02-11 12:59 GMT+01:00 Mark Thomas <ma...@apache.org>:
> 
>> JASPIC requires that an authentication module can wrap the
>> HttpServletRequest and/or HttpServletResponse passed to it and that the
>> wrapped instances are passed to the application. This creates a problem
>> since the authenticators are implemented as Valves which pass Tomcat's
>> o.a.c.connector.[Request|Response] objects. Tomcat's objects implement
>> the Servlet spec interfaces so, as long as no wrapping occurs, all is fine.
>>
> 
> Tomcat could simply use the already wrapped request/response later when
> invoking the filter chain. It would be set on the MessageInfo, right ? In
> that case, JASPIC should get Request.getRequest() and
> Response.getResponse() as its request/response objects and the wrapped
> request/response can go into two new fields in Request/Response. Would that
> work ?

That could work yes. I did think of that option but I rejected as I was
concerned about having two versions of the request and response. On
reflection, the end result is essentially the same for this option or
either of options 3 or 4 so maybe this is the way to go.

> Honestly, whatever JASPIC would do with its wrapping is likely irrelevant
> for the rest of the Cataline pipeline, so as long as the application sees
> it it should be fine.
> 
> There is a long standing enhancement request [1] for being able to use
>> wrapping with Valves. Rémy is -1 on implementing it.
>>
>> I'd still like to support JASPIC but I do understand where Rémy is
>> coming from w.r.t. the risks of wrapping. With all that in mind I have
>> been thinking through different implementation options and I have
>> reached the point where I think it makes sense to set out the options as
>> I see them for discussion.
>>
>> 1. Give up on JASPIC.
>>    Pros: All the design / integration issues go away
>>          No performance risk
>>    Cons: We lose the opportunity for a simple OAuth / SAML solution
>>
>> 2. Don't support wrapping with JASPIC.
>>    Pros: All the design / integration issues go away
>>    Cons: Not specification compliant
>>          We don't know how much implementations rely on this
>>
>> 3. Switch the Valve interface to use HttpServletRequest and
>>    HttpServletResponse.
>>    Pros: Enables the use of the associated Wrapper classes
>>    Cons: Valves need access to the internals. ValveBase could
>>          provide utility methods for accessing the wrapped
>>          o.a.c.connector.[Request|Response] implementations
>>          Breaks existing custom Valves
>>
>> 4. Make o.a.c.connector.[Request|Response] interfaces and provide
>>    separate concrete implementations.
>>    Pros: No change to Valve interface
>>    Cons: JASPIC would need additional code to bridge between these
>>          interfaces and the HttpServlet.*Wrapper classes
>>
>> 5. Move JASPIC processing to the start of the FilterChain since it
>>    uses HttpServlet[Request|Response]
>>    Pros: No / minimal API changes for Tomcat
>>    Cons: Would result in authentication happening in multiple places.
>>          Would need to be very careful to ensure requests couldn't
>>          bypass authentication, particularly during JASPIC provider
>>          (de)registratiob
>>          JASPIC would end up duplciating a lot of the authorization
>>          code in AuthenticatorBase
>>
>>
>> Of all of these, I think 4 holds the most promise. The first step for
>> that option would be reducing the current public interface of
>> o.a.c.connector.[Request|Response] to the minimum that is required. From
>> a design point of view that is a good thing to do anyway so I plan to
>> work on that while these options are discussed. Even if we don't go for
>> option 4, the work would be entirely pointless.
>>
>> If we do follow option 4 then the Javadoc will need updating to make it
>> very clear what is supported and what isn't. Generally, I think the
>> message needs to be "If you wrap these you are messing with Tomcat's
>> internals. Tread very, very carefully. Correct operation of Tomcat
>> depends on the correct behaviour of these methods. Before providing an
>> alternative implementation of any method you should review the standard
>> implementation and how the method is used."
>>
>> Mark
>>
>>
>> [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=45014
>>
>> On the other proposed solutions:
> 1) Well, we did so much with this junk, it would be a problem to give up.
> 2) It should be properly supported.
> 3) Meh. OTOH it's probably just some unwrapping.
> 4) I think 3 is better.
> 5) Servlet 3 did a lot to move some auth to the application layer, so if
> there's an auth step at this point, I don't think it is so horrible
> conceptually.
> 
> So maybe I like 3 better if my comment at the beginning cannot work.

Breaking all the existing Valves was my main concern with 3. I think
your earlier comment can work so I'll go that route for now.

Thanks for the review.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: JASPIC requirements for request and response wrapping

Posted by Rémy Maucherat <re...@apache.org>.
2016-02-11 12:59 GMT+01:00 Mark Thomas <ma...@apache.org>:

> JASPIC requires that an authentication module can wrap the
> HttpServletRequest and/or HttpServletResponse passed to it and that the
> wrapped instances are passed to the application. This creates a problem
> since the authenticators are implemented as Valves which pass Tomcat's
> o.a.c.connector.[Request|Response] objects. Tomcat's objects implement
> the Servlet spec interfaces so, as long as no wrapping occurs, all is fine.
>

Tomcat could simply use the already wrapped request/response later when
invoking the filter chain. It would be set on the MessageInfo, right ? In
that case, JASPIC should get Request.getRequest() and
Response.getResponse() as its request/response objects and the wrapped
request/response can go into two new fields in Request/Response. Would that
work ?

Honestly, whatever JASPIC would do with its wrapping is likely irrelevant
for the rest of the Cataline pipeline, so as long as the application sees
it it should be fine.

There is a long standing enhancement request [1] for being able to use
> wrapping with Valves. Rémy is -1 on implementing it.
>
> I'd still like to support JASPIC but I do understand where Rémy is
> coming from w.r.t. the risks of wrapping. With all that in mind I have
> been thinking through different implementation options and I have
> reached the point where I think it makes sense to set out the options as
> I see them for discussion.
>
> 1. Give up on JASPIC.
>    Pros: All the design / integration issues go away
>          No performance risk
>    Cons: We lose the opportunity for a simple OAuth / SAML solution
>
> 2. Don't support wrapping with JASPIC.
>    Pros: All the design / integration issues go away
>    Cons: Not specification compliant
>          We don't know how much implementations rely on this
>
> 3. Switch the Valve interface to use HttpServletRequest and
>    HttpServletResponse.
>    Pros: Enables the use of the associated Wrapper classes
>    Cons: Valves need access to the internals. ValveBase could
>          provide utility methods for accessing the wrapped
>          o.a.c.connector.[Request|Response] implementations
>          Breaks existing custom Valves
>
> 4. Make o.a.c.connector.[Request|Response] interfaces and provide
>    separate concrete implementations.
>    Pros: No change to Valve interface
>    Cons: JASPIC would need additional code to bridge between these
>          interfaces and the HttpServlet.*Wrapper classes
>
> 5. Move JASPIC processing to the start of the FilterChain since it
>    uses HttpServlet[Request|Response]
>    Pros: No / minimal API changes for Tomcat
>    Cons: Would result in authentication happening in multiple places.
>          Would need to be very careful to ensure requests couldn't
>          bypass authentication, particularly during JASPIC provider
>          (de)registratiob
>          JASPIC would end up duplciating a lot of the authorization
>          code in AuthenticatorBase
>
>
> Of all of these, I think 4 holds the most promise. The first step for
> that option would be reducing the current public interface of
> o.a.c.connector.[Request|Response] to the minimum that is required. From
> a design point of view that is a good thing to do anyway so I plan to
> work on that while these options are discussed. Even if we don't go for
> option 4, the work would be entirely pointless.
>
> If we do follow option 4 then the Javadoc will need updating to make it
> very clear what is supported and what isn't. Generally, I think the
> message needs to be "If you wrap these you are messing with Tomcat's
> internals. Tread very, very carefully. Correct operation of Tomcat
> depends on the correct behaviour of these methods. Before providing an
> alternative implementation of any method you should review the standard
> implementation and how the method is used."
>
> Mark
>
>
> [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=45014
>
> On the other proposed solutions:
1) Well, we did so much with this junk, it would be a problem to give up.
2) It should be properly supported.
3) Meh. OTOH it's probably just some unwrapping.
4) I think 3 is better.
5) Servlet 3 did a lot to move some auth to the application layer, so if
there's an auth step at this point, I don't think it is so horrible
conceptually.

So maybe I like 3 better if my comment at the beginning cannot work.

Rémy