Add SameSite support for cookies

[DavidLiedke]

Feature allready enabled by default in Chrome since version 63.
Mozilla enable this feature with Firefox 60.

https://www.bleepingcomputer.com/news/security/firefox-improves-csrf-protection-with-support-for-same-site-cookies/

[netniV]

I don't think that should be too difficult a request. I think we probably need to move to having a common routines so that things like headers can be output automatically. Are there any others you think could/should be included that are not currently?

Cacti does not rely on any third party sites so I personally think it could be set to strict at this point in time with the option for an admin to switch it to lax or off. The latter more for integration purposes if there is ever a specific need.

[DavidLiedke]

Yes i think the strict mode is the right one with a option to switch it.

What do you think about secure cookies?
https://www.sjoerdlangkemper.nl/2017/02/09/cookie-prefixes/
https://www.sjoerdlangkemper.nl/2017/10/25/strict-secure-cookies/

[netniV]

That should be optionally as often people run cacti inhouse without using SSL, so adding the host or secure prefixes wouldn't work as http isn't allowed to write those.

[netniV]

Also, having them enabled for the secure version after using the insecure version will cause any insecure cookie to be ignored due to the new optional prefix being used, having the effect of logging people out.

[DavidLiedke]

The secure cookie option can be combined with the "Force Connections over HTTPS" option.

[netniV]

I guess it could since that will force a redirect to the HTTPS option. Again, it would mean changing that option so it produces a warning when enabled because if logins have been using cookies for a long time, people may not remember what their password is, thus end up locked out :)

[TheWitness]

@netniV wasn't this fixed already by @cigamit?

[netniV]

Err, good question, let me check...

[netniV]

Unfortunately, whilst some code was put in, I don't believe that we are being consistent. For example, lib/auth.php has the following code within set_auth_cookie():

if (!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] != 'off') {
        setcookie('cacti_remembers', $user['username'] . ',' . $nssecret, time()+(86400*30), $config['url_path'], NULL, true, true);
} else {
        setcookie('cacti_remembers', $user['username'] . ',' . $nssecret, time()+(86400*30), $config['url_path']);
}

However, there are other uses of setcookie() that do not make the same check. So I think we need to move the above checks into a generic set_cookie() function and ensure we are consistent.

./auth_changepassword.php:86:   setcookie($cacti_session_name, null, -1, $config['url_path']);
./auth_login.php:486:   setcookie(session_name(), '', time() - 3600, $config['url_path']);
./lib/auth.php:44:                                      setcookie('cacti_remembers', '', time() - 3600, $config['url_path']);
./lib/auth.php:75:                      setcookie('cacti_remembers', $user['username'] . ',' . $nssecret, time()+(86400*30), $config['url_path'], NULL, true, true);
./lib/auth.php:77:                      setcookie('cacti_remembers', $user['username'] . ',' . $nssecret, time()+(86400*30), $config['url_path']);
./include/vendor/csrf/csrf-magic.php:141:               setcookie($GLOBALS['csrf']['cookie'], $val, time() + 3600, $GLOBALS['csrf']['url_path']);

[TheWitness]

Makes sense.

[TheWitness]

So, I did a review of this and have found that the cacti_remembers is more for session history control, I am making a change to global.php right now, that on php7.3 and above will implement this through the session_start() call, and for prior versions, through the Cookie header.

[TheWitness]

Confirmed with @netniV that PHP7.x does not like this setup. Making some additional changes.

[ddb4github]

CHANGELOG still have SameSite line

[TheWitness]

@ddb4github, Same site is done now. There was another change not pinned to this ticket.

[TheWitness]

Here is the commit:

f8b9483#diff-347d8bac8d944a3900b3284f46de3045

[ddb4github]

Here is the commit:

f8b9483#diff-347d8bac8d944a3900b3284f46de3045

Known now. Just look "Reject" in commit log and guess you rollback all previous change.