diff --git a/README.md b/README.md index 6bae169d..12c29afa 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,7 @@ Here is json equivalent for one of the default users | Device Authorization | yes | yes | [RFC 8628][10] | | mTLS | not yet | not yet | [RFC 8705][11] | | Back-Channel Logout | not yet | yes | OpenID Connect [Back-Channel Logout][12] 1.0 | +| Key Binding | yes | yes | OpenID Connect [Key Binding][13] 1.0 | [1]: https://openid.net/specs/openid-connect-core-1_0.html#CodeFlowAuth "3.1. Authentication using the Authorization Code Flow" [2]: https://openid.net/specs/openid-connect-core-1_0.html#ImplicitFlowAuth "3.2. Authentication using the Implicit Flow" @@ -163,6 +164,7 @@ Here is json equivalent for one of the default users [10]: https://www.rfc-editor.org/rfc/rfc8628.html "OAuth 2.0 Device Authorization Grant" [11]: https://www.rfc-editor.org/rfc/rfc8705.html "OAuth 2.0 Mutual-TLS Client Authentication and Certificate-Bound Access Tokens" [12]: https://openid.net/specs/openid-connect-backchannel-1_0.html "OpenID Connect Back-Channel Logout 1.0 incorporating errata set 1" +[13]: https://openid.net/specs/openid-connect-key-binding-1_0.html "OpenID Connect Key Binding 1.0 (draft 02)" ## Contributors diff --git a/UPGRADING.md b/UPGRADING.md index fba270d8..7905e5c3 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -5,6 +5,75 @@ All commands are executed from the root of the project that imports oidc package on non-GNU systems, such as MacOS. Alternatively, GNU sed can be installed on such systems. (`coreutils` package?). +## OpenID Connect Key Binding + +Key Binding is opt-in and off by default. Existing OPs and RPs need no changes: an OP only performs key binding once its storage implements `op.BoundKeyRequest`, so a storage predating this feature is unaffected even if its `Client.IsScopeAllowed` happens to permit the `bound_key` scope. + +Key Binding 1.0 is still an OpenID draft, so the API is marked `EXPERIMENTAL: may change until v4`. + +### Relying party + +Pass a `crypto.Signer` (including KMS- or HSM-backed ones) and its algorithm: + +```go +provider, err := rp.NewRelyingPartyOIDC(ctx, issuer, clientID, clientSecret, redirectURI, + scopes, rp.WithKeyBinding(signer, jose.ES256)) +``` + +The RP then appends the `bound_key` scope, sends `dpop_jkt` on the Authentication Request, signs a DPoP proof for the code and refresh token requests, and verifies that the returned ID Token is actually bound to `signer`. A stripped binding is an error rather than a silent downgrade to a bearer ID Token. + +The Device Authorization flow is supported as well: `rp.DeviceAuthorization` adds `bound_key` and `dpop_jkt`, and `rp.DeviceAccessToken` signs a proof bound to the `device_code` on every poll and verifies the returned ID Token. No code change is needed beyond `WithKeyBinding`. + +### OpenID Provider + +Two changes are required. + +1. Enable the feature, which advertises the `bound_key` scope in `scopes_supported` and populates `dpop_signing_alg_values_supported`: + +```go +provider, err := op.NewOpenIDProvider(issuer, config, storage, op.WithKeyBinding()) +``` + + Adding `oidc.ScopeBoundKey` to `op.Config.SupportedScopes` yourself is equivalent for discovery purposes. `op.WithKeyBinding()` is preferred because it also validates the storage at construction (see the device flow below). + +2. Persist `dpop_jkt` and expose it from your stored authorization request and refresh token request types by implementing the optional `op.BoundKeyRequest` interface: + +```go +type BoundKeyRequest interface { + GetDPoPJKT() string +} +``` + +Store `oidc.AuthRequest.DPoPJKT` when the Authentication Request is created and carry it across refresh token rotation. See `example/server/storage` for a complete implementation. + +Implementing `op.BoundKeyRequest` is the signal that your storage can honour a binding: + +- If the persisted request does not implement `op.BoundKeyRequest`, key binding is not integrated, so `bound_key` is ignored and an ordinary bearer ID Token is issued. Section 2.1 of the specification permits this, and a key-binding RP still fails closed because it verifies the returned `typ` and `cnf`. This is deliberate: `bound_key` is granted via `Client.IsScopeAllowed`, and a permissive implementation written before this scope existed may grant scopes it knows nothing about. Such an OP must not start returning errors. +- If the persisted request implements it but returns an empty or malformed `GetDPoPJKT()` for a request granted `bound_key`, that is a bug in the OP, not an unsupported feature, so the token endpoint fails with `server_error` rather than issuing an unbound token. + +The same check is enforced centrally in `CreateIDToken`, so flows that do not implement key binding, i.e. Token Exchange and the implicit/hybrid flows, reject a bound request rather than returning an unbound ID Token. + +### Device Authorization flow + +If you support the `device_code` grant and use `op.WithKeyBinding()`, your storage must also implement `op.BoundKeyDeviceAuthorizationStorage`. This is checked at construction and `op.NewOpenIDProvider` returns an error rather than letting the OP start in a state where key-bound device requests fail. + +```go +StoreBoundKeyDeviceAuthorization(ctx context.Context, clientID, deviceCode, userCode string, + expires time.Time, scopes []string, dpopJKT string) error +``` + +This exists as a separate optional interface because `StoreDeviceAuthorization` cannot gain a parameter without breaking every implementation. Persist `dpopJKT` and return it from `op.DeviceAuthorizationState.DPoPJKT` in `GetDeviceAuthorizatonState`; the proof is bound to the `device_code` (`c_s256 = SHA256(device_code)`). + +Implementing this interface is what enables key binding on the device flow, just as `op.BoundKeyRequest` does for the code and refresh flows. If the storage does not implement it, a `bound_key` device authorization request is downgraded to an ordinary unbound one: both `bound_key` and `dpop_jkt` are dropped before the authorization is stored. The scope has to be dropped along with the thumbprint because `op.DeviceAuthorizationState` always satisfies `op.BoundKeyRequest`, so a retained scope with no thumbprint would look like a lost binding and fail closed at the token endpoint. Because `op.WithKeyBinding()` rejects such a storage at construction, this downgrade can only affect an OP that never enabled key binding, and a key-binding RP still fails closed on the missing `cnf`. + +If the device grant also issues refresh tokens, carry the thumbprint onto the refresh token record as well — see the `getInfoFromRequest` case for `*op.DeviceAuthorizationState` in `example/server/storage`. + +Two storage requirements are worth calling out explicitly, because the library cannot enforce them for you: + +Access tokens remain bearer tokens; only the ID Token is bound. DPoP headers on requests without a binding are ignored, so DPoP-bound access-token clients are unaffected. + +A key-bound ID Token is rejected as a Token Exchange `subject_token` / `actor_token`, because possession of the binding key cannot be proven on that request; accepting it would convert a proof-of-possession token back into a bearer token. + ## Global `slog` logger migration OIDC now logs through the global `log/slog` functions. Configure the process-wide default before constructing an OP or RP: diff --git a/example/server/dynamic/op.go b/example/server/dynamic/op.go index 39821d0e..65e2b9aa 100644 --- a/example/server/dynamic/op.go +++ b/example/server/dynamic/op.go @@ -132,6 +132,8 @@ func newDynamicOP(ctx context.Context, storage op.Storage, key [32]byte, keyId s op.WithAllowInsecure(), //as an example on how to customize an endpoint this will change the authorization_endpoint from /authorize to /auth op.WithCustomAuthEndpoint(op.NewEndpoint("auth")), + // enable OpenID Connect Key Binding (advertises the bound_key scope) + op.WithKeyBinding(), ) if err != nil { return nil, err diff --git a/example/server/exampleop/op.go b/example/server/exampleop/op.go index c7c957e5..ee5cdfe3 100644 --- a/example/server/exampleop/op.go +++ b/example/server/exampleop/op.go @@ -153,6 +153,8 @@ func newOP( op.WithAllowInsecure(), // as an example on how to customize an endpoint this will change the authorization_endpoint from /authorize to /auth op.WithCustomAuthEndpoint(op.NewEndpoint("auth")), + // enable OpenID Connect Key Binding (advertises the bound_key scope) + op.WithKeyBinding(), }, extraOptions...)..., ) if err != nil { diff --git a/example/server/storage/client.go b/example/server/storage/client.go index f123b8a4..6c04b26a 100644 --- a/example/server/storage/client.go +++ b/example/server/storage/client.go @@ -108,9 +108,9 @@ func (c *Client) RestrictAdditionalAccessTokenScopes() func(scopes []string) []s } // IsScopeAllowed enables Client specific custom scopes validation -// in this example we allow the CustomScope for all clients +// in this example we allow the CustomScope and bound_key for all clients func (c *Client) IsScopeAllowed(scope string) bool { - return scope == CustomScope + return scope == CustomScope || scope == oidc.ScopeBoundKey } // IDTokenUserinfoClaimsAssertion allows specifying if claims of scope profile, email, phone and address are asserted into the id_token diff --git a/example/server/storage/oidc.go b/example/server/storage/oidc.go index 3d5d86b2..3175ed26 100644 --- a/example/server/storage/oidc.go +++ b/example/server/storage/oidc.go @@ -37,6 +37,7 @@ type AuthRequest struct { ResponseType oidc.ResponseType ResponseMode oidc.ResponseMode Nonce string + DPoPJKT string CodeChallenge *OIDCCodeChallenge done bool @@ -92,6 +93,10 @@ func (a *AuthRequest) GetNonce() string { return a.Nonce } +func (a *AuthRequest) GetDPoPJKT() string { + return a.DPoPJKT +} + func (a *AuthRequest) GetRedirectURI() string { return a.CallbackURI } @@ -165,6 +170,7 @@ func authRequestToInternal(authReq *oidc.AuthRequest, userID string) *AuthReques ResponseType: authReq.ResponseType, ResponseMode: authReq.ResponseMode, Nonce: authReq.Nonce, + DPoPJKT: authReq.DPoPJKT, CodeChallenge: codeChallenge, } } @@ -222,6 +228,10 @@ func (r *RefreshTokenRequest) GetClientID() string { return r.ApplicationID } +func (r *RefreshTokenRequest) GetDPoPJKT() string { + return r.DPoPJKT +} + func (r *RefreshTokenRequest) GetScopes() []string { return r.Scopes } diff --git a/example/server/storage/storage.go b/example/server/storage/storage.go index 253c241c..27d19a9f 100644 --- a/example/server/storage/storage.go +++ b/example/server/storage/storage.go @@ -280,7 +280,7 @@ func (s *Storage) CreateAccessAndRefreshTokens(ctx context.Context, request op.T } // get the information depending on the request type / implementation - applicationID, authTime, amr := getInfoFromRequest(request) + applicationID, authTime, amr, dpopJKT := getInfoFromRequest(request) // if currentRefreshToken is empty (Code Flow) we will have to create a new refresh token if currentRefreshToken == "" { @@ -289,7 +289,7 @@ func (s *Storage) CreateAccessAndRefreshTokens(ctx context.Context, request op.T if err != nil { return "", "", time.Time{}, err } - refreshToken, err := s.createRefreshToken(accessToken, amr, authTime) + refreshToken, err := s.createRefreshToken(accessToken, amr, authTime, dpopJKT) if err != nil { return "", "", time.Time{}, err } @@ -323,7 +323,7 @@ func (s *Storage) exchangeRefreshToken(ctx context.Context, request op.TokenExch return "", "", time.Time{}, err } - refreshToken, err := s.createRefreshToken(accessToken, nil, authTime) + refreshToken, err := s.createRefreshToken(accessToken, nil, authTime, "") if err != nil { return "", "", time.Time{}, err } @@ -591,7 +591,7 @@ func (s *Storage) Health(ctx context.Context) error { } // createRefreshToken will store a refresh_token in-memory based on the provided information -func (s *Storage) createRefreshToken(accessToken *Token, amr []string, authTime time.Time) (string, error) { +func (s *Storage) createRefreshToken(accessToken *Token, amr []string, authTime time.Time, dpopJKT string) (string, error) { s.lock.Lock() defer s.lock.Unlock() token := &RefreshToken{ @@ -604,6 +604,7 @@ func (s *Storage) createRefreshToken(accessToken *Token, amr []string, authTime Audience: accessToken.Audience, Expiration: time.Now().Add(5 * time.Hour), Scopes: accessToken.Scopes, + DPoPJKT: dpopJKT, AccessToken: accessToken.ID, } s.refreshTokens[token.ID] = token @@ -786,16 +787,16 @@ func (s *Storage) getTokenExchangeClaims(ctx context.Context, request op.TokenEx } // getInfoFromRequest returns the clientID, authTime and amr depending on the op.TokenRequest type / implementation -func getInfoFromRequest(req op.TokenRequest) (clientID string, authTime time.Time, amr []string) { +func getInfoFromRequest(req op.TokenRequest) (clientID string, authTime time.Time, amr []string, dpopJKT string) { authReq, ok := req.(*AuthRequest) // Code Flow (with scope offline_access) if ok { - return authReq.ApplicationID, authReq.authTime, authReq.GetAMR() + return authReq.ApplicationID, authReq.authTime, authReq.GetAMR(), authReq.DPoPJKT } refreshReq, ok := req.(*RefreshTokenRequest) // Refresh Token Request if ok { - return refreshReq.ApplicationID, refreshReq.AuthTime, refreshReq.AMR + return refreshReq.ApplicationID, refreshReq.AuthTime, refreshReq.AMR, refreshReq.DPoPJKT } - return "", time.Time{}, nil + return "", time.Time{}, nil, "" } // customClaim demonstrates how to return custom claims based on provided information diff --git a/example/server/storage/token.go b/example/server/storage/token.go index beab38cc..13ae0928 100644 --- a/example/server/storage/token.go +++ b/example/server/storage/token.go @@ -22,5 +22,6 @@ type RefreshToken struct { ApplicationID string Expiration time.Time Scopes []string + DPoPJKT string AccessToken string // Token.ID } diff --git a/pkg/client/key_binding_integration_test.go b/pkg/client/key_binding_integration_test.go new file mode 100644 index 00000000..75d59b95 --- /dev/null +++ b/pkg/client/key_binding_integration_test.go @@ -0,0 +1,159 @@ +package client_test + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + jose "github.com/go-jose/go-jose/v4" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zitadel/oidc/v3/example/server/exampleop" + "github.com/zitadel/oidc/v3/example/server/storage" + "github.com/zitadel/oidc/v3/pkg/client/rp" + "github.com/zitadel/oidc/v3/pkg/oidc" + "github.com/zitadel/oidc/v3/pkg/op" +) + +func TestNativeKeyBinding(t *testing.T) { + for _, wrapServer := range []bool{false, true} { + t.Run(fmt.Sprintf("legacy_server=%t", wrapServer), func(t *testing.T) { + testNativeKeyBinding(t, wrapServer) + }) + } +} + +func testNativeKeyBinding(t *testing.T, wrapServer bool) { + ctx := context.Background() + exampleStorage := storage.NewStorage(storage.NewUserStore("http://local-site")) + var deferred deferredHandler + opServer := httptest.NewServer(&deferred) + defer opServer.Close() + deferred.Handler = exampleop.SetupServer(opServer.URL, exampleStorage, Logger, wrapServer) + + clientID := "key-binding-" + uuid.NewString() + const clientSecret = "secret" + const redirectURI = "http://local-site/callback" + storage.RegisterClients(storage.WebClient(clientID, clientSecret, redirectURI)) + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + jkt, err := oidc.JWKThumbprint(&jose.JSONWebKey{Key: &key.PublicKey}) + require.NoError(t, err) + provider, err := rp.NewRelyingPartyOIDC( + ctx, + opServer.URL, + clientID, + clientSecret, + redirectURI, + []string{oidc.ScopeOpenID, oidc.ScopeOfflineAccess}, + rp.WithKeyBinding(key, jose.ES256), + ) + require.NoError(t, err) + + issuerCtx := op.ContextWithIssuer(ctx, opServer.URL) + authorizationURL, err := url.Parse(rp.AuthURL("state", provider)) + require.NoError(t, err) + query := authorizationURL.Query() + assert.Equal(t, jkt, query.Get(oidc.DPoPJKTParam)) + assert.Contains(t, query.Get("scope"), oidc.ScopeBoundKey) + authClient := &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }} + authResponse, err := authClient.Get(authorizationURL.String()) + require.NoError(t, err) + defer authResponse.Body.Close() + require.Equal(t, http.StatusFound, authResponse.StatusCode) + loginURL, err := authResponse.Location() + require.NoError(t, err) + authRequestID := loginURL.Query().Get("authRequestID") + require.NotEmpty(t, authRequestID) + authRequest, err := exampleStorage.AuthRequestByID(issuerCtx, authRequestID) + require.NoError(t, err) + boundRequest, ok := authRequest.(op.BoundKeyRequest) + require.True(t, ok) + assert.Equal(t, jkt, boundRequest.GetDPoPJKT()) + require.NoError(t, exampleStorage.CheckUsernamePassword("test-user@local-site", "verysecure", authRequest.GetID())) + code := uuid.NewString() + require.NoError(t, exampleStorage.SaveAuthCode(issuerCtx, authRequest.GetID(), code)) + plainProvider, err := rp.NewRelyingPartyOIDC( + ctx, + opServer.URL, + clientID, + clientSecret, + redirectURI, + []string{oidc.ScopeOpenID, oidc.ScopeOfflineAccess}, + ) + require.NoError(t, err) + _, err = rp.CodeExchange[*oidc.IDTokenClaims](ctx, code, plainProvider) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid_dpop_proof") + + tokens, err := rp.CodeExchange[*oidc.IDTokenClaims](ctx, code, provider) + require.NoError(t, err) + require.NotNil(t, tokens.IDTokenClaims.Confirmation) + assertConfirmationThumbprint(t, tokens.IDTokenClaims.Confirmation, jkt) + assertIDTokenType(t, tokens.IDToken, oidc.IDTokenTypeDPoP) + require.NotEmpty(t, tokens.RefreshToken) + + wrongKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + wrongKeyProvider, err := rp.NewRelyingPartyOIDC( + ctx, + opServer.URL, + clientID, + clientSecret, + redirectURI, + []string{oidc.ScopeOpenID}, + rp.WithKeyBinding(wrongKey, jose.ES256), + ) + require.NoError(t, err) + _, err = rp.RefreshTokens[*oidc.IDTokenClaims](ctx, wrongKeyProvider, tokens.RefreshToken, "", "") + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid_dpop_proof") + + refreshed, err := rp.RefreshTokens[*oidc.IDTokenClaims](ctx, provider, tokens.RefreshToken, "", "") + require.NoError(t, err) + require.NotNil(t, refreshed.IDTokenClaims.Confirmation) + assertConfirmationThumbprint(t, refreshed.IDTokenClaims.Confirmation, jkt) + assertIDTokenType(t, refreshed.IDToken, oidc.IDTokenTypeDPoP) + + unboundRequest, err := exampleStorage.CreateAuthRequest(issuerCtx, &oidc.AuthRequest{ + ClientID: clientID, + RedirectURI: redirectURI, + Scopes: oidc.SpaceDelimitedArray{oidc.ScopeOpenID}, + ResponseType: oidc.ResponseTypeCode, + }, "id1") + require.NoError(t, err) + require.NoError(t, exampleStorage.AuthRequestDone(unboundRequest.GetID())) + unboundCode := uuid.NewString() + require.NoError(t, exampleStorage.SaveAuthCode(issuerCtx, unboundRequest.GetID(), unboundCode)) + unboundTokens, err := rp.CodeExchange[*oidc.IDTokenClaims](ctx, unboundCode, plainProvider) + require.NoError(t, err) + assert.Nil(t, unboundTokens.IDTokenClaims.Confirmation) + assertIDTokenType(t, unboundTokens.IDToken, "JWT") +} + +func assertConfirmationThumbprint(t *testing.T, confirmation *oidc.Confirmation, want string) { + t.Helper() + var jwk jose.JSONWebKey + require.NoError(t, json.Unmarshal(confirmation.JWK, &jwk)) + got, err := oidc.JWKThumbprint(&jwk) + require.NoError(t, err) + assert.Equal(t, want, got) +} + +func assertIDTokenType(t *testing.T, token string, want jose.ContentType) { + t.Helper() + jws, err := jose.ParseSigned(token, []jose.SignatureAlgorithm{jose.RS256}) + require.NoError(t, err) + require.Len(t, jws.Signatures, 1) + assert.Equal(t, string(want), jws.Signatures[0].Header.ExtraHeaders[jose.HeaderType]) +} diff --git a/pkg/client/rp/device.go b/pkg/client/rp/device.go index ae95cd48..72f90046 100644 --- a/pkg/client/rp/device.go +++ b/pkg/client/rp/device.go @@ -82,5 +82,6 @@ func DeviceAccessToken(ctx context.Context, deviceCode string, interval time.Dur } } - return client.PollDeviceAccessTokenEndpointWithAuthFn(ctx, interval, req, tokenEndpointCaller{rp}, authFn) + + return client.PollDeviceAccessTokenEndpointWithAuthFn(ctx, interval, req, tokenEndpointCaller{RelyingParty: rp}, authFn) } diff --git a/pkg/client/rp/key_binding.go b/pkg/client/rp/key_binding.go new file mode 100644 index 00000000..24a513ec --- /dev/null +++ b/pkg/client/rp/key_binding.go @@ -0,0 +1,280 @@ +package rp + +import ( + "crypto" + "crypto/rsa" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "reflect" + "slices" + "time" + + "github.com/go-jose/go-jose/v4" + "github.com/go-jose/go-jose/v4/cryptosigner" + "github.com/google/uuid" + + "github.com/zitadel/oidc/v3/pkg/oidc" +) + +// This file implements the RP side of OpenID Connect Key Binding 1.0. The +// specification is still a draft, but it is already deployed by production +// IdPs, so the wire format is stable in practice; any adjustments to track the +// final specification will follow the normal deprecation process rather than +// changing without notice. + +var ( + ErrInvalidKeyBinding = errors.New("invalid key binding configuration") + ErrKeyBindingIDToken = errors.New("invalid key-bound ID token") + ErrKeyBindingConfirmation = errors.New("ID token confirmation does not match the binding key") +) + +type keyBinding struct { + signer jose.Signer + thumbprint string +} + +// KeyBindingRelyingParty is implemented by RPs configured with +// [WithKeyBinding]. Wrappers around a RelyingParty can preserve native key +// binding by forwarding these methods. +type KeyBindingRelyingParty interface { + RelyingParty + KeyBindingThumbprint() string + SignDPoPProof(method, htu, code string) (string, error) +} + +// WithKeyBinding enables OpenID Connect Key Binding for the authorization code, +// refresh and device authorization flows. The RP appends the `bound_key` scope, +// adds the `dpop_jkt` authorization request parameter, signs a DPoP proof for each +// token request, and verifies that the returned ID Token is actually bound to +// signer (`typ` and `cnf`), so a stripped binding fails closed. +// +// signer may be any [crypto.Signer], including a KMS- or HSM-backed one. alg must +// be an asymmetric JWS algorithm supported by signer's key. +func WithKeyBinding(signer crypto.Signer, alg jose.SignatureAlgorithm) Option { + return func(rp *relyingParty) error { + if rp.oauth2Only { + return fmt.Errorf("%w: key binding requires OpenID Connect", ErrInvalidOption) + } + if nilCryptoSigner(signer) || !isAsymmetricKeyBindingAlgorithm(alg) { + return ErrInvalidKeyBinding + } + // Reject a key the OP will reject anyway (oidc.ValidateDPoPKeyStrength + // is the same check the OP applies to the proof), so configuration + // fails immediately instead of at the token endpoint, after the user + // has already completed the authorization. + if err := oidc.ValidateDPoPKeyStrength(signer.Public()); err != nil { + return fmt.Errorf("%w: %v", ErrInvalidKeyBinding, err) + } + publicJWK := &jose.JSONWebKey{Key: signer.Public(), Algorithm: string(alg)} + thumbprint, err := oidc.JWKThumbprint(publicJWK) + if err != nil { + return fmt.Errorf("%w: %v", ErrInvalidKeyBinding, err) + } + opaqueSigner := cryptosigner.Opaque(joseCryptoSigner{Signer: signer}) + proofSigner, err := jose.NewSigner( + jose.SigningKey{Algorithm: alg, Key: keyBindingOpaqueSigner{OpaqueSigner: opaqueSigner, publicJWK: publicJWK}}, + (&jose.SignerOptions{EmbedJWK: true}).WithType(oidc.DPoPProofType), + ) + if err != nil { + return fmt.Errorf("%w: %v", ErrInvalidKeyBinding, err) + } + rp.keyBinding = &keyBinding{signer: proofSigner, thumbprint: thumbprint} + if !slices.Contains(rp.oauthConfig.Scopes, oidc.ScopeBoundKey) { + rp.oauthConfig.Scopes = append(slices.Clone(rp.oauthConfig.Scopes), oidc.ScopeBoundKey) + } + return nil + } +} + +// joseCryptoSigner corrects rsa.PSSSaltLengthAuto used by cryptosigner.Opaque +// to the hash-length salt required by JWA for PS256, PS384, and PS512. +type joseCryptoSigner struct { + crypto.Signer +} + +func (s joseCryptoSigner) Sign(random io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { + if pss, ok := opts.(*rsa.PSSOptions); ok { + corrected := *pss + corrected.SaltLength = rsa.PSSSaltLengthEqualsHash + opts = &corrected + } + return s.Signer.Sign(random, digest, opts) +} + +type keyBindingOpaqueSigner struct { + jose.OpaqueSigner + publicJWK *jose.JSONWebKey +} + +func (s keyBindingOpaqueSigner) Public() *jose.JSONWebKey { + return s.publicJWK +} + +func nilCryptoSigner(signer crypto.Signer) bool { + if signer == nil { + return true + } + value := reflect.ValueOf(signer) + switch value.Kind() { + case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, reflect.Ptr, reflect.Slice: + return value.IsNil() + default: + return false + } +} + +func (rp *relyingParty) KeyBindingThumbprint() string { + if rp.keyBinding == nil { + return "" + } + return rp.keyBinding.thumbprint +} + +func (rp *relyingParty) SignDPoPProof(method, htu, code string) (string, error) { + if rp.keyBinding == nil { + return "", ErrInvalidKeyBinding + } + return rp.keyBinding.proof(method, htu, code) +} + +func keyBindingRP(rp RelyingParty) (KeyBindingRelyingParty, bool) { + configured, ok := rp.(KeyBindingRelyingParty) + return configured, ok && configured.KeyBindingThumbprint() != "" +} + +func isAsymmetricKeyBindingAlgorithm(alg jose.SignatureAlgorithm) bool { + switch alg { + case jose.RS256, jose.RS384, jose.RS512, + jose.PS256, jose.PS384, jose.PS512, + jose.ES256, jose.ES384, jose.ES512, + jose.EdDSA: + return true + default: + return false + } +} + +func (k *keyBinding) proof(method, tokenEndpoint, code string) (string, error) { + claims := oidc.DPoPProofClaims{ + JWTID: uuid.NewString(), + HTTPMethod: method, + HTTPURI: tokenEndpoint, + IssuedAt: oidc.FromTime(time.Now()), + } + if code != "" { + claims.CodeHash = oidc.CodeHash(code) + } + payload, err := json.Marshal(claims) + if err != nil { + return "", err + } + signed, err := k.signer.Sign(payload) + if err != nil { + return "", err + } + return signed.CompactSerialize() +} + +type keyBindingTransport struct { + base http.RoundTripper + binding KeyBindingRelyingParty + code string + tokenEndpoint string +} + +func (t *keyBindingTransport) RoundTrip(req *http.Request) (*http.Response, error) { + htu := *req.URL + htu.RawQuery = "" + htu.ForceQuery = false + htu.Fragment = "" + htu.RawFragment = "" + + // Only ever sign a proof for the configured token endpoint. Without this, + // a 307/308 redirect from the token endpoint would make net/http replay the + // POST body (authorization code and client secret) to the redirect target, + // and this transport would helpfully mint a fresh proof for that host, + // disclosing c_s256 = SHA256(code) to it. It also stops the proof leaking + // if this client is accidentally reused for another request. + if t.tokenEndpoint == "" || htu.String() != t.tokenEndpoint { + return nil, fmt.Errorf("%w: refusing to sign a DPoP proof for %q, expected the token endpoint %q", + ErrInvalidKeyBinding, htu.String(), t.tokenEndpoint) + } + + proof, err := t.binding.SignDPoPProof(req.Method, htu.String(), t.code) + if err != nil { + return nil, err + } + req = req.Clone(req.Context()) + req.Header.Set(oidc.DPoPHeader, proof) + return t.base.RoundTrip(req) +} + +// keyBindingHTTPClient returns a shallow copy of client whose transport adds a +// DPoP proof to a single token-endpoint request. tokenEndpoint pins the only +// URL a proof will be signed for. +func keyBindingHTTPClient(client *http.Client, binding KeyBindingRelyingParty, code, tokenEndpoint string) *http.Client { + clone := http.Client{} + if client != nil { + clone = *client + } + base := clone.Transport + if base == nil { + base = http.DefaultTransport + } + clone.Transport = &keyBindingTransport{ + base: base, + binding: binding, + code: code, + tokenEndpoint: tokenEndpoint, + } + // Refuse redirects rather than re-POST the code to another host. + clone.CheckRedirect = func(req *http.Request, via []*http.Request) error { + return fmt.Errorf("%w: token endpoint redirect to %q refused", ErrInvalidKeyBinding, req.URL.Redacted()) + } + return &clone +} + +// verifyKeyBindingIDToken checks that token is actually bound to the RP's +// binding key, by requiring the protected `typ` header to be +// [oidc.IDTokenTypeDPoP] and cnf.jwk to be the key identified by expectedJKT. +// +// It only inspects the binding; the caller MUST have already verified the +// token's signature, issuer, audience and expiry with [VerifyTokens] over the +// same token string, and alg MUST be the algorithm of that verified signature. +// The token is re-parsed here solely to reach the protected header and the +// `cnf` claim, which the generic claims types do not expose. +func verifyKeyBindingIDToken(token string, alg jose.SignatureAlgorithm, expectedJKT string) error { + signed, err := jose.ParseSigned(token, []jose.SignatureAlgorithm{alg}) + if err != nil || len(signed.Signatures) != 1 { + return ErrKeyBindingIDToken + } + typ, _ := signed.Signatures[0].Header.ExtraHeaders[jose.HeaderType].(string) + if typ != string(oidc.IDTokenTypeDPoP) { + return fmt.Errorf("%w: unexpected typ %q", ErrKeyBindingIDToken, typ) + } + // Safe: the signature over this exact token string was already verified by + // the caller (see the contract above), so the payload is authentic. Parsing + // it again only to read `cnf` avoids re-implementing signature checks. + payload := signed.UnsafePayloadWithoutVerification() + var claims struct { + Confirmation *oidc.Confirmation `json:"cnf"` + } + if err := json.Unmarshal(payload, &claims); err != nil || claims.Confirmation == nil { + return fmt.Errorf("%w: missing cnf.jwk", ErrKeyBindingIDToken) + } + var jwk jose.JSONWebKey + if err := json.Unmarshal(claims.Confirmation.JWK, &jwk); err != nil || !jwk.Valid() || !jwk.IsPublic() { + return fmt.Errorf("%w: invalid cnf.jwk", ErrKeyBindingIDToken) + } + actualJKT, err := oidc.JWKThumbprint(&jwk) + if err != nil { + return fmt.Errorf("%w: invalid cnf.jwk", ErrKeyBindingIDToken) + } + if actualJKT != expectedJKT { + return ErrKeyBindingConfirmation + } + return nil +} diff --git a/pkg/client/rp/key_binding_test.go b/pkg/client/rp/key_binding_test.go new file mode 100644 index 00000000..28628dfd --- /dev/null +++ b/pkg/client/rp/key_binding_test.go @@ -0,0 +1,204 @@ +package rp + +import ( + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "net/http" + "strings" + "testing" + + "github.com/go-jose/go-jose/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/oauth2" + + "github.com/zitadel/oidc/v3/pkg/oidc" +) + +type wrappedCryptoSigner struct { + crypto.Signer +} + +func TestWithKeyBinding(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + configured := &relyingParty{oauthConfig: &oauth2.Config{Scopes: []string{oidc.ScopeOpenID}}} + require.NoError(t, WithKeyBinding(&wrappedCryptoSigner{Signer: key}, jose.ES256)(configured)) + require.NotNil(t, configured.keyBinding) + assert.Equal(t, []string{oidc.ScopeOpenID, oidc.ScopeBoundKey}, configured.oauthConfig.Scopes) + + require.NoError(t, WithKeyBinding(key, jose.ES256)(configured)) + assert.Equal(t, []string{oidc.ScopeOpenID, oidc.ScopeBoundKey}, configured.oauthConfig.Scopes) +} + +// TestWithKeyBindingDoesNotAliasCallerScopes ensures the caller's scope slice +// is never mutated in place when bound_key is appended. +func TestWithKeyBindingDoesNotAliasCallerScopes(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + callerScopes := make([]string, 1, 4) // spare capacity, so append would alias + callerScopes[0] = oidc.ScopeOpenID + configured := &relyingParty{oauthConfig: &oauth2.Config{Scopes: callerScopes}} + require.NoError(t, WithKeyBinding(key, jose.ES256)(configured)) + + assert.Equal(t, []string{oidc.ScopeOpenID}, callerScopes) + assert.Equal(t, []string{oidc.ScopeOpenID, oidc.ScopeBoundKey}, configured.oauthConfig.Scopes) +} + +func TestWithKeyBindingRejectsInvalidConfiguration(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + var nilKey *ecdsa.PrivateKey + + tests := []struct { + name string + rp *relyingParty + signer crypto.Signer + alg jose.SignatureAlgorithm + want error + }{ + {name: "oauth only", rp: &relyingParty{oauth2Only: true, oauthConfig: &oauth2.Config{}}, signer: key, alg: jose.ES256, want: ErrInvalidOption}, + {name: "nil signer", rp: &relyingParty{oauthConfig: &oauth2.Config{}}, alg: jose.ES256, want: ErrInvalidKeyBinding}, + {name: "typed nil signer", rp: &relyingParty{oauthConfig: &oauth2.Config{}}, signer: nilKey, alg: jose.ES256, want: ErrInvalidKeyBinding}, + {name: "symmetric algorithm", rp: &relyingParty{oauthConfig: &oauth2.Config{}}, signer: key, alg: jose.HS256, want: ErrInvalidKeyBinding}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := WithKeyBinding(tt.signer, tt.alg)(tt.rp) + assert.ErrorIs(t, err, tt.want) + }) + } +} + +func TestKeyBindingProof(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + configured := &relyingParty{oauthConfig: &oauth2.Config{}} + require.NoError(t, WithKeyBinding(&wrappedCryptoSigner{Signer: key}, jose.ES256)(configured)) + + const endpoint = "https://issuer.example.com/oauth/token" + const code = "code" + compact, err := configured.keyBinding.proof(http.MethodPost, endpoint, code) + require.NoError(t, err) + signed, err := jose.ParseSigned(compact, []jose.SignatureAlgorithm{jose.ES256}) + require.NoError(t, err) + require.Len(t, signed.Signatures, 1) + assert.Equal(t, string(oidc.DPoPProofType), signed.Signatures[0].Header.ExtraHeaders[jose.HeaderType]) + require.NotNil(t, signed.Signatures[0].Header.JSONWebKey) + assert.Equal(t, string(jose.ES256), signed.Signatures[0].Header.JSONWebKey.Algorithm) + payload, err := signed.Verify(&key.PublicKey) + require.NoError(t, err) + var claims oidc.DPoPProofClaims + require.NoError(t, json.Unmarshal(payload, &claims)) + assert.NotEmpty(t, claims.JWTID) + assert.Equal(t, http.MethodPost, claims.HTTPMethod) + assert.Equal(t, endpoint, claims.HTTPURI) + assert.Equal(t, oidc.CodeHash(code), claims.CodeHash) +} + +func TestKeyBindingProofPSSUsesHashLengthSalt(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + configured := &relyingParty{oauthConfig: &oauth2.Config{}} + require.NoError(t, WithKeyBinding(&wrappedCryptoSigner{Signer: key}, jose.PS256)(configured)) + + compact, err := configured.keyBinding.proof(http.MethodPost, "https://issuer.example.com/oauth/token", "code") + require.NoError(t, err) + parts := strings.Split(compact, ".") + require.Len(t, parts, 3) + signature, err := base64.RawURLEncoding.DecodeString(parts[2]) + require.NoError(t, err) + digest := sha256.Sum256([]byte(parts[0] + "." + parts[1])) + assert.NoError(t, rsa.VerifyPSS(&key.PublicKey, crypto.SHA256, digest[:], signature, &rsa.PSSOptions{ + SaltLength: rsa.PSSSaltLengthEqualsHash, + Hash: crypto.SHA256, + })) +} + +func TestVerifyKeyBindingIDToken(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + jwk, err := oidc.CanonicalJWK(&jose.JSONWebKey{Key: &key.PublicKey}) + require.NoError(t, err) + jkt, err := oidc.JWKThumbprint(&jose.JSONWebKey{Key: &key.PublicKey}) + require.NoError(t, err) + payload, err := json.Marshal(struct { + Confirmation *oidc.Confirmation `json:"cnf"` + }{Confirmation: &oidc.Confirmation{JWK: jwk}}) + require.NoError(t, err) + signer, err := jose.NewSigner( + jose.SigningKey{Algorithm: jose.ES256, Key: key}, + new(jose.SignerOptions).WithType(oidc.IDTokenTypeDPoP), + ) + require.NoError(t, err) + signed, err := signer.Sign(payload) + require.NoError(t, err) + compact, err := signed.CompactSerialize() + require.NoError(t, err) + + assert.NoError(t, verifyKeyBindingIDToken(compact, jose.ES256, jkt)) + assert.ErrorIs(t, verifyKeyBindingIDToken(compact, jose.ES256, "wrong"), ErrKeyBindingConfirmation) + + privateJWK, err := json.Marshal(jose.JSONWebKey{Key: key}) + require.NoError(t, err) + privatePayload, err := json.Marshal(struct { + Confirmation *oidc.Confirmation `json:"cnf"` + }{Confirmation: &oidc.Confirmation{JWK: privateJWK}}) + require.NoError(t, err) + privateSigned, err := signer.Sign(privatePayload) + require.NoError(t, err) + privateCompact, err := privateSigned.CompactSerialize() + require.NoError(t, err) + assert.ErrorIs(t, verifyKeyBindingIDToken(privateCompact, jose.ES256, jkt), ErrKeyBindingIDToken) +} + +// TestWithKeyBindingRejectsWeakKeys ensures the RP rejects a binding key the OP +// would reject when verifying the proof, so misconfiguration surfaces at setup +// instead of after the user has completed the authorization. +func TestWithKeyBindingRejectsWeakKeys(t *testing.T) { + weakRSA, err := rsa.GenerateKey(rand.Reader, 1024) + require.NoError(t, err) + + configured := &relyingParty{oauthConfig: &oauth2.Config{Scopes: []string{oidc.ScopeOpenID}}} + err = WithKeyBinding(weakRSA, jose.RS256)(configured) + require.ErrorIs(t, err, ErrInvalidKeyBinding) + assert.Nil(t, configured.keyBinding) + + strongRSA, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + assert.NoError(t, WithKeyBinding(strongRSA, jose.RS256)(configured)) +} + +// TestKeyBindingTransportRefusesForeignURL ensures a proof is only ever signed +// for the configured token endpoint, so a token-endpoint redirect cannot harvest +// the authorization code together with a valid proof over it. +func TestKeyBindingTransportRefusesForeignURL(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + configured := &relyingParty{oauthConfig: &oauth2.Config{Scopes: []string{oidc.ScopeOpenID}}} + require.NoError(t, WithKeyBinding(key, jose.ES256)(configured)) + + const tokenEndpoint = "https://op.example.com/token" + client := keyBindingHTTPClient(nil, configured, "thecode", tokenEndpoint) + transport := client.Transport + + req, err := http.NewRequest(http.MethodPost, "https://evil.example.com/token", nil) + require.NoError(t, err) + _, err = transport.RoundTrip(req) + require.ErrorIs(t, err, ErrInvalidKeyBinding) + assert.Empty(t, req.Header.Get(oidc.DPoPHeader)) + + // Redirects must be refused rather than re-POSTing the code elsewhere. + require.NotNil(t, client.CheckRedirect) + redirect, err := http.NewRequest(http.MethodPost, "https://evil.example.com/", nil) + require.NoError(t, err) + assert.ErrorIs(t, client.CheckRedirect(redirect, nil), ErrInvalidKeyBinding) +} diff --git a/pkg/client/rp/relying_party.go b/pkg/client/rp/relying_party.go index 8bca9e34..7e0ca30e 100644 --- a/pkg/client/rp/relying_party.go +++ b/pkg/client/rp/relying_party.go @@ -122,6 +122,7 @@ type relyingParty struct { idTokenVerifier *IDTokenVerifier verifierOpts []VerifierOption signer jose.Signer + keyBinding *keyBinding logger *slog.Logger } @@ -450,6 +451,9 @@ func AuthURL(state string, rp RelyingParty, opts ...AuthURLOpt) string { for _, opt := range opts { authOpts = append(authOpts, opt()...) } + if configured, ok := keyBindingRP(rp); ok { + authOpts = append(authOpts, oauth2.SetAuthURLParam(oidc.DPoPJKTParam, configured.KeyBindingThumbprint())) + } return rp.OAuthConfig().AuthCodeURL(state, authOpts...) } @@ -525,6 +529,11 @@ func verifyTokenResponse[C oidc.IDClaims](ctx context.Context, token *oauth2.Tok if err != nil { return nil, err } + if configured, ok := keyBindingRP(rp); ok { + if err := verifyKeyBindingIDToken(idTokenString, idToken.GetSignatureAlgorithm(), configured.KeyBindingThumbprint()); err != nil { + return nil, err + } + } return &oidc.Tokens[C]{Token: token, IDTokenClaims: idToken, IDToken: idTokenString}, nil } @@ -534,7 +543,11 @@ func CodeExchange[C oidc.IDClaims](ctx context.Context, code string, rp RelyingP ctx, codeExchangeSpan := client.Tracer.Start(ctx, "CodeExchange") defer codeExchangeSpan.End() - ctx = context.WithValue(ctx, oauth2.HTTPClient, rp.HttpClient()) + httpClient := rp.HttpClient() + if configured, ok := keyBindingRP(rp); ok { + httpClient = keyBindingHTTPClient(httpClient, configured, code, rp.OAuthConfig().Endpoint.TokenURL) + } + ctx = context.WithValue(ctx, oauth2.HTTPClient, httpClient) codeOpts := make([]oauth2.AuthCodeOption, 0) for _, opt := range opts { codeOpts = append(codeOpts, opt()...) @@ -800,12 +813,20 @@ func WithClientAssertionJWT(clientAssertion string) CodeExchangeOpt { type tokenEndpointCaller struct { RelyingParty + httpClient *http.Client } func (t tokenEndpointCaller) TokenEndpoint() string { return t.OAuthConfig().Endpoint.TokenURL } +func (t tokenEndpointCaller) HttpClient() *http.Client { + if t.httpClient != nil { + return t.httpClient + } + return t.RelyingParty.HttpClient() +} + type RefreshTokenRequest struct { RefreshToken string `schema:"refresh_token"` Scopes oidc.SpaceDelimitedArray `schema:"scope,omitempty"` @@ -864,14 +885,32 @@ func RefreshTokens[C oidc.IDClaims](ctx context.Context, rp RelyingParty, refres } } - newToken, err := client.CallTokenEndpointWithAuthFn(ctx, request, authFn, tokenEndpointCaller{RelyingParty: rp}) + httpClient := rp.HttpClient() + if configured, ok := keyBindingRP(rp); ok { + httpClient = keyBindingHTTPClient(httpClient, configured, "", rp.OAuthConfig().Endpoint.TokenURL) + } + caller := tokenEndpointCaller{RelyingParty: rp, httpClient: httpClient} + newToken, err := client.CallTokenEndpointWithAuthFn(ctx, request, authFn, caller) if err != nil { return nil, err } tokens, err := verifyTokenResponse[C](ctx, newToken, rp) - if err == nil || errors.Is(err, ErrMissingIDToken) { + if err == nil { + return tokens, nil + } + if errors.Is(err, ErrMissingIDToken) { // https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse // ...except that it might not contain an id_token. + // + // That allowance does not apply to a key-bound RP: the whole point of + // WithKeyBinding is to obtain a proof-of-possession ID Token, and + // ErrMissingIDToken is returned before the binding is ever checked. So + // an OP or MITM could just omit id_token to make the RP return + // (tokens, nil) with a nil IDTokenClaims, which callers would either + // nil-deref or paper over by continuing to use the previous ID Token. + if _, bound := keyBindingRP(rp); bound { + return nil, err + } return tokens, nil } return nil, err diff --git a/pkg/oidc/authorization.go b/pkg/oidc/authorization.go index fa37dbfe..29413ed4 100644 --- a/pkg/oidc/authorization.go +++ b/pkg/oidc/authorization.go @@ -32,6 +32,11 @@ const ( // that grants access to the End-User's UserInfo Endpoint even when the End-User is not present (not logged in). ScopeOfflineAccess = "offline_access" + // ScopeBoundKey defines the scope `bound_key` + // This (optional) scope value requests an ID Token bound to a proof-of-possession key, + // as defined by OpenID Connect Key Binding 1.0. + ScopeBoundKey = "bound_key" + // ResponseTypeCode for the Authorization Code Flow returning a code from the Authorization Server ResponseTypeCode ResponseType = "code" @@ -87,6 +92,14 @@ type AuthRequest struct { CodeChallenge string `json:"code_challenge" schema:"code_challenge"` CodeChallengeMethod CodeChallengeMethod `json:"code_challenge_method" schema:"code_challenge_method"` + // DPoPJKT is the `dpop_jkt` parameter defined by OpenID Connect Key + // Binding 1.0 / [RFC 9449, Section 10]. It carries the RFC 7638 JWK + // SHA-256 Thumbprint of the RP's proof-of-possession public key and, + // together with the `bound_key` scope, requests a key-bound ID Token. + // + // [RFC 9449, Section 10]: https://www.rfc-editor.org/rfc/rfc9449#section-10 + DPoPJKT string `json:"dpop_jkt,omitempty" schema:"dpop_jkt"` + // RequestParam enables OIDC requests to be passed in a single, self-contained parameter (as JWT, called Request Object) RequestParam string `schema:"request"` } diff --git a/pkg/oidc/discovery.go b/pkg/oidc/discovery.go index 11ba8064..305fe9a0 100644 --- a/pkg/oidc/discovery.go +++ b/pkg/oidc/discovery.go @@ -126,6 +126,9 @@ type DiscoveryConfiguration struct { // CodeChallengeMethodsSupported contains a list of Proof Key for Code Exchange (PKCE) code challenge methods supported by the OP. CodeChallengeMethodsSupported []CodeChallengeMethod `json:"code_challenge_methods_supported,omitempty"` + // DPoPSigningAlgValuesSupported contains the JWS algorithms accepted for DPoP proof JWTs. + DPoPSigningAlgValuesSupported []string `json:"dpop_signing_alg_values_supported,omitempty"` + // ServiceDocumentation is a URL where developers can get information about the OP and its usage. ServiceDocumentation string `json:"service_documentation,omitempty"` diff --git a/pkg/oidc/dpop.go b/pkg/oidc/dpop.go new file mode 100644 index 00000000..f0aa73c1 --- /dev/null +++ b/pkg/oidc/dpop.go @@ -0,0 +1,136 @@ +package oidc + +import ( + "bytes" + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rsa" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "fmt" + "reflect" + + jose "github.com/go-jose/go-jose/v4" +) + +// Parameters, claims and helpers for OpenID Connect Key Binding 1.0, which +// binds an ID Token to a proof-of-possession key using DPoP proofs. +// +// OpenID Connect Key Binding is a draft, but it is already deployed by +// production IdPs, so the wire format here is stable in practice. Any +// adjustments to track the final specification will follow the normal +// deprecation process rather than changing without notice. +const ( + // DPoPJKTParam is the authorization request parameter carrying the + // base64url-encoded SHA-256 JWK thumbprint of the binding key. + DPoPJKTParam = "dpop_jkt" + + // DPoPHeader carries a DPoP proof JWT. + DPoPHeader = "DPoP" + + // DPoPProofType is the media type used in a DPoP proof's typ header. + DPoPProofType jose.ContentType = "dpop+jwt" + + // IDTokenTypeDPoP is the media type used for a key-bound ID Token. + IDTokenTypeDPoP jose.ContentType = "dpop+id_token" +) + +// IDTokenTypeJWT is the media type used in the protected `typ` header of a +// bearer (unbound) ID Token. +const IDTokenTypeJWT jose.ContentType = "JWT" + +// Confirmation is the cnf claim of a key-bound ID Token. +type Confirmation struct { + JWK json.RawMessage `json:"jwk"` +} + +// DPoPProofClaims contains the claims used by a DPoP proof for OpenID +// Connect Key Binding. +type DPoPProofClaims struct { + JWTID string `json:"jti"` + HTTPMethod string `json:"htm"` + HTTPURI string `json:"htu"` + IssuedAt Time `json:"iat"` + CodeHash string `json:"c_s256,omitempty"` +} + +func (c *DPoPProofClaims) UnmarshalJSON(data []byte) error { + type claims DPoPProofClaims + var decoded claims + if err := json.Unmarshal(data, &decoded); err != nil { + return err + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(data, &fields); err != nil { + return err + } + if raw, ok := fields["iat"]; ok { + var issuedAt int64 + if bytes.Equal(raw, []byte("null")) { + return &json.UnmarshalTypeError{Value: "null", Type: reflect.TypeOf(issuedAt), Field: "iat"} + } + if err := json.Unmarshal(raw, &issuedAt); err != nil { + return err + } + decoded.IssuedAt = Time(issuedAt) + } + *c = DPoPProofClaims(decoded) + return nil +} + +// ValidDPoPJKT reports whether value is an unpadded base64url-encoded +// SHA-256 JWK thumbprint. +func ValidDPoPJKT(value string) bool { + decoded, err := base64.RawURLEncoding.Strict().DecodeString(value) + return err == nil && len(decoded) == sha256.Size && base64.RawURLEncoding.EncodeToString(decoded) == value +} + +// JWKThumbprint returns the RFC 7638 SHA-256 thumbprint of jwk, encoded +// with unpadded base64url. +func JWKThumbprint(jwk *jose.JSONWebKey) (string, error) { + thumbprint, err := jwk.Thumbprint(crypto.SHA256) + if err != nil { + return "", err + } + return base64.RawURLEncoding.EncodeToString(thumbprint), nil +} + +// CanonicalJWK returns the public key members of jwk without optional or +// caller-controlled JWK metadata such as kid, use, alg, or x5c. +func CanonicalJWK(jwk *jose.JSONWebKey) (json.RawMessage, error) { + canonical, err := json.Marshal(jose.JSONWebKey{Key: jwk.Key}) + if err != nil { + return nil, err + } + return json.RawMessage(canonical), nil +} + +// ValidateDPoPKeyStrength enforces minimum key strength for a DPoP +// proof-of-possession key: RSA 2048-8192 bits, EC curve P-256, P-384 or P-521, +// and Ed25519 (fixed strength). Other key types pass; callers are expected to +// have already restricted the key to a public asymmetric type. Shared by the OP +// and RP so the two cannot drift apart. +func ValidateDPoPKeyStrength(key any) error { + switch k := key.(type) { + case *rsa.PublicKey: + bits := k.N.BitLen() + if bits < 2048 || bits > 8192 { + return fmt.Errorf("RSA key size %d bits is not allowed", bits) + } + case *ecdsa.PublicKey: + switch k.Curve { + case elliptic.P256(), elliptic.P384(), elliptic.P521(): + default: + return fmt.Errorf("EC curve %s is not allowed", k.Curve.Params().Name) + } + } + return nil +} + +// CodeHash returns the c_s256 value for an authorization or device code. +func CodeHash(code string) string { + hash := sha256.Sum256([]byte(code)) + return base64.RawURLEncoding.EncodeToString(hash[:]) +} diff --git a/pkg/oidc/dpop_test.go b/pkg/oidc/dpop_test.go new file mode 100644 index 00000000..79fac9b3 --- /dev/null +++ b/pkg/oidc/dpop_test.go @@ -0,0 +1,80 @@ +package oidc_test + +import ( + "encoding/json" + "strings" + "testing" + + jose "github.com/go-jose/go-jose/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zitadel/oidc/v3/pkg/oidc" +) + +func TestJWKThumbprint(t *testing.T) { + const raw = `{"kty":"EC","crv":"P-256","x":"ukpv3fU6tqQKaUwcdBAQoK3IHvJIW__9yNd1oR7qvZc","y":"nBBxXrx0Nziwg_evfUMUUgnGKKUf2ATpWG9EojnUoU4"}` + var jwk jose.JSONWebKey + require.NoError(t, json.Unmarshal([]byte(raw), &jwk)) + + got, err := oidc.JWKThumbprint(&jwk) + require.NoError(t, err) + assert.Equal(t, "dnfb1T9jil_gOhti60baHs_WD_a4D8JN9VDJXbmBmGw", got) +} + +func TestValidDPoPJKT(t *testing.T) { + assert.True(t, oidc.ValidDPoPJKT("dnfb1T9jil_gOhti60baHs_WD_a4D8JN9VDJXbmBmGw")) + assert.False(t, oidc.ValidDPoPJKT("")) + assert.False(t, oidc.ValidDPoPJKT("too-short")) + assert.False(t, oidc.ValidDPoPJKT("dnfb1T9jil/gOhti60baHs_WD_a4D8JN9VDJXbmBmGw")) + assert.False(t, oidc.ValidDPoPJKT("dnfb1T9jil_gOhti60baHs_WD_a4D8JN9VDJXbmBmGw=")) + assert.False(t, oidc.ValidDPoPJKT("dnfb1T9jil_gOhti60baHs_WD_a4D8JN9VDJXbmBmGx")) +} + +func TestCanonicalJWK(t *testing.T) { + const raw = `{"kty":"EC","crv":"P-256","x":"ukpv3fU6tqQKaUwcdBAQoK3IHvJIW__9yNd1oR7qvZc","y":"nBBxXrx0Nziwg_evfUMUUgnGKKUf2ATpWG9EojnUoU4","kid":"client-key","use":"sig","alg":"ES256"}` + var jwk jose.JSONWebKey + require.NoError(t, json.Unmarshal([]byte(raw), &jwk)) + + got, err := oidc.CanonicalJWK(&jwk) + require.NoError(t, err) + assert.JSONEq(t, `{"kty":"EC","crv":"P-256","x":"ukpv3fU6tqQKaUwcdBAQoK3IHvJIW__9yNd1oR7qvZc","y":"nBBxXrx0Nziwg_evfUMUUgnGKKUf2ATpWG9EojnUoU4"}`, string(got)) +} + +func TestCodeHash(t *testing.T) { + assert.Equal(t, "o1uBp9eSe3DsmScN0jYriFgKKFdK-BLywC9WRpV5GG8", oidc.CodeHash("SplxlOBeZQQYbYS6WxSbIA")) +} + +func TestDPoPProofClaimsRejectsStringIssuedAt(t *testing.T) { + var claims oidc.DPoPProofClaims + assert.Error(t, json.Unmarshal([]byte(`{"iat":"2026-07-29T12:00:00Z"}`), &claims)) +} + +// TestIDTokenClaimsConfirmationRoundTrip pins the invariant that a `cnf` claim +// parses into IDTokenClaims.Confirmation. Several fail-closed checks depend on +// it (notably the OP refusing a key-bound ID Token as a Token Exchange +// subject_token); if `cnf` stopped unmarshalling here, those would silently +// become dead code and the checks would pass everything. +func TestIDTokenClaimsConfirmationRoundTrip(t *testing.T) { + const raw = `{"iss":"https://op.example.com","sub":"subject","cnf":{"jwk":{"kty":"EC","crv":"P-256","x":"a","y":"b"}}}` + + var claims oidc.IDTokenClaims + require.NoError(t, json.Unmarshal([]byte(raw), &claims)) + require.NotNil(t, claims.Confirmation) + assert.JSONEq(t, `{"kty":"EC","crv":"P-256","x":"a","y":"b"}`, string(claims.Confirmation.JWK)) + + // And it survives a marshal/unmarshal cycle without duplicating. + marshalled, err := json.Marshal(&claims) + require.NoError(t, err) + assert.Equal(t, 1, strings.Count(string(marshalled), `"cnf"`)) + + var again oidc.IDTokenClaims + require.NoError(t, json.Unmarshal(marshalled, &again)) + require.NotNil(t, again.Confirmation) + assert.JSONEq(t, string(claims.Confirmation.JWK), string(again.Confirmation.JWK)) + + // An unbound token must have no cnf at all. + unbound, err := json.Marshal(&oidc.IDTokenClaims{}) + require.NoError(t, err) + assert.NotContains(t, string(unbound), "cnf") +} diff --git a/pkg/oidc/error.go b/pkg/oidc/error.go index e9f7327e..a76d93b9 100644 --- a/pkg/oidc/error.go +++ b/pkg/oidc/error.go @@ -33,6 +33,12 @@ const ( // the requested target or audience is invalid. // [RFC 8693, Section 2.2.2: Error Response](https://www.rfc-editor.org/rfc/rfc8693#section-2.2.2) InvalidTarget errorType = "invalid_target" + + // InvalidDPoPProof error is returned by the token endpoint when the + // DPoP header is missing, malformed, or otherwise fails the checks + // defined in [RFC 9449, Section 4.3] or OpenID Connect Key Binding 1.0. + // [RFC 9449, Section 12.2: OAuth Extensions Error Registration](https://www.rfc-editor.org/rfc/rfc9449#section-12.2) + InvalidDPoPProof errorType = "invalid_dpop_proof" ) var ( @@ -126,6 +132,14 @@ var ( Description: "The requested audience or target is invalid.", } } + + // ErrInvalidDPoPProof is returned by the token endpoint when the DPoP + // proof JWT fails validation. + ErrInvalidDPoPProof = func() *Error { + return &Error{ + ErrorType: InvalidDPoPProof, + } + } ) type Error struct { diff --git a/pkg/oidc/token.go b/pkg/oidc/token.go index e1a0ae93..0ef5c9ab 100644 --- a/pkg/oidc/token.go +++ b/pkg/oidc/token.go @@ -147,7 +147,13 @@ type IDTokenClaims struct { UserInfoEmail UserInfoPhone Address *UserInfoAddress `json:"address,omitempty"` - Claims map[string]any `json:"-"` + + // Confirmation carries the `cnf` claim for a key-bound ID Token, as + // defined by OpenID Connect Key Binding 1.0, Section 4. It is nil for + // bearer (unbound) ID Tokens. + Confirmation *Confirmation `json:"cnf,omitempty"` + + Claims map[string]any `json:"-"` } // GetAccessTokenHash implements the IDTokenClaims interface diff --git a/pkg/op/auth_request.go b/pkg/op/auth_request.go index 70984fe7..3eca233d 100644 --- a/pkg/op/auth_request.go +++ b/pkg/op/auth_request.go @@ -45,6 +45,15 @@ type AuthRequestSessionState interface { GetSessionState() string } +// BoundKeyRequest should be implemented by persisted authorization and +// refresh-token requests that support OpenID Connect Key Binding. +type BoundKeyRequest interface { + // GetDPoPJKT returns the dpop_jkt value committed to by the + // Authentication Request, or an empty string if the request did not + // request a key-bound ID Token. + GetDPoPJKT() string +} + type Authorizer interface { Storage() Storage Decoder() httphelper.Decoder @@ -110,6 +119,7 @@ func Authorize(w http.ResponseWriter, r *http.Request, authorizer Authorizer) { } var client Client + customValidation := false validation := func(ctx context.Context, authReq *oidc.AuthRequest, storage Storage, verifier *IDTokenHintVerifier) (sub string, err error) { client, err = authorizer.Storage().GetClientByClientID(ctx, authReq.ClientID) if err != nil { @@ -119,6 +129,7 @@ func Authorize(w http.ResponseWriter, r *http.Request, authorizer Authorizer) { } if validator, ok := authorizer.(AuthorizeValidator); ok { validation = validator.ValidateAuthRequest + customValidation = true } userID, err := validation(ctx, authReq, authorizer.Storage(), authorizer.IDTokenHintVerifier(ctx)) if err != nil { @@ -142,6 +153,21 @@ func Authorize(w http.ResponseWriter, r *http.Request, authorizer Authorizer) { return } } + if customValidation && (authReq.DPoPJKT != "" || slices.Contains(authReq.Scopes, oidc.ScopeBoundKey)) { + if err = ValidateAuthReqRedirectURI(client, authReq.RedirectURI, authReq.ResponseType); err != nil { + AuthRequestError(w, r, authReq, err, authorizer) + return + } + authReq.Scopes, err = ValidateAuthReqScopes(client, authReq.Scopes) + if err != nil { + AuthRequestError(w, r, authReq, err, authorizer) + return + } + if err = ValidateAuthReqBoundKey(authReq); err != nil { + AuthRequestError(w, r, authReq, err, authorizer) + return + } + } req, err := authorizer.Storage().CreateAuthRequest(ctx, authReq, userID) if err != nil { AuthRequestError(w, r, authReq, oidc.DefaultToServerError(err, "unable to save auth request"), authorizer) @@ -238,6 +264,9 @@ func CopyRequestObjectToAuthRequest(authReq *oidc.AuthRequest, requestObject *oi if requestObject.CodeChallengeMethod != "" { authReq.CodeChallengeMethod = requestObject.CodeChallengeMethod } + if requestObject.DPoPJKT != "" { + authReq.DPoPJKT = requestObject.DPoPJKT + } authReq.RequestParam = "" } @@ -272,6 +301,9 @@ func ValidateAuthRequestClient(ctx context.Context, authReq *oidc.AuthRequest, c if err != nil { return "", err } + if err := ValidateAuthReqBoundKey(authReq); err != nil { + return "", err + } if err := ValidateAuthReqResponseType(client, authReq.ResponseType); err != nil { return "", err } @@ -293,6 +325,12 @@ func ValidateAuthReqPrompt(prompts []string, maxAge *uint) (_ *uint, err error) // ValidateAuthReqScopes validates the passed scopes and deletes any unsupported scopes. // An error is returned if scopes is empty. +// +// The standard OpenID Connect scopes listed below are always kept, without +// consulting the client. Every other scope, including extension scopes such as +// `bound_key`, is kept only if client.IsScopeAllowed reports it as allowed. +// So a client that must not use key binding simply has `bound_key` stripped +// here, and [ValidateAuthReqBoundKey] then treats the request as unbound. func ValidateAuthReqScopes(client Client, scopes []string) ([]string, error) { if len(scopes) == 0 { return nil, oidc.ErrInvalidRequest(). @@ -311,6 +349,33 @@ func ValidateAuthReqScopes(client Client, scopes []string) ([]string, error) { return scopes, nil } +// ValidateAuthReqBoundKey validates the pairing of the `bound_key` scope and the +// `dpop_jkt` parameter, as required by OpenID Connect Key Binding 1.0, Section 2.1. +// Without `bound_key`, dpop_jkt is cleared and ignored rather than rejected. +// +// Call only after [ValidateAuthReqScopes] has filtered the scopes and after +// redirect_uri validation, since it returns an [oidc.Error] that redirects to +// authReq.RedirectURI. +func ValidateAuthReqBoundKey(authReq *oidc.AuthRequest) error { + if !slices.Contains(authReq.Scopes, oidc.ScopeBoundKey) { + authReq.DPoPJKT = "" + return nil + } + if !slices.Contains(authReq.Scopes, oidc.ScopeOpenID) { + return oidc.ErrInvalidRequest().WithDescription("openid is required when the bound_key scope is requested") + } + if authReq.DPoPJKT == "" { + return oidc.ErrInvalidRequest().WithDescription("dpop_jkt is required when the bound_key scope is requested") + } + if !oidc.ValidDPoPJKT(authReq.DPoPJKT) { + return oidc.ErrInvalidRequest().WithDescription("dpop_jkt is not a valid JWK SHA-256 thumbprint") + } + if authReq.ResponseType != oidc.ResponseTypeCode { + return oidc.ErrInvalidRequest().WithDescription("bound_key is only supported with the authorization code flow") + } + return nil +} + // checkURIAgainstRedirects just checks against the valid redirect URIs and ignores // other factors. func checkURIAgainstRedirects(client Client, uri string) error { diff --git a/pkg/op/auth_request_test.go b/pkg/op/auth_request_test.go index 31dbbc59..3dbb3063 100644 --- a/pkg/op/auth_request_test.go +++ b/pkg/op/auth_request_test.go @@ -404,6 +404,74 @@ func TestValidateAuthReqScopes(t *testing.T) { } } +func TestValidateAuthReqBoundKey(t *testing.T) { + const validJKT = "dnfb1T9jil_gOhti60baHs_WD_a4D8JN9VDJXbmBmGw" + tests := []struct { + name string + request *oidc.AuthRequest + wantError bool + wantJKT string + }{ + { + name: "unbound request", + request: &oidc.AuthRequest{Scopes: []string{oidc.ScopeOpenID}}, + }, + { + name: "dpop_jkt without bound_key is ignored", + request: &oidc.AuthRequest{Scopes: []string{oidc.ScopeOpenID}, DPoPJKT: validJKT}, + }, + { + name: "bound_key requires dpop_jkt", + request: &oidc.AuthRequest{Scopes: []string{oidc.ScopeOpenID, oidc.ScopeBoundKey}}, + wantError: true, + }, + { + name: "bound_key requires openid", + request: &oidc.AuthRequest{Scopes: []string{oidc.ScopeBoundKey}, ResponseType: oidc.ResponseTypeCode, DPoPJKT: validJKT}, + wantError: true, + wantJKT: validJKT, + }, + { + name: "bound_key rejects malformed dpop_jkt", + request: &oidc.AuthRequest{Scopes: []string{oidc.ScopeOpenID, oidc.ScopeBoundKey}, DPoPJKT: "invalid"}, + wantError: true, + wantJKT: "invalid", + }, + { + name: "bound request", + request: &oidc.AuthRequest{Scopes: []string{oidc.ScopeOpenID, oidc.ScopeBoundKey}, ResponseType: oidc.ResponseTypeCode, DPoPJKT: validJKT}, + wantJKT: validJKT, + }, + { + name: "bound request rejects implicit flow", + request: &oidc.AuthRequest{Scopes: []string{oidc.ScopeOpenID, oidc.ScopeBoundKey}, ResponseType: oidc.ResponseTypeIDTokenOnly, DPoPJKT: validJKT}, + wantError: true, + wantJKT: validJKT, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := op.ValidateAuthReqBoundKey(tt.request) + if tt.wantError { + require.ErrorIs(t, err, oidc.ErrInvalidRequest()) + } else { + require.NoError(t, err) + } + assert.Equal(t, tt.wantJKT, tt.request.DPoPJKT) + }) + } +} + +func TestCopyRequestObjectToAuthRequest_DPoPJKT(t *testing.T) { + const jkt = "dnfb1T9jil_gOhti60baHs_WD_a4D8JN9VDJXbmBmGw" + authReq := &oidc.AuthRequest{RequestParam: "request-object"} + + op.CopyRequestObjectToAuthRequest(authReq, &oidc.RequestObject{AuthRequest: oidc.AuthRequest{DPoPJKT: jkt}}) + + assert.Equal(t, jkt, authReq.DPoPJKT) + assert.Empty(t, authReq.RequestParam) +} + func TestValidateAuthReqRedirectURI(t *testing.T) { type args struct { uri string diff --git a/pkg/op/discovery.go b/pkg/op/discovery.go index e3ca6035..8dde5ab3 100644 --- a/pkg/op/discovery.go +++ b/pkg/op/discovery.go @@ -3,6 +3,7 @@ package op import ( "context" "net/http" + "slices" jose "github.com/go-jose/go-jose/v4" @@ -60,6 +61,7 @@ func CreateDiscoveryConfig(ctx context.Context, config Configuration, storage Di RevocationEndpointAuthMethodsSupported: AuthMethodsRevocationEndpoint(config), ClaimsSupported: SupportedClaims(config), CodeChallengeMethodsSupported: CodeChallengeMethods(config), + DPoPSigningAlgValuesSupported: DPoPSigningAlgorithms(config), UILocalesSupported: config.SupportedUILocales(), RequestParameterSupported: config.RequestObjectSupported(), BackChannelLogoutSupported: config.BackChannelLogoutSupported(), @@ -93,6 +95,7 @@ func createDiscoveryConfigV2(ctx context.Context, config Configuration, storage RevocationEndpointAuthMethodsSupported: AuthMethodsRevocationEndpoint(config), ClaimsSupported: SupportedClaims(config), CodeChallengeMethodsSupported: CodeChallengeMethods(config), + DPoPSigningAlgValuesSupported: DPoPSigningAlgorithms(config), UILocalesSupported: config.SupportedUILocales(), RequestParameterSupported: config.RequestObjectSupported(), BackChannelLogoutSupported: config.BackChannelLogoutSupported(), @@ -101,13 +104,27 @@ func createDiscoveryConfigV2(ctx context.Context, config Configuration, storage } func Scopes(c Configuration) []string { - provider, ok := c.(*Provider) - if ok && provider.config.SupportedScopes != nil { - return provider.config.SupportedScopes + // Any Configuration may advertise its own scopes, not just *Provider, + // so that DPoPSigningAlgorithms and discovery agree on whether + // key binding is enabled. + if provider, ok := c.(interface{ ScopesSupported() []string }); ok { + if scopes := provider.ScopesSupported(); len(scopes) > 0 { + return scopes + } } return DefaultSupportedScopes } +// DPoPSigningAlgorithms returns the dpop_signing_alg_values_supported +// discovery metadata. It is empty unless the OP advertises the `bound_key` +// scope, since key binding is the only DPoP feature implemented. +func DPoPSigningAlgorithms(c Configuration) []string { + if !slices.Contains(Scopes(c), oidc.ScopeBoundKey) { + return nil + } + return DPoPSigAlgorithms(nil) +} + func ResponseTypes(c Configuration) []string { return []string{ string(oidc.ResponseTypeCode), diff --git a/pkg/op/discovery_test.go b/pkg/op/discovery_test.go index 4206d0da..e2112daf 100644 --- a/pkg/op/discovery_test.go +++ b/pkg/op/discovery_test.go @@ -77,6 +77,16 @@ func TestCreateDiscoveryConfig(t *testing.T) { } } +// scopeConfiguration is a minimal op.Configuration that is not a *op.Provider, +// used to verify that Scopes() honours any implementation advertising scopes. +// Only ScopesSupported is exercised, so the embedded interface stays nil. +type scopeConfiguration struct { + op.Configuration + scopes []string +} + +func (c scopeConfiguration) ScopesSupported() []string { return c.scopes } + func Test_scopes(t *testing.T) { type args struct { c op.Configuration @@ -96,6 +106,23 @@ func Test_scopes(t *testing.T) { args{newTestProvider(&op.Config{SupportedScopes: []string{"test1", "test2"}})}, []string{"test1", "test2"}, }, + { + // A Configuration advertising no scopes must fall back to the + // defaults rather than an empty scopes_supported. + "empty custom scopes", + args{newTestProvider(&op.Config{SupportedScopes: []string{}})}, + op.DefaultSupportedScopes, + }, + { + "non-provider configuration advertising scopes", + args{scopeConfiguration{scopes: []string{oidc.ScopeOpenID, oidc.ScopeBoundKey}}}, + []string{oidc.ScopeOpenID, oidc.ScopeBoundKey}, + }, + { + "non-provider configuration advertising no scopes", + args{scopeConfiguration{}}, + op.DefaultSupportedScopes, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -105,6 +132,15 @@ func Test_scopes(t *testing.T) { } } +func TestDPoPSigningAlgorithms(t *testing.T) { + assert.Nil(t, op.DPoPSigningAlgorithms(newTestProvider(&op.Config{}))) + assert.Equal(t, + op.DPoPSigAlgorithms(op.DefaultDPoPSigningAlgs), + op.DPoPSigningAlgorithms(newTestProvider(&op.Config{SupportedScopes: []string{oidc.ScopeOpenID, oidc.ScopeBoundKey}})), + ) + assert.Empty(t, op.DPoPSigAlgorithms([]jose.SignatureAlgorithm{jose.HS256})) +} + func Test_ResponseTypes(t *testing.T) { type args struct { c op.Configuration @@ -631,3 +667,62 @@ func Test_CodeChallengeMethods(t *testing.T) { }) } } + +// nonDeviceStorage is a minimal op.Storage used to construct a provider in the +// WithKeyBinding tests. +type nonDeviceStorage struct{ op.Storage } + +// TestWithKeyBinding covers the provider option: it must make discovery +// advertise the feature and must not mutate the caller's Config. +func TestWithKeyBinding(t *testing.T) { + newProvider := func(t *testing.T, config *op.Config, storage op.Storage, opts ...op.Option) (*op.Provider, error) { + t.Helper() + return op.NewOpenIDProvider(testIssuer, config, storage, + append([]op.Option{op.WithAllowInsecure()}, opts...)...) + } + + t.Run("advertises bound_key and DPoP algorithms", func(t *testing.T) { + config := &op.Config{CryptoKey: testConfig.CryptoKey} + provider, err := newProvider(t, config, nonDeviceStorage{}, op.WithKeyBinding()) + require.NoError(t, err) + + assert.Contains(t, provider.ScopesSupported(), oidc.ScopeBoundKey) + assert.Equal(t, op.DPoPSigAlgorithms(nil), op.DPoPSigningAlgorithms(provider)) + // The caller's Config must be left alone. + assert.Nil(t, config.SupportedScopes) + }) + + t.Run("disabled by default", func(t *testing.T) { + provider, err := newProvider(t, &op.Config{CryptoKey: testConfig.CryptoKey}, nonDeviceStorage{}) + require.NoError(t, err) + + assert.NotContains(t, provider.ScopesSupported(), oidc.ScopeBoundKey) + assert.Empty(t, op.DPoPSigningAlgorithms(provider)) + }) + + t.Run("appends to custom scopes without duplicating", func(t *testing.T) { + provider, err := newProvider(t, + &op.Config{CryptoKey: testConfig.CryptoKey, SupportedScopes: []string{oidc.ScopeOpenID}}, + nonDeviceStorage{}, op.WithKeyBinding()) + require.NoError(t, err) + assert.Equal(t, []string{oidc.ScopeOpenID, oidc.ScopeBoundKey}, provider.ScopesSupported()) + + // Already advertised explicitly: must not be added twice. + provider, err = newProvider(t, + &op.Config{CryptoKey: testConfig.CryptoKey, SupportedScopes: []string{oidc.ScopeOpenID, oidc.ScopeBoundKey}}, + nonDeviceStorage{}, op.WithKeyBinding()) + require.NoError(t, err) + assert.Equal(t, []string{oidc.ScopeOpenID, oidc.ScopeBoundKey}, provider.ScopesSupported()) + }) + + t.Run("option order does not matter", func(t *testing.T) { + first, err := newProvider(t, &op.Config{CryptoKey: testConfig.CryptoKey}, nonDeviceStorage{}, + op.WithKeyBinding(), op.WithAllowInsecure()) + require.NoError(t, err) + second, err := newProvider(t, &op.Config{CryptoKey: testConfig.CryptoKey}, nonDeviceStorage{}, + op.WithAllowInsecure(), op.WithKeyBinding()) + require.NoError(t, err) + assert.Equal(t, first.ScopesSupported(), second.ScopesSupported()) + assert.Contains(t, first.ScopesSupported(), oidc.ScopeBoundKey) + }) +} diff --git a/pkg/op/dpop.go b/pkg/op/dpop.go new file mode 100644 index 00000000..79764215 --- /dev/null +++ b/pkg/op/dpop.go @@ -0,0 +1,466 @@ +package op + +import ( + "context" + "crypto/subtle" + "encoding/json" + "fmt" + "net" + "net/http" + "net/url" + "slices" + "strings" + "time" + + jose "github.com/go-jose/go-jose/v4" + + "github.com/zitadel/oidc/v3/pkg/oidc" +) + +// This file implements the OP side of OpenID Connect Key Binding 1.0. The +// specification is still a draft, but it is already deployed by production +// IdPs, so the wire format is stable in practice; any adjustments to track the +// final specification will follow the normal deprecation process rather than +// changing without notice. + +// DefaultDPoPSigningAlgs is the default, and recommended, allow-list of JWS +// algorithms accepted for DPoP proof JWTs. It excludes "none" and symmetric +// (HMAC) algorithms, which [RFC 9449, Section 4.2] forbids. It also derives the +// dpop_signing_alg_values_supported metadata (see [DPoPSigAlgorithms]), so +// discovery and the token endpoint cannot drift apart. +// +// [RFC 9449, Section 4.2]: https://www.rfc-editor.org/rfc/rfc9449#section-4.2 +var DefaultDPoPSigningAlgs = []jose.SignatureAlgorithm{ + jose.ES256, jose.ES384, jose.ES512, + jose.PS256, jose.PS384, jose.PS512, + jose.RS256, jose.RS384, jose.RS512, + jose.EdDSA, +} + +// DefaultDPoPProofMaxAge is the default acceptance window for the `iat` +// claim of a DPoP proof JWT, in either direction. Since this implementation +// does not maintain a `jti` replay cache (see [RFC 9449, Section 11.1]), a +// short window is used to bound how long a captured proof remains usable. +// +// [RFC 9449, Section 11.1]: https://www.rfc-editor.org/rfc/rfc9449#section-11.1 +const DefaultDPoPProofMaxAge = time.Minute + +// DPoPProofVerifier validates DPoP proof JWTs per [RFC 9449, Section 4.3], +// plus the additional c_s256 and dpop_jkt bindings required by OpenID +// Connect Key Binding 1.0, Sections 2.3, 3.3 and 5. +// +// It does not implement `jti` replay detection ([RFC 9449, Section 11.1]); +// callers relying solely on this verifier should keep ProofMaxAge short. +// +// NOTE: the fields below apply only when you call Verify yourself. An OP built +// with [NewOpenIDProvider] always uses [DefaultDPoPSigningAlgs] and +// [DefaultDPoPProofMaxAge], which is what discovery advertises. +type DPoPProofVerifier struct { + // SupportedSignAlgs is the allow-list of JWS algorithms accepted for + // DPoP proof JWTs. Defaults to [DefaultDPoPSigningAlgs]. It MUST NOT + // contain "none" or a symmetric (HMAC) algorithm. + SupportedSignAlgs []jose.SignatureAlgorithm + + // ProofMaxAge bounds how far the `iat` claim of a proof may be from the + // current time, in either direction. Defaults to [DefaultDPoPProofMaxAge]. + ProofMaxAge time.Duration + + // Now returns the current time. Defaults to time.Now; overridable for + // tests. + Now func() time.Time +} + +// NewDPoPProofVerifier returns a DPoPProofVerifier configured with +// [DefaultDPoPSigningAlgs] and [DefaultDPoPProofMaxAge]. +func NewDPoPProofVerifier() *DPoPProofVerifier { + return &DPoPProofVerifier{ + SupportedSignAlgs: DefaultDPoPSigningAlgs, + ProofMaxAge: DefaultDPoPProofMaxAge, + Now: time.Now, + } +} + +func (v *DPoPProofVerifier) signAlgs() []jose.SignatureAlgorithm { + algs := v.SupportedSignAlgs + if len(algs) == 0 { + algs = DefaultDPoPSigningAlgs + } + return slices.DeleteFunc(slices.Clone(algs), func(alg jose.SignatureAlgorithm) bool { + return !isAsymmetricDPoPAlgorithm(alg) + }) +} + +func isAsymmetricDPoPAlgorithm(alg jose.SignatureAlgorithm) bool { + switch alg { + case jose.ES256, jose.ES384, jose.ES512, + jose.PS256, jose.PS384, jose.PS512, + jose.RS256, jose.RS384, jose.RS512, + jose.EdDSA: + return true + default: + return false + } +} + +func (v *DPoPProofVerifier) now() time.Time { + if v.Now != nil { + return v.Now() + } + return time.Now() +} + +func (v *DPoPProofVerifier) maxAge() time.Duration { + if v.ProofMaxAge > 0 { + return v.ProofMaxAge + } + return DefaultDPoPProofMaxAge +} + +// Verify validates the single DPoP proof JWT in header against the request's +// method and target URI (htu, without query or fragment), and confirms it was +// created for the key committed to by expectedJKT. On success it returns the +// confirmation to embed in the ID Token's `cnf` claim. +// +// A non-empty boundCode (authorization code or device code) requires the proof's +// c_s256 claim to match its SHA-256 hash, per Key Binding 1.0 Sections 2.3 and +// 3.3; an empty boundCode is the Refresh Request case (Section 5), where no +// c_s256 is required. +func (v *DPoPProofVerifier) Verify(header http.Header, method, htu, expectedJKT, boundCode string) (*oidc.Confirmation, error) { + proof, err := singleHeaderValue(header, oidc.DPoPHeader) + if err != nil { + return nil, oidc.ErrInvalidDPoPProof().WithParent(err).WithDescription("%s", err.Error()) + } + + // RFC 9449 §4.3 checks 2 and 5. Do NOT substitute ParseSigned: it accepts the + // JWS JSON Serialization, whose unprotected header is not covered by the + // signature, letting an attacker inject `typ` and `jwk` and pass an unrelated + // JWS off as a proof. A proof is a JWT, so compact serialization is required + // (RFC 7519 §3). This also enforces alg is in v.signAlgs(). + jws, err := jose.ParseSignedCompact(proof, v.signAlgs()) + if err != nil { + return nil, oidc.ErrInvalidDPoPProof().WithParent(err).WithDescription("malformed DPoP proof: %s", err.Error()) + } + if len(jws.Signatures) != 1 { + return nil, oidc.ErrInvalidDPoPProof().WithDescription("DPoP proof must have exactly one signature") + } + sig := jws.Signatures[0] + + // Every header below MUST be read from sig.Protected, never sig.Header: + // sig.Header merges the protected and unprotected headers and is therefore + // not integrity protected. Compact serialization has no unprotected + // header, but read from Protected regardless so this stays correct if the + // parser above is ever changed. + if len(sig.Unprotected.ExtraHeaders) > 0 || sig.Unprotected.JSONWebKey != nil { + return nil, oidc.ErrInvalidDPoPProof().WithDescription("DPoP proof must not have an unprotected header") + } + + // Check 4: typ must be dpop+jwt, in the protected header. + typ, _ := sig.Protected.ExtraHeaders[jose.HeaderType].(string) + if typ != string(oidc.DPoPProofType) { + return nil, oidc.ErrInvalidDPoPProof().WithDescription("typ must be %q", oidc.DPoPProofType) + } + + // Checks 6 and 7: jwk header must be present, integrity protected, and a + // public key. jose already rejects an embedded private or symmetric key + // (see (*rawJSONWebSignature).sanitized), but we check explicitly for a + // precise error and defense in depth. + jwk := sig.Protected.JSONWebKey + if jwk == nil { + return nil, oidc.ErrInvalidDPoPProof().WithDescription("missing jwk header") + } + if !jwk.Valid() || !jwk.IsPublic() { + return nil, oidc.ErrInvalidDPoPProof().WithDescription("jwk header must be a public key") + } + if err := oidc.ValidateDPoPKeyStrength(jwk.Key); err != nil { + return nil, oidc.ErrInvalidDPoPProof().WithParent(err).WithDescription("%s", err.Error()) + } + + // Check 6: signature verifies with the embedded key. Reuse the + // already-parsed jwk; do not re-parse the key material. + payload, err := jws.Verify(jwk) + if err != nil { + return nil, oidc.ErrInvalidDPoPProof().WithParent(err).WithDescription("signature verification failed") + } + + claims := new(oidc.DPoPProofClaims) + if err := json.Unmarshal(payload, claims); err != nil { + return nil, oidc.ErrInvalidDPoPProof().WithParent(err).WithDescription("malformed proof claims") + } + + // Check 3: required claims. + if claims.JWTID == "" { + return nil, oidc.ErrInvalidDPoPProof().WithDescription("missing jti claim") + } + if claims.HTTPMethod == "" { + return nil, oidc.ErrInvalidDPoPProof().WithDescription("missing htm claim") + } + if claims.HTTPURI == "" { + return nil, oidc.ErrInvalidDPoPProof().WithDescription("missing htu claim") + } + if claims.IssuedAt == 0 { + return nil, oidc.ErrInvalidDPoPProof().WithDescription("missing iat claim") + } + + // Check 8: htm matches. + if claims.HTTPMethod != method { + return nil, oidc.ErrInvalidDPoPProof().WithDescription("htm does not match the request method") + } + + // Check 9: htu matches, ignoring query and fragment, with syntax- and + // scheme-based normalization (RFC 3986 §§6.2.2, 6.2.3). + wantHTU, err := normalizeHTU(htu) + if err != nil { + return nil, oidc.ErrInvalidDPoPProof().WithParent(err).WithDescription("invalid configured token endpoint") + } + gotHTU, err := normalizeHTU(claims.HTTPURI) + if err != nil { + return nil, oidc.ErrInvalidDPoPProof().WithParent(err).WithDescription("invalid htu claim") + } + if gotHTU != wantHTU { + return nil, oidc.ErrInvalidDPoPProof().WithDescription("htu does not match the request URI") + } + + // Check 11: iat is within the acceptance window. + iat := claims.IssuedAt.AsTime() + if age := v.now().Sub(iat); age > v.maxAge() || age < -v.maxAge() { + return nil, oidc.ErrInvalidDPoPProof().WithDescription("iat is outside the acceptable window") + } + + // OpenID Connect Key Binding 1.0, §§2.3/3.3: the proof must be bound to + // the authorization code (or device code) of this token request. + if boundCode != "" { + want := oidc.CodeHash(boundCode) + if !constantTimeEqual(claims.CodeHash, want) { + return nil, oidc.ErrInvalidDPoPProof().WithDescription("c_s256 does not match the presented code") + } + } + + // OpenID Connect Key Binding 1.0, §2.1: the proof key must match the + // thumbprint committed to in the dpop_jkt Authentication Request + // parameter. + gotJKT, err := oidc.JWKThumbprint(jwk) + if err != nil { + return nil, oidc.ErrInvalidDPoPProof().WithParent(err).WithDescription("unable to compute JWK thumbprint") + } + if !constantTimeEqual(gotJKT, expectedJKT) { + return nil, oidc.ErrInvalidDPoPProof().WithDescription("the proof key does not match the committed dpop_jkt") + } + + // Canonicalize before use: strips kid/use/alg/x5c and any other + // attacker- or client-supplied extras, and keeps the value stable + // across a refresh (OpenID Connect Key Binding 1.0, Section 5). + canonical, err := oidc.CanonicalJWK(jwk) + if err != nil { + return nil, oidc.ErrInvalidDPoPProof().WithParent(err).WithDescription("unable to canonicalize JWK") + } + return &oidc.Confirmation{JWK: canonical}, nil +} + +// verifyBoundKey verifies a token request only when its persisted request is +// key-bound. Requests without a binding ignore DPoP headers, as required for +// compatibility with DPoP-bound access-token clients. +func verifyBoundKey(header http.Header, method, htu string, request IDTokenRequest, boundCode string) (*oidc.Confirmation, error) { + jkt, err := boundKeyThumbprint(request) + if err != nil || jkt == "" { + return nil, err + } + return NewDPoPProofVerifier().Verify(header, method, htu, jkt, boundCode) +} + +// keyBindingIntegrated reports whether the OP's storage has integrated key +// binding, which is true exactly when the persisted request type implements +// [BoundKeyRequest]. +func keyBindingIntegrated(request IDTokenRequest) (BoundKeyRequest, bool) { + boundRequest, ok := request.(BoundKeyRequest) + return boundRequest, ok +} + +func boundKeyThumbprint(request IDTokenRequest) (string, error) { + boundRequest, integrated := keyBindingIntegrated(request) + if !integrated { + // Key binding is not implemented by this storage; ignore it. + return "", nil + } + jkt := boundRequest.GetDPoPJKT() + if jkt == "" { + // The storage does know about key binding, so a granted bound_key with + // no persisted thumbprint is a bug in the OP, not an unsupported + // feature. Fail closed rather than issue an unbound token. + if slices.Contains(request.GetScopes(), oidc.ScopeBoundKey) { + return "", oidc.ErrServerError().WithDescription("bound_key request is missing its persisted dpop_jkt") + } + return "", nil + } + if !oidc.ValidDPoPJKT(jkt) { + return "", oidc.ErrServerError().WithDescription("bound_key request has an invalid persisted dpop_jkt") + } + return jkt, nil +} + +func verifyProviderBoundKey(ctx context.Context, header http.Header, method string, request IDTokenRequest, boundCode string, provider any) (*oidc.Confirmation, error) { + jkt, err := boundKeyThumbprint(request) + if err != nil || jkt == "" { + return nil, err + } + tokenEndpointer, ok := provider.(interface{ TokenEndpoint() *Endpoint }) + if !ok || tokenEndpointer.TokenEndpoint() == nil { + return nil, oidc.ErrServerError().WithDescription("unable to determine token endpoint for bound_key request") + } + htu := tokenEndpointer.TokenEndpoint().Absolute(IssuerFromContext(ctx)) + return NewDPoPProofVerifier().Verify(header, method, htu, jkt, boundCode) +} + +// singleHeaderValue returns the single value of the named HTTP header, +// erroring if it is absent or repeated (RFC 9449 §4.3, check 1). +func singleHeaderValue(header http.Header, name string) (string, error) { + values := header.Values(name) + switch len(values) { + case 0: + return "", fmt.Errorf("missing %s header", name) + case 1: + return values[0], nil + default: + return "", fmt.Errorf("multiple %s headers", name) + } +} + +// normalizeHTU applies RFC 3986 §6.2.2 (syntax-based) and §6.2.3 +// (scheme-based) normalization to raw and strips any query and fragment, +// as recommended by RFC 9449 §4.3 for comparing the htu claim. +func normalizeHTU(raw string) (string, error) { + u, err := url.Parse(raw) + if err != nil { + return "", err + } + u.Scheme = strings.ToLower(u.Scheme) + if u.Scheme != "http" && u.Scheme != "https" || u.Host == "" || u.User != nil { + return "", fmt.Errorf("htu must be an absolute HTTP URI without user information") + } + hostname := strings.ToLower(u.Hostname()) + port := u.Port() + if port == "" || u.Scheme == "http" && port == "80" || u.Scheme == "https" && port == "443" { + if strings.Contains(hostname, ":") { + u.Host = "[" + hostname + "]" + } else { + u.Host = hostname + } + } else { + u.Host = net.JoinHostPort(hostname, port) + } + escapedPath, err := normalizePercentEncoding(u.EscapedPath()) + if err != nil { + return "", err + } + if escapedPath == "" { + escapedPath = "/" + } + escapedPath = removeDotSegments(escapedPath) + u.Path, err = url.PathUnescape(escapedPath) + if err != nil { + return "", err + } + u.RawPath = escapedPath + u.RawQuery = "" + u.Fragment = "" + u.RawFragment = "" + return u.String(), nil +} + +func normalizePercentEncoding(value string) (string, error) { + var normalized strings.Builder + normalized.Grow(len(value)) + for i := 0; i < len(value); i++ { + if value[i] != '%' { + normalized.WriteByte(value[i]) + continue + } + if i+2 >= len(value) { + return "", fmt.Errorf("invalid percent encoding") + } + decoded, err := url.PathUnescape(value[i : i+3]) + if err != nil || len(decoded) != 1 { + return "", fmt.Errorf("invalid percent encoding") + } + if isUnreserved(decoded[0]) { + normalized.WriteByte(decoded[0]) + } else { + normalized.WriteByte('%') + normalized.WriteString(strings.ToUpper(value[i+1 : i+3])) + } + i += 2 + } + return normalized.String(), nil +} + +func isUnreserved(value byte) bool { + return value >= 'a' && value <= 'z' || value >= 'A' && value <= 'Z' || + value >= '0' && value <= '9' || strings.ContainsRune("-._~", rune(value)) +} + +// removeDotSegments implements RFC 3986 Section 5.2.4 without collapsing +// empty path segments. +func removeDotSegments(input string) string { + var output string + for input != "" { + switch { + case strings.HasPrefix(input, "../"): + input = input[3:] + case strings.HasPrefix(input, "./"): + input = input[2:] + case strings.HasPrefix(input, "/./"): + input = input[2:] + case input == "/.": + input = "/" + case strings.HasPrefix(input, "/../"): + input = input[3:] + output = removeLastPathSegment(output) + case input == "/..": + input = "/" + output = removeLastPathSegment(output) + case input == "." || input == "..": + input = "" + default: + segmentEnd := strings.IndexByte(input[1:], '/') + if input[0] != '/' { + segmentEnd = strings.IndexByte(input, '/') + if segmentEnd < 0 { + segmentEnd = len(input) + } + } else if segmentEnd < 0 { + segmentEnd = len(input) + } else { + segmentEnd++ + } + output += input[:segmentEnd] + input = input[segmentEnd:] + } + } + return output +} + +func removeLastPathSegment(value string) string { + if index := strings.LastIndexByte(value, '/'); index >= 0 { + return value[:index] + } + return "" +} + +func constantTimeEqual(a, b string) bool { + return subtle.ConstantTimeCompare([]byte(a), []byte(b)) == 1 +} + +// DPoPSigAlgorithms returns the JWS algorithm names to advertise as +// dpop_signing_alg_values_supported in discovery, derived from algs so +// that discovery and the DPoPProofVerifier can never drift apart. +func DPoPSigAlgorithms(algs []jose.SignatureAlgorithm) []string { + if len(algs) == 0 { + algs = DefaultDPoPSigningAlgs + } + out := make([]string, 0, len(algs)) + for _, alg := range algs { + if isAsymmetricDPoPAlgorithm(alg) { + out = append(out, string(alg)) + } + } + return out +} diff --git a/pkg/op/dpop_internal_test.go b/pkg/op/dpop_internal_test.go new file mode 100644 index 00000000..94b85621 --- /dev/null +++ b/pkg/op/dpop_internal_test.go @@ -0,0 +1,256 @@ +package op + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestNormalizeHTU covers the RFC 3986 syntax-based normalization applied to +// both the DPoP proof `htu` claim and the actual request URI before they are +// compared. Over-normalizing would let a proof minted for one endpoint be +// replayed against another, so the encoding-sensitive cases matter. +func TestNormalizeHTU(t *testing.T) { + tests := []struct { + name string + raw string + want string + wantErr bool + }{ + { + name: "already normalized", + raw: "https://op.example.com/oauth/v2/token", + want: "https://op.example.com/oauth/v2/token", + }, + { + name: "scheme and host lowercased", + raw: "HTTPS://OP.Example.COM/token", + want: "https://op.example.com/token", + }, + { + name: "default https port removed", + raw: "https://op.example.com:443/token", + want: "https://op.example.com/token", + }, + { + name: "default http port removed", + raw: "http://op.example.com:80/token", + want: "http://op.example.com/token", + }, + { + name: "non-default port retained", + raw: "https://op.example.com:8443/token", + want: "https://op.example.com:8443/token", + }, + { + name: "http on 443 is not a default port", + raw: "http://op.example.com:443/token", + want: "http://op.example.com:443/token", + }, + { + name: "query and fragment removed", + raw: "https://op.example.com/token?foo=bar#frag", + want: "https://op.example.com/token", + }, + { + name: "empty path becomes root", + raw: "https://op.example.com", + want: "https://op.example.com/", + }, + { + name: "IPv6 host lowercased and bracketed", + raw: "https://[2001:DB8::1]/token", + want: "https://[2001:db8::1]/token", + }, + { + name: "IPv6 host with non-default port", + raw: "https://[2001:DB8::1]:8443/token", + want: "https://[2001:db8::1]:8443/token", + }, + { + // %2F is reserved: decoding it would merge distinct paths and + // allow cross-endpoint proof replay. + name: "encoded slash is preserved", + raw: "https://op.example.com/%2Ftoken", + want: "https://op.example.com/%2Ftoken", + }, + { + name: "lowercase hex in reserved escape uppercased", + raw: "https://op.example.com/%2ftoken", + want: "https://op.example.com/%2Ftoken", + }, + { + name: "unreserved escape decoded", + raw: "https://op.example.com/%7Euser", + want: "https://op.example.com/~user", + }, + { + name: "lowercase unreserved escape decoded", + raw: "https://op.example.com/%7euser", + want: "https://op.example.com/~user", + }, + { + name: "dot segments removed", + raw: "https://op.example.com/a/../../b", + want: "https://op.example.com/b", + }, + { + // Empty segments are meaningful to some routers, so they must + // survive normalization. + name: "empty path segments preserved", + raw: "https://op.example.com//a//b", + want: "https://op.example.com//a//b", + }, + {name: "relative URI rejected", raw: "/token", wantErr: true}, + {name: "missing host rejected", raw: "https:///token", wantErr: true}, + {name: "non-http scheme rejected", raw: "ftp://op.example.com/token", wantErr: true}, + {name: "userinfo rejected", raw: "https://user@op.example.com/token", wantErr: true}, + {name: "userinfo with password rejected", raw: "https://user:pw@op.example.com/token", wantErr: true}, + {name: "truncated percent encoding rejected", raw: "https://op.example.com/token%", wantErr: true}, + {name: "invalid percent encoding rejected", raw: "https://op.example.com/token%zz", wantErr: true}, + {name: "unparseable URI rejected", raw: "https://op.example.com/token\x7f\x00", wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := normalizeHTU(tt.raw) + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +// TestNormalizeHTUIsIdempotent guards against a normalization step that keeps +// changing its own output, which would make comparison order-dependent. +func TestNormalizeHTUIsIdempotent(t *testing.T) { + for _, raw := range []string{ + "HTTPS://OP.Example.COM:443/a/../b/%7euser?q=1#f", + "http://op.example.com:80//a//b/%2Fc", + "https://[2001:DB8::1]:8443/", + } { + once, err := normalizeHTU(raw) + require.NoError(t, err) + twice, err := normalizeHTU(once) + require.NoError(t, err) + assert.Equal(t, once, twice, "normalizeHTU(%q) is not idempotent", raw) + } +} + +func TestNormalizePercentEncoding(t *testing.T) { + tests := []struct { + name string + value string + want string + wantErr bool + }{ + {name: "empty", value: "", want: ""}, + {name: "no escapes", value: "/oauth/v2/token", want: "/oauth/v2/token"}, + {name: "unreserved alpha decoded", value: "%41", want: "A"}, + {name: "unreserved digit decoded", value: "%30", want: "0"}, + {name: "unreserved tilde decoded", value: "%7E", want: "~"}, + {name: "unreserved hyphen decoded", value: "%2D", want: "-"}, + {name: "unreserved period decoded", value: "%2E", want: "."}, + {name: "unreserved underscore decoded", value: "%5F", want: "_"}, + {name: "lowercase hex unreserved decoded", value: "%7e", want: "~"}, + {name: "reserved slash kept uppercase", value: "%2F", want: "%2F"}, + {name: "reserved slash hex uppercased", value: "%2f", want: "%2F"}, + {name: "reserved space kept", value: "%20", want: "%20"}, + {name: "null byte kept", value: "%00", want: "%00"}, + {name: "multibyte utf8 kept per octet", value: "%c3%a9", want: "%C3%A9"}, + {name: "mixed literal and escapes", value: "/a%2Fb/%7ec", want: "/a%2Fb/~c"}, + {name: "trailing percent rejected", value: "abc%", wantErr: true}, + {name: "single hex digit rejected", value: "%4", wantErr: true}, + {name: "non hex digits rejected", value: "%zz", wantErr: true}, + {name: "second digit non hex rejected", value: "%4z", wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := normalizePercentEncoding(tt.value) + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestIsUnreserved(t *testing.T) { + for _, value := range []byte{'a', 'z', 'A', 'Z', '0', '9', '-', '.', '_', '~'} { + assert.True(t, isUnreserved(value), "expected %q to be unreserved", value) + } + for _, value := range []byte{'/', ':', '?', '#', '[', ']', '@', '!', '%', '+', ' ', 0x00, 0x7f, 0xc3} { + assert.False(t, isUnreserved(value), "expected %q to be reserved", value) + } +} + +// TestRemoveDotSegments verifies RFC 3986 Section 5.2.4, including the +// deliberate deviation that empty path segments are not collapsed. +func TestRemoveDotSegments(t *testing.T) { + tests := []struct { + input string + want string + }{ + {"", ""}, + {"/", "/"}, + {"/a/b/c", "/a/b/c"}, + {"/a/./b", "/a/b"}, + {"/./a", "/a"}, + {"/a/../b", "/b"}, + {"/a/b/../c", "/a/c"}, + {"/a/../../b", "/b"}, + {"/a/.", "/a/"}, + {"/a/..", "/"}, + {"/..", "/"}, + {"/../", "/"}, + {"/../a", "/a"}, + {".", ""}, + {"..", ""}, + {"./a", "a"}, + {"../a", "a"}, + // RFC 3986 Section 5.2.4 example. + {"/a/b/c/./../../g", "/a/g"}, + // Empty segments are preserved rather than collapsed. + {"//a//b", "//a//b"}, + {"/a//../b", "/a/b"}, + {"/a/b//", "/a/b//"}, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + assert.Equal(t, tt.want, removeDotSegments(tt.input)) + }) + } +} + +func TestRemoveLastPathSegment(t *testing.T) { + tests := []struct { + value string + want string + }{ + {"", ""}, + {"/", ""}, + {"/a", ""}, + {"/a/b", "/a"}, + {"/a/b/c", "/a/b"}, + {"a", ""}, + {"//a", "/"}, + } + for _, tt := range tests { + t.Run(tt.value, func(t *testing.T) { + assert.Equal(t, tt.want, removeLastPathSegment(tt.value)) + }) + } +} + +func TestConstantTimeEqual(t *testing.T) { + assert.True(t, constantTimeEqual("", "")) + assert.True(t, constantTimeEqual("abc", "abc")) + assert.False(t, constantTimeEqual("abc", "abd")) + assert.False(t, constantTimeEqual("abc", "ab")) + assert.False(t, constantTimeEqual("", "a")) +} diff --git a/pkg/op/dpop_test.go b/pkg/op/dpop_test.go new file mode 100644 index 00000000..c125cc8c --- /dev/null +++ b/pkg/op/dpop_test.go @@ -0,0 +1,655 @@ +package op_test + +import ( + "crypto/ecdsa" + "crypto/ed25519" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "encoding/json" + "math/big" + "net/http" + "strings" + "testing" + "time" + + jose "github.com/go-jose/go-jose/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zitadel/oidc/v3/pkg/oidc" + "github.com/zitadel/oidc/v3/pkg/op" +) + +const ( + testHTM = http.MethodPost + testHTU = "https://server.example.com/token" +) + +type dpopProofOpts struct { + typ jose.ContentType + iat time.Time + jti string + htm string + htu string + codeHash string +} + +func defaultDPoPProofOpts() dpopProofOpts { + return dpopProofOpts{ + typ: oidc.DPoPProofType, + iat: time.Now(), + jti: "e1j3V_bKic8-LAEB", + htm: testHTM, + htu: testHTU, + } +} + +// signDPoPProof builds a DPoP proof JWT, signed by key using alg, with an +// embedded public JWK header, as described by RFC 9449, Section 4.2. +func signDPoPProof(t *testing.T, key any, alg jose.SignatureAlgorithm, opts dpopProofOpts) string { + t.Helper() + so := (&jose.SignerOptions{EmbedJWK: true}).WithType(opts.typ) + signer, err := jose.NewSigner(jose.SigningKey{Algorithm: alg, Key: key}, so) + require.NoError(t, err) + + claims := oidc.DPoPProofClaims{ + JWTID: opts.jti, + HTTPMethod: opts.htm, + HTTPURI: opts.htu, + IssuedAt: oidc.FromTime(opts.iat), + CodeHash: opts.codeHash, + } + payload, err := json.Marshal(claims) + require.NoError(t, err) + + jws, err := signer.Sign(payload) + require.NoError(t, err) + proof, err := jws.CompactSerialize() + require.NoError(t, err) + return proof +} + +func dpopHeader(proof string) http.Header { + h := make(http.Header) + if proof != "" { + h.Set(oidc.DPoPHeader, proof) + } + return h +} + +func jktOf(t *testing.T, pub any) string { + t.Helper() + jwk := &jose.JSONWebKey{Key: pub} + jkt, err := oidc.JWKThumbprint(jwk) + require.NoError(t, err) + return jkt +} + +func newVerifier(now time.Time) *op.DPoPProofVerifier { + v := op.NewDPoPProofVerifier() + v.Now = func() time.Time { return now } + return v +} + +func TestDPoPProofVerifier_Verify_EC(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + now := time.Now() + jkt := jktOf(t, &key.PublicKey) + + proof := signDPoPProof(t, key, jose.ES256, defaultDPoPProofOpts()) + v := newVerifier(now) + + cnf, err := v.Verify(dpopHeader(proof), testHTM, testHTU, jkt, "") + require.NoError(t, err) + require.NotNil(t, cnf) + assert.NotEmpty(t, cnf.JWK) + + var m map[string]any + require.NoError(t, json.Unmarshal(cnf.JWK, &m)) + assert.ElementsMatch(t, []string{"kty", "crv", "x", "y"}, keysOf(m)) +} + +func TestDPoPProofVerifier_Verify_RSA(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + now := time.Now() + jkt := jktOf(t, &key.PublicKey) + + proof := signDPoPProof(t, key, jose.RS256, defaultDPoPProofOpts()) + v := newVerifier(now) + + cnf, err := v.Verify(dpopHeader(proof), testHTM, testHTU, jkt, "") + require.NoError(t, err) + require.NotNil(t, cnf) +} + +func TestDPoPProofVerifier_Verify_EdDSA(t *testing.T) { + pub, priv, err := ed25519.GenerateKey(rand.Reader) + require.NoError(t, err) + now := time.Now() + jkt := jktOf(t, pub) + + proof := signDPoPProof(t, priv, jose.EdDSA, defaultDPoPProofOpts()) + v := newVerifier(now) + + cnf, err := v.Verify(dpopHeader(proof), testHTM, testHTU, jkt, "") + require.NoError(t, err) + require.NotNil(t, cnf) +} + +func TestDPoPProofVerifier_Verify_CodeBinding(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + now := time.Now() + jkt := jktOf(t, &key.PublicKey) + const code = "SplxlOBeZQQYbYS6WxSbIA" + + opts := defaultDPoPProofOpts() + opts.codeHash = oidc.CodeHash(code) + proof := signDPoPProof(t, key, jose.ES256, opts) + v := newVerifier(now) + + cnf, err := v.Verify(dpopHeader(proof), testHTM, testHTU, jkt, code) + require.NoError(t, err) + require.NotNil(t, cnf) +} + +func TestDPoPProofVerifier_Verify_NormalizedHTU(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + now := time.Now() + jkt := jktOf(t, &key.PublicKey) + opts := defaultDPoPProofOpts() + opts.htu = "HTTPS://SERVER.EXAMPLE.COM:443/a/../token?ignored=true#ignored" + proof := signDPoPProof(t, key, jose.ES256, opts) + + cnf, err := newVerifier(now).Verify(dpopHeader(proof), testHTM, testHTU, jkt, "") + require.NoError(t, err) + require.NotNil(t, cnf) +} + +func TestDPoPProofVerifier_Verify_DoesNotCollapseEmptyPathSegments(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + now := time.Now() + jkt := jktOf(t, &key.PublicKey) + opts := defaultDPoPProofOpts() + opts.htu = "https://server.example.com/a//token" + proof := signDPoPProof(t, key, jose.ES256, opts) + + _, err = newVerifier(now).Verify(dpopHeader(proof), testHTM, "https://server.example.com/a/token", jkt, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "htu does not match") +} + +func TestDPoPProofVerifier_CustomAlgorithmsCannotEnableHMAC(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + now := time.Now() + v := newVerifier(now) + v.SupportedSignAlgs = []jose.SignatureAlgorithm{jose.HS256} + + _, err = v.Verify(dpopHeader(hmacSignedProof(t, defaultDPoPProofOpts())), testHTM, testHTU, jktOf(t, &key.PublicKey), "") + require.Error(t, err) + assert.Contains(t, err.Error(), "malformed DPoP proof") +} + +// TestDPoPProofVerifier_Verify_RejectsJSONSerialization ensures a DPoP proof +// must use the JWS Compact Serialization. +// +// This is a regression test for a real bypass. jose.ParseSigned also accepts +// the JWS JSON Serialization, whose unprotected header is NOT covered by the +// signature, and Signature.Header merges the protected and unprotected +// headers. Reading typ/jwk from the merged header therefore let an attacker +// take any JWS signed by the binding key whose protected header happened to +// omit typ and jwk, re-serialize it as flattened JSON with attacker-chosen +// unprotected typ and jwk, and have it accepted as a valid proof. Key reuse +// across protocols is only a SHOULD-NOT in the spec, so this was reachable. +func TestDPoPProofVerifier_Verify_RejectsJSONSerialization(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + now := time.Now() + jkt := jktOf(t, &key.PublicKey) + pub := &jose.JSONWebKey{Key: key.Public()} + + // A JWS by the binding key whose protected header carries only alg. + signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.ES256, Key: key}, nil) + require.NoError(t, err) + payload, err := json.Marshal(oidc.DPoPProofClaims{ + JWTID: "e1j3V_bKic8-LAEB", HTTPMethod: testHTM, + HTTPURI: testHTU, IssuedAt: oidc.FromTime(now), + }) + require.NoError(t, err) + jws, err := signer.Sign(payload) + require.NoError(t, err) + + // Without a protected typ/jwk the compact form is correctly rejected. + compact, err := jws.CompactSerialize() + require.NoError(t, err) + _, err = newVerifier(now).Verify(dpopHeader(compact), testHTM, testHTU, jkt, "") + require.Error(t, err) + + // Smuggling typ and jwk through the unprotected header must not help. + var flat map[string]any + require.NoError(t, json.Unmarshal([]byte(jws.FullSerialize()), &flat)) + pubJSON, err := pub.MarshalJSON() + require.NoError(t, err) + var pubMap map[string]any + require.NoError(t, json.Unmarshal(pubJSON, &pubMap)) + flat["header"] = map[string]any{"typ": string(oidc.DPoPProofType), "jwk": pubMap} + forged, err := json.Marshal(flat) + require.NoError(t, err) + + _, err = newVerifier(now).Verify(dpopHeader(string(forged)), testHTM, testHTU, jkt, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "malformed DPoP proof") + + // The general JSON serialization must be rejected too, even when the + // protected header is fully populated. + valid := signDPoPProof(t, key, jose.ES256, defaultDPoPProofOpts()) + parts := strings.Split(valid, ".") + require.Len(t, parts, 3) + general, err := json.Marshal(map[string]any{ + "payload": parts[1], + "signatures": []map[string]any{{ + "protected": parts[0], + "signature": parts[2], + }}, + }) + require.NoError(t, err) + _, err = newVerifier(now).Verify(dpopHeader(string(general)), testHTM, testHTU, jkt, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "malformed DPoP proof") +} + +// TestDPoPProofVerifier_Verify_SpecFixture verifies the exact non-normative +// DPoP proof example from OpenID Connect Key Binding 1.0, Section 2.3, +// including the dpop_jkt value from the Section 2.1 example Authentication +// Request. Both values were independently recomputed and cross-checked +// against RFC 7638 and this implementation's CodeHash before being pinned +// here. +func TestDPoPProofVerifier_Verify_SpecFixture(t *testing.T) { + const proof = "eyJhbGciOiJFUzI1NiIsImp3ayI6eyJjcnYiOiJQLTI1NiIsImt0eSI6IkVDIiwieCI6InVrcHYzZlU2dHFRS2FVd2NkQkFRb0szSUh2SklXX185eU5kMW9SN3F2WmMiLCJ5IjoibkJCeFhyeDBOeml3Z19ldmZVTVVVZ25HS0tVZjJBVHBXRzlFb2puVW9VNCJ9LCJ0eXAiOiJkcG9wK2p3dCJ9.eyJjX3MyNTYiOiJvMXVCcDllU2UzRHNtU2NOMGpZcmlGZ0tLRmRLLUJMeXdDOVdScFY1R0c4IiwiaHRtIjoiUE9TVCIsImh0dSI6Imh0dHBzOi8vc2VydmVyLmV4YW1wbGUuY29tL3Rva2VuIiwiaWF0IjoxNzYxOTM3NDQ5LCJqdGkiOiJJUVM1dFlQLWJwQlB0SnNvclQ0ejdnIn0.ay7H-sV7o_NE19Qfdq7oFNZ_oH-8LRw7_dgiTRQAUusLjEhgzNYR1ZU1T6IZGopiTEk55LPu_g0gKKku96d4kA" + const dpopJKT = "dnfb1T9jil_gOhti60baHs_WD_a4D8JN9VDJXbmBmGw" + const code = "SplxlOBeZQQYbYS6WxSbIA" + + v := newVerifier(time.Unix(1761937449, 0)) + cnf, err := v.Verify(dpopHeader(proof), http.MethodPost, "https://server.example.com/token", dpopJKT, code) + require.NoError(t, err) + require.NotNil(t, cnf) + + var m map[string]any + require.NoError(t, json.Unmarshal(cnf.JWK, &m)) + assert.Equal(t, "EC", m["kty"]) + assert.Equal(t, "P-256", m["crv"]) +} + +func TestDPoPProofVerifier_Verify_Negative(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + now := time.Now() + jkt := jktOf(t, &key.PublicKey) + + tests := []struct { + name string + header http.Header + htm string + htu string + jkt string + code string + wantErr string + }{ + { + name: "missing DPoP header", + header: dpopHeader(""), + htm: testHTM, + htu: testHTU, + jkt: jkt, + wantErr: "missing DPoP header", + }, + { + name: "multiple DPoP headers", + header: func() http.Header { + h := dpopHeader(signDPoPProof(t, key, jose.ES256, defaultDPoPProofOpts())) + h.Add(oidc.DPoPHeader, "another-value") + return h + }(), + htm: testHTM, + htu: testHTU, + jkt: jkt, + wantErr: "multiple DPoP headers", + }, + { + name: "malformed JWT", + header: dpopHeader("not-a-jwt"), + htm: testHTM, + htu: testHTU, + jkt: jkt, + wantErr: "malformed DPoP proof", + }, + { + name: "unsupported alg HS256", + header: dpopHeader(hmacSignedProof(t, defaultDPoPProofOpts())), + htm: testHTM, + htu: testHTU, + jkt: jkt, + wantErr: "malformed DPoP proof", + }, + { + name: "alg none", + header: dpopHeader(unsecuredJWS(t, map[string]any{"alg": "none", "typ": "dpop+jwt"}, defaultDPoPProofOpts())), + htm: testHTM, + htu: testHTU, + jkt: jkt, + wantErr: "malformed DPoP proof", + }, + { + name: "wrong typ", + header: dpopHeader(func() string { + o := defaultDPoPProofOpts() + o.typ = "JWT" + return signDPoPProof(t, key, jose.ES256, o) + }()), + htm: testHTM, + htu: testHTU, + jkt: jkt, + wantErr: "typ must be", + }, + { + name: "missing jwk header", + header: dpopHeader(noJWKProof(t, key, defaultDPoPProofOpts())), + htm: testHTM, + htu: testHTU, + jkt: jkt, + wantErr: "missing jwk header", + }, + { + name: "htm mismatch", + header: dpopHeader(func() string { + o := defaultDPoPProofOpts() + o.htm = http.MethodGet + return signDPoPProof(t, key, jose.ES256, o) + }()), + htm: testHTM, + htu: testHTU, + jkt: jkt, + wantErr: "htm does not match", + }, + { + name: "htu mismatch", + header: dpopHeader(func() string { + o := defaultDPoPProofOpts() + o.htu = "https://attacker.example.com/token" + return signDPoPProof(t, key, jose.ES256, o) + }()), + htm: testHTM, + htu: testHTU, + jkt: jkt, + wantErr: "htu does not match", + }, + { + name: "stale iat", + header: dpopHeader(func() string { + o := defaultDPoPProofOpts() + o.iat = now.Add(-10 * time.Minute) + return signDPoPProof(t, key, jose.ES256, o) + }()), + htm: testHTM, + htu: testHTU, + jkt: jkt, + wantErr: "iat is outside", + }, + { + name: "future iat", + header: dpopHeader(func() string { + o := defaultDPoPProofOpts() + o.iat = now.Add(10 * time.Minute) + return signDPoPProof(t, key, jose.ES256, o) + }()), + htm: testHTM, + htu: testHTU, + jkt: jkt, + wantErr: "iat is outside", + }, + { + name: "missing jti", + header: dpopHeader(func() string { + o := defaultDPoPProofOpts() + o.jti = "" + return signDPoPProof(t, key, jose.ES256, o) + }()), + htm: testHTM, + htu: testHTU, + jkt: jkt, + wantErr: "missing jti", + }, + { + name: "c_s256 missing", + header: dpopHeader( + signDPoPProof(t, key, jose.ES256, defaultDPoPProofOpts()), + ), + htm: testHTM, + htu: testHTU, + jkt: jkt, + code: "SplxlOBeZQQYbYS6WxSbIA", + wantErr: "c_s256 does not match", + }, + { + name: "c_s256 mismatch", + header: dpopHeader(func() string { + o := defaultDPoPProofOpts() + o.codeHash = oidc.CodeHash("some-other-code") + return signDPoPProof(t, key, jose.ES256, o) + }()), + htm: testHTM, + htu: testHTU, + jkt: jkt, + code: "SplxlOBeZQQYbYS6WxSbIA", + wantErr: "c_s256 does not match", + }, + { + name: "thumbprint mismatch", + header: dpopHeader(signDPoPProof(t, key, jose.ES256, defaultDPoPProofOpts())), + htm: testHTM, + htu: testHTU, + jkt: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", + wantErr: "does not match the committed dpop_jkt", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v := newVerifier(now) + _, err := v.Verify(tt.header, tt.htm, tt.htu, tt.jkt, tt.code) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + }) + } +} + +// hmacSignedProof builds a DPoP-shaped proof signed with HS256, which must +// be rejected because DefaultDPoPSigningAlgs excludes symmetric algorithms. +func hmacSignedProof(t *testing.T, opts dpopProofOpts) string { + t.Helper() + secret := make([]byte, 32) + _, err := rand.Read(secret) + require.NoError(t, err) + so := (&jose.SignerOptions{}).WithType(opts.typ) + signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.HS256, Key: secret}, so) + require.NoError(t, err) + claims := oidc.DPoPProofClaims{ + JWTID: opts.jti, + HTTPMethod: opts.htm, + HTTPURI: opts.htu, + IssuedAt: oidc.FromTime(opts.iat), + } + payload, err := json.Marshal(claims) + require.NoError(t, err) + jws, err := signer.Sign(payload) + require.NoError(t, err) + proof, err := jws.CompactSerialize() + require.NoError(t, err) + return proof +} + +// unsecuredJWS hand-builds a compact JWS with an arbitrary header and no +// signature, to simulate an alg:none downgrade attack. +func unsecuredJWS(t *testing.T, header map[string]any, opts dpopProofOpts) string { + t.Helper() + claims := oidc.DPoPProofClaims{ + JWTID: opts.jti, + HTTPMethod: opts.htm, + HTTPURI: opts.htu, + IssuedAt: oidc.FromTime(opts.iat), + } + payloadBytes, err := json.Marshal(claims) + require.NoError(t, err) + headerBytes, err := json.Marshal(header) + require.NoError(t, err) + return b64url(headerBytes) + "." + b64url(payloadBytes) + "." +} + +// noJWKProof signs a proof without embedding a jwk header, which is +// required by RFC 9449, Section 4.2. +func noJWKProof(t *testing.T, key any, opts dpopProofOpts) string { + t.Helper() + so := (&jose.SignerOptions{}).WithType(opts.typ) + signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.ES256, Key: key}, so) + require.NoError(t, err) + claims := oidc.DPoPProofClaims{ + JWTID: opts.jti, + HTTPMethod: opts.htm, + HTTPURI: opts.htu, + IssuedAt: oidc.FromTime(opts.iat), + } + payload, err := json.Marshal(claims) + require.NoError(t, err) + jws, err := signer.Sign(payload) + require.NoError(t, err) + proof, err := jws.CompactSerialize() + require.NoError(t, err) + return proof +} + +// fixedSizeBytes returns the big-endian bytes of n, left-padded with zeros +// to size bytes, as required for JWK EC coordinate encoding. +func fixedSizeBytes(n *big.Int, size int) []byte { + b := n.Bytes() + if len(b) >= size { + return b + } + out := make([]byte, size) + copy(out[size-len(b):], b) + return out +} + +func b64url(b []byte) string { + const alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_" + var out []byte + for i := 0; i < len(b); i += 3 { + chunk := b[i:min(i+3, len(b))] + var n int + for _, c := range chunk { + n = n<<8 | int(c) + } + n <<= uint(8 * (3 - len(chunk))) + nChars := len(chunk) + 1 + for j := 0; j < nChars; j++ { + out = append(out, alphabet[(n>>uint(18-6*j))&0x3f]) + } + } + return string(out) +} + +func min(a, b int) int { + if a < b { + return a + } + return b +} + +func TestDPoPProofVerifier_Verify_UndersizedRSAKey(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 1024) + require.NoError(t, err) + now := time.Now() + jkt := jktOf(t, &key.PublicKey) + + proof := signDPoPProof(t, key, jose.RS256, defaultDPoPProofOpts()) + v := newVerifier(now) + + _, err = v.Verify(dpopHeader(proof), testHTM, testHTU, jkt, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "RSA key size") +} + +// TestDPoPProofVerifier_Verify_UnsupportedECCurve uses a hand-crafted +// (not validly signed) proof, because go-jose's ES256 signer itself +// enforces the P-256 curve and would refuse to produce a P-224-keyed +// proof. +// +// go-jose's own JWK decoder only recognizes the P-256/P-384/P-521 curve +// names and already rejects a "P-224" member at parse time -- before this +// implementation's own curve allow-list in checkDPoPKeyStrength ever runs. +// The two layers overlap by construction (checkDPoPKeyStrength allows +// exactly the curves go-jose can parse), so this test only asserts that +// the request is rejected end-to-end with a curve-related error, not +// which layer caught it. +func TestDPoPProofVerifier_Verify_UnsupportedECCurve(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P224(), rand.Reader) + require.NoError(t, err) + now := time.Now() + // go-jose refuses to compute a thumbprint (or marshal) a P-224 key (it + // only recognizes P-256/384/521), so the expected dpop_jkt is left as + // an arbitrary placeholder: checkDPoPKeyStrength must reject the curve + // before the thumbprint comparison is ever reached. + const jkt = "irrelevant-key-strength-is-checked-first" + + // The JWK itself is built by hand from the raw coordinates for the + // same reason. + byteLen := (key.Curve.Params().BitSize + 7) / 8 + header := map[string]any{ + "alg": "ES256", + "typ": "dpop+jwt", + "jwk": map[string]any{ + "kty": "EC", + "crv": "P-224", + "x": b64url(fixedSizeBytes(key.X, byteLen)), + "y": b64url(fixedSizeBytes(key.Y, byteLen)), + }, + } + proof := handcraftedDPoPProof(t, header, defaultDPoPProofOpts(), make([]byte, 64)) + v := newVerifier(now) + + _, err = v.Verify(dpopHeader(proof), testHTM, testHTU, jkt, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "curve") +} + +// handcraftedDPoPProof builds a compact JWS by hand from header and the +// claims described by opts, with an arbitrary (not necessarily valid) +// signature. Useful for constructing structurally-valid proofs that +// go-jose's own signer would refuse to produce. +func handcraftedDPoPProof(t *testing.T, header map[string]any, opts dpopProofOpts, sig []byte) string { + t.Helper() + claims := oidc.DPoPProofClaims{ + JWTID: opts.jti, + HTTPMethod: opts.htm, + HTTPURI: opts.htu, + IssuedAt: oidc.FromTime(opts.iat), + CodeHash: opts.codeHash, + } + payloadBytes, err := json.Marshal(claims) + require.NoError(t, err) + headerBytes, err := json.Marshal(header) + require.NoError(t, err) + return b64url(headerBytes) + "." + b64url(payloadBytes) + "." + b64url(sig) +} + +func keysOf(m map[string]any) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + return out +} diff --git a/pkg/op/op.go b/pkg/op/op.go index bb789a39..48ce88ae 100644 --- a/pkg/op/op.go +++ b/pkg/op/op.go @@ -5,6 +5,7 @@ import ( "fmt" "log/slog" "net/http" + "slices" "time" "github.com/go-chi/chi/v5" @@ -79,6 +80,7 @@ var ( "Accept-Language", "Authorization", "Content-Type", + oidc.DPoPHeader, "X-Requested-With", }, AllowedMethods: []string{ @@ -314,6 +316,7 @@ type Provider struct { accessTokenVerifierOpts []AccessTokenVerifierOpt idTokenHintVerifierOpts []IDTokenHintVerifierOpt corsOpts *cors.Options + keyBinding bool } func (o *Provider) IssuerFromRequest(r *http.Request) string { @@ -427,6 +430,20 @@ func (o *Provider) SupportedUILocales() []language.Tag { return o.config.SupportedUILocales } +// ScopesSupported returns the scopes advertised by discovery. +func (o *Provider) ScopesSupported() []string { + scopes := DefaultSupportedScopes + if o.config.SupportedScopes != nil { + scopes = o.config.SupportedScopes + } + // Added here rather than by mutating config.SupportedScopes, so the option + // is order-independent and the caller's Config is left untouched. + if o.keyBinding && !slices.Contains(scopes, oidc.ScopeBoundKey) { + scopes = slices.Concat(scopes, []string{oidc.ScopeBoundKey}) + } + return scopes +} + func (o *Provider) DeviceAuthorization() DeviceAuthorizationConfig { return o.config.DeviceAuthorization } @@ -522,6 +539,21 @@ func WithAllowInsecure() Option { } } +// WithKeyBinding enables OpenID Connect Key Binding 1.0 on the provider. +// +// It advertises the `bound_key` scope in `scopes_supported` and populates +// `dpop_signing_alg_values_supported`. +// +// This controls advertisement only, not whether the code and refresh flows +// honour a binding: that is driven by the storage implementing +// [BoundKeyRequest], and per-client by Client.IsScopeAllowed. +func WithKeyBinding() Option { + return func(o *Provider) error { + o.keyBinding = true + return nil + } +} + func WithCustomAuthEndpoint(endpoint *Endpoint) Option { return func(o *Provider) error { if err := endpoint.Validate(); err != nil { diff --git a/pkg/op/server_http.go b/pkg/op/server_http.go index 274cabd3..c86b38a5 100644 --- a/pkg/op/server_http.go +++ b/pkg/op/server_http.go @@ -234,6 +234,9 @@ func (s *webServer) authorize(ctx context.Context, r *Request[oidc.AuthRequest]) if err := ValidateAuthReqRedirectURI(cr.Client, authReq.RedirectURI, authReq.ResponseType); err != nil { return nil, err } + if err := ValidateAuthReqBoundKey(authReq); err != nil { + return nil, err + } if err := ValidateAuthReqResponseType(cr.Client, authReq.ResponseType); err != nil { return nil, err } diff --git a/pkg/op/server_legacy.go b/pkg/op/server_legacy.go index 44d66e51..f18608c3 100644 --- a/pkg/op/server_legacy.go +++ b/pkg/op/server_legacy.go @@ -241,7 +241,11 @@ func (s *LegacyServer) CodeExchange(ctx context.Context, r *ClientRequest[oidc.A if r.Data.RedirectURI != authReq.GetRedirectURI() { return nil, oidc.ErrInvalidGrant().WithDescription("redirect_uri does not correspond") } - resp, err := CreateTokenResponse(ctx, authReq, r.Client, s.provider, true, r.Data.Code, "") + confirmation, err := verifyBoundKey(r.Header, r.Method, s.endpoints.Token.Absolute(IssuerFromContext(ctx)), authReq, r.Data.Code) + if err != nil { + return nil, err + } + resp, err := createTokenResponse(ctx, authReq, r.Client, s.provider, true, r.Data.Code, "", confirmation) if err != nil { return nil, err } @@ -262,10 +266,17 @@ func (s *LegacyServer) RefreshToken(ctx context.Context, r *ClientRequest[oidc.R if r.Client.GetID() != request.GetClientID() { return nil, oidc.ErrInvalidGrant() } - if err = ValidateRefreshTokenScopes(r.Data.Scopes, request); err != nil { + if err = validateRefreshTokenScopes(r.Data.Scopes, request); err != nil { return nil, err } - resp, err := CreateTokenResponse(ctx, request, r.Client, s.provider, true, "", r.Data.RefreshToken) + confirmation, err := verifyBoundKey(r.Header, r.Method, s.endpoints.Token.Absolute(IssuerFromContext(ctx)), request, "") + if err != nil { + return nil, err + } + if len(r.Data.Scopes) > 0 { + request.SetCurrentScopes(r.Data.Scopes) + } + resp, err := createTokenResponse(ctx, request, r.Client, s.provider, true, "", r.Data.RefreshToken, confirmation) if err != nil { return nil, err } diff --git a/pkg/op/signer.go b/pkg/op/signer.go index 5c3dd6a8..814e5b71 100644 --- a/pkg/op/signer.go +++ b/pkg/op/signer.go @@ -4,6 +4,8 @@ import ( "errors" jose "github.com/go-jose/go-jose/v4" + + "github.com/zitadel/oidc/v3/pkg/oidc" ) var ErrSignerCreationFailed = errors.New("signer creation failed") @@ -15,13 +17,18 @@ type SigningKey interface { } func SignerFromKey(key SigningKey) (jose.Signer, error) { + return SignerFromKeyAndType(key, oidc.IDTokenTypeJWT) +} + +// SignerFromKeyAndType creates a signer with typ in the protected header. +func SignerFromKeyAndType(key SigningKey, typ jose.ContentType) (jose.Signer, error) { signer, err := jose.NewSigner(jose.SigningKey{ Algorithm: key.SignatureAlgorithm(), Key: &jose.JSONWebKey{ Key: key.Key(), KeyID: key.ID(), }, - }, (&jose.SignerOptions{}).WithType("JWT")) + }, (&jose.SignerOptions{}).WithType(typ)) if err != nil { return nil, ErrSignerCreationFailed // TODO: log / wrap error? } diff --git a/pkg/op/token.go b/pkg/op/token.go index 7991a984..45c5a2f4 100644 --- a/pkg/op/token.go +++ b/pkg/op/token.go @@ -28,6 +28,10 @@ type AccessTokenClient interface { } func CreateTokenResponse(ctx context.Context, request IDTokenRequest, client Client, creator TokenCreator, createAccessToken bool, code, refreshToken string) (*oidc.AccessTokenResponse, error) { + return createTokenResponse(ctx, request, client, creator, createAccessToken, code, refreshToken, nil) +} + +func createTokenResponse(ctx context.Context, request IDTokenRequest, client Client, creator TokenCreator, createAccessToken bool, code, refreshToken string, confirmation *oidc.Confirmation) (*oidc.AccessTokenResponse, error) { ctx, span := Tracer.Start(ctx, "CreateTokenResponse") defer span.End() @@ -40,7 +44,7 @@ func CreateTokenResponse(ctx context.Context, request IDTokenRequest, client Cli return nil, err } } - idToken, err := CreateIDToken(ctx, IssuerFromContext(ctx), request, client.IDTokenLifetime(), accessToken, code, creator.Storage(), client) + idToken, err := createIDToken(ctx, IssuerFromContext(ctx), request, client.IDTokenLifetime(), accessToken, code, creator.Storage(), client, confirmation) if err != nil { return nil, err } @@ -199,6 +203,10 @@ type IDTokenRequest interface { } func CreateIDToken(ctx context.Context, issuer string, request IDTokenRequest, validity time.Duration, accessToken, code string, storage Storage, client Client) (string, error) { + return createIDToken(ctx, issuer, request, validity, accessToken, code, storage, client, nil) +} + +func createIDToken(ctx context.Context, issuer string, request IDTokenRequest, validity time.Duration, accessToken, code string, storage Storage, client Client, confirmation *oidc.Confirmation) (string, error) { ctx, span := Tracer.Start(ctx, "CreateIDToken") defer span.End() @@ -259,13 +267,45 @@ func CreateIDToken(ctx context.Context, issuer string, request IDTokenRequest, v } claims.CodeHash = codeHash } - signer, err := SignerFromKey(signingKey) + + if err := requireBoundKeyConfirmation(request, confirmation); err != nil { + return "", err + } + + // Registered claims must win over storage-supplied extra claims. + delete(claims.Claims, "cnf") + + typ := oidc.IDTokenTypeJWT + if confirmation != nil { + typ = oidc.IDTokenTypeDPoP + claims.Confirmation = confirmation + } + signer, err := SignerFromKeyAndType(signingKey, typ) if err != nil { return "", err } return crypto.Sign(claims, signer) } +// requireBoundKeyConfirmation returns an error if request asked for a key-bound +// ID Token but no verified DPoP confirmation was produced for it. +func requireBoundKeyConfirmation(request IDTokenRequest, confirmation *oidc.Confirmation) error { + if confirmation != nil { + return nil + } + boundRequest, integrated := keyBindingIntegrated(request) + if !integrated { + return nil + } + // Deliberately checks the granted scopes rather than the scopes filtered by + // RestrictAdditionalIdTokenScopes, so a client-level scope restriction + // cannot strip `bound_key` and disable this check. + if slices.Contains(request.GetScopes(), oidc.ScopeBoundKey) || boundRequest.GetDPoPJKT() != "" { + return oidc.ErrServerError().WithDescription("bound_key is not supported for this flow") + } + return nil +} + func removeUserinfoScopes(scopes []string) []string { newScopeList := make([]string, 0, len(scopes)) for _, scope := range scopes { diff --git a/pkg/op/token_code.go b/pkg/op/token_code.go index 753709fd..bbf9d00e 100644 --- a/pkg/op/token_code.go +++ b/pkg/op/token_code.go @@ -29,7 +29,12 @@ func CodeExchange(w http.ResponseWriter, r *http.Request, exchanger Exchanger) { RequestError(w, r, err, nil) return } - resp, err := CreateTokenResponse(r.Context(), authReq, client, exchanger, true, tokenReq.Code, "") + confirmation, err := verifyProviderBoundKey(r.Context(), r.Header, r.Method, authReq, tokenReq.Code, exchanger) + if err != nil { + RequestError(w, r, err, nil) + return + } + resp, err := createTokenResponse(r.Context(), authReq, client, exchanger, true, tokenReq.Code, "", confirmation) if err != nil { RequestError(w, r, err, nil) return diff --git a/pkg/op/token_exchange.go b/pkg/op/token_exchange.go index f8dc0f84..15717319 100644 --- a/pkg/op/token_exchange.go +++ b/pkg/op/token_exchange.go @@ -325,6 +325,12 @@ func GetTokenIDAndSubjectFromToken( break } + // A key-bound ID Token must never be accepted as a bearer credential. + // TODO: Add key-binding DPoP support for Token Exchange + if idTokenClaims.Confirmation != nil { + break + } + tokenIDOrToken, subject, claims, ok = token, idTokenClaims.Subject, idTokenClaims.Claims, true } diff --git a/pkg/op/token_internal_test.go b/pkg/op/token_internal_test.go new file mode 100644 index 00000000..4299caf4 --- /dev/null +++ b/pkg/op/token_internal_test.go @@ -0,0 +1,134 @@ +package op + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zitadel/oidc/v3/pkg/oidc" +) + +type boundKeyIDTokenRequest struct { + scopes []string + jkt string +} + +func (r boundKeyIDTokenRequest) GetAMR() []string { return nil } +func (r boundKeyIDTokenRequest) GetAudience() []string { return []string{"client"} } +func (r boundKeyIDTokenRequest) GetAuthTime() time.Time { return time.Time{} } +func (r boundKeyIDTokenRequest) GetClientID() string { return "client" } +func (r boundKeyIDTokenRequest) GetScopes() []string { return r.scopes } +func (r boundKeyIDTokenRequest) GetSubject() string { return "subject" } + +// plainIDTokenRequest does not implement BoundKeyRequest at all, modelling a +// storage that has not been updated for key binding. +type plainIDTokenRequest struct { + boundKeyIDTokenRequest +} + +type jktIDTokenRequest struct { + boundKeyIDTokenRequest +} + +func (r jktIDTokenRequest) GetDPoPJKT() string { return r.jkt } + +func TestRequireBoundKeyConfirmation(t *testing.T) { + confirmation := &oidc.Confirmation{JWK: []byte(`{"kty":"EC"}`)} + openid := []string{oidc.ScopeOpenID} + bound := []string{oidc.ScopeOpenID, oidc.ScopeBoundKey} + + tests := []struct { + name string + request IDTokenRequest + confirmation *oidc.Confirmation + wantErr bool + }{ + { + name: "unbound request without confirmation is allowed", + request: plainIDTokenRequest{boundKeyIDTokenRequest{scopes: openid}}, + }, + { + name: "bound request with confirmation is allowed", + request: jktIDTokenRequest{boundKeyIDTokenRequest{scopes: bound, jkt: "thumbprint"}}, + confirmation: confirmation, + }, + { + // The core downgrade case. + name: "bound_key scope without confirmation is rejected", + request: jktIDTokenRequest{boundKeyIDTokenRequest{scopes: bound, jkt: "thumbprint"}}, + wantErr: true, + }, + { + name: "bound_key scope on a non-BoundKeyRequest is ignored", + request: plainIDTokenRequest{boundKeyIDTokenRequest{scopes: bound}}, + }, + { + // Defends against a flow that drops bound_key from the scopes but + // still has a persisted binding commitment. + name: "persisted jkt without the scope is rejected", + request: jktIDTokenRequest{boundKeyIDTokenRequest{scopes: openid, jkt: "thumbprint"}}, + wantErr: true, + }, + { + name: "no scope and no jkt is allowed", + request: jktIDTokenRequest{boundKeyIDTokenRequest{scopes: openid}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := requireBoundKeyConfirmation(tt.request, tt.confirmation) + if !tt.wantErr { + assert.NoError(t, err) + return + } + require.Error(t, err) + var oidcErr *oidc.Error + require.ErrorAs(t, err, &oidcErr) + assert.Equal(t, oidc.ServerError, oidcErr.ErrorType) + }) + } +} + +// TestIDTokenClaimsCannotForgeConfirmation ensures a storage-supplied extra +// claim named "cnf" cannot fake a key binding on an unbound ID Token. +func TestIDTokenClaimsCannotForgeConfirmation(t *testing.T) { + claims := &oidc.IDTokenClaims{ + Claims: map[string]any{"cnf": map[string]any{"jwk": map[string]any{"kty": "EC"}}}, + } + + // Mirrors what createIDToken does before signing. + delete(claims.Claims, "cnf") + + marshalled, err := claims.MarshalJSON() + require.NoError(t, err) + assert.NotContains(t, string(marshalled), "cnf") +} + +// legacyAuthRequest models an authorization request from a storage written +// before key binding existed: it has no GetDPoPJKT method. +type legacyAuthRequest struct { + boundKeyIDTokenRequest +} + +func TestLegacyStorageCannotBreakOnBoundKey(t *testing.T) { + request := legacyAuthRequest{boundKeyIDTokenRequest{ + scopes: []string{oidc.ScopeOpenID, oidc.ScopeBoundKey}, + }} + + _, integrated := keyBindingIntegrated(request) + require.False(t, integrated) + + jkt, err := boundKeyThumbprint(request) + require.NoError(t, err) + assert.Empty(t, jkt, "no binding should be attempted") + assert.NoError(t, requireBoundKeyConfirmation(request, nil)) + + integratedRequest := jktIDTokenRequest{boundKeyIDTokenRequest{ + scopes: []string{oidc.ScopeOpenID, oidc.ScopeBoundKey}, + }} + _, err = boundKeyThumbprint(integratedRequest) + require.Error(t, err) + assert.Error(t, requireBoundKeyConfirmation(integratedRequest, nil)) +} diff --git a/pkg/op/token_refresh.go b/pkg/op/token_refresh.go index 631a3096..fca862f8 100644 --- a/pkg/op/token_refresh.go +++ b/pkg/op/token_refresh.go @@ -33,12 +33,20 @@ func RefreshTokenExchange(w http.ResponseWriter, r *http.Request, exchanger Exch RequestError(w, r, err, nil) return } - validatedRequest, client, err := ValidateRefreshTokenRequest(r.Context(), tokenReq, exchanger) + validatedRequest, client, err := validateRefreshTokenRequest(r.Context(), tokenReq, exchanger, false) if err != nil { RequestError(w, r, err, nil) return } - resp, err := CreateTokenResponse(r.Context(), validatedRequest, client, exchanger, true, "", tokenReq.RefreshToken) + confirmation, err := verifyProviderBoundKey(r.Context(), r.Header, r.Method, validatedRequest, "", exchanger) + if err != nil { + RequestError(w, r, err, nil) + return + } + if len(tokenReq.Scopes) > 0 { + validatedRequest.SetCurrentScopes(tokenReq.Scopes) + } + resp, err := createTokenResponse(r.Context(), validatedRequest, client, exchanger, true, "", tokenReq.RefreshToken, confirmation) if err != nil { RequestError(w, r, err, nil) return @@ -59,6 +67,10 @@ func ParseRefreshTokenRequest(r *http.Request, decoder httphelper.Decoder) (*oid // ValidateRefreshTokenRequest validates the refresh_token request parameters including authorization check of the client // and returns the data representing the original auth request corresponding to the refresh_token func ValidateRefreshTokenRequest(ctx context.Context, tokenReq *oidc.RefreshTokenRequest, exchanger Exchanger) (RefreshTokenRequest, Client, error) { + return validateRefreshTokenRequest(ctx, tokenReq, exchanger, true) +} + +func validateRefreshTokenRequest(ctx context.Context, tokenReq *oidc.RefreshTokenRequest, exchanger Exchanger, setCurrentScopes bool) (RefreshTokenRequest, Client, error) { ctx, span := Tracer.Start(ctx, "ValidateRefreshTokenRequest") defer span.End() @@ -72,9 +84,12 @@ func ValidateRefreshTokenRequest(ctx context.Context, tokenReq *oidc.RefreshToke if client.GetID() != request.GetClientID() { return nil, nil, oidc.ErrInvalidGrant() } - if err = ValidateRefreshTokenScopes(tokenReq.Scopes, request); err != nil { + if err = validateRefreshTokenScopes(tokenReq.Scopes, request); err != nil { return nil, nil, err } + if setCurrentScopes && len(tokenReq.Scopes) > 0 { + request.SetCurrentScopes(tokenReq.Scopes) + } return request, client, nil } @@ -82,15 +97,22 @@ func ValidateRefreshTokenRequest(ctx context.Context, tokenReq *oidc.RefreshToke // it will set the requested scopes as current scopes onto RefreshTokenRequest // if empty the original scopes will be used func ValidateRefreshTokenScopes(requestedScopes []string, authRequest RefreshTokenRequest) error { + if err := validateRefreshTokenScopes(requestedScopes, authRequest); err != nil { + return err + } if len(requestedScopes) == 0 { return nil } + authRequest.SetCurrentScopes(requestedScopes) + return nil +} + +func validateRefreshTokenScopes(requestedScopes []string, authRequest RefreshTokenRequest) error { for _, scope := range requestedScopes { if !slices.Contains(authRequest.GetScopes(), scope) { return oidc.ErrInvalidScope() } } - authRequest.SetCurrentScopes(requestedScopes) return nil }