Project

General

Profile

Actions

Feature #7814

closed

Consider supporting SameSite attribute on cookies

Added by Bruce Toll over 3 years ago. Updated 11 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Start date:
11/02/2020
Due date:
% Done:

100%

Estimated time:

Description

WApplication::setCookie() does not currently support setting the SameSite attribute. This results in a default interpretation by the browser. For Firefox versions 76 and higher, it also results in a warning on the developer console.

This warning was reported in #6593-8, and I encountered the warning independently while debugging some new code.

The warning can be produced with the hello.wt application. With Firefox 82.0 on Linux, the following messages appear on the console:

Some cookies are misusing the recommended “SameSite“ attribute
Cookie “jscookietest” will be soon rejected because it has the “SameSite” attribute set to “None” or an invalid value, without the “secure” attribute. To know more about the “SameSite“ attribute, read https://developer.mozilla.org/docs/Web/HTTP/Headers/Set-Cookie/SameSite test:167
Cookie “jscookietest” will be soon rejected because it has the “SameSite” attribute set to “None” or an invalid value, without the “secure” attribute. To know more about the “SameSite“ attribute, read https://developer.mozilla.org/docs/Web/HTTP/Headers/Set-Cookie/SameSite test:170
Cookie “WtTestCookie” will be soon rejected because it has the “SameSite” attribute set to “None” or an invalid value, without the “secure” attribute. To know more about the “SameSite“ attribute, read https://developer.mozilla.org/docs/Web/HTTP/Headers/Set-Cookie/SameSite test:173:59

With the Firefox network.cookie.sameSite.laxByDefault config value set to true (the expected future setting), the following messages appear on the console when testing the hello.wt application:

Some cookies are misusing the “SameSite“ attribute, so it won’t work as expected
Cookie “jscookietest” has “SameSite” policy set to “Lax” because it is missing a “SameSite” attribute, and “SameSite=Lax” is the default value for this attribute. test:167
Cookie “jscookietest” has “SameSite” policy set to “Lax” because it is missing a “SameSite” attribute, and “SameSite=Lax” is the default value for this attribute. test:170
Cookie “WtTestCookie” has “SameSite” policy set to “Lax” because it is missing a “SameSite” attribute, and “SameSite=Lax” is the default value for this attribute. test:173:59

Attached for your review is a patch that enhances setCookie with a sameSite parameter. It also tries to set reasonable SameSite values for cookies used by Wt. The patch has been lightly tested against github Wt 4.4.0-28-g6e0c1758, but would benefit from additional review and testing (particularly for cookies that set SameSite=Strict). There are additional notes in the patch file.

Additional references:

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite
https://hacks.mozilla.org/2020/08/changes-to-samesite-cookie-behavior/
https://bugzilla.mozilla.org/show_bug.cgi?id=1454781
https://www.chromium.org/updates/same-site/test-debug
https://www.chromium.org/updates/same-site/faq


Files

Actions #1

Updated by Bruce Toll over 3 years ago

I noticed some problems with the initial patch and plan to submit an update later this week.

Actions #2

Updated by Bruce Toll over 3 years ago

I've attached an updated patch (0001-Add-SameSite-support-to-setCookie_rev2.patch). It corrects the text of a LOG_WARN message. It also updates the callers of WebRenderer::setCookie to explicitly provide a CookieSameSite argument (and removes the default argument). This seems more consistent with the handling of the secure attribute.

Notes on testing:

  1. Most of my testing was done using the hangman program, modifying the wt_config.xml, e.g. with tracking=auto and reload-is-new-session=false.
  2. Testing was done almost exclusively with Firefox 82 under Linux. Cursory testing was done with Chromium 86 under Linux.
  3. I temporarily patched the AuthModel::setRememberMeCookie to request a CookieSameSite::None and verified that a warning was logged.
  4. I did not do any testing with https (secure mode).
Actions #3

Updated by Roel Standaert over 2 years ago

  • Target version set to future
Actions #4

Updated by Roel Standaert over 2 years ago

  • Target version changed from future to 4.7.0
Actions #5

Updated by Korneel Dumon over 2 years ago

  • Status changed from New to InProgress
  • Assignee set to Korneel Dumon
Actions #6

Updated by Korneel Dumon over 2 years ago

  • Status changed from InProgress to Review
  • Assignee deleted (Korneel Dumon)
Actions #7

Updated by Roel Standaert about 2 years ago

  • Target version changed from 4.7.0 to 4.8.0

Moving this to 4.8.0 since I didn't have the opportunity to properly review these changes yet.

Actions #8

Updated by Roel Standaert almost 2 years ago

  • Target version changed from 4.8.0 to 4.9.0
Actions #9

Updated by Roel Standaert over 1 year ago

  • Target version changed from 4.9.0 to 4.10.0
Actions #10

Updated by Roel Standaert about 1 year ago

  • Status changed from Review to Implemented @Emweb
  • Assignee set to Korneel Dumon
  • % Done changed from 0 to 100
Actions #11

Updated by Roel Standaert about 1 year ago

  • Status changed from Implemented @Emweb to Resolved
Actions #12

Updated by Matthias Van Ceulebroeck 11 months ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF