Cookie code refactor, Use CGI::Cookie and support samesite

[taniwallach]

Refactor the Cookie code:

1. Use CGI::Cookie instead of Apache2:Cookie, as the new code needs support for the samesite attribute.
2. Added CGI::Cookie to bin/check_modules.pl and Dockerfile.
   • Note: The support for samesite dates to June 2019 in CGI::Cookie 4.45.
3. Remove obsolete, commented out, code using cookies from lib/WeBWorK/ContentGenerator/Logout.pm.
4. Drop the constant COOKIE_LIFESPAN and instead allow setting cookie lifespan using site / course environment configuration variables.
   • $CookieLifeTime - for when cookie based session management is in use - default to 6 hours.
   • $CookieLifeTime2 - for when cookie based session management is not in use, defaults to 30 days.
5. Allow setting value of cookies samesite and secure attribute using site / course environment configuration variable:
   • $CookieSameSite
   • $CookieSecure

---

This is motivated by the impending changes to Firefox and Chrome which would was expected to reject the webwork cookies in the future, as no samesite atttribute is set, so it would be treated was mistakenly expected to be treated as none without the secure attribute set. See: #1072

Update: As explained below, the change will be to treat cookies without an explicit samesite setting as Lax so they should not break for "same site" contexts. Thus, regular webwork sites are not likely to have problems in the near future. However, the changes will allow explicit use of both cookie settings, if and when they may be needed.

---

Depending on when Firefox and Chrome begin to enforce the new policy, back-porting this as a hotfix to WW 2.15 may be needed.

[taniwallach]

Note: The code depends on having CGI::Cookie version 4.45 (June 2019) or later. Many servers will need to update CGI.pm to get a new enough version.

See: https://metacpan.org/changes/distribution/CGI

[taniwallach]

Sample lines added to course.conf:

$CookieSameSite = "None";
$CookieSecure = 1;
$CookieLifeTime = "+7d";    # 7 day Cookie lifetime - for when cookie session management is in use
$CookieLifeTime2 = "+14d"; # 7 day Cookie lifetime - for when cookie session management is in use

The time settings can use the syntax of CGI.pm - search for 30 seconds from now at https://metacpan.org/pod/CGI to see the various options.

[drgrice1]

This is needed. I am not sure how long it will be until Firefox and Chrome enforce this policy, but it is coming.

I think that defaults for $CookeSameSite, $CookieSecure, $CookieLifeTime, and $CookieLifeTime2 should be added to the appropriate configuration file. Probably site.conf.dist. If the defaults are only implemented as fall back values in the code, then they aren't very visible. If the default values are listed (and documented) in the appropriate configuration file, then those configuring servers can find them.

[taniwallach]

This is needed. I am not sure how long it will be until Firefox and Chrome enforce this policy, but it is coming.

I realized that the problem is less urgent than I thought yesterday. It seems that the intention is to change to default to Lax (instead of None) which means the problems would mainly occur in embedded pages, and not for users who only use their webwork server directly.

I think that defaults for $CookeSameSite, $CookieSecure, $CookieLifeTime, and $CookieLifeTime2 should be added to the appropriate configuration file. Probably site.conf.dist. If the defaults are only implemented as fall back values in the code, then they aren't very visible. If the default values are listed (and documented) in the appropriate configuration file, then those configuring servers can find them.

Since defaults.conf has the setting $session_management_via = "session_cookie"; and instructs that an override should be made in localOverrides.conf I think the best place is in localOverrides.conf.dist For now I put sample lines there.

[taniwallach]

I think we want to get the secure cookie handling in for WW 2.16.

[drgrice1]

I agree.

[drgrice1]

This generally looks good, and works as it should.

There is one thing that might be nice, and that is to tie things in tighter between the $sessionKeyTimout and the $CookieLifeTime settings. Probably there should only be one fo these for when managing the session via cookies. It certainly should be the case that the $CookieLifeTime should be the greater of the two.

[taniwallach]

@drgrice1 wrote:

There is one thing that might be nice, and that is to tie things in tighter between the $sessionKeyTimout and the $CookieLifeTime settings. Probably there should only be one of these for when managing the session via cookies. It certainly should be the case that the $CookieLifeTime should be the greater of the two.

Implemented partially - see #1149 (comment) which probably should have been here instead.

[drgrice1]

This looks good.

Could someone else please take a look.

This is an important pull request that will be needed soon.

[taniwallach]

This looks good.

Could someone else please take a look.

This is an important pull request that will be needed soon.

I added requests that @pstaabp and @drdrew42 review, as I think @mgage is too busy, and @Alex-Jordan is planning to work on testing the LTI patches, so I'ld like to free him from needing to test this one.

[drgrice1]

I am bumping the priority on this (assuming priority 1 is a higher priority). It won't be long until browsers start enforcing this beyond just a console warning. There is a good chance this will happen before the next release of webwork, so lets get this in now.

[drdrew42]

Can I get an elaboration on how I can tell whether or not this PR is working? I don't get any console logs about cookies with or without this PR...

[drgrice1]

One way is to use Firefox. Firefox shows warnings without this pull request, but not with.

So I decided to give the information on this a closer read. I think that Firefox's console warning is a bit misleading, and this may not be as serious as I thought.

As I understand it Google Chrome is already using the new standards by default, but Firefox is not yet. You can enable the new standards in Firefox to test things.

The new standards are that if a cookie is sent with SameSite not set then the default value that will be used is Lax. Furthermore, if SameSite is set to None, then Secure must be true or the cookie will be rejected.

So basically, at this point the webwork cookies are being used with SameSite set to Lax on Google Chrome. Those cookies aren't rejected, but they also aren't secure. Firefox is still using the old standards, and sets SameSite to None. Those cookies are also not secure, and they aren't rejected because again Firefox is still using the old standards.

Firefox's is using the current policy to set the default of SameSite to None, but is warning about that not having Secure set to true as per the future policy. This warning does not take into account that the future policy also says that the default for SameSite will be Lax, for which Secure does not need to be true.

So the upshot of this is that things will still continue to work for webwork with or without this pull request. There is just the issue of whether or not we want the cookies to be secure. So I am not sure how serious this is.

I just looked back at issue #1072, and @taniwallach said some of this in his last post there.

[drgrice1]

Back to priority 2. I see @taniwallach (or someone) classified the priorities. I don't think there really was any doubt though.

[pstaabp]

I think I need a bit more to test this. It appears that this may only be a problem using LTI. Is this true. I don't have that setup currently.

Is there a setting in FF or chrome to turn on the site-same cookie for the standard WW setup to check this?

[drgrice1]

This affects all cookie based authentication. Not just LTI. Currently cookie based authentication is the default for webwork.

[pstaabp]

Not sure this is full tested because I can't see the effect this has on Chrome, but overall, this appears to future-proof us.

[pstaabp]

Overall, everything seems to work, I did verify what @drgrice1 sees on Firefox.

I've tried setting the flags for SameSite in Chrome, but am not getting any warnings on the develop branch, which is what I expect. According to https://www.chromium.org/updates/same-site/faq there is site checking for the right flags and the current chrome (89) seems to have all the correct flags set. Not sure what is going on.

[taniwallach]

I've tried setting the flags for SameSite in Chrome, but am not getting any warnings on the develop branch,

From what I understand from https://www.chromium.org/updates/same-site/faq - warning will be issued by Chrome only for cross-site requests: "Chrome is displaying warnings in the Console in DevTools which highlight each cross-site request where cookies would be affected" (emphasis is mine). Firefox on the other hand seems to issue warnings far more broadly, even when they probably would not really have problems.

[drgrice1]

We have two reviews on this. I think it is ready to be merged in. We will need to make sure that the installation documentation notes the a new Ubuntu package is needed (if on Ubuntu 20.04) or a new perl module needs to be installed via cpan or cpanm (if on Ubuntu 18.04).

[drgrice1]

I missed this before. This gives an error as CookieLifeTime is not numeric.

[drgrice1]

   * Note: The support for samesite dates to June 2019 in CGI::Cookie 4.45.

Looking closely at this, it seems that CGI::Cookie had the samesite setting before version 4.45. See https://metacpan.org/pod/release/LEEJO/CGI-4.38/lib/CGI/Cookie.pm. Version 4.38 had the samesite setting but only allowed values of strict or lax, and not none. I also tested this on my server running Ubuntu 18.04 and the Ubuntu libcgi-pm-perl package (which contains CGI::Cookie version 4.38), and this worked fine. I was using the default samesite value of strict.

I installed the cpan version 4.45 to be safe, but there may be no need for concern if the Ubuntu package on 18.04 is used.

[taniwallach]

   * Note: The support for samesite dates to June 2019 in CGI::Cookie 4.45.

Looking closely at this, it seems that CGI::Cookie had the samesite setting before version 4.45. See https://metacpan.org/pod/release/LEEJO/CGI-4.38/lib/CGI/Cookie.pm. Version 4.38 had the samesite setting but only allowed values of strict or lax, and not none. I also tested this on my server running Ubuntu 18.04 and the Ubuntu libcgi-pm-perl package (which contains CGI::Cookie version 4.38), and this worked fine. I was using the default samesite value of strict.

I installed the cpan version 4.45 to be safe, but there may be no need for concern if the Ubuntu package on 18.04 is used.

Good point, but it depends on whether a site's WW content will be embedded in a cross-origin setting.

Long response below.

Bottom line - I think it suffices to explain in the release notes under what circumstances a server is likely to need to be able to enable SameSite=None and inform administrators of the need for a more modern version of CGI::Cookie when that is needed.

---

I can trigger (with some work) a test where browser same-site cookie enforcement seems to require the SameSite=None setting - see below. (Lax by default triggers "bad" behavior for an end user.)

Regular" webwork servers use only first-party cookies, so should at the most trigger warnings (mainly in FireFox) at present, if they don't make an explicit setting.

That means that the version in Ubuntu 18.04 suffices to set an explicit value of lax (the new default or even strict), and that should suffice to bypass the warnings if they bother someone. For many use cases - that is more than enough.

I suspect that the sites which are likely to have a real possible need of this new code are precisely those which embed WeBWorK content in some external setting where the issue of a cross-origin request can be an issue. It seems that in such a setting - the WeBWorK cookie might sometime be needed in a cross-origin context, so would need to have the SameSite=None setting .

---

All the testing below was done without any special settings for SameSite on the WeBWorK servers, to check behavior with the current WW default settings.

I did the testing using Chrome Version 89.0.4389.90 (Official Build) (64-bit) on Linux and with a special Chrome flag enabled:

• chrome://flags/#same-site-by-default-cookies
• SameSite by default cookies
• Treat cookies that don't specify a SameSite attribute as if they were SameSite=Lax. Sites must specify SameSite=None in order to enable third-party usage. – Mac, Windows, Linux, Chrome OS, Android

The flag I enabled is being dropped in Chrome 91, as the new behavior will be forced in that version:

• https://www.chromium.org/updates/same-site

That should be forcing my currently installed browser to enforce what will be enforced by default in version 91 (release scheduled for late May 2021 - https://chromiumdash.appspot.com/schedule).

When I disable that flag, the "embedded" pages which fail to work properly do work.

I also do not hit actual problems yet with FireFox version 78.8.0esr (64-bit).

Other useful references:

• https://www.chromium.org/updates/same-site/faq
• https://www.chromium.org/updates/same-site
• https://samesite-sandbox.glitch.me/

---

The problems occurred (on Chrome with the flag mentioned above enables) when I tested a Moodle LTI link from moodle.technion.ac.il to a course on webwork.iucc.ac.il where the target page is embedded (in an iframe), the initial connection works, but navigation using the menu kicks me out to the login page.

It seems that the cookie sent at the time of the initial connection were not stored (do not appear where cookies would be shown in the developer tools) so when a navigation request is made it is sent without the cookie (none is listed in "Request Headers").

However, a copy of the same LTI link which was set to open in a new windows - does store a cookie, and internal course navigation does work, as a new windows creates a "first party" context for the original cookie sent by the WW server.

---

From what I see so far, in many common situations, embedding a given institutions locally hosted WW content in a locally hosted Moodle page via "LTI as embedded" works fine. This is apparently related to how browsers define "cross-domain" - see:

• https://web.dev/same-site-same-origin/
• https://publicsuffix.org/list/public_suffix_list.dat

The fact that SameSite looks at eTLD+1 apparently means in my "local" test case that moodle.technion.ac.il and webwork.technion.ac.il are both in technion.ac.il domain for the purposes of SameSite.