fix(security): prevent silent inbound auth bypass in DistHTTPAuth (v20.1)#107
Merged
fix(security): prevent silent inbound auth bypass in DistHTTPAuth (v20.1)#107
Conversation
….0.1) Previously, configuring `DistHTTPAuth` with only `ClientSign` (no `Token` and no `ServerVerify`) flipped the internal `configured()` predicate to true — causing the node to sign outbound traffic — while `verify()` had no inbound material and silently accepted every request. An operator wiring only one side of an HMAC scheme would end up with a signed-out / open-in node that appeared authenticated. Changes: - Split the single `configured()` predicate into `inboundConfigured()` (Token or ServerVerify present) and an outbound-specific check inside `sign()`; `wrapAuth` now gates on `inboundConfigured()` only. - Add `DistHTTPAuth.validate()`, called in `NewDistMemory`, that returns `sentinel.ErrInsecureAuthConfig` when `ClientSign` is set without an inbound verifier and the operator has not opted in. - Add `DistHTTPAuth.AllowAnonymousInbound` as an explicit opt-in for asymmetric signed-out / open-in deployments (e.g. inbound gated by an L4 firewall or service mesh). - Add `sentinel.ErrInsecureAuthConfig` sentinel error. - Bump `github.com/valyala/fasthttp` to v1.71.0. - Add three new integration tests: `TestDistHTTPAuth_RejectsClientSignOnlyConfig`, `TestDistHTTPAuth_AnonymousInboundOptIn`, `TestDistHTTPAuth_TokenWithClientSignOverride`. - Document all changes in CHANGELOG.md under [2.0.1]. - Remove stale `//nolint:revive` directives from histogram collector test. BREAKING CHANGE: `NewDistMemory` now returns `sentinel.ErrInsecureAuthConfig` for the previously-accepted `ClientSign`-only config. Operators relying on that shape must either add `Token`/`ServerVerify` for inbound enforcement or set `AllowAnonymousInbound: true`.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens DistHTTPAuth to prevent a silent inbound authentication bypass when only outbound signing (ClientSign) is configured, and adds an explicit opt-in for intentionally open-inbound deployments.
Changes:
- Split auth enablement into inbound-only gating (
inboundConfigured()) and outbound signing behavior (sign()), so inbound auth wrapping is never enabled by outbound-only config. - Add constructor-time validation (
DistHTTPAuth.validate()inNewDistMemory) that rejectsClientSign-only configs unlessAllowAnonymousInboundis explicitly set, returningsentinel.ErrInsecureAuthConfig. - Add/adjust tests, changelog entry, and bump
fasthttpto v1.71.0.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/dist_http_auth_test.go |
Adds integration tests covering the new insecure-config rejection and the explicit anonymous-inbound opt-in behavior. |
pkg/stats/histogramcollector_test.go |
Cleans up revive nolint placement around forced GC in the memory soak test. |
pkg/backend/dist_memory.go |
Validates DistHTTPAuth during construction and fails fast on insecure configs. |
pkg/backend/dist_http_server.go |
Introduces AllowAnonymousInbound, splits inbound configuration detection, and updates auth wrapping to gate only on inbound config. |
internal/sentinel/sentinel.go |
Adds sentinel.ErrInsecureAuthConfig sentinel error. |
go.mod / go.sum |
Bumps indirect github.com/valyala/fasthttp dependency to v1.71.0. |
CHANGELOG.md |
Documents the security fix and the new opt-in flag under [2.0.1]. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+314
to
+324
| 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, | ||
| } |
Comment on lines
+354
to
+376
| // (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). |
|
|
||
| ## [Unreleased] | ||
|
|
||
| ## [2.0.1] — 2026-05-05 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, configuring
DistHTTPAuthwith onlyClientSign(noTokenand noServerVerify) flipped the internalconfigured()predicate to true — causing the node to sign outbound traffic — whileverify()had no inbound material and silently accepted every request. An operator wiring only one side of an HMAC scheme would end up with a signed-out / open-in node that appeared authenticated.Changes:
configured()predicate intoinboundConfigured()(Token or ServerVerify present) and an outbound-specific check insidesign();wrapAuthnow gates oninboundConfigured()only.DistHTTPAuth.validate(), called inNewDistMemory, that returnssentinel.ErrInsecureAuthConfigwhenClientSignis set without an inbound verifier and the operator has not opted in.DistHTTPAuth.AllowAnonymousInboundas an explicit opt-in for asymmetric signed-out / open-in deployments (e.g. inbound gated by an L4 firewall or service mesh).sentinel.ErrInsecureAuthConfigsentinel error.github.com/valyala/fasthttpto v1.71.0.TestDistHTTPAuth_RejectsClientSignOnlyConfig,TestDistHTTPAuth_AnonymousInboundOptIn,TestDistHTTPAuth_TokenWithClientSignOverride.//nolint:revivedirectives from histogram collector test.BREAKING CHANGE:
NewDistMemorynow returnssentinel.ErrInsecureAuthConfigfor the previously-acceptedClientSign-only config. Operators relying on that shape must either addToken/ServerVerifyfor inbound enforcement or setAllowAnonymousInbound: true.