diff --git a/service/internal/auth/authn.go b/service/internal/auth/authn.go index caedd9847b..860f92dc63 100644 --- a/service/internal/auth/authn.go +++ b/service/internal/auth/authn.go @@ -377,6 +377,17 @@ func (e *DPoPNonceMalformedError) Error() string { return e.Message } +// DPoPProofError marks a non-retryable DPoP proof rejection (tampered htu/htm, +// bad ath, replayed jti, malformed nonce). Handlers translate it into a +// WWW-Authenticate: DPoP error="invalid_dpop_proof" challenge per RFC 9449 §7.1. +type DPoPProofError struct { + err error +} + +func (e *DPoPProofError) Error() string { return e.err.Error() } + +func (e *DPoPProofError) Unwrap() error { return e.err } + func normalizeURL(o string, u *url.URL) string { // Currently this does not do a full normatlization ou, err := url.Parse(o) @@ -471,6 +482,22 @@ func (a Authentication) MuxHandler(handler http.Handler) http.Handler { http.Error(w, "unauthenticated", http.StatusUnauthorized) return } + // Other DPoP proof failures get an invalid_dpop_proof challenge (RFC 9449 §7.1). + var proofErr *DPoPProofError + if errors.As(err, &proofErr) { + if a.dpopNonceManager.requireNonce { + w.Header().Set("DPoP-Nonce", a.dpopNonceManager.getCurrentNonce()) + } + w.Header().Set("WWW-Authenticate", `DPoP error="invalid_dpop_proof"`) + log.WarnContext( + ctxWithAuthX, + "unauthenticated", + slog.Any("error", err), + slog.Any("dpop", dp), + ) + http.Error(w, "unauthenticated", http.StatusUnauthorized) + return + } log.WarnContext( ctxWithAuthX, "unauthenticated", @@ -616,6 +643,16 @@ func (a Authentication) ConnectAuthNInterceptor() connect.UnaryInterceptorFunc { connectErr.Meta().Set("WWW-Authenticate", `DPoP error="use_dpop_nonce"`) return nil, connectErr } + // Other DPoP proof failures get an invalid_dpop_proof challenge (RFC 9449 §7.1). + var proofErr *DPoPProofError + if errors.As(err, &proofErr) { + connectErr := connect.NewError(connect.CodeUnauthenticated, errors.New("unauthenticated")) + if a.dpopNonceManager.requireNonce { + connectErr.Meta().Set("DPoP-Nonce", a.dpopNonceManager.getCurrentNonce()) + } + connectErr.Meta().Set("WWW-Authenticate", `DPoP error="invalid_dpop_proof"`) + return nil, connectErr + } return nil, connect.NewError(connect.CodeUnauthenticated, errors.New("unauthenticated")) } @@ -1061,11 +1098,14 @@ func (a *Authentication) checkToken(ctx context.Context, authHeader []string, dp if err != nil { var nonceErr *DPoPNonceError if errors.As(err, &nonceErr) { + // Retryable nonce challenge: returned unwrapped so handlers issue use_dpop_nonce. a.logger.DebugContext(ctx, "dpop nonce challenge issued", slog.String("reason", nonceErr.Message)) - } else { - a.logger.WarnContext(ctx, "failed to validate dpop", slog.Any("err", err)) + return nil, nil, err } - return nil, nil, err + // Any other DPoP proof failure (tampered htu/htm, bad ath, replayed jti, + // malformed nonce) becomes an invalid_dpop_proof challenge. + a.logger.WarnContext(ctx, "failed to validate dpop", slog.Any("err", err)) + return nil, nil, &DPoPProofError{err: err} } ctx = ctxAuth.ContextWithAuthNInfo(ctx, dpopKey, accessToken, tokenRaw) return accessToken, ctx, nil diff --git a/service/internal/auth/authn_test.go b/service/internal/auth/authn_test.go index b55c97d461..b17aff60b3 100644 --- a/service/internal/auth/authn_test.go +++ b/service/internal/auth/authn_test.go @@ -844,6 +844,61 @@ func (s *AuthSuite) Test_ConnectAuthNInterceptor_RequiresHeaderWithExistingConte s.Require().ErrorAs(err, &connectErr) } +func (s *AuthSuite) Test_MuxHandler_DPoPProofError_IssuesInvalidProofChallenge() { + auth := s.newAuthDPoP(false) + rec := s.muxAuthErrorRecorder(auth, &DPoPProofError{err: errors.New("incorrect `htu` claim in DPoP JWT")}) + + s.Equal(http.StatusUnauthorized, rec.Code) + s.Equal(`DPoP error="invalid_dpop_proof"`, rec.Header().Get("WWW-Authenticate")) + // Without RequireNonce there is nothing to retry against, so no nonce is issued. + s.Empty(rec.Header().Get("DPoP-Nonce")) +} + +func (s *AuthSuite) Test_MuxHandler_DPoPProofError_IncludesNonceWhenRequired() { + auth := s.newAuthDPoP(true) + rec := s.muxAuthErrorRecorder(auth, &DPoPProofError{err: errors.New("DPoP proof replay detected")}) + + s.Equal(http.StatusUnauthorized, rec.Code) + s.Equal(`DPoP error="invalid_dpop_proof"`, rec.Header().Get("WWW-Authenticate")) + s.NotEmpty(rec.Header().Get("DPoP-Nonce"), "a fresh nonce aids the client's retry") +} + +func (s *AuthSuite) Test_MuxHandler_DPoPNonceError_IssuesUseNonceChallenge() { + auth := s.newAuthDPoP(true) + rec := s.muxAuthErrorRecorder(auth, &DPoPNonceError{Message: "nonce required for retry"}) + + s.Equal(http.StatusUnauthorized, rec.Code) + s.Equal(`DPoP error="use_dpop_nonce"`, rec.Header().Get("WWW-Authenticate")) + s.NotEmpty(rec.Header().Get("DPoP-Nonce")) +} + +func (s *AuthSuite) Test_ConnectAuthNInterceptor_DPoPProofError_IssuesInvalidProofChallenge() { + auth := s.newAuthDPoP(false) + connectErr := s.connectAuthError(auth, &DPoPProofError{err: errors.New("incorrect `htu` claim in DPoP JWT")}) + + s.Equal(connect.CodeUnauthenticated, connectErr.Code()) + s.Equal(`DPoP error="invalid_dpop_proof"`, connectErr.Meta().Get("WWW-Authenticate")) + s.Empty(connectErr.Meta().Get("DPoP-Nonce")) +} + +func (s *AuthSuite) Test_ConnectAuthNInterceptor_DPoPProofError_IncludesNonceWhenRequired() { + auth := s.newAuthDPoP(true) + connectErr := s.connectAuthError(auth, &DPoPProofError{err: errors.New("DPoP proof replay detected")}) + + s.Equal(connect.CodeUnauthenticated, connectErr.Code()) + s.Equal(`DPoP error="invalid_dpop_proof"`, connectErr.Meta().Get("WWW-Authenticate")) + s.NotEmpty(connectErr.Meta().Get("DPoP-Nonce"), "a fresh nonce aids the client's retry") +} + +func (s *AuthSuite) Test_ConnectAuthNInterceptor_DPoPNonceError_IssuesUseNonceChallenge() { + auth := s.newAuthDPoP(true) + connectErr := s.connectAuthError(auth, &DPoPNonceError{Message: "nonce required for retry"}) + + s.Equal(connect.CodeUnauthenticated, connectErr.Code()) + s.Equal(`DPoP error="use_dpop_nonce"`, connectErr.Meta().Get("WWW-Authenticate")) + s.NotEmpty(connectErr.Meta().Get("DPoP-Nonce")) +} + func (s *AuthSuite) Test_CheckToken_When_Authorization_Header_Invalid_Expect_Error() { _, _, err := s.auth.checkToken(context.Background(), []string{"BPOP "}, receiverInfo{}, nil) s.Require().Error(err) @@ -1441,6 +1496,48 @@ func (s *AuthSuite) Test_RoleRequestForConnectProcedure() { } } +// dpopChallengeRoute is a non-public route used by the DPoP challenge handler tests. +// checkToken is stubbed in those tests, so the exact procedure value is irrelevant. +const dpopChallengeRoute = "/dpop.test/Challenge" + +// muxAuthErrorRecorder drives MuxHandler with checkToken stubbed to return retErr and +// returns the recorded response. The request carries DPoP Authorization + proof headers. +func (s *AuthSuite) muxAuthErrorRecorder(auth *Authentication, retErr error) *httptest.ResponseRecorder { + auth._testCheckTokenFunc = func(context.Context, []string, receiverInfo, []string) (jwt.Token, context.Context, error) { + return nil, nil, retErr + } + handler := auth.MuxHandler(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, dpopChallengeRoute, nil) + req.Header.Set("Authorization", "DPoP token") + req.Header.Set("DPoP", "proof") + handler.ServeHTTP(rec, req) + return rec +} + +// connectAuthError drives ConnectAuthNInterceptor with checkToken stubbed to return retErr +// and returns the resulting *connect.Error. +func (s *AuthSuite) connectAuthError(auth *Authentication, retErr error) *connect.Error { + auth._testCheckTokenFunc = func(context.Context, []string, receiverInfo, []string) (jwt.Token, context.Context, error) { + return nil, nil, retErr + } + interceptor := auth.ConnectAuthNInterceptor() + next := func(context.Context, connect.AnyRequest) (connect.AnyResponse, error) { + return connect.NewResponse(&kas.RewrapResponse{}), nil + } + req := &authnTestRequest{ + Request: connect.NewRequest(&kas.RewrapRequest{}), + procedure: dpopChallengeRoute, + } + req.Header().Set("Authorization", "DPoP token") + req.Header().Set("DPoP", "proof") + _, err := interceptor(next)(s.T().Context(), req) + s.Require().Error(err) + var connectErr *connect.Error + s.Require().ErrorAs(err, &connectErr) + return connectErr +} + func Test_GetClientIDFromToken(t *testing.T) { tests := []struct { name string diff --git a/service/internal/auth/dpop_nonce_test.go b/service/internal/auth/dpop_nonce_test.go index 97f61d8ae1..3a4329cdb6 100644 --- a/service/internal/auth/dpop_nonce_test.go +++ b/service/internal/auth/dpop_nonce_test.go @@ -7,6 +7,7 @@ import ( "crypto/rsa" "crypto/sha256" "encoding/base64" + "errors" "net/http" "testing" "time" @@ -103,6 +104,31 @@ func TestDPoPNonceError(t *testing.T) { }) } +func TestDPoPProofError(t *testing.T) { + inner := errors.New("incorrect `htu` claim in DPoP JWT") + var err error = &DPoPProofError{err: inner} + + t.Run("error message delegates to inner", func(t *testing.T) { + // Handlers/tests rely on substring matching of the underlying error. + assert.Equal(t, inner.Error(), err.Error()) + }) + + t.Run("unwraps to inner error", func(t *testing.T) { + assert.ErrorIs(t, err, inner) + }) + + t.Run("detected via errors.As", func(t *testing.T) { + var proofErr *DPoPProofError + require.ErrorAs(t, err, &proofErr) + }) + + t.Run("does not match DPoPNonceError", func(t *testing.T) { + // A wrapped non-nonce failure must not be mistaken for a retryable challenge. + var nonceErr *DPoPNonceError + assert.NotErrorAs(t, err, &nonceErr) + }) +} + func TestDPoPAlgorithmRestrictions(t *testing.T) { testCases := []struct { alg jwa.SignatureAlgorithm @@ -131,6 +157,12 @@ func TestDPoPAlgorithmRestrictions(t *testing.T) { // newAuthWithNonce creates an Authentication using the suite's OIDC server with RequireNonce=true. func (s *AuthSuite) newAuthWithNonce() *Authentication { + return s.newAuthDPoP(true) +} + +// newAuthDPoP creates a DPoP-enforcing Authentication backed by the suite's OIDC server, +// with the nonce challenge toggled by requireNonce. +func (s *AuthSuite) newAuthDPoP(requireNonce bool) *Authentication { auth, err := NewAuthenticator( context.Background(), Config{ @@ -141,7 +173,7 @@ func (s *AuthSuite) newAuthWithNonce() *Authentication { DPoPSkew: time.Hour, TokenSkew: time.Minute, DPoP: DPoPConfig{ - RequireNonce: true, + RequireNonce: requireNonce, NonceExpiration: 5 * time.Minute, StrictHTU: false, }, @@ -285,6 +317,10 @@ func (s *AuthSuite) TestDPoP_MalformedNonce_Returns_DPoPNonceMalformedError() { // Confirm it does NOT match DPoPNonceError, so handlers hard-reject rather than issue a challenge. var nonceErr *DPoPNonceError s.Require().NotErrorAs(err, &nonceErr) + + // checkToken wraps it in DPoPProofError so handlers issue an invalid_dpop_proof challenge. + var proofErr *DPoPProofError + s.Require().ErrorAs(err, &proofErr) } func (s *AuthSuite) TestDPoP_WrongNonce_Returns_DPoPNonceError() {