fix(security): harden httpx — XFF hops, __Host- cookie, isolation headers
REVIEW-001 (M0) perimeter findings: - H1: resolveForwarded walked X-Forwarded-For left→right and returned the leftmost untrusted entry. An append-style proxy (nginx's default) appends the peer it saw to the RIGHT of whatever the client sent, so the leftmost entry is attacker-controlled — a fresh rate-limit bucket per request (A3) and a forged client_ip in the audit trail (A11/D-031). Walk right→left and return the first untrusted entry, falling back to the peer. - L1s: the session cookie now carries the `__Host-` prefix when it is Secure (https), so it is pinned to the exact origin and cannot be shadowed from a sibling subdomain. Conditional on purpose: a browser rejects a `__Host-` cookie without Secure, which would break the http loopback bootstrap. - L4s: add Permissions-Policy, Cross-Origin-Opener-Policy and Cross-Origin-Resource-Policy. - Add httpx.IsLoopbackIP: a RESOLVED client IP test that fails closed on an empty/unparseable value (the setup-mode gate, H2), sharing a classifier with IsLoopbackHost whose "unknown" direction is the opposite, each fail-closed for its own decision. Tests updated: the unit test encoding the vulnerable leftmost expectation is rewritten, plus right→left/forged-entry/host-classification coverage.
This commit is contained in:
parent
f5c6b95c27
commit
03a9db03dc
5 changed files with 181 additions and 30 deletions
|
|
@ -64,9 +64,11 @@ func (t TrustedProxies) Contains(ip net.IP) bool {
|
|||
//
|
||||
// - the immediate peer (RemoteAddr) is NOT trusted → any X-Forwarded-For is
|
||||
// ignored and the peer is the client (a client cannot spoof past the gate);
|
||||
// - the peer IS trusted → walk X-Forwarded-For left to right and return the
|
||||
// first address that is not itself a trusted proxy (the real client); if
|
||||
// every hop is trusted, the last hop is returned.
|
||||
// - the peer IS trusted → walk X-Forwarded-For RIGHT to LEFT and return the
|
||||
// first address that is not itself a trusted proxy (the real client). Each
|
||||
// trusted hop appended the peer it saw to the right, so the rightmost
|
||||
// untrusted entry is the one closest to us that a client could not have
|
||||
// forged (H1); if every hop is trusted, the peer is returned.
|
||||
//
|
||||
// The resolved value is written back with Header.Set (replace), never Add.
|
||||
func ClientIP(r *http.Request, trusted TrustedProxies) string {
|
||||
|
|
@ -85,26 +87,29 @@ func ClientIP(r *http.Request, trusted TrustedProxies) string {
|
|||
return s
|
||||
}
|
||||
|
||||
// resolveForwarded returns the leftmost non-trusted address in the header,
|
||||
// falling back to the last hop, then to the peer.
|
||||
// resolveForwarded returns the RIGHTMOST address in the header that is not a
|
||||
// trusted proxy, falling back to the peer.
|
||||
//
|
||||
// Walking right→left is the defence (H1): an append-style proxy (nginx's
|
||||
// default) appends the peer it saw to the RIGHT of whatever the client sent, so
|
||||
// the leftmost entries are attacker-controlled. The rightmost untrusted entry
|
||||
// is the closest hop we can trust — it is the first address not appended by a
|
||||
// trusted proxy. Returning the leftmost entry (the old behaviour) let a client
|
||||
// forge a fresh rate-limit bucket per request (A3) and a forged client_ip in
|
||||
// the audit trail (A11/D-031).
|
||||
func resolveForwarded(header string, trusted TrustedProxies, peer net.IP) net.IP {
|
||||
if strings.TrimSpace(header) == "" {
|
||||
return peer
|
||||
}
|
||||
var last net.IP
|
||||
for _, part := range strings.Split(header, ",") {
|
||||
ip := net.ParseIP(strings.TrimSpace(part))
|
||||
parts := strings.Split(header, ",")
|
||||
for i := len(parts) - 1; i >= 0; i-- {
|
||||
ip := net.ParseIP(strings.TrimSpace(parts[i]))
|
||||
if ip == nil {
|
||||
continue
|
||||
}
|
||||
last = ip
|
||||
if !trusted.Contains(ip) {
|
||||
return ip // the real client
|
||||
return ip // the closest untrusted hop — the real client
|
||||
}
|
||||
}
|
||||
if last != nil {
|
||||
return last
|
||||
}
|
||||
// Every hop (or the whole header) is trusted or unparseable: the peer is
|
||||
// the only address we can vouch for.
|
||||
return peer
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -8,10 +8,27 @@ package httpx
|
|||
|
||||
import "net/http"
|
||||
|
||||
// SessionCookieName is the name of the session cookie (SPEC-003 §6). The value
|
||||
// is an opaque random id; the server-side session row is authoritative.
|
||||
// SessionCookieName is the base name of the session cookie (SPEC-003 §6). The
|
||||
// value is an opaque random id; the server-side session row is authoritative.
|
||||
const SessionCookieName = "myday_session"
|
||||
|
||||
// HostPrefix is the `__Host-` cookie-name prefix (L1s). A browser only accepts
|
||||
// a `__Host-` cookie when it is Secure · Path=/ · has no Domain attribute, so
|
||||
// the cookie is pinned to the exact origin — a sibling subdomain cannot shadow
|
||||
// it (a cookie-fixation / session-shadowing DoS).
|
||||
const HostPrefix = "__Host-"
|
||||
|
||||
// SessionCookieNameFor returns the session cookie's name for the transport:
|
||||
// `__Host-myday_session` over https (Secure is set — required by the prefix)
|
||||
// and the bare name otherwise, because a browser REJECTS a `__Host-` cookie
|
||||
// sent without Secure over plain http (the loopback bootstrap / human test).
|
||||
func SessionCookieNameFor(secure bool) string {
|
||||
if secure {
|
||||
return HostPrefix + SessionCookieName
|
||||
}
|
||||
return SessionCookieName
|
||||
}
|
||||
|
||||
// SetSessionCookie writes the hardened session cookie (SPEC-003 §6):
|
||||
// HttpOnly · Secure · SameSite=Lax · Path=/. The lifetime is server-side, so
|
||||
// no Max-Age is set (a browser-session cookie).
|
||||
|
|
|
|||
|
|
@ -22,7 +22,18 @@ func SetSecurityHeaders(w http.ResponseWriter, secure bool) {
|
|||
h.Set("Referrer-Policy", "no-referrer")
|
||||
h.Set("X-Frame-Options", "DENY")
|
||||
h.Set("Content-Security-Policy", ContentSecurityPolicy)
|
||||
// L4s — the isolation trio: deny the powerful features the app never uses,
|
||||
// and keep the document and its subresources same-origin (so a compromised
|
||||
// or mis-hosted sibling cannot open, embed, or read MyDay).
|
||||
h.Set("Permissions-Policy", PermissionsPolicy)
|
||||
h.Set("Cross-Origin-Opener-Policy", "same-origin")
|
||||
h.Set("Cross-Origin-Resource-Policy", "same-origin")
|
||||
if secure {
|
||||
h.Set("Strict-Transport-Security", "max-age=31536000; includeSubDomains")
|
||||
}
|
||||
}
|
||||
|
||||
// PermissionsPolicy disables the browser features MyDay does not use (L4s).
|
||||
// An empty allowlist means "no origin, including self, may use it".
|
||||
const PermissionsPolicy = "accelerometer=(), camera=(), geolocation=(), " +
|
||||
"gyroscope=(), magnetometer=(), microphone=(), payment=(), usb=()"
|
||||
|
|
|
|||
|
|
@ -80,29 +80,107 @@ func TestClientIPUntrustedPeerIgnoresXFF(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestClientIPTrustedPeerResolvesLeftmostUntrusted(t *testing.T) {
|
||||
func TestClientIPTrustedPeerResolvesRightmostUntrusted(t *testing.T) {
|
||||
tp, _ := httpx.ParseTrustedProxies([]string{"127.0.0.1", "10.0.0.0/8"})
|
||||
r := httptest.NewRequest("GET", "/", nil)
|
||||
r.RemoteAddr = "127.0.0.1:5555"
|
||||
r.Header.Set("X-Forwarded-For", "198.51.100.7, 10.0.0.5")
|
||||
// An append-style proxy (nginx default) appends the peer it SAW to the
|
||||
// right of whatever the client sent. Here a client forged "1.2.3.4" and the
|
||||
// proxy appended the real client "198.51.100.7" (H1).
|
||||
r.Header.Set("X-Forwarded-For", "1.2.3.4, 198.51.100.7, 10.0.0.5")
|
||||
|
||||
got := httpx.ClientIP(r, tp)
|
||||
if got != "198.51.100.7" {
|
||||
t.Errorf("ClientIP = %q, want the leftmost non-trusted 198.51.100.7", got)
|
||||
t.Errorf("ClientIP = %q, want the rightmost untrusted 198.51.100.7 (H1: the leftmost entry is forgeable)", got)
|
||||
}
|
||||
if xff := r.Header.Get("X-Forwarded-For"); xff != "198.51.100.7" {
|
||||
t.Errorf("X-Forwarded-For = %q, want it SET (replaced), not appended", xff)
|
||||
}
|
||||
}
|
||||
|
||||
// TestClientIPIgnoresForgedLeftmostEntry is the H1 regression: the leftmost
|
||||
// entry must never win, or an append-style proxy gives the client a fresh
|
||||
// rate-limit bucket and a forged audit client_ip (A3/A11).
|
||||
func TestClientIPIgnoresForgedLeftmostEntry(t *testing.T) {
|
||||
tp, _ := httpx.ParseTrustedProxies([]string{"127.0.0.1"})
|
||||
r := httptest.NewRequest("GET", "/", nil)
|
||||
r.RemoteAddr = "127.0.0.1:5555"
|
||||
r.Header.Set("X-Forwarded-For", "9.9.9.9, 198.51.100.7")
|
||||
|
||||
if got := httpx.ClientIP(r, tp); got != "198.51.100.7" {
|
||||
t.Errorf("ClientIP = %q, want 198.51.100.7 (the forged 9.9.9.9 must be ignored, H1)", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestClientIPAllHopsTrusted(t *testing.T) {
|
||||
tp, _ := httpx.ParseTrustedProxies([]string{"127.0.0.1", "10.0.0.0/8"})
|
||||
r := httptest.NewRequest("GET", "/", nil)
|
||||
r.RemoteAddr = "127.0.0.1:5555"
|
||||
r.Header.Set("X-Forwarded-For", "10.0.0.1, 10.0.0.2")
|
||||
|
||||
if got := httpx.ClientIP(r, tp); got != "10.0.0.2" {
|
||||
t.Errorf("ClientIP = %q, want the last hop 10.0.0.2", got)
|
||||
// Every hop is trusted: the peer is the only address we can vouch for.
|
||||
if got := httpx.ClientIP(r, tp); got != "127.0.0.1" {
|
||||
t.Errorf("ClientIP = %q, want the peer 127.0.0.1 when every hop is trusted", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestIsLoopbackHost pins the nudge's origin predicate (SPEC-006 §8): a NAME
|
||||
// is not loopback, and only an empty/unknown host fails closed to loopback.
|
||||
func TestIsLoopbackHost(t *testing.T) {
|
||||
cases := []struct {
|
||||
host string
|
||||
want bool
|
||||
}{
|
||||
{"127.0.0.1", true},
|
||||
{"127.0.0.1:8080", true},
|
||||
{"localhost", true},
|
||||
{"localhost:8080", true},
|
||||
{"[::1]:8080", true},
|
||||
{"example.com", false},
|
||||
{"myday.example.com:443", false},
|
||||
{"203.0.113.9", false},
|
||||
{"", true}, // unknown → suppress the nudge (fail closed)
|
||||
}
|
||||
for _, tc := range cases {
|
||||
if got := httpx.IsLoopbackHost(tc.host); got != tc.want {
|
||||
t.Errorf("IsLoopbackHost(%q) = %v, want %v", tc.host, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestIsLoopbackIPFailsClosed pins the setup-mode gate's predicate (H2): an
|
||||
// empty or unparseable resolved client IP must NOT count as loopback.
|
||||
func TestIsLoopbackIPFailsClosed(t *testing.T) {
|
||||
cases := []struct {
|
||||
ip string
|
||||
want bool
|
||||
}{
|
||||
{"127.0.0.1", true},
|
||||
{"127.0.0.5", true},
|
||||
{"::1", true},
|
||||
{"localhost", true},
|
||||
{"[::1]", true},
|
||||
{"203.0.113.9", false},
|
||||
{"10.0.0.5", false},
|
||||
{"", false}, // unparseable: fail closed
|
||||
{"not-an-ip", false},
|
||||
{"0.0.0.0", false},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
if got := httpx.IsLoopbackIP(tc.ip); got != tc.want {
|
||||
t.Errorf("IsLoopbackIP(%q) = %v, want %v", tc.ip, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestSessionCookieNameForPrefix pins L1s: the `__Host-` prefix is used exactly
|
||||
// when the cookie is Secure (a browser rejects a `__Host-` cookie over http).
|
||||
func TestSessionCookieNameForPrefix(t *testing.T) {
|
||||
if got := httpx.SessionCookieNameFor(true); got != "__Host-myday_session" {
|
||||
t.Errorf("SessionCookieNameFor(true) = %q, want the __Host- prefix", got)
|
||||
}
|
||||
if got := httpx.SessionCookieNameFor(false); got != httpx.SessionCookieName {
|
||||
t.Errorf("SessionCookieNameFor(false) = %q, want the bare name", got)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -155,6 +233,16 @@ func TestSetSecurityHeaders(t *testing.T) {
|
|||
if !strings.Contains(h.Get("Content-Security-Policy"), "default-src 'self'") {
|
||||
t.Errorf("CSP = %q", h.Get("Content-Security-Policy"))
|
||||
}
|
||||
// L4s: the isolation trio.
|
||||
if got := h.Get("Permissions-Policy"); !strings.Contains(got, "geolocation=()") {
|
||||
t.Errorf("Permissions-Policy = %q, want the feature allowlist", got)
|
||||
}
|
||||
if got := h.Get("Cross-Origin-Opener-Policy"); got != "same-origin" {
|
||||
t.Errorf("Cross-Origin-Opener-Policy = %q, want same-origin", got)
|
||||
}
|
||||
if got := h.Get("Cross-Origin-Resource-Policy"); got != "same-origin" {
|
||||
t.Errorf("Cross-Origin-Resource-Policy = %q, want same-origin", got)
|
||||
}
|
||||
if h.Get("Strict-Transport-Security") != "" {
|
||||
t.Error("HSTS set for an insecure request")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,20 +11,50 @@ import (
|
|||
// It gates the passkey nudge (SPEC-006 §8): a passkey registered at
|
||||
// `127.0.0.1` binds the Relying Party ID `127.0.0.1` and is UNUSABLE at the
|
||||
// public domain (O-13/D-041), so the nudge is suppressed on loopback. An
|
||||
// empty/unknown host is treated as loopback — fail closed, so an origin we
|
||||
// cannot classify never triggers the nudge.
|
||||
// empty/unknown host counts as loopback — fail closed, so an origin we cannot
|
||||
// classify never triggers the nudge.
|
||||
func IsLoopbackHost(hostport string) bool {
|
||||
loopback, known := classifyHost(hostport)
|
||||
return !known || loopback
|
||||
}
|
||||
|
||||
// IsLoopbackIP reports whether a RESOLVED client IP is loopback. It gates the
|
||||
// per-request setup-mode check (SPEC-003 §7, H2) and therefore FAILS CLOSED in
|
||||
// the other direction: an empty or unparseable value is NOT loopback.
|
||||
//
|
||||
// A bind-time check alone is not enough (D-025): a reverse proxy on 127.0.0.1
|
||||
// relaying remote clients makes `/setup` reachable past the bind check, so the
|
||||
// door must re-check every request.
|
||||
func IsLoopbackIP(ip string) bool {
|
||||
loopback, known := classifyHost(ip)
|
||||
return known && loopback
|
||||
}
|
||||
|
||||
// classifyHost reports whether hostport ("host", "host:port", or a bare IP) is
|
||||
// loopback, and whether it could be classified at all. The two callers above
|
||||
// resolve "unknown" in opposite directions on purpose: each is fail-closed for
|
||||
// its own decision.
|
||||
//
|
||||
// "Unknown" is only an empty/blank value: a NAME that is not `localhost` is
|
||||
// known — and it is not loopback.
|
||||
func classifyHost(hostport string) (loopback, known bool) {
|
||||
host := strings.TrimSpace(hostport)
|
||||
if host == "" {
|
||||
return false, false
|
||||
}
|
||||
if h, _, err := net.SplitHostPort(host); err == nil {
|
||||
host = h
|
||||
}
|
||||
host = strings.Trim(strings.TrimSpace(host), "[]")
|
||||
if host == "" {
|
||||
return true
|
||||
return false, false
|
||||
}
|
||||
if strings.EqualFold(host, "localhost") {
|
||||
return true
|
||||
if strings.EqualFold(host, "localhost") || strings.EqualFold(host, "localhost.") {
|
||||
return true, true
|
||||
}
|
||||
ip := net.ParseIP(host)
|
||||
return ip != nil && ip.IsLoopback()
|
||||
if ip := net.ParseIP(host); ip != nil {
|
||||
return ip.IsLoopback(), true
|
||||
}
|
||||
// A name we cannot map to loopback: known, and not loopback.
|
||||
return false, true
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue