Repository navigation
WW-5841 fix(boot): restrict session tracking to cookies and set SameSite=Lax - #2117
Conversation
…SameSite=Lax Without a web.xml, the embedded Tomcat context defaulted to COOKIE+URL session tracking, so a session id supplied only via ";jsessionid=" in the URL was honoured with no fresh Set-Cookie -- a session-fixation vector. Live-reproduced before this fix: a session established by cookie was resumed by replaying the id via the URL parameter with no Cookie header sent at all. Restricts session tracking to COOKIE only, and the session cookie now carries SameSite=Lax (previously unset). Secure is deliberately not forced: Boot opens a plain HTTP listener by default, and an operator terminating TLS in front of it sets Secure themselves, as with any other servlet container. Boot is part of 7.5.0 (unreleased, no production deployments yet), so this lands before anyone is exposed to it. Tests: StrutsBootTomcatTest gains testSessionCookieHasSameSiteLax and a URL-supplied-session-id-is-not-honoured test, with a cookie-based-session- still-works control. Full boot suite: 52/0 failures.
lukaszlenart
left a comment
There was a problem hiding this comment.
Thanks, the change is the right one and I verified it locally: both new tests fail against main's StrutsBootServer (the URL-supplied id resumed the session, counter went to 2; no SameSite) and the whole StrutsBootTomcatTest is 16/16 green on this branch. One more effect worth stating in the ticket: on main, the first response of a Boot app already rewrites the id into every link rendered by <s:url> / <s:a> (URL=/sessionProbe.action;jsessionid=…), because Tomcat's encodeURL appends it whenever the session did not arrive by cookie and URL tracking is effective. With this branch those links are clean. That is the more visible symptom and the reason cookie-only is the right default, independent of how the id gets replayed.
Changes requested:
THREAT_MODEL.md§5 lists the Boot container defaults the framework owns ("A deviation from these defaults is a Struts finding"). Please add two bullets there: session tracking is cookie-only, and the session cookie carriesSameSite=Lax. Without them the new defaults are not part of the contract.- The
Secureparagraph in the Javadoc is not true for Boot — see the inline comment. Please drop the sentence; astruts.boot.*setting for the cookie attributes can be a separate ticket. - Comments: keep them functional and short — what the code does, not the scenario it prevents. Inline comments mark the spots. Same for the test Javadoc and the
// GREEN:marker. - Naming: the two defaults are one decision; give them one method (inline).
- Commit headline: please use the repo form, e.g.
WW-5841 fix(boot): restrict session tracking to cookies and set SameSite=Lax.
The branch is 4 commits behind main; none touch boot/, so no rebase is needed for this review.
…nts, threat model bullets - fold the tracking-mode initializer and the SameSite cookie processor into one method, restrictSessionTracking(Context) - drop the Secure claim from the Javadoc and reduce the comments to what the code does - reduce the test Javadoc and drop the narrating comment lines - THREAT_MODEL.md section 5: add the cookie-only tracking and SameSite=Lax defaults to the Boot container defaults list
|
Addressed in bfc956e:
I'll update the ticket text to drop the |
Fixes WW-5841
Without a
web.xml, the embedded Tomcat contextStrutsBootServerstarts defaulted to the servlet-spec COOKIE+URL session tracking modes, so a session id supplied only via;jsessionid=in the URL (no cookie at all) was accepted and kept alive with no freshSet-Cookie. Live-reproduced before this fix: a session established by cookie was resumed by replaying the id via the URL parameter with no Cookie header sent at all.Restricts session tracking to COOKIE only, and the session cookie now carries
SameSite=Lax. Boot is part of 7.5.0, unreleased, so this lands before anyone is exposed to it.Verified:
StrutsBootTomcatTestgainstestSessionCookieHasSameSiteLaxand a URL-supplied-session-id test, with a cookie-based-session control. Fullboot: 52/0 failures.