You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by "Craig R. McClanahan" <Cr...@eng.sun.com> on 2000/11/10 18:50:34 UTC
Ready for 3.2b7?
It's been a week now, and I've committed > 20 patches to the 3.2 tree,
ranging from simple tweaks to security problems to spec compliance
bugs. I believe that I've gotten all of the critical bug reports
submitted on the mailing lists or via BugRat. Does anyone know of any
I've missed (see below for one issue I know is outstanding)?
What I'd like to do is build a "beta 7" release this afternoon and post
it. That will give people a chance to pound on it. Any critical bugs
we find will need to be fixed, but we need to hold off on changing
non-essential stuff so we can get a final 3.2 release out the door.
NOTE: One issue that's been discussed in the last couple of days is
problems supporting the "load balancing" feature for root webapps. I
haven't seen a proposed patch for this, but understand from the comments
of people that have tried kludges to work around it -- and it seems
unreasonable to risk destabilizing things at this late date. I suggest
that any work on fixing this problem be deferred to a post-3.2-final
maintenance cycle.
Craig McClanahan
PS: Thanks to everyone for all the bug reports, and to Larry and Nacho
for chipping in on the commits!
PPS: When the 3.2 final release is completed, my personal focus is
going to return to the Tomcat 4.0 code base (which does not suffer from
any of the bugs patched in 3.2, although I did find one 4.0 bug along
the way :-). If and when bugs show up in 3.2 final, I will be happy to
commit patches that people supply -- but any big debugging effort or
major new work on the 3.x track will need to be done by someone else.
Re: [PATCH] Re: Ready for 3.2b7?
Posted by Colin Evans <ce...@bitmo.com>.
> 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."
>
For what its worth, a lot of browsers don't seem to do this correctly. One
of the developers that I work with was telling me that versions of Netscape
4.7 don't do this correctly, and I've had experiences with the Phone.com WAP
browser that indicate that the browser is *very* broken in how and when it
handles, replaces, deletes, and expires cookies.
For instance, the Phone.com 4.0 WAP browser emulator doesn't do cookie
replacement, which means that it will pass up multiple JSESSIONID cookies
for the same path attribute. This violates section 4.3.3 of RFC 2109, but
the Phone.com browser is in every WAP phone in north america, which makes it
a de facto standard:
"If a user agent receives a Set-Cookie response header whose NAME is the
same as a pre-existing cookie, and whose Domain and Path attribute values
exactly (string) match those of a pre-existing cookie, the new cookie
supersedes the old."
I would recommend testing cookie behavior with a large number of browsers
before committing to any strategy for handling cookies.
-Colin
--
Colin Evans
Bitmo, Inc. (http://www.bitmo.com)
(415)920.7225 / cevans@bitmo.com
Re: [PATCH] Re: Ready for 3.2b7?
Posted by "Craig R. McClanahan" <Cr...@eng.sun.com>.
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
[PATCH] Re: Ready for 3.2b7?
Posted by Paul Frieden <pf...@dChain.com>.
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.
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.
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.
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
"Craig R. McClanahan" wrote:
>
> It's been a week now, and I've committed > 20 patches to the 3.2 tree,
> ranging from simple tweaks to security problems to spec compliance
> bugs. I believe that I've gotten all of the critical bug reports
> submitted on the mailing lists or via BugRat. Does anyone know of any
> I've missed (see below for one issue I know is outstanding)?
>
> What I'd like to do is build a "beta 7" release this afternoon and post
> it. That will give people a chance to pound on it. Any critical bugs
> we find will need to be fixed, but we need to hold off on changing
> non-essential stuff so we can get a final 3.2 release out the door.
>
> NOTE: One issue that's been discussed in the last couple of days is
> problems supporting the "load balancing" feature for root webapps. I
> haven't seen a proposed patch for this, but understand from the comments
> of people that have tried kludges to work around it -- and it seems
> unreasonable to risk destabilizing things at this late date. I suggest
> that any work on fixing this problem be deferred to a post-3.2-final
> maintenance cycle.
>
> Craig McClanahan
>
> PS: Thanks to everyone for all the bug reports, and to Larry and Nacho
> for chipping in on the commits!
>
> PPS: When the 3.2 final release is completed, my personal focus is
> going to return to the Tomcat 4.0 code base (which does not suffer from
> any of the bugs patched in 3.2, although I did find one 4.0 bug along
> the way :-). If and when bugs show up in 3.2 final, I will be happy to
> commit patches that people supply -- but any big debugging effort or
> major new work on the 3.x track will need to be done by someone else.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org