diff --git a/CHANGELOG.md b/CHANGELOG.md index 896d508..0e29d4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,34 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +## [2.0.1] — 2026-05-05 + +### Security + +- **Fixed silent inbound auth bypass when `DistHTTPAuth.ClientSign` was + set without a matching inbound verifier.** Previously, a config of + `DistHTTPAuth{ClientSign: hmacSign}` flipped the internal `configured` + predicate to true (causing the auto-client to sign outbound traffic), + but `verify()` had no inbound material and silently allowed every + request — so an operator wiring half of an HMAC scheme could end up + with signed-out / open-in nodes that looked authenticated. The + internal predicate is now split into `inboundConfigured()` / + outbound-path checks, and `NewDistMemory` rejects this shape at + construction with `sentinel.ErrInsecureAuthConfig`. Operators who + legitimately want signed-out / open-in deployments (e.g. inbound is + gated by an L4 firewall or service mesh below this server) must opt + in via the new `DistHTTPAuth.AllowAnonymousInbound` field. All other + configurations (`Token`-only, `Token+ServerVerify`, `Token+ClientSign`, + `ServerVerify`-only) are unaffected. Reported by the post-tag + security review; addressed before any v2.0.0 public announcement. + +### Added + +- `DistHTTPAuth.AllowAnonymousInbound` — explicit opt-in for asymmetric + signed-out / open-in configurations. +- `sentinel.ErrInsecureAuthConfig` — surfaced from `NewDistMemory` when + the auth policy would silently disable inbound enforcement. + ## [2.0.0] — 2026-05-04 A modernization release. The headline themes: diff --git a/go.mod b/go.mod index 54f77f6..a14c7c7 100644 --- a/go.mod +++ b/go.mod @@ -30,7 +30,7 @@ require ( github.com/rogpeppe/go-internal v1.14.1 // indirect github.com/tinylib/msgp v1.6.4 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect - github.com/valyala/fasthttp v1.70.0 // indirect + github.com/valyala/fasthttp v1.71.0 // indirect go.uber.org/atomic v1.11.0 // indirect golang.org/x/crypto v0.50.0 // indirect golang.org/x/net v0.53.0 // indirect diff --git a/go.sum b/go.sum index 2f26566..987a387 100644 --- a/go.sum +++ b/go.sum @@ -63,8 +63,8 @@ github.com/ugorji/go/codec v1.3.1 h1:waO7eEiFDwidsBN6agj1vJQ4AG7lh2yqXyOXqhgQuyY github.com/ugorji/go/codec v1.3.1/go.mod h1:pRBVtBSKl77K30Bv8R2P+cLSGaTtex6fsA2Wjqmfxj4= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= -github.com/valyala/fasthttp v1.70.0 h1:LAhMGcWk13QZWm85+eg8ZBNbrq5mnkWFGbHMUJHIdXA= -github.com/valyala/fasthttp v1.70.0/go.mod h1:oDZEHHkJ/Buyklg6uURmYs19442zFSnCIfX3j1FY3pE= +github.com/valyala/fasthttp v1.71.0 h1:tepR7H+Guh9VUqxxcPggYi8R3lGUu2Rsdh+z7/FCY3k= +github.com/valyala/fasthttp v1.71.0/go.mod h1:z1sDUvOShhXq/C9mwH/fSm1Vb71tUJwmQdgkBrBNwnA= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/xyproto/randomstring v1.0.5 h1:YtlWPoRdgMu3NZtP45drfy1GKoojuR7hmRcnhZqKjWU= diff --git a/internal/sentinel/sentinel.go b/internal/sentinel/sentinel.go index 0a5a575..c77c6fb 100644 --- a/internal/sentinel/sentinel.go +++ b/internal/sentinel/sentinel.go @@ -72,6 +72,11 @@ var ( // ErrUnauthorized is returned when an HTTP request to the dist transport is missing or carries an invalid auth token. ErrUnauthorized = ewrap.New("unauthorized") + // ErrInsecureAuthConfig is returned by NewDistMemory when a DistHTTPAuth value would silently disable inbound + // authentication despite outbound signing being configured (ClientSign set with neither Token nor ServerVerify). + // Operators who genuinely want asymmetric auth must set DistHTTPAuth.AllowAnonymousInbound explicitly. + ErrInsecureAuthConfig = ewrap.New("dist HTTP auth: ClientSign without inbound verifier (set Token, ServerVerify, or AllowAnonymousInbound)") + // ErrTypeMismatch is returned by the typed cache wrapper when a stored value is not assertable to the wrapper's V parameter. ErrTypeMismatch = ewrap.New("cached value type mismatch") diff --git a/pkg/backend/dist_http_server.go b/pkg/backend/dist_http_server.go index df73ca6..051e3d1 100644 --- a/pkg/backend/dist_http_server.go +++ b/pkg/backend/dist_http_server.go @@ -49,24 +49,31 @@ type distHTTPServer struct { serveErr atomic.Pointer[error] } -// DistHTTPAuth configures bearer-token authentication for the dist HTTP -// server (inbound) and the auto-created HTTP client (outbound). Zero-value -// disables auth — current behavior. When configured, *all* dist endpoints -// (including /health) require a valid token; operators who want a public -// health endpoint can supply a custom ServerVerify that exempts that path. +// DistHTTPAuth configures authentication for the dist HTTP server +// (inbound) and the auto-created HTTP client (outbound). The two sides +// are independent: ServerVerify+Token govern inbound validation, while +// ClientSign+Token govern outbound signing. Zero-value disables both. // -// Most clusters need only Token: every node sets the same string, the -// server validates incoming Authorization: Bearer headers via -// constant-time compare, and the client sends the same header on every -// outgoing request. +// Symmetric clusters use Token alone: every node sets the same string, +// the server validates incoming `Authorization: Bearer ` via +// constant-time compare, and the client sends the same header. // -// ServerVerify and ClientSign are escape hatches for JWT, mTLS-derived -// identity, HMAC signing, etc. When set they fully replace the default -// token check / header injection. +// ServerVerify (inbound) and ClientSign (outbound) are escape hatches +// for JWT, mTLS-derived identity, HMAC signing, etc. When set, each +// fully replaces the corresponding Token-based default on its side. +// +// Asymmetric configs are valid but require explicit intent. In +// particular, setting ClientSign without any inbound verifier (Token +// or ServerVerify) is dangerous — the node would sign outbound traffic +// while accepting unauthenticated inbound. NewDistMemory rejects that +// shape with sentinel.ErrInsecureAuthConfig. Operators who genuinely +// want signed-out / open-in (e.g. inbound is gated by an L4 firewall +// or service mesh) must opt in via AllowAnonymousInbound. type DistHTTPAuth struct { - // Token is the shared bearer string. When set (and ServerVerify is - // nil), the server requires `Authorization: Bearer ` on every - // request. The auto-created client sends the same header. + // Token is the shared bearer string. When set, the server requires + // `Authorization: Bearer ` on every request (unless + // ServerVerify overrides) and the auto-created client sends the + // same header (unless ClientSign overrides). Token string // ServerVerify (optional) inspects each incoming request and returns // non-nil to reject with HTTP 401. Use for JWT, OAuth introspection, @@ -76,17 +83,46 @@ type DistHTTPAuth struct { // Use for HMAC signing, mTLS-derived headers, etc. When set it // replaces the default `Authorization: Bearer ` header. ClientSign func(*http.Request) error + // AllowAnonymousInbound permits this node to accept inbound requests + // without authentication when no inbound verifier is configured + // (neither Token nor ServerVerify) but ClientSign is. Without this + // flag, that combination is rejected at construction time to prevent + // silent inbound bypass when an operator wires only one side of an + // HMAC scheme. Setting this flag is an explicit acknowledgment that + // inbound traffic is protected at a layer below this server (L4 + // firewall, service mesh mTLS, etc.). + AllowAnonymousInbound bool +} + +// inboundConfigured reports whether server-side validation is active — +// drives whether incoming requests are auth-checked. ClientSign alone +// does NOT count: it is an outbound concern. Outbound signing has no +// equivalent predicate because sign() is already path-specific (it +// short-circuits when both Token and ClientSign are zero). +func (a DistHTTPAuth) inboundConfigured() bool { + return a.Token != "" || a.ServerVerify != nil } -// configured reports whether the auth policy is active. -func (a DistHTTPAuth) configured() bool { - return a.Token != "" || a.ServerVerify != nil || a.ClientSign != nil +// validate enforces the inbound/outbound coherence rules at construction +// time. Returns sentinel.ErrInsecureAuthConfig when ClientSign is set +// without any inbound verifier and the operator has not explicitly +// opted into anonymous inbound — the configuration shape that previously +// caused a silent inbound auth bypass. +func (a DistHTTPAuth) validate() error { + signOnly := a.ClientSign != nil && !a.inboundConfigured() + if signOnly && !a.AllowAnonymousInbound { + return sentinel.ErrInsecureAuthConfig + } + + return nil } -// verify validates the incoming request against the configured policy. -// Returns nil when the request is authorized, non-nil otherwise. The -// default (Token-only) check uses constant-time compare to defeat timing -// side-channels. +// verify validates the incoming request against the configured inbound +// policy. Returns nil when the request is authorized, non-nil otherwise. +// The default (Token-only) check uses constant-time compare to defeat +// timing side-channels. Callers must gate this behind inboundConfigured() +// — verify itself returns nil when no inbound check is configured, which +// is the intended behavior only when inbound is deliberately open. func (a DistHTTPAuth) verify(fctx fiber.Ctx) error { if a.ServerVerify != nil { return a.ServerVerify(fctx) @@ -255,10 +291,15 @@ func (s *distHTTPServer) LastServeError() error { } // wrapAuth returns an auth-checking wrapper around the supplied handler -// when the server's auth policy is configured; otherwise returns the -// handler untouched (zero overhead for unauthenticated deployments). +// when the server's *inbound* auth policy is configured; otherwise +// returns the handler untouched (zero overhead for unauthenticated +// deployments). Outbound-only configs (ClientSign without Token or +// ServerVerify) intentionally fall through to the bare handler — that +// shape is rejected at NewDistMemory unless AllowAnonymousInbound is +// set, which is the operator's explicit acknowledgment that inbound is +// protected by a layer below this server. func (s *distHTTPServer) wrapAuth(handler fiber.Handler) fiber.Handler { - if !s.auth.configured() { + if !s.auth.inboundConfigured() { return handler } diff --git a/pkg/backend/dist_memory.go b/pkg/backend/dist_memory.go index 90f64e6..97679fc 100644 --- a/pkg/backend/dist_memory.go +++ b/pkg/backend/dist_memory.go @@ -616,6 +616,11 @@ func WithDistHTTPLimits(limits DistHTTPLimits) DistMemoryOption { // requests with HTTP 401. Like WithDistHTTPLimits this only affects the // internal transport; an externally-supplied DistTransport is the // caller's responsibility to authenticate. +// +// NewDistMemory validates the resulting policy and returns +// sentinel.ErrInsecureAuthConfig if ClientSign is set without a +// matching inbound verifier (Token or ServerVerify) and +// AllowAnonymousInbound is not set — see DistHTTPAuth for the rationale. func WithDistHTTPAuth(auth DistHTTPAuth) DistMemoryOption { return func(dm *DistMemory) { dm.httpAuth = auth } } @@ -643,6 +648,17 @@ func NewDistMemory(ctx context.Context, opts ...DistMemoryOption) (IBackend[Dist opt(dm) } + // Reject incoherent auth configs (e.g. ClientSign-only) before + // any subsystem captures the policy. validate returns + // sentinel.ErrInsecureAuthConfig for the misconfiguration that + // previously caused silent inbound bypass. + authErr := dm.httpAuth.validate() + if authErr != nil { + lifeCancel() + + return nil, authErr + } + dm.ensureShardConfig() dm.initMembershipIfNeeded() // Pass the lifecycle ctx to subsystems that capture it (HTTP handlers, diff --git a/pkg/stats/histogramcollector_test.go b/pkg/stats/histogramcollector_test.go index 479d6f1..a457e7b 100644 --- a/pkg/stats/histogramcollector_test.go +++ b/pkg/stats/histogramcollector_test.go @@ -267,6 +267,8 @@ func TestHistogramStatsCollector_GetStatsSnapshotIsolated(t *testing.T) { // TestHistogramStatsCollector_NoMemoryLeak verifies the bounded sample window // keeps memory usage flat under sustained recording. The previous // implementation appended forever and would grow unbounded. +// +//nolint:revive func TestHistogramStatsCollector_NoMemoryLeak(t *testing.T) { t.Parallel() @@ -283,7 +285,8 @@ func TestHistogramStatsCollector_NoMemoryLeak(t *testing.T) { c.Histogram(constants.StatHistogram, int64(i)) } - runtime.GC() //nolint:revive + // Force a GC to clean up any garbage from priming the buffer, so we start with a clean slate. + runtime.GC() var before runtime.MemStats @@ -296,7 +299,7 @@ func TestHistogramStatsCollector_NoMemoryLeak(t *testing.T) { c.Histogram(constants.StatHistogram, int64(i)) } - runtime.GC() //nolint:revive + runtime.GC() var after runtime.MemStats diff --git a/tests/dist_http_auth_test.go b/tests/dist_http_auth_test.go index a7d8d9f..565714f 100644 --- a/tests/dist_http_auth_test.go +++ b/tests/dist_http_auth_test.go @@ -10,6 +10,7 @@ import ( fiber "github.com/gofiber/fiber/v3" + "github.com/hyp3rd/hypercache/internal/sentinel" "github.com/hyp3rd/hypercache/pkg/backend" cache "github.com/hyp3rd/hypercache/pkg/cache/v2" ) @@ -268,6 +269,133 @@ func TestDistHTTPAuth_ClientSignsRequests(t *testing.T) { t.Fatalf("replication did not propagate to nodeB — client likely failed to sign requests") } +// errClientSignInvoked is the sentinel returned by the client-sign hook +// in TestDistHTTPAuth_RejectsClientSignOnlyConfig — a value the test +// can identify if the hook were ever invoked (it should not be: the +// constructor must reject before any HTTP traffic). +var errClientSignInvoked = errors.New("client sign hook should not run") + +// TestDistHTTPAuth_RejectsClientSignOnlyConfig pins the +// constructor-time guard that prevents the silent-inbound-bypass shape +// (CVE-style: ClientSign set, no Token, no ServerVerify, no opt-in). +// Without this guard the dist HTTP server would have signed outbound +// traffic while accepting unauthenticated inbound — see +// sentinel.ErrInsecureAuthConfig for the rationale. +func TestDistHTTPAuth_RejectsClientSignOnlyConfig(t *testing.T) { + t.Parallel() + + addr := AllocatePort(t) + + bi, err := backend.NewDistMemory(context.Background(), + backend.WithDistNode("auth-reject", addr), + backend.WithDistReplication(1), + backend.WithDistHTTPAuth(backend.DistHTTPAuth{ + ClientSign: func(*http.Request) error { return errClientSignInvoked }, + }), + ) + if !errors.Is(err, sentinel.ErrInsecureAuthConfig) { + t.Fatalf("expected ErrInsecureAuthConfig, got err=%v bi=%v", err, bi) + } + + if bi != nil { + t.Fatalf("expected nil backend on validation failure, got %T", bi) + } +} + +// TestDistHTTPAuth_AnonymousInboundOptIn confirms operators can +// deliberately wire signed-out / open-in deployments by setting +// AllowAnonymousInbound — used when an L4 firewall or service mesh +// gates inbound at a layer below this server. The server must accept +// anonymous /internal/* requests while the auto-client still signs +// outbound. +func TestDistHTTPAuth_AnonymousInboundOptIn(t *testing.T) { + t.Parallel() + + var signCalls atomic.Int64 + + auth := backend.DistHTTPAuth{ + ClientSign: func(req *http.Request) error { + signCalls.Add(1) + req.Header.Set("X-Asymmetric-Sig", "ok") + + return nil + }, + AllowAnonymousInbound: true, + } + + dm := newAuthDistNode(t, auth) + + // Inbound /internal/get without any auth header must succeed + // (returns 404 not-owner because no key is set, but importantly + // not 401 — auth is skipped per the explicit opt-in). + req, err := http.NewRequestWithContext( + context.Background(), + http.MethodGet, + "http://"+dm.LocalNodeAddr()+"/internal/get?key=anything", + nil, + ) + if err != nil { + t.Fatalf("build request: %v", err) + } + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("do request: %v", err) + } + + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode == http.StatusUnauthorized { + t.Fatalf("AllowAnonymousInbound did not skip auth wrap; got 401") + } +} + +// TestDistHTTPAuth_TokenWithClientSignOverride covers the asymmetric +// (but valid) shape where Token validates inbound and ClientSign +// overrides the default outbound header — e.g. a node fronting an HMAC +// peer mesh while still gating its own inbound on a shared bearer. +func TestDistHTTPAuth_TokenWithClientSignOverride(t *testing.T) { + t.Parallel() + + var signCalls atomic.Int64 + + auth := backend.DistHTTPAuth{ + Token: authTestToken, + ClientSign: func(req *http.Request) error { + signCalls.Add(1) + req.Header.Set("Authorization", "Bearer "+authTestToken) + + return nil + }, + } + + // Construction must succeed — Token covers inbound, ClientSign + // overrides outbound, no insecure shape. + dm := newAuthDistNode(t, auth) + + // Inbound without a token still 401s (Token-driven inbound). + req, err := http.NewRequestWithContext( + context.Background(), + http.MethodGet, + "http://"+dm.LocalNodeAddr()+"/internal/get?key=anything", + nil, + ) + if err != nil { + t.Fatalf("build request: %v", err) + } + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("do request: %v", err) + } + + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusUnauthorized { + t.Fatalf("Token-inbound did not enforce; got %d", resp.StatusCode) + } +} + // TestDistHTTPAuth_CustomVerify proves the ServerVerify escape hatch is // invoked for every request and can deny on its own logic — used here // to allow /health while requiring the bearer token elsewhere.