Skip to content

Commit

Permalink
ws: Restrict our cookie to the login host only
Browse files Browse the repository at this point in the history
Mark our cookie as `SameSite: Strict` [1]. The current `None` default
will soon be moved to `Lax` by Firefox and Chromium, and recent versions
started to throw a warning about it.

[1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite

https://bugzilla.redhat.com/show_bug.cgi?id=1891944
  • Loading branch information
martinpitt committed Jan 11, 2021
1 parent 54d1c62 commit 46f6839
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 3 deletions.
4 changes: 2 additions & 2 deletions src/ws/cockpitauth.c
Expand Up @@ -1598,7 +1598,7 @@ cockpit_auth_login_finish (CockpitAuth *self,
force_secure = connection ? !G_IS_SOCKET_CONNECTION (connection) : TRUE;
cookie_name = application_cookie_name (cockpit_creds_get_application (creds));
cookie_b64 = g_base64_encode ((guint8 *)session->cookie, strlen (session->cookie));
header = g_strdup_printf ("%s=%s; Path=/; %s HttpOnly",
header = g_strdup_printf ("%s=%s; Path=/; SameSite=Strict;%s HttpOnly",
cookie_name, cookie_b64,
force_secure ? " Secure;" : "");
g_free (cookie_b64);
Expand Down Expand Up @@ -1734,7 +1734,7 @@ cockpit_auth_empty_cookie_value (const gchar *path, gboolean secure)

/* this is completely security irrelevant, but security scanners complain
* about the lack of Secure (rhbz#1677767) */
gchar *cookie_line = g_strdup_printf ("%s=deleted; PATH=/;%s HttpOnly",
gchar *cookie_line = g_strdup_printf ("%s=deleted; PATH=/; SameSite=strict;%s HttpOnly",
cookie,
secure ? " Secure;" : "");

Expand Down
2 changes: 1 addition & 1 deletion src/ws/test-handlers.c
Expand Up @@ -523,7 +523,7 @@ static const DefaultFixture fixture_shell_path_login = {
.org_path = "/path/system/host",
.auth = NULL,
.expect = "HTTP/1.1 200*"
"Set-Cookie: cockpit=deleted; PATH=/; HttpOnly\r*"
"Set-Cookie: cockpit=deleted; PATH=/; SameSite=strict; HttpOnly\r*"
"<html>*"
"<base href=\"/path/\">*"
"login-button*"
Expand Down
3 changes: 3 additions & 0 deletions test/verify/check-connection
Expand Up @@ -103,6 +103,7 @@ class TestConnection(MachineCase):
# cookie should not be marked as secure, it's not https
cookie = b.cookie("cockpit")
self.assertTrue(cookie["httpOnly"])
self.assertEqual(cookie["sameSite"], "Strict")
self.assertFalse(cookie["secure"])

# take cockpit-ws down on the server page
Expand Down Expand Up @@ -421,12 +422,14 @@ class TestConnection(MachineCase):
# cookie should be marked as secure
self.assertTrue(cookie["httpOnly"])
self.assertTrue(cookie["secure"])
self.assertEqual(cookie["sameSite"], "Strict")
# same after logout
b.logout()
cookie = b.cookie("cockpit")
self.assertEqual(cookie["value"], "deleted")
self.assertTrue(cookie["httpOnly"])
self.assertTrue(cookie["secure"])
self.assertEqual(cookie["sameSite"], "Strict")

# http on localhost should not redirect to https
self.assertIn("HTTP/1.1 200 OK", m.execute("curl --head http://127.0.0.1:9090"))
Expand Down

0 comments on commit 46f6839

Please sign in to comment.