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

Check if we need to add the SameSite=None setting to cookies, if so probably needs to be a hotfix for versions in production use, and will require a recent CGI.pm #1072

Closed
taniwallach opened this issue Jan 31, 2020 · 2 comments

Comments

@taniwallach
Copy link
Member

taniwallach commented Jan 31, 2020

Background: Google Chrome 80 (to be released soon) will be requiring the use of SameSite=None for cross-site cookies.

Tim Payer ( http://webwork.maa.org/moodle/mod/forum/discuss.php?d=4719 ) raised the issue of whether this scheduled change in Chrome will cause problems with using WW over LTI.

Preliminary analysis:

  1. I suspect the problem will not occur when LTI is used to open WeBWorK in a new window.
  2. It seems to me that when WeBWorK is used via LTI or the html2xml and embedded in an iFrame, the WeBWorK cookie will treated as a cross-site cookie, and will probably stop working with the upcoming version of Chrome.

WeBWorK's cookies - a quick look + initial ideas:

WeBWorK seems to set the cookies in

  • lib/WeBWorK/Authen.pm
  • lib/WeBWorK/ContentGenerator/Logout.pm
    using a call to $r->headers_out->set("Set-Cookie" => $cookie->as_string) where the $cookie is a WeBWorK::Cookie object which (under mod_perl 2) is essentially Apache2::Cookie.

Apache2::Cookie does not seem to be maintained, and does not seem to support the SameSite attribute.

However, it seems that we can transition to using CGI::Cookie ( https://metacpan.org/pod/CGI::Cookie ) which does support the SameSite attribute.

CGI::Cookie

  • can take an mod_perl request object as an argument to the bake() method (and will then call $r->headers_out->add('Set-Cookie' => $self->as_string);
  • or we can call the as_string() method to create the string for the cookie,
  • however, it seems from https://metacpan.org/pod/CGI::Cookie that "Mod_perl users can set cookies using the request object's header_out() method' using just $r->headers_out->set('Set-Cookie' => $c); where $c is the CGI::Cookie cookie.

Given these options - making the transition should be pretty easy, but...

CGI.pm only added support for setting SameSite=None in version 4.45 (released in June 2019, and probably newer than the version most systems have installed). Thus, if we need to set SameSite=None then WW systems which need this patch will probably need to upgrade the installed version of CGI.pm.

Also, setting SameSite=None requires setting the Secure flag also.

  • As a result, this will work only over https connection, so is a problem for any sites still using plain http for WeBWorK pages after authentication.
  • Current WeBWorK cookies do not enable the Secure flag, probably exactly in order to allow WW to work over plain http.

As a result - it seems that any change to add use of ``SameSite=None` should be regulated by control settings from the main configuration files, and possible overridden by course level configuration files.

References:

@taniwallach
Copy link
Member Author

taniwallach commented Oct 25, 2020

The WeBWorK session cookie seems likely to be rejected by Firefox in the near future, so I worked on a PR to address this. For now it is targeted to the develop branch.

See: #1149


In FireFox's developer tools, I see the message:

Cookie “WeBWorKCourseAuthen.C2_devel” 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

Update That warning is less critical than I originally understood, as the policy change will change the default of samesite to Lax, so the current WW code would probably create a cookie which will work so long as activity of the tab/window remains on the webwork server's URL. Cookies for embedded content are likely to fail (ex. LTI in an iFrame) until the improved WW cookie code from the PR is put into use (possibly backported to older versions of WW).

@drgrice1
Copy link
Sponsor Member

This has been fixed for a while now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants