Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of SameSite cookie attribute #165

Closed
wants to merge 6 commits into from
Closed

Implementation of SameSite cookie attribute #165

wants to merge 6 commits into from

Conversation

farnulfo
Copy link

Introduction

Hi folks, this pull request is an attemp to implement the SameSite cookie attribute according to https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-03 .

The main objective is to reduce the CSRF vulnerability.
Some articles :
https://web.dev/samesite-cookies-explained
https://www.owasp.org/index.php/SameSite

As this my first real pull request I'm listening to you :-)

Implementation:

This implementation is on two level:

  • In Cookie / CookieGenerator classes
    • Add variable and getter/setter for this attribute
    • Generate the Set-Cookie header with this attribute if needed
  • In Apache Tomcat Context and subclasses
    • Enable the use of SameSite attribute for the session cookie (JSESSIONID) which can’t be easily done

Implementation details:
Classes patched based on what was needed to add HttpOnly (thanks to Mark Thomas comment http://tomcat.10.x6.nabble.com/Support-SameSite-cookie-attribute-in-Tomcat-td5075308.html that helped to identify which classes to edit).

Design :
Name of the cookie attribute : SameSiteEnforcement (with getSameSite/setSameSite methods)
No boolean/method like isSameSite:
If not null, the “SameSite” attribute with only allow the following valid value : None, Lax, Strict
I don’t see the need for a isSameSite because we need to get the value which is not a boolean (not the case with httpOnly attribute).
Choice open for discussion. FYI Undertow has isSame and (get|set)SameSiteMode : undertow-io/undertow#499

To set the SameSiteEnforcement for session cookie, set it on the Context Container like httpOnly before it was true by default or before it was part of Java EE standard.

Not sure about ApplicationSessionCookieConfig.java modification : do the ApplicationSessionCookieConfig may be have a samesitesite ?

Next ?

If you’re ok with the idea, I can go further :
Add/Polish Javadocs
Polish context.xml documentation
Add tests
...

Thansk !

@farnulfo
Copy link
Author

I'm really sorry I didn't see the PR #162 .
I closed mine.

@farnulfo farnulfo closed this May 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant