From 0fdff0c79b51d1621150b8b353ca29d009d4d63b Mon Sep 17 00:00:00 2001
From: Kristoffer Dalby <kristoffer@tailscale.com>
Date: Sat, 6 Jun 2026 23:34:50 +0000
Subject: [PATCH] oidc: set SameSite=Lax on state/nonce CSRF cookies

setCSRFCookie set no SameSite attribute despite a nolint comment claiming it did, so the OIDC state/nonce cookies relied on the browser default. Set Lax explicitly (Strict would drop the cookie on the cross-site IdP->callback navigation and break login), hardening browsers that do not default to Lax against OIDC login CSRF.
---
 hscontrol/oidc.go      |  8 +++++++-
 hscontrol/oidc_test.go | 23 +++++++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/hscontrol/oidc.go b/hscontrol/oidc.go
index 140e8683..7f1e3fae 100644
--- a/hscontrol/oidc.go
+++ b/hscontrol/oidc.go
@@ -935,7 +935,7 @@ func setCSRFCookie(w http.ResponseWriter, r *http.Request, name string) (string,
 		return val, err
 	}
 
-	//nolint:gosec // G124: Secure set conditionally via r.TLS; HttpOnly + SameSite already set
+	//nolint:gosec // G124: Secure set conditionally via r.TLS; HttpOnly + SameSite set below
 	c := &http.Cookie{
 		Path:     "/oidc/callback",
 		Name:     getCookieName(name, val),
@@ -943,6 +943,12 @@ func setCSRFCookie(w http.ResponseWriter, r *http.Request, name string) (string,
 		MaxAge:   int(time.Hour.Seconds()),
 		Secure:   r.TLS != nil,
 		HttpOnly: true,
+		// Lax, not Strict: the OIDC callback is a cross-site top-level GET
+		// redirect from the IdP that must still carry this cookie. Strict
+		// would drop it and break login. Setting it explicitly also stops
+		// pre-Lax-default browsers from sending it on other cross-site
+		// requests.
+		SameSite: http.SameSiteLaxMode,
 	}
 	http.SetCookie(w, c)
 
diff --git a/hscontrol/oidc_test.go b/hscontrol/oidc_test.go
index c6ce382c..a77ba1bc 100644
--- a/hscontrol/oidc_test.go
+++ b/hscontrol/oidc_test.go
@@ -1,9 +1,13 @@
 package hscontrol
 
 import (
+	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/juanfont/headscale/hscontrol/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestDoOIDCAuthorization(t *testing.T) {
@@ -171,3 +175,22 @@ func TestDoOIDCAuthorization(t *testing.T) {
 		})
 	}
 }
+
+// TestSetCSRFCookieSameSite verifies the OIDC state/nonce CSRF cookies carry an
+// explicit SameSite=Lax attribute. Lax (not Strict) is required because the
+// OIDC callback is a cross-site top-level GET navigation from the IdP that must
+// still carry the cookie — Strict would drop it and break login. The cookie
+// previously set no SameSite (despite a comment claiming it did), leaving
+// browsers that do not default to Lax sending it on cross-site requests.
+func TestSetCSRFCookieSameSite(t *testing.T) {
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest(http.MethodGet, "/auth/abcdef0123456789", nil)
+
+	_, err := setCSRFCookie(w, r, "state")
+	require.NoError(t, err)
+
+	cookies := w.Result().Cookies()
+	require.Len(t, cookies, 1)
+	assert.Equal(t, http.SameSiteLaxMode, cookies[0].SameSite,
+		"OIDC CSRF cookie must explicitly set SameSite=Lax")
+}