Bind every changing form's token to its session (issue 1284)

2026-10-19 22:18:03 +02:00
parent 8e0ef84060
commit 3ee80c98ab
2 changed files with 181 additions and 0 deletions
@@ -0,0 +1,49 @@
+package main
+
+// The form token bound to the session (novox/hq ADR 0440 §3's last clause, issue 1284): every form that
+// changes something — today mail_send, once issue 1267 builds the write page — carries a hidden field with
+// the session's own token (a random 32 bytes, made once per session, Sessions.Create); a POST missing it, or
+// carrying another session's, is refused 403 before the change it would have made ever runs, compared in
+// constant time so the check is never a timing side-channel.
+
+import (
+	"crypto/subtle"
+	"html/template"
+	"net/http"
+)
+
+// FormTokenField is the hidden field's own name every changing form carries.
+const FormTokenField = "csrf_token"
+
+// FormTokenInput is the hidden field a changing form's own template includes (issue 1284/AC1): bound to the
+// session handed to the page that renders it (RequireSession, SessionFrom).
+func FormTokenInput(sess Session) template.HTML {
+	return template.HTML(`<input type="hidden" name="` + FormTokenField + `" value="` +
+		template.HTMLEscapeString(sess.Token) + `">`)
+}
+
+// RequireFormToken wraps a POST handler that changes something (novox/hq issue 1284): it calls next only once
+// the session RequireSession handed on (SessionFrom) and the form's own FormTokenField compare equal, in
+// constant time; a POST with no token, or another session's, is answered 403 and next is never called, so the
+// change it would have made — today mail_send — never runs (AC2, AC3). RequireFormToken is placed inside
+// RequireSession (RequireSession(RequireFormToken(next))): a POST with no session at all is the sign-in
+// redirect RequireSession itself gives, never reaching here.
+func RequireFormToken(next http.HandlerFunc) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		sess, ok := SessionFrom(r)
+		if !ok {
+			http.Error(w, "forbidden", http.StatusForbidden)
+			return
+		}
+		if err := r.ParseForm(); err != nil {
+			http.Error(w, "forbidden", http.StatusForbidden)
+			return
+		}
+		given := r.PostFormValue(FormTokenField)
+		if given == "" || subtle.ConstantTimeCompare([]byte(given), []byte(sess.Token)) != 1 {
+			http.Error(w, "forbidden", http.StatusForbidden)
+			return
+		}
+		next(w, r)
+	}
+}
@@ -0,0 +1,132 @@
+package main
+
+// The form token bound to the session (novox/hq ADR 0440 §3, issue 1284). There is no write page to test
+// against yet (issue 1267 builds it); these tests stand in for it with a minimal changing handler ("send"),
+// proving the mechanism issue 1267 will wire mail_send behind: a session, and that session's own token.
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+)
+
+// sessionAndCookie starts a session directly (bypassing sign-in, which auth_test.go already covers on its
+// own) and answers it with the cookie a request would carry.
+func sessionAndCookie(t *testing.T, sessions *Sessions, operator string) (Session, *http.Cookie) {
+	t.Helper()
+	id, err := sessions.Create(operator)
+	if err != nil {
+		t.Fatal(err)
+	}
+	sess, ok := sessions.Get(id)
+	if !ok {
+		t.Fatal("the session just created is not there")
+	}
+	return sess, &http.Cookie{Name: SessionCookie, Value: id}
+}
+
+// 1284/AC1: a signed-in session's own form carries that session's token.
+func TestFormTokenInputCarriesTheSessionsOwnToken(t *testing.T) {
+	sess, _ := sessionAndCookie(t, NewSessions(nil), "alice")
+	html := string(FormTokenInput(sess))
+	if !strings.Contains(html, `name="csrf_token"`) || !strings.Contains(html, sess.Token) {
+		t.Fatalf("the form's hidden field does not carry the session's token: %s", html)
+	}
+}
+
+// changingHandler is the stand-in for mail_send (issue 1267 is the real one): it counts how many times it was
+// actually reached.
+func changingHandler(calls *int) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		*calls++
+		w.WriteHeader(http.StatusOK)
+	}
+}
+
+func postSend(t *testing.T, handler http.HandlerFunc, cookie *http.Cookie, token string) *httptest.ResponseRecorder {
+	t.Helper()
+	form := url.Values{}
+	if token != "" {
+		form.Set(FormTokenField, token)
+	}
+	r := httptest.NewRequest(http.MethodPost, "/send", strings.NewReader(form.Encode()))
+	r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	if cookie != nil {
+		r.AddCookie(cookie)
+	}
+	w := httptest.NewRecorder()
+	handler(w, r)
+	return w
+}
+
+// 1284/AC2: a POST of a send without the token is answered 403, and the change (here, the stand-in) is never
+// called.
+func TestPOSTWithoutTheTokenIsRefusedAndTheChangeIsNotCalled(t *testing.T) {
+	sessions := NewSessions(nil)
+	_, cookie := sessionAndCookie(t, sessions, "alice")
+	auth := &Auth{Sessions: sessions}
+	var calls int
+	handler := auth.RequireSession(RequireFormToken(changingHandler(&calls)))
+
+	w := postSend(t, handler, cookie, "")
+	if w.Code != http.StatusForbidden {
+		t.Fatalf("status %d, want 403", w.Code)
+	}
+	if calls != 0 {
+		t.Fatalf("the change was called %d time(s) with no token", calls)
+	}
+}
+
+// 1284/AC3: a POST carrying another session's token is answered 403, and the change is never called.
+func TestPOSTWithAnotherSessionsTokenIsRefusedAndTheChangeIsNotCalled(t *testing.T) {
+	sessions := NewSessions(nil)
+	_, cookie := sessionAndCookie(t, sessions, "alice")
+	otherSess, _ := sessionAndCookie(t, sessions, "bob")
+	auth := &Auth{Sessions: sessions}
+	var calls int
+	handler := auth.RequireSession(RequireFormToken(changingHandler(&calls)))
+
+	w := postSend(t, handler, cookie, otherSess.Token)
+	if w.Code != http.StatusForbidden {
+		t.Fatalf("status %d, want 403", w.Code)
+	}
+	if calls != 0 {
+		t.Fatalf("the change was called %d time(s) with another session's token", calls)
+	}
+}
+
+// 1284/AC4: a POST carrying its own session's token proceeds.
+func TestPOSTWithItsOwnSessionsTokenProceeds(t *testing.T) {
+	sessions := NewSessions(nil)
+	sess, cookie := sessionAndCookie(t, sessions, "alice")
+	auth := &Auth{Sessions: sessions}
+	var calls int
+	handler := auth.RequireSession(RequireFormToken(changingHandler(&calls)))
+
+	w := postSend(t, handler, cookie, sess.Token)
+	if w.Code != http.StatusOK {
+		t.Fatalf("status %d, want 200: %s", w.Code, w.Body.String())
+	}
+	if calls != 1 {
+		t.Fatalf("the change was called %d time(s), want 1", calls)
+	}
+}
+
+// No session at all: RequireSession's own redirect runs, RequireFormToken is never reached, and the change is
+// never called either (a POST attempted with no session is treated the same as a page with no session).
+func TestPOSTWithNoSessionDoesNotReachTheFormTokenCheckOrTheChange(t *testing.T) {
+	fi := newFakeIssuer(t)
+	auth := testAuth(t, fi, operatorsFile(t, "alice"), nil)
+	var calls int
+	handler := auth.RequireSession(RequireFormToken(changingHandler(&calls)))
+
+	w := postSend(t, handler, nil, "whatever")
+	if w.Code != http.StatusFound {
+		t.Fatalf("status %d, want 302 (redirect to sign in)", w.Code)
+	}
+	if calls != 0 {
+		t.Fatalf("the change was called %d time(s) with no session", calls)
+	}
+}