~bigbes/sr-ht-ecore

ede9467aeff49bfa6205b12e16914dac560eb790 — Eugene Blikh 9 days ago 4d7f4c4
csrf: the shared same-origin guard

Five services carried their own copy of the Origin/Referer check and the
copies had drifted, not only in wording: tokens, cover and bench apply it
as router-wide middleware, while dolt and specs call a predicate from
individual handlers, so a form added to either of those goes out
unguarded. Require is therefore the API this package leads with, and
SameOrigin is the escape hatch for a service migrating one route at a
time.
2 files changed, 654 insertions(+), 0 deletions(-)

A csrf/csrf.go
A csrf/csrf_test.go
A csrf/csrf.go => csrf/csrf.go +230 -0
@@ 0,0 1,230 @@
// Package csrf is the same-origin guard that stands in front of the forms of
// every custom service on a self-hosted SourceHut instance (compare, spec,
// dolt, cover, bench, ...), and is the one copy of that rule.
//
// Before this package each service carried its own: tokens.sr.ht and bench and
// cover as a router-wide middleware, dolt as a predicate three handlers
// remember to call, spec as a predicate one handler pair calls. The copies had
// already drifted — byte-equal versus case-insensitive host comparison, four
// different refusal sentences, and, in the two services that check per handler,
// a form added later that nobody guards at all. This is a security rule, so the
// interesting drift is not the wording: it is that in two of five services the
// default for a new POST route is *unprotected*. Hence Require, the middleware,
// is the API this package leads with, and SameOrigin exists mainly so a service
// mid-migration is not forced to move its whole router in one commit.
//
// # Why a header check and not a token
//
// There is no CSRF token on any of these services and nowhere to keep one.
// Identity is meta.sr.ht's unified-login cookie, set on the parent domain: no
// individual service issues it, and none of them can set its SameSite
// attribute. A synchronizer-token scheme would mean every daemon inventing a
// session store of its own for the sake of two forms.
//
// What is left is what the browser itself says about where the request came
// from. Origin — and Referer, when Origin is absent — are set by the user agent
// and cannot be forged from script across origins, which is exactly the
// attacker this defends against: a page on another site submitting a form at us
// with the viewer's cookie attached.
//
// # The rule
//
// Read Origin; failing that, read Referer; compare scheme and host against the
// service's own origin; and refuse a request that carries neither.
//
// That last clause is the one a reader is tempted to relax, and it is the one
// holding the guard up. A request that will not say where it came from cannot be
// shown to have come from us. Our own forms are same-origin, every engine has
// sent Origin on a form POST for years, and a request arriving with neither
// header is a script, a stripped proxy or a hand-rolled client — none of which
// is the browser this check exists to protect. Waving it through would reduce
// the whole guard to a header an attacker's page simply omits.
//
// Only scheme and host are compared, and the host comparison includes the port,
// because that is what an origin is (RFC 6454 §4): a page served from :8443 is
// not this origin whatever its hostname says, and http:// is not https://
// whatever the host says. The path a Referer carries is ignored — a Referer is
// a whole URL and any page of ours is an acceptable referrer for our own form.
// The comparison is case-insensitive in both components, since RFC 3986 §3.1
// and §3.2.2 say they are: browsers send them lower-cased, but the service's own
// origin comes from a config.ini line a human typed, and an operator who wrote
// https://Bench.Example.org would otherwise get a service whose every form
// answers 403 with nothing in the logs to explain it.
//
// Safe methods are exempt for RFC 9110 §9.2.1's reason — they change nothing —
// and because every read on these surfaces has to keep working from a bookmark,
// a README's <img>, a probe on /healthz and a curl with no headers at all.
//
// # Usage
//
// The middleware is the one to reach for, on the whole router:
//
//	r.Use(csrf.Require(selfOrigin, func(w http.ResponseWriter, r *http.Request) {
//	    s.renderError(w, r, http.StatusForbidden, csrf.Message)
//	}))
//
// Installed there rather than per handler, the guard covers the routes that are
// not written yet as well as the ones that are: a POST added to the table below
// is protected by having been registered, which is the only form of "do not
// forget" that survives a year. Install it after whatever sets the cache
// headers, so the refusal page carries the same private, no-store every other
// answer of the surface does. It also runs before routing, so a POST to an
// address the surface does not serve is refused rather than 404'd — the right
// way round, since an unrouted POST answering differently from a routed one
// would be a way to enumerate which of them exist without ever passing the
// check.
//
// A service whose bearer-token API lives in its own mux keeps that exemption by
// not mounting this middleware there, which is a fact about where Mount is
// called and not a path test this package could get wrong. There is deliberately
// no "is this /api?" option here and there must never be one: every escape,
// every case fold and every dot segment a client can spell would then be a way
// to ask for the exemption.
//
// selfOrigin is the service's own configured origin, e.g.
// config.GetOrigin(conf, "bench.sr.ht", true). It is parsed once, when the
// middleware is built. If it is not a URL with a scheme and a host, every
// mutating request is refused — the failure is closed, because the alternative
// is a guard that compares against nothing and admits everyone. Services
// validate their origin at startup, so this is unreachable in practice; it is
// written down because it is the arm nobody would notice in production except
// as "all forms 403".
package csrf

import (
	"net/http"
	"net/url"
	"strings"
)

// Message is what a refused mutation tells the viewer, shared so the five
// services answer the same sentence.
//
// It is deliberately not the services' ownership or visibility refusal: this
// says nothing about whether the viewer may do the thing — they may very well
// own the object — only that there is no evidence they asked for it. A human
// who meets it has usually reached the form through something that stripped the
// header, which is worth saying out loud.
const Message = "That request did not come from this site, so it was not carried out."

// Require builds the middleware that refuses a mutating request which cannot
// show it came from selfOrigin.
//
// deny renders the refusal; it is the service's own error page, so that the
// refusal looks like the rest of the surface. It should answer 403 and not 400
// — the request is well-formed and the viewer may well be logged in, what is
// missing is evidence that they asked for this — and it must not redirect: a
// redirect after a POST drops the body and would silently turn a refused
// mutation into a page that looks like it worked. A nil deny answers a plain
// text 403 carrying Message, which is a usable default rather than an invitation
// to skip the middleware.
//
// The returned value has the shape every net/http middleware chain expects,
// including chi's Use.
func Require(selfOrigin string, deny http.HandlerFunc) func(http.Handler) http.Handler {
	// Parsed once here rather than per request: the origin is process-wide
	// configuration, and a middleware built against a broken one should not pay
	// to rediscover that on every POST. ok == false means every mutating request
	// is refused; see the package comment.
	self, ok := parseSelf(selfOrigin)
	if deny == nil {
		deny = denyPlain
	}
	return func(next http.Handler) http.Handler {
		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			if SafeMethod(r.Method) || (ok && claimMatches(r, self)) {
				next.ServeHTTP(w, r)
				return
			}
			deny(w, r)
		})
	}
}

// SameOrigin reports whether r names selfOrigin in its Origin header — or,
// failing that, in its Referer.
//
// This is the predicate behind Require, exported for the handler that cannot
// yet sit behind the middleware: a route mounted outside the guarded router, or
// a service migrating one handler at a time. Prefer Require. A predicate is
// protection somebody has to remember, and the services this package was
// extracted from are the proof: the two that check per handler are the two
// where a form added later would go out unguarded.
//
// It does not consult the method — a caller reaching for it has already decided
// the request mutates. Pair it with SafeMethod if that is not true.
func SameOrigin(r *http.Request, selfOrigin string) bool {
	self, ok := parseSelf(selfOrigin)
	if !ok {
		return false
	}
	return claimMatches(r, self)
}

// SafeMethod reports whether a method may not change state and is therefore
// exempt (RFC 9110 §9.2.1). Everything else — POST today, a PUT, PATCH or
// DELETE tomorrow — is checked, which is the direction this list has to fail
// in: an unknown method is guarded, not waved through.
//
// TRACE is on the list because RFC 9110 defines it as safe and because net/http
// neither routes nor echoes it by default; leaving it off would have made this
// list disagree with the four donors for no gain.
func SafeMethod(method string) bool {
	switch method {
	case http.MethodGet, http.MethodHead, http.MethodOptions, http.MethodTrace:
		return true
	default:
		return false
	}
}

// claimMatches applies the rule to a request whose own origin has already
// parsed.
//
// Origin is consulted first and, when present, alone: a browser that sends both
// means them to agree, and treating Referer as a second chance after Origin has
// already said "somewhere else" would turn the stronger statement into the
// weaker one.
func claimMatches(r *http.Request, self *url.URL) bool {
	if origin := r.Header.Get("Origin"); origin != "" {
		return originMatches(origin, self)
	}
	if referer := r.Header.Get("Referer"); referer != "" {
		return originMatches(referer, self)
	}
	// Neither header: refuse rather than assume same-origin.
	return false
}

// originMatches reports whether raw — a whole URL, which is what both headers
// carry — has self's scheme and host.
//
// A value that does not parse matches nothing, and so does one that parses to
// no scheme or no host: the "null" that a sandboxed iframe or a form redirected
// across origins posts is the everyday example, and it must not be mistaken for
// "no header", which is refused anyway.
func originMatches(raw string, self *url.URL) bool {
	u, err := url.Parse(raw)
	if err != nil {
		return false
	}
	return strings.EqualFold(u.Scheme, self.Scheme) && strings.EqualFold(u.Host, self.Host)
}

// parseSelf parses the service's own origin, reporting false for anything that
// could not be an origin. Both components are required: a host with no scheme
// would compare equal to a protocol-relative "//host/..." claim, and a value
// with no host would compare equal to every claim that also has none.
func parseSelf(selfOrigin string) (*url.URL, bool) {
	u, err := url.Parse(selfOrigin)
	if err != nil || u.Scheme == "" || u.Host == "" {
		return nil, false
	}
	return u, true
}

// denyPlain is the refusal Require uses when the caller supplies none: the
// shared sentence, as text, with the status the services' own error pages use.
func denyPlain(w http.ResponseWriter, _ *http.Request) {
	http.Error(w, Message, http.StatusForbidden)
}

A csrf/csrf_test.go => csrf/csrf_test.go +424 -0
@@ 0,0 1,424 @@
package csrf

import (
	"net/http"
	"net/http/httptest"
	"strings"
	"testing"

	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
)

// selfOrigin is the origin under test throughout: an https service on the
// default port, spelled the way a config.ini line would be.
const selfOrigin = "https://bench.example.org"

// request builds a POST carrying the given headers; a header with an empty
// value is not set at all, which is how a request that omits it looks to
// Header.Get.
func request(method string, headers map[string]string) *http.Request {
	r := httptest.NewRequest(method, "/settings", nil)
	for k, v := range headers {
		if v != "" {
			r.Header.Set(k, v)
		}
	}
	return r
}

func TestSameOrigin(t *testing.T) {
	tests := []struct {
		name    string
		self    string
		origin  string
		referer string
		want    bool
	}{
		{
			name:   "matching Origin",
			self:   selfOrigin,
			origin: "https://bench.example.org",
			want:   true,
		},
		{
			name:   "matching Origin differing only in case",
			self:   "https://Bench.Example.org",
			origin: "https://bench.example.org",
			want:   true,
			// The operator types the config line; the browser sends the
			// lower-cased form. Byte equality would 403 every form on this
			// instance with nothing in the logs to explain it.
		},
		{
			name:   "mismatching Origin",
			self:   selfOrigin,
			origin: "https://evil.example.com",
			want:   false,
		},
		{
			name:   "Origin whose host is a suffix of ours",
			self:   selfOrigin,
			origin: "https://notbench.example.org",
			want:   false,
		},
		{
			name:   "Origin that only prefixes ours",
			self:   selfOrigin,
			origin: "https://bench.example.org.evil.com",
			want:   false,
		},
		{
			name:   "scheme mismatch",
			self:   selfOrigin,
			origin: "http://bench.example.org",
			want:   false,
		},
		{
			name:   "port mismatch",
			self:   selfOrigin,
			origin: "https://bench.example.org:8443",
			want:   false,
			// The port is part of the origin (RFC 6454 §4): a page served from
			// :8443 is not this origin whatever its hostname says.
		},
		{
			name:   "our port spelled out where ours has none",
			self:   selfOrigin,
			origin: "https://bench.example.org:443",
			want:   false,
			// Refused deliberately: this compares hosts, not effective ports.
			// Browsers elide the default port, so a real request never looks
			// like this, and normalising it would mean teaching this package
			// every scheme's default.
		},
		{
			name:    "matching Referer with no Origin",
			self:    selfOrigin,
			referer: "https://bench.example.org/~bigbes/foo",
			want:    true,
			// The path is ignored: any page of ours may refer to our own form.
		},
		{
			name:    "matching Referer, bare origin with no path",
			self:    selfOrigin,
			referer: "https://bench.example.org",
			want:    true,
		},
		{
			name:    "mismatching Referer",
			self:    selfOrigin,
			referer: "https://evil.example.com/attack.html",
			want:    false,
		},
		{
			name: "neither header",
			self: selfOrigin,
			want: false,
			// The clause the whole guard rests on. A request that will not say
			// where it came from cannot be shown to have come from us, and
			// admitting it would reduce the check to a header an attacker's
			// page simply omits.
		},
		{
			name:    "mismatching Origin beats a matching Referer",
			self:    selfOrigin,
			origin:  "https://evil.example.com",
			referer: "https://bench.example.org/~bigbes/foo",
			want:    false,
			// Referer is the fallback for an absent Origin, not a second chance
			// after Origin has already said "somewhere else".
		},
		{
			name:    "matching Origin outvotes a foreign Referer",
			self:    selfOrigin,
			origin:  "https://bench.example.org",
			referer: "https://evil.example.com/attack.html",
			want:    true,
		},
		{
			name:   "the null origin of a sandboxed iframe",
			self:   selfOrigin,
			origin: "null",
			want:   false,
		},
		{
			name:   "malformed Origin",
			self:   selfOrigin,
			origin: "http://[::1",
			want:   false,
		},
		{
			name:   "Origin that is not a URL at all",
			self:   selfOrigin,
			origin: "not a url",
			want:   false,
		},
		{
			name:   "protocol-relative Origin naming our host",
			self:   selfOrigin,
			origin: "//bench.example.org",
			want:   false,
			// Parses to our host with no scheme. Refused, because the guard
			// compares schemes too and the empty one is not https.
		},
		{
			name:   "malformed own origin",
			self:   "http://[::1",
			origin: "http://[::1",
			want:   false,
			// Fails closed: a broken config line refuses every mutation rather
			// than making every claim match a nil comparison.
		},
		{
			name:   "own origin with no scheme",
			self:   "bench.example.org",
			origin: "https://bench.example.org",
			want:   false,
		},
		{
			name:   "empty own origin",
			self:   "",
			origin: "https://bench.example.org",
			want:   false,
		},
		{
			name: "empty own origin and no headers",
			self: "",
			want: false,
		},
	}

	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			r := request(http.MethodPost, map[string]string{
				"Origin":  tt.origin,
				"Referer": tt.referer,
			})
			assert.Equal(t, tt.want, SameOrigin(r, tt.self))
		})
	}
}

// SameOrigin says nothing about the method — a caller reaching for the
// predicate has already decided the request mutates.
func TestSameOriginIgnoresTheMethod(t *testing.T) {
	for _, method := range []string{http.MethodGet, http.MethodPost, http.MethodDelete} {
		r := request(method, nil)
		assert.False(t, SameOrigin(r, selfOrigin), "%s with no headers", method)
	}
}

func TestSafeMethod(t *testing.T) {
	tests := []struct {
		method string
		want   bool
	}{
		{http.MethodGet, true},
		{http.MethodHead, true},
		{http.MethodOptions, true},
		{http.MethodTrace, true},
		{http.MethodPost, false},
		{http.MethodPut, false},
		{http.MethodPatch, false},
		{http.MethodDelete, false},
		{http.MethodConnect, false},
		{"PROPFIND", false},
		{"", false},
		{"get", false}, // methods are case-sensitive; an unknown one is guarded
	}

	for _, tt := range tests {
		t.Run(tt.method, func(t *testing.T) {
			assert.Equal(t, tt.want, SafeMethod(tt.method))
		})
	}
}

func TestRequire(t *testing.T) {
	tests := []struct {
		name       string
		self       string
		method     string
		origin     string
		referer    string
		wantServed bool
	}{
		{
			name:       "matching Origin on a POST",
			self:       selfOrigin,
			method:     http.MethodPost,
			origin:     "https://bench.example.org",
			wantServed: true,
		},
		{
			name:       "mismatching Origin on a POST",
			self:       selfOrigin,
			method:     http.MethodPost,
			origin:     "https://evil.example.com",
			wantServed: false,
		},
		{
			name:       "matching Referer with no Origin",
			self:       selfOrigin,
			method:     http.MethodPost,
			referer:    "https://bench.example.org/settings",
			wantServed: true,
		},
		{
			name:       "mismatching Referer",
			self:       selfOrigin,
			method:     http.MethodPost,
			referer:    "https://evil.example.com/attack.html",
			wantServed: false,
		},
		{
			name:       "POST with neither header",
			self:       selfOrigin,
			method:     http.MethodPost,
			wantServed: false,
		},
		{
			name:       "DELETE with neither header",
			self:       selfOrigin,
			method:     http.MethodDelete,
			wantServed: false,
			// The methods nobody has written a route for yet are guarded by
			// default; that is the whole reason this is a middleware.
		},
		{
			name:       "PUT with a matching Origin",
			self:       selfOrigin,
			method:     http.MethodPut,
			origin:     "https://bench.example.org",
			wantServed: true,
		},
		{
			name:       "GET with no headers at all",
			self:       selfOrigin,
			method:     http.MethodGet,
			wantServed: true,
			// A bookmark, a README's <img>, a probe on /healthz.
		},
		{
			name:       "HEAD from another site",
			self:       selfOrigin,
			method:     http.MethodHead,
			origin:     "https://evil.example.com",
			wantServed: true,
		},
		{
			name:       "OPTIONS preflight from another site",
			self:       selfOrigin,
			method:     http.MethodOptions,
			origin:     "https://evil.example.com",
			wantServed: true,
		},
		{
			name:       "malformed Origin on a POST",
			self:       selfOrigin,
			method:     http.MethodPost,
			origin:     "http://[::1",
			wantServed: false,
		},
		{
			name:       "broken own origin refuses a matching POST",
			self:       "not-an-origin",
			method:     http.MethodPost,
			origin:     "https://bench.example.org",
			wantServed: false,
		},
		{
			name:       "broken own origin still serves a GET",
			self:       "not-an-origin",
			method:     http.MethodGet,
			wantServed: true,
			// Fail-closed applies to mutations. A misconfigured origin must not
			// take the whole read surface down with it.
		},
	}

	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			var served, denied bool
			next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
				served = true
				w.WriteHeader(http.StatusNoContent)
			})
			deny := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
				denied = true
				http.Error(w, Message, http.StatusForbidden)
			})

			w := httptest.NewRecorder()
			Require(tt.self, deny)(next).ServeHTTP(w, request(tt.method, map[string]string{
				"Origin":  tt.origin,
				"Referer": tt.referer,
			}))

			assert.Equal(t, tt.wantServed, served, "handler reached")
			assert.Equal(t, !tt.wantServed, denied, "refusal rendered")
			if tt.wantServed {
				assert.Equal(t, http.StatusNoContent, w.Code)
			} else {
				assert.Equal(t, http.StatusForbidden, w.Code)
			}
		})
	}
}

// A nil deny is a usable default rather than a reason to skip the middleware.
func TestRequireDefaultRefusal(t *testing.T) {
	next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		t.Error("handler must not be reached")
	})

	w := httptest.NewRecorder()
	Require(selfOrigin, nil)(next).ServeHTTP(w, request(http.MethodPost, nil))

	require.Equal(t, http.StatusForbidden, w.Code)
	assert.Equal(t, Message, strings.TrimSpace(w.Body.String()))
}

// The refused request's body must not reach the handler, and the refusal must
// not be a redirect: a redirect after a POST drops the body and turns a refused
// mutation into a page that looks like it worked.
func TestRequireRefusalIsNotARedirect(t *testing.T) {
	w := httptest.NewRecorder()
	next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		t.Error("handler must not be reached")
	})
	r := httptest.NewRequest(http.MethodPost, "/settings", strings.NewReader("name=x"))
	r.Header.Set("Origin", "https://evil.example.com")

	Require(selfOrigin, nil)(next).ServeHTTP(w, r)

	require.Equal(t, http.StatusForbidden, w.Code)
	assert.Empty(t, w.Header().Get("Location"))
}

// The guard runs before routing, so a mutating request to an address the
// service does not serve is refused rather than answered 404 — an unrouted POST
// that answered differently would enumerate which routes exist without ever
// passing the check.
func TestRequireRunsBeforeRouting(t *testing.T) {
	mux := http.NewServeMux()
	mux.HandleFunc("/settings", func(w http.ResponseWriter, r *http.Request) {
		w.WriteHeader(http.StatusNoContent)
	})
	guarded := Require(selfOrigin, nil)(mux)

	for _, target := range []string{"/settings", "/no/such/route"} {
		w := httptest.NewRecorder()
		r := httptest.NewRequest(http.MethodPost, target, nil)
		r.Header.Set("Origin", "https://evil.example.com")
		guarded.ServeHTTP(w, r)
		assert.Equal(t, http.StatusForbidden, w.Code, "POST %s", target)
	}
}

// Message is part of the package's contract: the five services answer the same
// sentence, and a service's own error page renders it verbatim.
func TestMessageIsShared(t *testing.T) {
	assert.Equal(t, "That request did not come from this site, so it was not carried out.", Message)
}