Paul and others,
See intermixed comments below.
Paul Frieden wrote:
> I've attached 1 patch, 2 modified interceptors, and one new
> interceptor. These all work with 3.2b6. Please consider these for
> 3.2b7. Here is what they do:
>
> CookieTools.patch:
> This patch fixes cookie deletion. The problem was that to delete a
> cookie, you use
> setMaxAge(0). The implementation of CookieTools adds the max age onto
> the current time of
> the server. Thats fine, but the RFCs refer to setting the time into
> the past for
> deletion. This patch sets the expire time on the cookie way back into
> the past. It at
> least fixes it for me.
>
Makes perfect sense. I patched this for both 3.2 and 4.0 (since I had copied
CookieUtils
from the 3.2 tree :-).
>
> SessionInterceptor1.java & StandardSessionInterceptor1.java:
> This patch modifies the behavior of session cookies. The way the
> original worked was
> SessionInterceptor would find the first cookie named JSESSIONID, and
> store it as the
> requested session ID. StandardSessionInterceptor would then check the
> requested session
> ID to see if it was valid in the selected context. This version
> removes the cookie
> checking from SessionInterceptor, and moves all the cookie handling to
> StandardSessionInterceptor. This gives us access to the context so
> that we can check all
> the cookies named JSESSIONID to find a valid one for that context.
> That allows a / and a
> non-/ cookie to both be present and it will still pick the right one.
> These two new
> interceptors should be used together, and replace the original
> SessionInterceptor and
> StandardSessionInterceptor. These also set the jvmRoute on the /
> context. We can do that
> now because it will always get the right session ID for the context.
>
I like the thinking here, and I'm currently experimenting with these two new
interceptors to
make sure nothing else breaks. I do have one suggested change, though.
Right now, your patch tries to find the "correct" JSESSIONID cookie by looking for a
valid
one in the current context. Not only will this fail in the (admittedly rare) case
where the
same session id is assigned by two different webapps, it also hides a useful piece of
information.
Consider that I have a cookie-maintained session, and I go to lunch (so my session
expires)
before submitting the next request. My request will still include the cookie for this
web
app, so it should be reported via request.getRequestedSessionId() even though it's not
valid
-- which means request.isRequestedSessionIdValid() should return false.
What I suggest we do instead (for both 3.2 and 4.0, by the way) is to treat the *first*
JSESSIONID occurrence that is submitted as the one that is relevant for this web
application. This is based on a requirement in the cookie spec (RFC 2109) that the
browsers
actually seem to be obeying:
"If multiple cookies satisfy the criteria above, they are
ordered in the Cookie header such that those with more
specific Path attributes precede those with less specific."
This conveniently matches the rules that servlet containers use to map requests to
context
paths -- the longest matching context path wins. Therefore, if I have active sessions
in
three contexts with the following context paths:
/
/foo
/foo/bar
then if I submit a request for "/foo/bar/index.jsp" I will get be sent to the
"/foo/bar"
context, and will receive all three cookies -- the one for the /foo/bar context being
first.
What do you think?
>
> SessionCookieSanitizer.java:
> This is a new interceptor that will hide all cookies named JSESSIONID
> that are not valid
> sessions from the webapp. In the case of a good app and a malicious
> app, if the one
> user logged in on the good app, and then accessed the malicious app,
> the malicious app
> could send the session ID to someone who could then masquerade as the
> user in the good
> app.
>
I haven't gotten to this one yet, but it sounds reasonable. If implemented, this
should
*definitely* be in a separate interceptor so it can be commented out -- otherwise, you
could
not write a web app that acts like a "proxy" and therefore needs access to all of the
incoming headers.
>
> Both SessionInterceptor1 and StandardSessionInterceptor1 are mostly the
> same as the originals, but some code has been moved. Sending these as
> patches might make it unclear where the changes had occured and why, so
> I'm sending them in their entirity.
>
> Paul Frieden
>
Thanks for the patches!!!
Craig
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
