diff --git a/management/internals/shared/grpc/proxy.go b/management/internals/shared/grpc/proxy.go index 40b0914ef..cee50b270 100644 --- a/management/internals/shared/grpc/proxy.go +++ b/management/internals/shared/grpc/proxy.go @@ -1311,7 +1311,7 @@ func (s *ProxyServiceServer) authenticateHeader(ctx context.Context, serviceID s lastErr = err continue } - return true, "header-user", proxyauth.MethodHeader + return true, proxyauth.HeaderUserID, proxyauth.MethodHeader } if lastErr != nil { diff --git a/proxy/auth/auth.go b/proxy/auth/auth.go index 78f0097d5..5512bf003 100644 --- a/proxy/auth/auth.go +++ b/proxy/auth/auth.go @@ -30,6 +30,12 @@ const ( SessionJWTIssuer = "netbird-management" ) +// HeaderUserID is the synthetic user id recorded for header-authenticated +// requests. Header auth validates a per-service secret and resolves no user +// record, so proxy access logs and management-minted session tokens both +// attribute the request to this id. +const HeaderUserID = "header-user" + // ResolveProto determines the protocol scheme based on the forwarded proto // configuration. When set to "http" or "https" the value is used directly. // Otherwise TLS state is used: if conn is non-nil "https" is returned, else "http". diff --git a/proxy/internal/auth/header.go b/proxy/internal/auth/header.go index 194800a49..64d5da8f1 100644 --- a/proxy/internal/auth/header.go +++ b/proxy/internal/auth/header.go @@ -1,36 +1,33 @@ package auth import ( + "crypto/sha256" "errors" - "fmt" "net/http" + "sync" "github.com/netbirdio/netbird/proxy/auth" - "github.com/netbirdio/netbird/proxy/internal/types" - "github.com/netbirdio/netbird/shared/management/proto" + "github.com/netbirdio/netbird/shared/hash/argon2id" ) -// ErrHeaderAuthFailed indicates that the header was present but the -// credential did not validate. Callers should return 401 instead of -// falling through to other auth schemes. -var ErrHeaderAuthFailed = errors.New("header authentication failed") - -// Header implements header-based authentication. The proxy checks for the -// configured header in each request and validates its value via gRPC. +// Header implements header-based authentication. The service mapping carries +// the argon2id hash of every value accepted for the header, so the proxy +// verifies the credential locally rather than round-tripping to management. type Header struct { - id types.ServiceID - accountId types.AccountID headerName string - client authenticator + hashes []string + verified *verifiedValues } -// NewHeader creates a Header authentication scheme for the given header name. -func NewHeader(client authenticator, id types.ServiceID, accountId types.AccountID, headerName string) Header { +// NewHeader creates a Header authentication scheme accepting any value whose +// argon2id hash appears in hashes. An empty hashes slice rejects every request +// carrying the header, so a mapping that arrived without its hashes fails +// closed instead of leaving the service unprotected. +func NewHeader(headerName string, hashes []string) Header { return Header{ - id: id, - accountId: accountId, - headerName: headerName, - client: client, + headerName: http.CanonicalHeaderKey(headerName), + hashes: hashes, + verified: &verifiedValues{seen: make(map[[32]byte]struct{}, len(hashes))}, } } @@ -39,31 +36,64 @@ func (Header) Type() auth.Method { return auth.MethodHeader } -// Authenticate checks for the configured header in the request. If absent, -// returns empty (unauthenticated). If present, validates via gRPC. -func (h Header) Authenticate(r *http.Request) (string, string, error) { +// Authenticate satisfies Scheme. Header credentials are resolved by Verify +// before the scheme loop runs, so a request that reaches here never carries +// the header and there is no credential to prompt for. +func (Header) Authenticate(*http.Request) (string, string, error) { + return "", "", nil +} + +// Verify reports whether the request carries the configured header and, when +// it does, whether the value matches one of the service's hashes. +// +// A non-nil unusable is a diagnostic rather than a request error: a stored hash +// could not be decoded, so no credential can ever match it and the header stays +// unauthenticatable until the service is saved again. Folding that into an +// ordinary mismatch would hide the misconfiguration behind a permanent 401. +func (h Header) Verify(r *http.Request) (present, matched bool, unusable error) { value := r.Header.Get(h.headerName) if value == "" { - return "", "", nil + return false, false, nil } - res, err := h.client.Authenticate(r.Context(), &proto.AuthenticateRequest{ - Id: string(h.id), - AccountId: string(h.accountId), - Request: &proto.AuthenticateRequest_HeaderAuth{ - HeaderAuth: &proto.HeaderAuthRequest{ - HeaderValue: value, - HeaderName: h.headerName, - }, - }, - }) - if err != nil { - return "", "", fmt.Errorf("authenticate header: %w", err) + digest := sha256.Sum256([]byte(value)) + if h.verified.has(digest) { + return true, true, nil } - if res.GetSuccess() { - return res.GetSessionToken(), "", nil + for _, hash := range h.hashes { + err := argon2id.Verify(value, hash) + if err == nil { + h.verified.add(digest) + return true, true, nil + } + if !errors.Is(err, argon2id.ErrMismatchedHashAndPassword) { + unusable = err + } } - - return "", "", ErrHeaderAuthFailed + return true, false, unusable +} + +// verifiedValues remembers which header values already passed argon2id +// verification. argon2id is deliberately expensive (19 MiB, two passes) and +// header credentials repeat on every request, so re-deriving per request would +// dominate the hot path. The set cannot outgrow the number of configured +// hashes, and a mapping update builds a fresh scheme with an empty set. +// Values are keyed by digest so the plaintext credential is not retained. +type verifiedValues struct { + mu sync.Mutex + seen map[[32]byte]struct{} +} + +func (v *verifiedValues) has(digest [32]byte) bool { + v.mu.Lock() + defer v.mu.Unlock() + _, ok := v.seen[digest] + return ok +} + +func (v *verifiedValues) add(digest [32]byte) { + v.mu.Lock() + defer v.mu.Unlock() + v.seen[digest] = struct{}{} } diff --git a/proxy/internal/auth/middleware.go b/proxy/internal/auth/middleware.go index 72630b085..8abdf2923 100644 --- a/proxy/internal/auth/middleware.go +++ b/proxy/internal/auth/middleware.go @@ -146,7 +146,7 @@ func (mw *Middleware) Protect(next http.Handler) http.Handler { return } - if mw.forwardWithHeaderAuth(w, r, host, config, next) { + if mw.forwardWithHeaderAuth(w, r, config, next) { return } @@ -325,6 +325,16 @@ func (mw *Middleware) forwardWithSessionCookie(w http.ResponseWriter, r *http.Re if err != nil { return false } + + // Header auth is checked per request against the mapping's hashes and mints + // no session, so a header-method token can only predate that. Honouring it + // would keep a rotated credential working until the token expired. + if method == auth.MethodHeader.String() { + mw.logger.WithField("host", host). + Debug("ignoring header-auth session cookie; the header is required on every request") + return false + } + if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil { cd.SetUserID(userID) cd.SetUserEmail(email) @@ -436,73 +446,44 @@ func isTunnelSourceIP(ip netip.Addr) bool { // forwardWithHeaderAuth checks for a Header auth scheme. If the header validates, // the request is forwarded directly (no redirect), which is important for API clients. -func (mw *Middleware) forwardWithHeaderAuth(w http.ResponseWriter, r *http.Request, host string, config DomainConfig, next http.Handler) bool { +func (mw *Middleware) forwardWithHeaderAuth(w http.ResponseWriter, r *http.Request, config DomainConfig, next http.Handler) bool { + var presented []string for _, scheme := range config.Schemes { hdr, ok := scheme.(Header) if !ok { continue } - handled := mw.tryHeaderScheme(w, r, host, config, hdr, next) - if handled { + present, matched, unusable := hdr.Verify(r) + if matched { + if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil { + cd.SetUserID(auth.HeaderUserID) + cd.SetAuthMethod(auth.MethodHeader.String()) + } + next.ServeHTTP(w, r) return true } + if unusable != nil { + mw.logger.WithFields(log.Fields{ + "host": r.Host, + "header": hdr.headerName, + }).WithError(unusable).Error("header auth: a configured hash cannot be decoded, so this header can never authenticate; re-save the service") + } + if present { + presented = append(presented, hdr.headerName) + } } - return false -} -func (mw *Middleware) tryHeaderScheme(w http.ResponseWriter, r *http.Request, host string, config DomainConfig, hdr Header, next http.Handler) bool { - token, _, err := hdr.Authenticate(r) - if err != nil { - return mw.handleHeaderAuthError(w, r, err) - } - if token == "" { + if len(presented) == 0 { return false } - result, err := mw.validateSessionToken(r.Context(), host, token, config.SessionPublicKey, auth.MethodHeader) - if err != nil { - setHeaderCapturedData(r.Context(), "", "", nil, nil) - status := http.StatusBadRequest - msg := "invalid session token" - if errors.Is(err, errValidationUnavailable) { - status = http.StatusBadGateway - msg = "authentication service unavailable" - } - http.Error(w, msg, status) - return true - } - - if !result.Valid { - setHeaderCapturedData(r.Context(), result.UserID, result.UserEmail, result.Groups, result.GroupNames) - http.Error(w, "Unauthorized", http.StatusUnauthorized) - return true - } - - setSessionCookie(w, token, config.SessionExpiration) - if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil { - cd.SetUserID(result.UserID) - cd.SetUserEmail(result.UserEmail) - cd.SetUserGroups(result.Groups) - cd.SetUserGroupNames(result.GroupNames) - cd.SetAuthMethod(auth.MethodHeader.String()) - } - - next.ServeHTTP(w, r) - return true -} - -func (mw *Middleware) handleHeaderAuthError(w http.ResponseWriter, r *http.Request, err error) bool { - if errors.Is(err, ErrHeaderAuthFailed) { - setHeaderCapturedData(r.Context(), "", "", nil, nil) - http.Error(w, "Unauthorized", http.StatusUnauthorized) - return true - } - mw.logger.WithField("scheme", "header").Warnf("header auth infrastructure error: %v", err) - if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil { - cd.SetOrigin(proxy.OriginAuth) - } - http.Error(w, "authentication service unavailable", http.StatusBadGateway) + mw.logger.WithFields(log.Fields{ + "host": r.Host, + "headers": presented, + }).Debug("header auth rejected: no presented header matched a configured hash") + setHeaderCapturedData(r.Context(), "", "", nil, nil) + http.Error(w, "Unauthorized", http.StatusUnauthorized) return true } diff --git a/proxy/internal/auth/middleware_test.go b/proxy/internal/auth/middleware_test.go index 6608c2b22..9220ce790 100644 --- a/proxy/internal/auth/middleware_test.go +++ b/proxy/internal/auth/middleware_test.go @@ -16,6 +16,7 @@ import ( "time" log "github.com/sirupsen/logrus" + logtest "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/grpc" @@ -25,6 +26,7 @@ import ( "github.com/netbirdio/netbird/proxy/internal/proxy" "github.com/netbirdio/netbird/proxy/internal/restrict" "github.com/netbirdio/netbird/proxy/internal/types" + "github.com/netbirdio/netbird/shared/hash/argon2id" "github.com/netbirdio/netbird/shared/management/proto" ) @@ -1023,38 +1025,24 @@ func TestProtect_OIDCWithOtherMethodShowsLoginPage(t *testing.T) { assert.Equal(t, http.StatusUnauthorized, rec.Code, "should show login page when multiple methods exist") } -// mockAuthenticator is a minimal mock for the authenticator gRPC interface -// used by the Header scheme. -type mockAuthenticator struct { - fn func(ctx context.Context, req *proto.AuthenticateRequest) (*proto.AuthenticateResponse, error) -} - -func (m *mockAuthenticator) Authenticate(ctx context.Context, in *proto.AuthenticateRequest, _ ...grpc.CallOption) (*proto.AuthenticateResponse, error) { - return m.fn(ctx, in) -} - -// newHeaderSchemeWithToken creates a Header scheme backed by a mock that -// returns a signed session token when the expected header value is provided. -func newHeaderSchemeWithToken(t *testing.T, kp *sessionkey.KeyPair, headerName, expectedValue string) Header { +// newHeaderScheme creates a Header scheme accepting each of the given values, +// hashed the way management hashes them before putting them on the mapping. +func newHeaderScheme(t *testing.T, headerName string, acceptedValues ...string) Header { t.Helper() - token, err := sessionkey.SignToken(kp.PrivateKey, "header-user", "", "example.com", auth.MethodHeader, nil, nil, time.Hour) - require.NoError(t, err) - - mock := &mockAuthenticator{fn: func(_ context.Context, req *proto.AuthenticateRequest) (*proto.AuthenticateResponse, error) { - ha := req.GetHeaderAuth() - if ha != nil && ha.GetHeaderValue() == expectedValue { - return &proto.AuthenticateResponse{Success: true, SessionToken: token}, nil - } - return &proto.AuthenticateResponse{Success: false}, nil - }} - return NewHeader(mock, "svc1", "acc1", headerName) + hashes := make([]string, 0, len(acceptedValues)) + for _, v := range acceptedValues { + hash, err := argon2id.Hash(v) + require.NoError(t, err, "hashing an accepted header value must succeed") + hashes = append(hashes, hash) + } + return NewHeader(headerName, hashes) } func TestProtect_HeaderAuth_ForwardsOnSuccess(t *testing.T) { mw := NewMiddleware(log.StandardLogger(), nil, nil) kp := generateTestKeyPair(t) - hdr := newHeaderSchemeWithToken(t, kp, "X-API-Key", "secret-key") + hdr := newHeaderScheme(t, "X-API-Key", "secret-key") require.NoError(t, mw.AddDomain("example.com", []Scheme{hdr}, kp.PublicKey, time.Hour, "acc1", "svc1", nil, false)) var backendCalled bool @@ -1075,19 +1063,12 @@ func TestProtect_HeaderAuth_ForwardsOnSuccess(t *testing.T) { assert.Equal(t, http.StatusOK, rec.Code) assert.Equal(t, "ok", rec.Body.String()) - // Session cookie should be set. - var sessionCookie *http.Cookie + // The credential rides on every request, so no session cookie is issued. for _, c := range rec.Result().Cookies() { - if c.Name == auth.SessionCookieName { - sessionCookie = c - break - } + assert.NotEqual(t, auth.SessionCookieName, c.Name, "header auth must not issue a session cookie") } - require.NotNil(t, sessionCookie, "session cookie should be set after successful header auth") - assert.True(t, sessionCookie.HttpOnly) - assert.True(t, sessionCookie.Secure) - assert.Equal(t, "header-user", capturedData.GetUserID()) + assert.Equal(t, auth.HeaderUserID, capturedData.GetUserID()) assert.Equal(t, "header", capturedData.GetAuthMethod()) } @@ -1095,7 +1076,7 @@ func TestProtect_HeaderAuth_MissingHeaderFallsThrough(t *testing.T) { mw := NewMiddleware(log.StandardLogger(), nil, nil) kp := generateTestKeyPair(t) - hdr := newHeaderSchemeWithToken(t, kp, "X-API-Key", "secret-key") + hdr := newHeaderScheme(t, "X-API-Key", "secret-key") // Also add a PIN scheme so we can verify fallthrough behavior. pinScheme := &stubScheme{method: auth.MethodPIN, promptID: "pin"} require.NoError(t, mw.AddDomain("example.com", []Scheme{hdr, pinScheme}, kp.PublicKey, time.Hour, "acc1", "svc1", nil, false)) @@ -1114,10 +1095,7 @@ func TestProtect_HeaderAuth_WrongValueReturns401(t *testing.T) { mw := NewMiddleware(log.StandardLogger(), nil, nil) kp := generateTestKeyPair(t) - mock := &mockAuthenticator{fn: func(_ context.Context, _ *proto.AuthenticateRequest) (*proto.AuthenticateResponse, error) { - return &proto.AuthenticateResponse{Success: false}, nil - }} - hdr := NewHeader(mock, "svc1", "acc1", "X-API-Key") + hdr := newHeaderScheme(t, "X-API-Key", "secret-key") require.NoError(t, mw.AddDomain("example.com", []Scheme{hdr}, kp.PublicKey, time.Hour, "acc1", "svc1", nil, false)) capturedData := proxy.NewCapturedData("") @@ -1131,93 +1109,282 @@ func TestProtect_HeaderAuth_WrongValueReturns401(t *testing.T) { assert.Equal(t, http.StatusUnauthorized, rec.Code) assert.Equal(t, "header", capturedData.GetAuthMethod()) + assert.Empty(t, hdr.verified.seen, "a rejected value must not be memoized") } -func TestProtect_HeaderAuth_InfraErrorReturns502(t *testing.T) { +// TestProtect_HeaderAuth_MatchesAnyConfiguredHeader covers a client that carries +// a valid credential on one configured header while also sending an unrelated +// value on another — an app-level Authorization alongside an API key, say. +// Schemes OR across header names, so the valid credential admits the request no +// matter which order the mapping happened to list the headers in. +func TestProtect_HeaderAuth_MatchesAnyConfiguredHeader(t *testing.T) { + tests := []struct { + name string + matchedLast bool + }{ + {name: "unmatched header listed first", matchedLast: true}, + {name: "matched header listed first", matchedLast: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mw := NewMiddleware(log.StandardLogger(), nil, nil) + kp := generateTestKeyPair(t) + + authz := newHeaderScheme(t, "Authorization", "Bearer proxy-secret") + apiKey := newHeaderScheme(t, "X-Api-Key", "secret-key") + schemes := []Scheme{apiKey, authz} + if tt.matchedLast { + schemes = []Scheme{authz, apiKey} + } + require.NoError(t, mw.AddDomain("example.com", schemes, kp.PublicKey, time.Hour, "acc1", "svc1", nil, false)) + + var backendCalled bool + handler := mw.Protect(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + backendCalled = true + w.WriteHeader(http.StatusOK) + })) + + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req.Header.Set("X-Api-Key", "secret-key") + req.Header.Set("Authorization", "Bearer app-level-token") + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.True(t, backendCalled, "a valid credential on one header must admit the request") + assert.Equal(t, http.StatusOK, rec.Code) + }) + } +} + +// TestProtect_HeaderAuth_RejectsWhenEveryPresentedHeaderFails is the other half +// of the OR: trying all schemes before rejecting must not turn into admitting a +// request that satisfied none of them. +func TestProtect_HeaderAuth_RejectsWhenEveryPresentedHeaderFails(t *testing.T) { mw := NewMiddleware(log.StandardLogger(), nil, nil) kp := generateTestKeyPair(t) - mock := &mockAuthenticator{fn: func(_ context.Context, _ *proto.AuthenticateRequest) (*proto.AuthenticateResponse, error) { - return nil, errors.New("gRPC unavailable") - }} - hdr := NewHeader(mock, "svc1", "acc1", "X-API-Key") - require.NoError(t, mw.AddDomain("example.com", []Scheme{hdr}, kp.PublicKey, time.Hour, "acc1", "svc1", nil, false)) - - handler := mw.Protect(newPassthroughHandler()) - - req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) - req.Header.Set("X-API-Key", "some-key") - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - - assert.Equal(t, http.StatusBadGateway, rec.Code) -} - -func TestProtect_HeaderAuth_SubsequentRequestUsesSessionCookie(t *testing.T) { - mw := NewMiddleware(log.StandardLogger(), nil, nil) - kp := generateTestKeyPair(t) - - hdr := newHeaderSchemeWithToken(t, kp, "X-API-Key", "secret-key") - require.NoError(t, mw.AddDomain("example.com", []Scheme{hdr}, kp.PublicKey, time.Hour, "acc1", "svc1", nil, false)) + authz := newHeaderScheme(t, "Authorization", "Bearer proxy-secret") + apiKey := newHeaderScheme(t, "X-Api-Key", "secret-key") + require.NoError(t, mw.AddDomain("example.com", []Scheme{authz, apiKey}, kp.PublicKey, time.Hour, "acc1", "svc1", nil, false)) + var backendCalled bool handler := mw.Protect(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + backendCalled = true + w.WriteHeader(http.StatusOK) + })) + + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req.Header.Set("X-Api-Key", "wrong-key") + req.Header.Set("Authorization", "Bearer wrong-token") + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.False(t, backendCalled) + assert.Equal(t, http.StatusUnauthorized, rec.Code) +} + +// TestProtect_HeaderAuth_ReportsUndecodableHash covers a stored hash the proxy +// cannot decode. No credential can ever match it, so the header is permanently +// unauthenticatable — an operator fault that has to surface loudly instead of +// hiding behind the same quiet 401 a wrong credential earns. +func TestProtect_HeaderAuth_ReportsUndecodableHash(t *testing.T) { + validHash, err := argon2id.Hash("secret-key") + require.NoError(t, err) + + tests := []struct { + name string + hashes []string + wantErrLog bool + }{ + {name: "stored hash cannot be decoded", hashes: []string{"$argon2id$v=19$garbage"}, wantErrLog: true}, + {name: "wrong credential against a good hash", hashes: []string{validHash}, wantErrLog: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + logger, hook := logtest.NewNullLogger() + logger.SetLevel(log.DebugLevel) + mw := NewMiddleware(logger, nil, nil) + kp := generateTestKeyPair(t) + + require.NoError(t, mw.AddDomain("example.com", []Scheme{NewHeader("X-Api-Key", tt.hashes)}, + kp.PublicKey, time.Hour, "acc1", "svc1", nil, false)) + + handler := mw.Protect(newPassthroughHandler()) + + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req.Header.Set("X-Api-Key", "wrong-key") + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + require.Equal(t, http.StatusUnauthorized, rec.Code, "either way the request is denied") + + var errored []string + for _, entry := range hook.AllEntries() { + if entry.Level == log.ErrorLevel { + errored = append(errored, entry.Message) + } + } + + if !tt.wantErrLog { + assert.Empty(t, errored, "a wrong credential is not an operator fault") + return + } + require.Len(t, errored, 1, "an undecodable hash must be reported once") + assert.Contains(t, errored[0], "cannot be decoded") + }) + } +} + +// TestProtect_HeaderAuth_NoHashesFailsClosed covers a mapping that names a +// header but carries no hash for it: the check cannot be evaluated, so the +// request must be denied rather than let through unauthenticated. +func TestProtect_HeaderAuth_NoHashesFailsClosed(t *testing.T) { + mw := NewMiddleware(log.StandardLogger(), nil, nil) + kp := generateTestKeyPair(t) + + hdr := NewHeader("X-API-Key", nil) + require.NoError(t, mw.AddDomain("example.com", []Scheme{hdr}, kp.PublicKey, time.Hour, "acc1", "svc1", nil, false)) + + var backendCalled bool + handler := mw.Protect(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + backendCalled = true + w.WriteHeader(http.StatusOK) + })) + + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req.Header.Set("X-API-Key", "any-key") + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusUnauthorized, rec.Code) + assert.False(t, backendCalled, "a header auth with no hashes must not admit the request") +} + +// TestProtect_HeaderAuth_SubsequentRequestRequiresHeader verifies that header +// auth grants no ambient session: a follow-up request that drops the header is +// treated as unauthenticated. +func TestProtect_HeaderAuth_SubsequentRequestRequiresHeader(t *testing.T) { + mw := NewMiddleware(log.StandardLogger(), nil, nil) + kp := generateTestKeyPair(t) + + hdr := newHeaderScheme(t, "X-API-Key", "secret-key") + require.NoError(t, mw.AddDomain("example.com", []Scheme{hdr}, kp.PublicKey, time.Hour, "acc1", "svc1", nil, false)) + + var backendCalls int + handler := mw.Protect(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + backendCalls++ w.WriteHeader(http.StatusOK) })) - // First request with header auth. req1 := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) req1.Header.Set("X-API-Key", "secret-key") req1 = req1.WithContext(proxy.WithCapturedData(req1.Context(), proxy.NewCapturedData(""))) rec1 := httptest.NewRecorder() handler.ServeHTTP(rec1, req1) require.Equal(t, http.StatusOK, rec1.Code) + require.Equal(t, 1, backendCalls) - // Extract session cookie. - var sessionCookie *http.Cookie - for _, c := range rec1.Result().Cookies() { - if c.Name == auth.SessionCookieName { - sessionCookie = c - break - } - } - require.NotNil(t, sessionCookie) - - // Second request with only the session cookie (no header). - capturedData2 := proxy.NewCapturedData("") + // Same client, second request, header omitted: no cookie was handed out, so + // there is nothing to carry the earlier success forward. req2 := httptest.NewRequest(http.MethodGet, "http://example.com/other", nil) - req2.AddCookie(sessionCookie) - req2 = req2.WithContext(proxy.WithCapturedData(req2.Context(), capturedData2)) + for _, c := range rec1.Result().Cookies() { + req2.AddCookie(c) + } rec2 := httptest.NewRecorder() handler.ServeHTTP(rec2, req2) - assert.Equal(t, http.StatusOK, rec2.Code) - assert.Equal(t, "header-user", capturedData2.GetUserID()) - assert.Equal(t, "header", capturedData2.GetAuthMethod()) + assert.Equal(t, http.StatusUnauthorized, rec2.Code, "dropping the header must revoke access") + assert.Equal(t, 1, backendCalls, "backend must not be reached without the header") } -// TestProtect_HeaderAuth_MultipleValuesSameHeader verifies that the proxy -// correctly handles multiple valid credentials for the same header name. -// In production, the mgmt gRPC authenticateHeader iterates all configured -// header auths and accepts if any hash matches (OR semantics). The proxy -// creates one Header scheme per entry, but a single gRPC call checks all. +// TestProtect_HeaderAuth_LegacySessionCookieIsIgnored covers the upgrade +// window. Header auth used to mint a session token, so cookies with +// method=header survive a proxy upgrade and stay signature-valid for their full +// lifetime. They must not stand in for the header, or a credential rotated +// right after the upgrade would keep working until every such token expired. +func TestProtect_HeaderAuth_LegacySessionCookieIsIgnored(t *testing.T) { + mw := NewMiddleware(log.StandardLogger(), nil, nil) + kp := generateTestKeyPair(t) + + hdr := newHeaderScheme(t, "X-API-Key", "secret-key") + require.NoError(t, mw.AddDomain("example.com", []Scheme{hdr}, kp.PublicKey, time.Hour, "acc1", "svc1", nil, false)) + + // A token management would have minted for header auth before the upgrade. + legacyToken, err := sessionkey.SignToken(kp.PrivateKey, auth.HeaderUserID, "", "example.com", auth.MethodHeader, nil, nil, time.Hour) + require.NoError(t, err) + + var backendCalls int + handler := mw.Protect(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + backendCalls++ + w.WriteHeader(http.StatusOK) + })) + + t.Run("cookie alone is rejected", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req.AddCookie(&http.Cookie{Name: auth.SessionCookieName, Value: legacyToken}) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusUnauthorized, rec.Code, "a header-auth cookie must not authenticate on its own") + assert.Equal(t, 0, backendCalls, "backend must not be reached without the header") + }) + + t.Run("cookie does not block the header path", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req.AddCookie(&http.Cookie{Name: auth.SessionCookieName, Value: legacyToken}) + req.Header.Set("X-API-Key", "secret-key") + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code, "a client sending both must still be admitted by the header") + assert.Equal(t, 1, backendCalls) + }) +} + +// TestProtect_HeaderAuth_RepeatedValueIsMemoized verifies the KDF is run once +// per distinct accepted value. argon2id is deliberately expensive, so a +// credential that repeats on every request must not be re-derived each time. +func TestProtect_HeaderAuth_RepeatedValueIsMemoized(t *testing.T) { + mw := NewMiddleware(log.StandardLogger(), nil, nil) + kp := generateTestKeyPair(t) + + hdr := newHeaderScheme(t, "X-API-Key", "key-a", "key-b") + require.NoError(t, mw.AddDomain("example.com", []Scheme{hdr}, kp.PublicKey, time.Hour, "acc1", "svc1", nil, false)) + + handler := mw.Protect(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + + get := func(value string) int { + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req.Header.Set("X-API-Key", value) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + return rec.Code + } + + require.Equal(t, http.StatusOK, get("key-a")) + require.Equal(t, http.StatusOK, get("key-a")) + assert.Len(t, hdr.verified.seen, 1, "the same value must be memoized once") + + require.Equal(t, http.StatusOK, get("key-b")) + assert.Len(t, hdr.verified.seen, 2, "each accepted value gets its own entry") + + require.Equal(t, http.StatusUnauthorized, get("key-c")) + assert.Len(t, hdr.verified.seen, 2, "rejected values must not grow the set") +} + +// TestProtect_HeaderAuth_MultipleValuesSameHeader verifies that a service with +// several accepted credentials for one header name accepts any of them. +// Management applied these OR semantics while it still validated the value; the +// proxy preserves them by carrying every hash for a name on one scheme. func TestProtect_HeaderAuth_MultipleValuesSameHeader(t *testing.T) { mw := NewMiddleware(log.StandardLogger(), nil, nil) kp := generateTestKeyPair(t) - // Mock simulates mgmt behavior: accepts either token-a or token-b. - accepted := map[string]bool{"Bearer token-a": true, "Bearer token-b": true} - mock := &mockAuthenticator{fn: func(_ context.Context, req *proto.AuthenticateRequest) (*proto.AuthenticateResponse, error) { - ha := req.GetHeaderAuth() - if ha != nil && accepted[ha.GetHeaderValue()] { - token, err := sessionkey.SignToken(kp.PrivateKey, "header-user", "", "example.com", auth.MethodHeader, nil, nil, time.Hour) - require.NoError(t, err) - return &proto.AuthenticateResponse{Success: true, SessionToken: token}, nil - } - return &proto.AuthenticateResponse{Success: false}, nil - }} - - // Single Header scheme (as if one entry existed), but the mock checks both values. - hdr := NewHeader(mock, "svc1", "acc1", "Authorization") + hdr := newHeaderScheme(t, "Authorization", "Bearer token-a", "Bearer token-b") require.NoError(t, mw.AddDomain("example.com", []Scheme{hdr}, kp.PublicKey, time.Hour, "acc1", "svc1", nil, false)) var backendCalled bool diff --git a/proxy/server.go b/proxy/server.go index aee748339..38477fb87 100644 --- a/proxy/server.go +++ b/proxy/server.go @@ -20,6 +20,7 @@ import ( "net/url" "path/filepath" "reflect" + "slices" "sync" "time" @@ -2062,9 +2063,7 @@ func (s *Server) updateMapping(ctx context.Context, mapping *proto.ProxyMapping) if mapping.GetAuth().GetOidc() { schemes = append(schemes, auth.NewOIDC(s.mgmtClient, svcID, accountID, s.ForwardedProto)) } - for _, ha := range mapping.GetAuth().GetHeaderAuths() { - schemes = append(schemes, auth.NewHeader(s.mgmtClient, svcID, accountID, ha.GetHeader())) - } + schemes = append(schemes, headerAuthSchemes(mapping.GetAuth().GetHeaderAuths())...) ipRestrictions := s.parseRestrictions(mapping) s.warnIfGeoUnavailable(mapping.GetDomain(), mapping.GetAccessRestrictions()) @@ -2088,6 +2087,32 @@ func (s *Server) updateMapping(ctx context.Context, mapping *proto.ProxyMapping) return nil } +// headerAuthSchemes builds one scheme per canonical header name, carrying every +// hash configured for that name so any of them is accepted — the OR semantics +// management applied while it still validated the credential itself. No entry is +// ever dropped: a name that arrives blank, or without a hash, still yields a +// scheme, because a mapping that lost its only scheme would fall through +// Protect's no-schemes pass-through and serve the domain unauthenticated. +func headerAuthSchemes(headerAuths []*proto.HeaderAuth) []auth.Scheme { + names := make([]string, 0, len(headerAuths)) + hashes := make(map[string][]string, len(headerAuths)) + for _, ha := range headerAuths { + name := http.CanonicalHeaderKey(ha.GetHeader()) + if !slices.Contains(names, name) { + names = append(names, name) + } + if hash := ha.GetHashedValue(); hash != "" { + hashes[name] = append(hashes[name], hash) + } + } + + schemes := make([]auth.Scheme, 0, len(names)) + for _, name := range names { + schemes = append(schemes, auth.NewHeader(name, hashes[name])) + } + return schemes +} + // initMiddlewareManager wires the middleware subsystem at boot. It configures // the per-process FactoryContext concrete middlewares consult, installs the // live-service check, and binds the resolver to the registry concrete diff --git a/proxy/server_test.go b/proxy/server_test.go index f0c4765db..9cef63b95 100644 --- a/proxy/server_test.go +++ b/proxy/server_test.go @@ -6,6 +6,8 @@ import ( "fmt" "io" "net" + "net/http" + "net/http/httptest" "testing" "time" @@ -15,8 +17,10 @@ import ( "go.opentelemetry.io/otel/metric/noop" "google.golang.org/grpc" + "github.com/netbirdio/netbird/proxy/internal/auth" proxymetrics "github.com/netbirdio/netbird/proxy/internal/metrics" "github.com/netbirdio/netbird/proxy/internal/types" + "github.com/netbirdio/netbird/shared/hash/argon2id" "github.com/netbirdio/netbird/shared/management/proto" ) @@ -209,6 +213,62 @@ func TestRedactMappingForLog_HandlesEmptyOrNilFields(t *testing.T) { assert.Empty(t, redacted.Path, "empty Path must remain empty") } +// headerSchemeAccepts reports whether the scheme admits value for headerName. +func headerSchemeAccepts(t *testing.T, scheme auth.Scheme, headerName, value string) bool { + t.Helper() + hdr, ok := scheme.(auth.Header) + require.True(t, ok, "header auths must produce Header schemes") + + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req.Header.Set(headerName, value) + _, matched, _ := hdr.Verify(req) + return matched +} + +func TestHeaderAuthSchemes_GroupsValuesByCanonicalHeaderName(t *testing.T) { + hashOf := func(v string) string { + hash, err := argon2id.Hash(v) + require.NoError(t, err) + return hash + } + + schemes := headerAuthSchemes([]*proto.HeaderAuth{ + {Header: "Authorization", HashedValue: hashOf("Bearer a")}, + {Header: "authorization", HashedValue: hashOf("Bearer b")}, + {Header: "X-Api-Key", HashedValue: hashOf("key-1")}, + }) + + require.Len(t, schemes, 2, "entries differing only in header-name case must collapse into one scheme") + + assert.True(t, headerSchemeAccepts(t, schemes[0], "Authorization", "Bearer a"), "first value for the header must be accepted") + assert.True(t, headerSchemeAccepts(t, schemes[0], "Authorization", "Bearer b"), "second value for the same header must be accepted") + assert.False(t, headerSchemeAccepts(t, schemes[0], "Authorization", "Bearer c"), "unconfigured value must be rejected") + assert.True(t, headerSchemeAccepts(t, schemes[1], "X-Api-Key", "key-1"), "a second header name keeps its own scheme") +} + +// TestHeaderAuthSchemes_MissingHashFailsClosed covers a mapping that names a +// header but carries no hash for it. Dropping the scheme would leave a service +// whose only auth is that header wide open, so the scheme is kept and denies. +func TestHeaderAuthSchemes_MissingHashFailsClosed(t *testing.T) { + schemes := headerAuthSchemes([]*proto.HeaderAuth{{Header: "X-Api-Key"}}) + + require.Len(t, schemes, 1, "a header without a hash must still register a scheme") + assert.False(t, headerSchemeAccepts(t, schemes[0], "X-Api-Key", "anything"), + "a header auth without a hash must reject every value") +} + +// TestHeaderAuthSchemes_BlankNameFailsClosed covers a mapping row whose header +// name is empty. Skipping it would leave a service whose only auth is that entry +// with no schemes at all, which Protect treats as an unprotected domain, so the +// entry is kept and the domain stays gated. +func TestHeaderAuthSchemes_BlankNameFailsClosed(t *testing.T) { + schemes := headerAuthSchemes([]*proto.HeaderAuth{{Header: "", HashedValue: "$argon2id$not-a-real-hash"}}) + + require.Len(t, schemes, 1, "a blank header name must still register a scheme") + assert.False(t, headerSchemeAccepts(t, schemes[0], "X-Api-Key", "anything"), + "a blank header auth must not admit any request") +} + type statusUpdateOnlyClient struct { proto.ProxyServiceClient }