panel: __Host- session cookie and duplicate-cookie detection

Phase 14.B. The cookie already satisfied everything the __Host- prefix
requires — Secure, Path=/, no Domain — but as a promise the server makes, not
one the browser enforces. With the prefix the browser refuses to store a
same-named cookie carrying a Domain attribute, which is the second lever the
same-site neighbour from 14.A has: set selfpost_session for the parent domain,
and the browser sends two cookies of that name, oldest first. r.Cookie
returned that first one, so the admin logged in successfully and landed back
on the login form, for as long as the planted cookie lived.

The name has to stay conditional: __Host- is only valid on a Secure cookie, so
with PANEL_COOKIE_SECURE=false the browser would discard the Set-Cookie and
the dev instance would fail to log in with no visible reason. Hence the test
on that branch specifically, not just the production one.

requireAuth now reads r.Cookies() and refuses a request carrying more than one
cookie of the name, with a log line naming the cause. That is the only place
the overwrite becomes visible at all, and unlike the prefix it also works in
the dev shape. Sign-out clears both names, so the upgrade does not leave the
old cookie behind; it does sign the administrator out once, which costs
nothing given sessions live in memory and die on restart anyway.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

internal/web/handlers_account.go

@@ -142,12 +142,12 @@ func (s *Server) submitAccount(w http.ResponseWriter, r *http.Request) {
 	// Keep this session usable under the new name, and — when the password
 	// changed — drop every other session so a cookie captured under the old
 	// password stops working.
-	if c, err := r.Cookie(sessionCookie); err == nil {
+	if token, ok := s.sessionToken(r); ok {
 		if renaming {
-			s.sessions.Rename(c.Value, username)
+			s.sessions.Rename(token, username)
 		}
 		if repassword {
-			s.sessions.DestroyOthers(c.Value)
+			s.sessions.DestroyOthers(token)
 		}
 	}
 

internal/web/handlers_auth.go

@@ -9,8 +9,81 @@ import (
 	"golang.org/x/crypto/bcrypt"
 )
 
-// sessionCookie is the name of the panel session cookie.
-const sessionCookie = "selfpost_session"
+// The panel session cookie's two possible names. In the production shape —
+// TLS in front, so CookieSecure — it carries the __Host- prefix, which turns
+// what the cookie's attributes merely promise into something the browser
+// enforces: Secure, Path=/ and, the point of the exercise, no Domain
+// attribute, so no other host may set a cookie by this name (phase 14.B).
+// The prefix is only valid on a Secure cookie, so a development instance on
+// plain HTTP has to keep the bare name: with the prefix the browser would
+// discard the Set-Cookie outright and logging in would silently never stick.
+const (
+	sessionCookieBase     = "selfpost_session"
+	sessionCookiePrefixed = "__Host-" + sessionCookieBase
+)
+
+// sessionCookie is the session cookie's name for this deployment.
+func (s *Server) sessionCookie() string {
+	if s.cfg.CookieSecure {
+		return sessionCookiePrefixed
+	}
+	return sessionCookieBase
+}
+
+// sessionToken returns the session token the request carries, if exactly one
+// cookie of that name is present.
+//
+// It walks r.Cookies() rather than calling r.Cookie, which silently returns
+// the first match. Two cookies with the same name mean somebody other than
+// this panel set one of them — a host on the same registrable domain can,
+// with Domain=example.com, and the browser will then send both — and RFC 6265
+// makes the older one come first, so "the first match" is precisely the
+// attacker's. The value cannot be forged into a valid session, so the effect
+// is denial of service, not compromise; refusing the request and saying so in
+// the log is what makes it diagnosable instead of an endless login loop. The
+// __Host- prefix prevents this outright, but only where it applies — this
+// check also covers the plain-HTTP development shape (phase 14.B).
+func (s *Server) sessionToken(r *http.Request) (string, bool) {
+	name := s.sessionCookie()
+	var token string
+	var n int
+	for _, c := range r.Cookies() {
+		if c.Name == name {
+			n++
+			token = c.Value
+		}
+	}
+	switch n {
+	case 0:
+		return "", false
+	case 1:
+		return token, true
+	default:
+		logf("panel: %s %s carries %d cookies named %q — treating the request as signed out; "+
+			"another host on this domain is overwriting the session cookie, clear the cookies for the parent domain",
+			r.Method, r.URL.Path, n, name)
+		return "", false
+	}
+}
+
+// clearSessionCookies expires the session cookie under both names, so an
+// upgrade that switches to the __Host- prefix does not leave the old cookie
+// sitting in the browser until it is closed.
+func (s *Server) clearSessionCookies(w http.ResponseWriter) {
+	for _, name := range []string{sessionCookieBase, sessionCookiePrefixed} {
+		http.SetCookie(w, &http.Cookie{
+			Name:     name,
+			Value:    "",
+			Path:     "/",
+			MaxAge:   -1,
+			HttpOnly: true,
+			// The prefixed name is only accepted at all when Secure is set,
+			// including on the expiring copy.
+			Secure:   s.cfg.CookieSecure || name == sessionCookiePrefixed,
+			SameSite: http.SameSiteLaxMode,
+		})
+	}
+}
 
 // handleLogin serves the login form (GET) and authenticates (POST). Until an
 // administrator exists there is nobody to log in, so it points at setup.
@@ -80,7 +153,7 @@ func (s *Server) submitLogin(w http.ResponseWriter, r *http.Request) {
 
 	token := s.sessions.Create(admin.Username)
 	http.SetCookie(w, &http.Cookie{
-		Name:     sessionCookie,
+		Name:     s.sessionCookie(),
 		Value:    token,
 		Path:     "/",
 		HttpOnly: true,
@@ -97,17 +170,15 @@ func (s *Server) handleLogout(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
 		return
 	}
-	if c, err := r.Cookie(sessionCookie); err == nil {
+	// Destroy every token presented under the session cookie's name: if a
+	// shadowing duplicate is present (see sessionToken) one of them is the
+	// real session, and a value that names no session is simply not found.
+	name := s.sessionCookie()
+	for _, c := range r.Cookies() {
+		if c.Name == name {
 			s.sessions.Destroy(c.Value)
 		}
-	http.SetCookie(w, &http.Cookie{
-		Name:     sessionCookie,
-		Value:    "",
-		Path:     "/",
-		MaxAge:   -1,
-		HttpOnly: true,
-		Secure:   s.cfg.CookieSecure,
-		SameSite: http.SameSiteLaxMode,
-	})
+	}
+	s.clearSessionCookies(w)
 	http.Redirect(w, r, "/login", http.StatusSeeOther)
 }

internal/web/handlers_auth_test.go

@@ -0,0 +1,112 @@
+package web
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+// The __Host- prefix is only valid on a Secure cookie: getting this condition
+// backwards would make the development instance fail to log in at all, and
+// silently — the browser discards the Set-Cookie and the panel just shows the
+// login form again.
+func TestSessionCookieNameFollowsCookieSecure(t *testing.T) {
+	secure := &Server{cfg: Config{CookieSecure: true}}
+	if got := secure.sessionCookie(); got != "__Host-selfpost_session" {
+		t.Errorf("with TLS the cookie is named %q, want the __Host- prefixed name", got)
+	}
+	plain := &Server{cfg: Config{CookieSecure: false}}
+	if got := plain.sessionCookie(); got != "selfpost_session" {
+		t.Errorf("without TLS the cookie is named %q, want the bare name", got)
+	}
+}
+
+// A neighbouring host on the same registrable domain can set a cookie by the
+// same name; the browser then sends both, oldest first. Picking one at random
+// would leave the administrator in a login loop with no explanation, so the
+// request counts as signed out instead.
+func TestSessionTokenRejectsDuplicates(t *testing.T) {
+	s := &Server{cfg: Config{CookieSecure: false}}
+	r := httptest.NewRequest(http.MethodGet, "http://panel.example.com/domains", nil)
+	r.AddCookie(&http.Cookie{Name: "selfpost_session", Value: "planted-by-a-neighbour"})
+	r.AddCookie(&http.Cookie{Name: "selfpost_session", Value: "the-real-session"})
+
+	if token, ok := s.sessionToken(r); ok {
+		t.Fatalf("duplicate cookies accepted, token = %q", token)
+	}
+}
+
+func TestSessionTokenReadsOneCookie(t *testing.T) {
+	s := &Server{cfg: Config{CookieSecure: true}}
+	r := httptest.NewRequest(http.MethodGet, "http://panel.example.com/domains", nil)
+	r.AddCookie(&http.Cookie{Name: "__Host-selfpost_session", Value: "the-real-session"})
+
+	token, ok := s.sessionToken(r)
+	if !ok || token != "the-real-session" {
+		t.Fatalf("sessionToken = %q, %t; want the cookie's value", token, ok)
+	}
+}
+
+// A cookie under the other deployment's name is not this deployment's session:
+// after an upgrade the pre-14 cookie must not be honoured as if it were the
+// prefixed one.
+func TestSessionTokenIgnoresTheOtherName(t *testing.T) {
+	s := &Server{cfg: Config{CookieSecure: true}}
+	r := httptest.NewRequest(http.MethodGet, "http://panel.example.com/domains", nil)
+	r.AddCookie(&http.Cookie{Name: "selfpost_session", Value: "left-over-from-an-older-build"})
+
+	if _, ok := s.sessionToken(r); ok {
+		t.Fatal("the unprefixed cookie was accepted on a TLS deployment")
+	}
+}
+
+func TestRequireAuthRejectsDuplicateCookies(t *testing.T) {
+	s := &Server{cfg: Config{CookieSecure: false}, sessions: newSessionStore()}
+	token := s.sessions.Create("admin")
+
+	reached := false
+	h := s.requireAuth(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { reached = true }))
+
+	r := httptest.NewRequest(http.MethodGet, "http://panel.example.com/domains", nil)
+	r.AddCookie(&http.Cookie{Name: "selfpost_session", Value: "planted-by-a-neighbour"})
+	r.AddCookie(&http.Cookie{Name: "selfpost_session", Value: token})
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, r)
+
+	if reached {
+		t.Fatal("the handler ran even though the session cookie was shadowed")
+	}
+	if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/login" {
+		t.Fatalf("status = %d, Location = %q; want a redirect to /login", rec.Code, rec.Header().Get("Location"))
+	}
+}
+
+// Signing out has to expire the cookie under both names, or the cookie left
+// over from a pre-__Host- build stays in the browser for the rest of its life.
+func TestLogoutClearsBothCookieNames(t *testing.T) {
+	s := &Server{cfg: Config{CookieSecure: true}, sessions: newSessionStore()}
+	token := s.sessions.Create("admin")
+
+	r := httptest.NewRequest(http.MethodPost, "http://panel.example.com/logout", nil)
+	r.Host = "panel.example.com"
+	r.AddCookie(&http.Cookie{Name: "__Host-selfpost_session", Value: token})
+	rec := httptest.NewRecorder()
+	s.handleLogout(rec, r)
+
+	if _, ok := s.sessions.Lookup(token); ok {
+		t.Error("the session survived sign-out")
+	}
+	set := rec.Header().Values("Set-Cookie")
+	for _, name := range []string{"selfpost_session=", "__Host-selfpost_session="} {
+		var found bool
+		for _, c := range set {
+			if strings.HasPrefix(c, name) && strings.Contains(c, "Max-Age=0") {
+				found = true
+			}
+		}
+		if !found {
+			t.Errorf("sign-out does not expire a cookie named %q: %v", strings.TrimSuffix(name, "="), set)
+		}
+	}
+}

internal/web/middleware.go

@@ -14,12 +14,12 @@ const usernameKey ctxKey = 0
 // username is stashed in the request context for downstream handlers.
 func (s *Server) requireAuth(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		c, err := r.Cookie(sessionCookie)
-		if err != nil {
+		token, ok := s.sessionToken(r)
+		if !ok {
 			http.Redirect(w, r, "/login", http.StatusSeeOther)
 			return
 		}
-		username, ok := s.sessions.Lookup(c.Value)
+		username, ok := s.sessions.Lookup(token)
 		if !ok {
 			http.Redirect(w, r, "/login", http.StatusSeeOther)
 			return