tomcat-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simon Kulessa <kule...@flexsecure.de>
Subject Re: HttpServletRequest - getHeaders() vs getCookies()
Date Tue, 22 Jul 2014 09:47:50 GMT
Hi Christopher,

see below

Am 17.07.2014 16:18, schrieb Christopher Schultz:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Simon,
>
> On 7/17/14, 3:52 AM, Simon Kulessa wrote:
>> Hi Christopher,
>>
>> Am 16.07.2014 14:45, schrieb Christopher Schultz: Simon,
>>
>> On 7/16/14, 4:02 AM, Simon Kulessa wrote:
>>>>> Am 15.07.2014 16:12, schrieb Christopher Schultz: Simon,
>>>>>
>>>>> On 7/9/14, 4:51 AM, Simon Kulessa wrote:
>>>>>>>> I had a look at the documentation and the tomcat source
>>>>>>>> to get a better understanding of what the
>>>>>>>> '|org.apache.catalina.connector.RECYCLE_FACADE'
>>>>>>>> parameter actually does.|
>>>>>>>>
>>>>>>>> I have seen that Tomcat objects like Cookies, Request
>>>>>>>> etc. are designed to be reusable.
>>>>> Requests, yes. I haven't looked to see if Cookies would be
>>>>> re-usable but it seems plausible.
>>>>>
>>>>>>>> What I currently do not understand is: In which
>>>>>>>> scenario and what context are they going to be reused?
>>>>>>>> I see there are Endpoints classes (like NIOEndpoint)
>>>>>>>> which are used to process the different requests. This
>>>>>>>> seems to be the most likely entry point into the
>>>>>>>> scenario.
>>>>>>>>
>>>>>>>> Maybe somebody can provide some general outline of how
>>>>>>>> requests and the reusing of the object actually works
>>>>>>>> together? Is there some kind of relation to the IP of
>>>>>>>> an incoming request?
>>>>> The client's IP is irrelevant: Tomcat uses a pool of objects
>>>>> and re-fills each object with data from an incoming request.
>>>>> These objects, as far as the web application should be
>>>>> concerned, should have a valid lifetime equal to the request
>>>>> itself. The servlet spec requires these semantics, so this
>>>>> isn't some weird Tomcat thing. Tomcat has chosen to pool
>>>>> these objects for a small performance gain when it comes to
>>>>> memory management and garbage collection.
>>>>>
>>>>> If your application retains references to these objects after
>>>>> they become invalid, they may contain invalid data or valid
>>>>> data from another request after they should have become
>>>>> invalid form the perspective of the original request.
>>>>>
>>>>> If you need data from a request, cookie, etc. then you should
>>>>> copy it somewhere safe before the request ends.
>>>>>> We do not cache any request specific information. The IP
>>>>>> relation itself is irrelevant - it seems that the reused
>>>>>> object's contains more 'old' informations than I
>>>>>> previously assumed. The header itself and the
>>>>>> requestedSessionId seems to be changed, the sourceIP and
>>>>>> the cookie stay the same.
>> I checked, and Tomcat does not appear to cache Cookie objects in
>> any way. If the request object is cached and there is a terrible
>> bug, it might be possible to re-read an incorrect session cookie
>> from an old set of request headers.
>>
>>> At least the cookies have a recycle() method, which is called if
>>> a request is recycled.
> I don't see a recycle() method in the Cookie class. Where do you see that?

I checked out the src for Tomcat the Tags 7.0.29 and 7.0.54 from
http://svn.apache.org/repos/asf/tomcat

In both version org.apache.tomcat.util.http.Cookies
as well as org.apache.tomcat.util.http.ServerCookie
have a recycle() method

>> I honestly think it's unlikely that there is such a bug in Tomcat.
>>
>>> There was a bug in Tomcat (CVE-2011-3375
>>> <http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2011-3375>)
>>> related to an incorrect recycling of the objects which seemed to
>>> be there for quite some time, but was fixed for 7.0.22.
> Yes, a bug was found in the past. Does that mean there could be a bug
> here? Sure, but I think it's unlikely that you have found one.
>
>>> I doubt as well that this is actually a bug in Tomcat, but I
>>> wouldn't rule this out completely.
>> At this point, I think you're probably going to have to create a
>> proof-of-concept web application that reproduces this problem. Do
>> you think you could boil-down the problem to a single servlet that
>> demonstrates what you are observing?
>>
>>> I tried to do that (it's a little bit more complex than just a
>>> single servlet), but so far I did not succeed.
> If we can't reproduce it, we can't investigate it. Please keep trying.
> There are ways to securely deliver a test case if you can't divorce
> the problem from your confidential code.
>
>>> But I can at least reproduce the Session Id change mentioned
>>> above if that would be interesting to anyone.
> I don't even understand the "session id change" that you described so
> it would be helpful to have a test case for that.
Basically it looks like this

0 request comes in via Servlet.doPost(...)

1 HttpSession session = request.getSession();
2 String sId1 = request.getSession().getId();
3 session.invalidate();
4 String sId2 = request.getSession().getId();

5 response is written

The session from 1 is saved in global map, so 3 is actually done from 
another thread that has nothing to do with the request.
The other steps are done in same request and thread.

Problem is that 'sometimes' sId1 does not equal sId2.

>>> Recently I start to wonder whether the Implementation of
>>> javax.servlet.AsyncContext might actually cache the original
>>> request. The interface at least suggest that it could have these
>>> information. I did not look at the implementation yet.
>>> At least the AsyncContext is created based on the Request. But
>>> the whole point of this thing is to be shared between requests,
>>> or am I wrong there?
> Are you caching the AsyncContext yourself, or are you using an
> AsyncListener to capture events and using the AsyncContext that comes
> from the event?
We are caching the AsyncContext and we have a Listener as well to react 
on the events.
In the listener we mostly (with the exception of onComplete, see below) 
use the cached
reference, not the one from the event.
> Can you even give us some pseudocode that shows what you are trying to
> do? So far, all you've really said is "Tomcat is messing-up my
> sessions". It took you 3 posts to mention that you were using Servlet
> 3.0 async, which might be the most important piece of information
> after the version you are using. Since your version is old, you really
> should upgrade regardless of the outcome of this conversation. In
> fact, there have been so many changes and tweaks to the async-related
> code over the years (/literally/ years since your release came out)
> that this problem may not even exist anymore.
The code for starting the context looks like this:

// listener is an application scoped class that is used for all 
AsyncContext's
public void startAsync(HttpServletRequest servletRequest, AsyncListener 
listener) {

         AsyncContext asyncContext = servletRequest.startAsync();
         if (asyncContext == null) {
             throw new IllegalStateException();
         }

         HttpSession session = ((HttpServletRequest) 
asyncContext.getRequest()).getSession();
         if (session == null) {
             asyncContext.complete();
             throw new IllegalStateException();
         }

         asyncContext.addListener(listener);
         // store asyncContext in global map, mapped to the current session
}

At this point of time the processing of the request which came from a 
Servlet is done.
The listener itself looks like this:

     public void onTimeout(AsyncEvent event) throws IOException {

           HttpSession httpSession = ((HttpServletRequest) 
event.getSuppliedRequest()).getSession(false);
           dispose(httpSession.getId());
     }

     public void onError(AsyncEvent event) throws IOException {

           HttpSession httpSession = ((HttpServletRequest) 
event.getSuppliedRequest()).getSession(false);
           dispose(httpSession.getId());
     }

     private void dispose(String sessionId) {
           // retrieve related session from global map
           AsyncContext ctx = ...
           if (ctx != null) {
               // Send some message that we close the session
               Message message = ...
               sendMessageOnAsyncContext(message, ctx);
           }

           httpSession.invalidate();
     }

     public void onComplete(AsyncEvent event) throws IOException {

          // retrieve session based on the sessionId contained in the 
event from the global map and take the AsyncContext from there
          AsyncContext ctx = ...

          if (event.getAsyncContext() == ctx) {  // <--- maybe equals 
would be better here? but the object should have the same reference
             // if same remove async context from global map
          }
     }

     public void onStartAsync(AsyncEvent event) throws IOException {
       // does nothing
     }

And then there is the sending of something which is used by the listener 
and other places

    // message is some internal used object to contain our message
     public static void sendMessageOnAsyncContext(Message message, 
AsyncContext context) throws IOException {

             try {
                // send something on the context to inform the client 
that we are closing it
             } finally {
                ctx.complete();
                // remove async context from global map
             }
         }
     }

> Can you reproduce the problem in a test environment with 7.0.29?
Currently not
> - -chris
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1
> Comment: GPGTools -http://gpgtools.org
> Comment: Using GnuPG with Thunderbird -http://www.enigmail.net/
>
> iQIcBAEBCAAGBQJTx9s6AAoJEBzwKT+lPKRYivIQAKGxXTjPPswuwAlvB8i2AYuL
> MJV6epY4HyAU1X7ovHcBcmUZ86Vt/zx0ehCgmKSToT/VcWMy8LROgzyDZa9kM3He
> /LNMCQpUACt6RRw08lntuOgQDxyNchedWdXFWbI2glpWrVGHNfzszDZ66pHJAkxG
> U8umNY7v6DWeNhjOehwF4x5JwSr/mozhtbK82l/ycl/QhUFeHm1zTBufVnBEtais
> L7N8G0L4heOt2F1Vh8IEyaEt8IvK+5kpQnLdRpkj4qplVfjbtpirJJpF5vY96VHa
> haTKKwHvM9axFY0CRhlV1+MFUnAfMUivRqqqGWPHXesi5Ao/pEDMcFJHy3IJZhG+
> rRaHBKH8JAkUNqnQxNaV1W4iZGfRtpEjdTVheat4B/Skrn2aIHmV5MQLVgtXCw8i
> jg2Lo9IgcXdYphBluOS9xd47r603b34wNNEyzAOO8sWL51kjfR48tAwcPD9WM11W
> Sh8+OtWZd4HOjm/zOAGUHZodH/JuAr/F4yn9CEDjoxsMU5I4R+0pY8j5EHY0YXhI
> L08M3OqyLItuA6Y0kYI7ZPnCedg+K+xN57h0plEEw5orvkyNEKjKPpBwgQkDb/AV
> qMTvU2AcXOj0zYOZCfppHXvtpjsjAuYDoXtY+NrQTTo1w91V4n1QhXr8aq+zlUcp
> /oX1KuwaNyo3rVaPS+B0
> =31cb
> -----END PGP SIGNATURE-----
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:users-unsubscribe@tomcat.apache.org
> For additional commands, e-mail:users-help@tomcat.apache.org
>
>


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


Mime
View raw message