Session Cookie = Lax breaks SAML auth in many cases with PHP 7.3 and above.

[kesselb]

This change breaks SAML auth in many cases with PHP 7.3 and above.

During a Nextcloud-SAML login, the user attempts to goto the nextcloud frontpage to login, nextcloud sets up the session cookies right away. Then redirects the user to login via SAML.

After the SAML login, there is a POST request with the SAML authorization back to nextcloud:
https:///index.php/apps/user_saml/saml/acs

With these cookies set to LAX, the POST doesn't contain the needed session cookies, and the auth fails entirely.

If we drop the PHP version to 7.2, the cookie doesn't get set to LAX, and the SAML response POST will contain the needed session cookie, and login via SAML is successful.

This phenomenon is documented here, https://php.watch/articles/PHP-Samesite-cookies :

When a cookie is marked samesite=Lax, that cookie will not be passed for any cross-domain requests unless it's a regular link that navigates user to the target site. Other requests methods (such as POST and PUT) and XHR requests will not contain this cookie.

I can confirm this is the source of the issue two different ways: drop the server php version to 7.2, issue gone; makes sense since this change code only sets the flag if php is 7.3 or above.

Also in chrome hitting nextcloud 19 with php 7.3 or 7.4, after the failed SAML login attempt from this issue, you can manually modify the session cookies in the F12 dev tools to remove the LAX flag from the two session cookies, then retrying login succeeds.

A workaround is setting up the SAML IdP server on the same subdomain as the nextcloud instance:

sso.mydomain.net
nextcloud.mydomain.net

In this scenario, the bug doesn't manifest, since they are both part of the same subdomain, LAX allows the cookie to be sent even in a POST.

Unfortunately changing the hosted FQDN of the Identity provider to match your nextcloud instance may not be an option, or feasible.

Perhaps an override for this cookie change can be put in place so SAML auth app users can continue to function as before?

Originally posted by @peacepenguin in #17075 (comment)

[kesselb]

cc @rullzer @peacepenguin

[peacepenguin]

In case anyone runs into this in production and downgrading to php 7.2 isn't an option, nor is changing FQDNs of the SAML server, here's a simple workaround to edit the running nextcloud php files that basically undo the security enhancement merge that led to this bug: #17075

If the session cookies "samesite" property is set to either "None" or not set at all, as is the case with php 7.2, the SAML login works fine. But the new default of "Lax" when using php 7.3 and above causes the browser to not send the session cookies in the POST after the login.

Note these changes will probly be overwritten on any updates to Nextcloud, and setting to "None" may introduce new security problems, so use caution until a real fix can found.

Fix it by setting these cookies from "Lax" to "None" (see #17075)

sudo -u www-data nano /var/www/nextcloud/lib/private/Session/CryptoWrapper.php

# line ~ 103: set to   'None' 
'samesite' => 'None'

sudo -u www-data nano /var/www/nextcloud/lib/private/Session/Internal.php 

#  line 222:  set to  'None' here too instead of 'Lax' :
[['cookie_samesite' => 'None']])

also the bug only occurs when the subdomain of the SAML server does NOT match the nextcloud server.

sso.domain123.com
nextcloud.domain123.com <- works fine with "Lax" cookies, the subdomain is common

sso.hostedsaml.com
nextcloud.domain123.com <- not working anymore with new "Lax" cookies

There are no errors when this bug happens, the user is just shown a page that says "null". Makes sense since there was no session information sent, so nextcloud throws it out as a rouge request and not an error.

[rullzer]

Mmmm so I'm wondering how others solve it. As Chrome now by default assumes everything is lax if nothing is set (including the php session cookie).

I would also swear I tried this and all worked fine. BUt maybe other caching magic was in effect.
Anyways to the debug machine!

[rullzer]

Ok. I think I have a nice work around. Let me push something here and to the user_saml app and then we can test.

[rullzer]

So I think it is fixed with

#21479 for server
and nextcloud/user_saml#430 for user_saml

At least then all is fine with my test setups.

[peacepenguin]

Yes I was surprised removing the LAX flag actually worked around the issue, as my understanding was chrome v80+ assumed LAX for all cookies. But doesn't appear to be the case still for some reason in chrome 83.

Thanks for checking this out and proposing a fix so quick, I'll try out the changes you've proposed. Thanks!

[peacepenguin]

Just tested your patches, they worked perfect over here too! And the session cookies remain set at LAX for the original benefit. Amazing work @rullzer!