Carsten Been pondering this one some more and thrown some notes down below. Caveat to all of them though: I am not so familiar with the recent Felix codebase in this area; and my experience of real-world HTTP security definitely has gaps!
So my take is that some apps (obviously not many as we're the 1st to hit this in 6 months since the change!) would expected a session invalidate() to destroy every aspect of that session. All attributes definitely, and also most would expect the ID to be destroyed and not re-used. I haven't checked the Servlet or HTTP specs for that last part, maybe it isn’t stated in a contract anywhere. I could certainly see apps that hold their session information in some external cache not in attributes getting tripped up with narrow timing windows where an old session ID comes in again before a cache is cleared. Bad coding by that app - sure, almost certainly. But also caught out by non-standard behaviour. Of course it sounds like we have just as many if not more HTTP whiteboard users that would get caught by this changed. I'm not going to argue the current re-use of IDs is wrong, and that we need to refactor since we risk breaking things for these users, and that is just as bad. So my proposal is (hopefully) a simple one. We add a property that lets users (like us) tripped up by the wrapper behaviour, have the raw Jetty Session object returned rather than the wrapper. That way no Felix code gets in the way of normal Jetty handling for those not using HTTP whiteboard. Looking at the code, I think there's a very small number of places this would change. Obviously getSession() is one. And then in Dispatcher we'd need to do an instanceof test in dispatch before calling getExpiredSessionContextNames. And I think again something similar in ServiceController. We could of course add a higher level wrapper, but that seems to get heavyweight. One added benefit of the above is it allows use of the normal Jetty SecureHandler code which does an automatic session invalidate/renew as part of the auth steps. But this gets torpedoed by the use of the Felix wrapper due to an (admittedly also nasty) instanceof test in the Jetty code. Cheers --Rob -----Original Message----- From: Rob Walker Sent: 22 August 2018 18:49 To: Carsten Ziegeler <cziege...@apache.org>; dev@felix.apache.org Subject: RE: Session invalidation I didn’t check for that, will take a look. The duplicate ID was what I noticed, and one of the things tripping me up. -R -----Original Message----- From: Carsten Ziegeler [mailto:cziege...@apache.org] Sent: 22 August 2018 17:59 To: dev@felix.apache.org; Rob Walker <r...@ascert.com> Subject: Re: Session invalidation The new session has the same ID (and I'm not trying to imply that this is good), but it should be a different session object being empty Regards Carsten Rob Walker wrote > What I'm seeing is that I get the same session with the same ID back, > and it never gets invalidated. I think it has something to do with the > wrapper not invalidating the delegate. In our local code I've patched > the wrapper to put the delegate.invalidate() call back, and it seems > then to be creating a new session for me (or returning null) -R > > -----Original Message----- > From: Carsten Ziegeler [mailto:cziege...@apache.org] > Sent: 22 August 2018 17:33 > To: dev@felix.apache.org; Rob Walker <r...@ascert.com> > Subject: Re: Session invalidation > > Hmm, maybe I'm missing something here, so you call getSession().invalidate(). > This should invalidate this session and clear all attributes. > > If another request comes in using the same session id, a > getSession(false) should return null. > > A getSession() should return a new session which is empty. > > So this should follow the servlet API contract. > > Or do you experience different results here? > > Regards > > Carsten > > > Rob Walker wrote >> Well for the short term, I've just copied the source for HttpSessionWrapper >> into our gradle re-bundling script, and just added back in the >> delegate.invalidate() call at the end. So at least for now, we've got a >> local solution that lets us deal with this. >> >> I'm afraid I'm not sure what the proper / general solution is. >> >> It would seem to me though that the current code doesn't honour the Servlet >> contact i.e. getSession().invalidate() does not invalidate the underlying >> HTTP session. >> >> I'm not sure I fully understood the issue with the http whiteboard as it's >> not a service we use. But it feels like in order to allow that to work, we >> are actually breaking an underlying fundamental operation of HTTP servlets. >> That feels kinda wrong to me, since it could break all sorts of usage. In >> our case, it allows session hijacking by cloning the session ID from the >> cookie or URL parameter. That's a pretty nasty security breach, and one that >> possibly could hit others who are relying on session invalidate to clear >> tokens. >> >> Anyhow, enough from me, wanted to highlight the issue at least >> >> -Rob >> >> -----Original Message----- >> From: Carsten Ziegeler [mailto:cziege...@apache.org] >> Sent: 22 August 2018 17:08 >> To: dev@felix.apache.org; Rob Walker <r...@ascert.com> >> Subject: Re: Session invalidation >> >> This is a problem of the http whiteboard wrt to session handling. For each >> context managed by the whiteboard you get a separate session. These sessions >> are managed in the real http session. >> >> Now I assume you invalidate one of the per context managed sessions. >> This does not invalidate the real http session as the http whiteboard >> implementation does not know whether anything else still wants to use it. >> Imagine you have another application running which happens to use the same >> http session - and that app is not managed by the whiteboard. >> >> The basic idea is that the real session times out eventually. >> >> I'm not sure what a good way of invalidating the http session in this >> case is :( >> >> Regards >> Carsten >> >> Rob Walker wrote >>> So one more from me today - I'm a little perplexed on session invalidation. >>> >>> >>> >>> In common with general security best practice on HTTP, we invalidate >>> the session ID obtained during initial logon and create a new one >>> for the auth'd and logged on user. This helps prevent session >>> sniffing and spoofing because the initial session ID can become visible and >>> disclosed. >>> >>> >>> >>> While updating to newer Felix HTTP Jetty the session ID never seems >>> to get invalidated. We always seem to get the same ID back even >>> after we try and invalidate >>> >>> >>> >>> Digging into the code of HttpSessionWrapper shows that the Jetty >>> delegate invalidate never gets called. >>> >>> >>> >>> Here's where it gets weird though. It looks like a mod was committed >>> by Carsten on 29/3/2018 to explicitly remove the delegate invalidate >>> quiet recently >>> >>> >>> >>> SHA-1: f86428f2689e62aafe750d1905fff4f5136ab67e >>> >>> >>> >>> ** FELIX-5819 : Container session should not be invalidated* >>> >>> >>> >>> git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1827956 >>> 13f79535-47bb-0310-9956-ffa450edef68 >>> >>> >>> >>> >>> >>> At which point I get thoroughly confused! Clearly there must be >>> something I'm missing >>> >>> >>> >>> ---- >>> >>> Rob Walker >>> >>> >>> >>> cid:image001.jpg@01D39C2C.9DA64510 >>> >>> >>> >>> www.ascert.com >>> >>> r...@ascert.com >>> >>> SA +27 21 300 2028 >>> >>> UK +44 20 7488 3470 ext 5119 >>> >>> >>> >> -- >> Carsten Ziegeler >> Adobe Research Switzerland >> cziege...@apache.org >> > -- > Carsten Ziegeler > Adobe Research Switzerland > cziege...@apache.org > -- Carsten Ziegeler Adobe Research Switzerland cziege...@apache.org
