diff --git a/docs/arch/18-spiffe-association-declarations.md b/docs/arch/18-spiffe-association-declarations.md index cbed41457e..5842fc6d8d 100644 --- a/docs/arch/18-spiffe-association-declarations.md +++ b/docs/arch/18-spiffe-association-declarations.md @@ -78,21 +78,44 @@ The durably-claimed record is always an inert placeholder — a client with no g On every startup, the server reconstructs the static registry and its overlay from serialized configuration. A restart with the same configuration produces the same associations and reconciles cleanly against the previous run's placeholders. A **changed** association (a different fingerprint — scopes, audiences, resources, grant types, response types, or SPIFFE identity — at the same client ID) does not take effect: `ReconcileConfiguredClient` fails and the server refuses to start, exactly as described under "Startup collision handling" above. A **removed** association's active policy does take effect on a successful restart — the in-memory overlay is rebuilt from the current configuration, so a client with no matching association is no longer served as a static client. What does *not* clean up is its durable reservation: nothing currently deletes the inert placeholder `ReconcileConfiguredClient` claimed for that client ID, so it persists in storage indefinitely, preventing the ID from being reused by DCR or a delegate client (tracked as [#6477](https://github.com/stacklok/toolhive/issues/6477)). Dynamic clients remain subject to the storage backend's own persistence, but no stale static client is restored from storage — the in-memory overlay's clients always come from the current configuration, never from a prior run's storage state. +## JWT-SVID client authentication + +The dispatch and validation logic for JWT-SVID client authentication is implemented for configured associations, though it is not yet reachable end to end (see "Security and delivery scope" below). The immutable dispatcher selects the SPIFFE JWT arm whenever any `client_assertion_type` form value is the SPIFFE JWT type — the shared dispatcher itself is otherwise untouched, so an entirely non-SPIFFE request (including RFC 7523 private-key JWT) still reaches Fosite's default strategy unchanged. Within the SPIFFE JWT arm, a duplicate or otherwise ambiguous `client_assertion_type` is rejected. Absent any SPIFFE-selecting value, the request falls through to Fosite's default strategy unless an ambient SPIFFE X.509 identity is present; that identity selects the fail-closed, not-yet-implemented X.509 arm instead. + +The authorization server accepts a serialized JWT-SVID client assertion, limited to 16 KiB, and validates it with go-spiffe `jwtsvid.ParseAndValidate` against the configured JWT bundle source (`AuthorizationServerParams.SPIFFEJWTBundleSource`). Validation requires the assertion's sole audience to be the configured authorization-server issuer. It does not require an `iss` claim: RFC 7519 makes `iss` OPTIONAL, and the SPIFFE JWT-SVID specification does not mandate it either, so a conformant SVID signed by a bundle-trusted key is accepted whether or not it carries one. This path reaches go-jose/v4's default one-minute claim leeway through go-spiffe v2.7.0; the leeway is inherited and not configurable in this code path. The server also rejects an assertion whose remaining validity exceeds six minutes, representing the recommended five-minute JWT-SVID issuer lifetime plus that one-minute clock-skew allowance. + +`client_id` is optional for this authentication method. When it is omitted, the registry derives the configured OAuth client from the verified SPIFFE ID association. When it is supplied, it is treated as an exact selector and must match that association's configured client ID; the server does not normalize the value. For example: + +```console +curl -X POST https://auth.example.com/oauth/token \ + --data-urlencode 'grant_type=urn:ietf:params:oauth:grant-type:token-exchange' \ + --data-urlencode "subject_token=$SUBJECT_TOKEN" \ + --data-urlencode 'subject_token_type=urn:ietf:params:oauth:token-type:access_token' \ + --data-urlencode 'client_assertion_type=urn:ietf:params:oauth:client-assertion-type:jwt-spiffe' \ + --data-urlencode "client_assertion=$JWT_SVID" +``` + +After validation, the authentication strategy calls the shared `SPIFFEAssociationRegistry.Resolve` path synchronously through its JWT resolver. The registry verifies the enabled JWT method and either derives the association's configured client ID or checks the supplied selector's ownership, then returns the configured immutable static OAuth client. The derived identity context is not propagated to downstream request handling. + +After the SPIFFE JWT arm is selected, malformed request fields use generic OAuth `invalid_request` errors; validation, association, and mixed-credential failures use generic `invalid_client` errors. In that arm, an HTTP Basic authorization header or any `client_secret` form field causes client authentication to fail. Credential material is not logged or included in errors. + +JWT-SVID assertions currently have no application-level replay protection, `jti` persistence, nonce, or proof-of-possession binding. A captured valid assertion can therefore be reused until its expiry, subject to the six-minute maximum remaining-validity policy and any acceptance allowed by the inherited claim leeway. + ## Security and delivery scope -Configuration is not authentication. In particular, a client ID, a declared association, a request header, an unverified SPIFFE-looking URI, or a client-supplied trust domain is never workload identity. Until credential validation is implemented, configured SPIFFE clients remain non-public OAuth clients without a secret and token requests cannot authenticate through these declarations. +Configuration and loaded bundles are not authentication by themselves. A client ID, a declared association, a request header, an unverified SPIFFE-looking URI, a client-supplied trust domain, or a loaded bundle is never workload identity. JWT-SVID validation establishes identity only after the assertion validates against configured trust material and the association registry authorizes the resulting SPIFFE ID and configured client ID. + +The JWT-SVID client-authentication path described above is implemented by [#6203](https://github.com/stacklok/toolhive/issues/6203), but `newServer` (`pkg/authserver/server_impl.go`) does not yet construct and wire in the JWT bundle source that path validates assertions against, so `jwtsvid.ParseAndValidate` is unreachable with real trust material today and every JWT-SVID authentication attempt fails closed. Issue [#6201](https://github.com/stacklok/toolhive/issues/6201) loads and rotates trust bundles and will supply that source. The following remain separate and pending: + +- validate X.509-SVIDs ([#6202](https://github.com/stacklok/toolhive/issues/6202)); +- integrate SPIFFE methods with grants or discovery metadata ([#6204](https://github.com/stacklok/toolhive/issues/6204)); and +- deploy SPIRE or mount Workload API sockets ([#6205](https://github.com/stacklok/toolhive/issues/6205)). -Configured SPIFFE associations establish configuration, policy, and static-client ownership only. They do **not**: +For [#6205](https://github.com/stacklok/toolhive/issues/6205), `workloadapi.X509Source` implements both `x509svid.Source` and `x509bundle.Source`, so one Workload API connection can also provide the authorization server's own certificate when deployment wiring is added. The v1alpha1 `ClientCASecretRef` plus `subPath` shape cannot support a rotating bundle and must not be reused for this purpose. -- fetch, load, or rotate a trust bundle, even though a `bundle_source` is declared; -- authenticate workloads with X.509-SVIDs or JWT-SVIDs; -- authenticate token requests or issue tokens through SPIFFE; -- pair users with applications; -- advertise discovery metadata for SPIFFE methods; -- deploy SPIRE or mount Workload API sockets; or -- claim full SPIFFE interoperability or end-to-end coverage. +Configured SPIFFE associations remain non-deployable independent of the above: `RunConfig.Validate()` still hard-rejects any non-empty `spiffeTrustDomains` (see "Current status" above), so none of this runs against a real deployment yet. -Future credential-validation code must establish identity from validated SVIDs and then resolve that verified identity through this registry. It must fail closed for missing associations, client-ID ownership mismatches, unknown trust domains, and methods not enabled by policy. +Future X.509-SVID credential-validation code must establish identity from validated SVIDs and then resolve that verified identity through this registry. It must fail closed for missing associations, client-ID ownership mismatches, unknown trust domains, and methods not enabled by policy. ## Related documentation diff --git a/docs/arch/README.md b/docs/arch/README.md index 43a1ceef52..6747568579 100644 --- a/docs/arch/README.md +++ b/docs/arch/README.md @@ -143,8 +143,9 @@ Welcome to the ToolHive architecture documentation. This directory contains comp 18. **[SPIFFE Association Declarations](18-spiffe-association-declarations.md)** - Not yet deployable: rejected at startup pending real SVID verification - Configuration-only SPIFFE trust, association, and static-client model + - JWT-SVID client-authentication dispatch and validation logic implemented but not yet wired to a trust bundle source - Fail-closed policy validation and durable, restart-safe static-client reservation - - Explicit authentication and bundle-loading delivery boundaries + - X.509-SVID validation, trust-bundle loading, and SPIFFE grant/discovery integration remain pending ### Existing Documentation diff --git a/pkg/authserver/server/provider.go b/pkg/authserver/server/provider.go index 7aeec05324..bb5693bfc0 100644 --- a/pkg/authserver/server/provider.go +++ b/pkg/authserver/server/provider.go @@ -132,7 +132,7 @@ type AuthorizationServerConfig struct { // each independently extending this struct. SPIFFEX509BundleSource x509bundle.Source // SPIFFEJWTBundleSource provides JWT bundles for verifying SPIFFE - // JWT-SVID client assertions. See SPIFFEX509BundleSource. + // JWT-SVID client assertions, read by newSPIFFEClientAuthenticationStrategy. SPIFFEJWTBundleSource jwtbundle.Source } @@ -213,7 +213,7 @@ type AuthorizationServerParams struct { // struct. SPIFFEX509BundleSource x509bundle.Source // SPIFFEJWTBundleSource provides JWT bundles for verifying SPIFFE - // JWT-SVID client assertions. See SPIFFEX509BundleSource. + // JWT-SVID client assertions, copied through to AuthorizationServerConfig. SPIFFEJWTBundleSource jwtbundle.Source } @@ -476,7 +476,8 @@ func NewAuthorizationServer( return provider.DefaultClientAuthenticationStrategy }() fositeConfig.ClientAuthenticationStrategy = newSPIFFEClientAuthenticationStrategy( - defaultStrategy, providerConfig.SPIFFEClientResolver, + defaultStrategy, config.GetAccessTokenIssuer(), + config.SPIFFEJWTBundleSource, providerConfig.SPIFFEClientResolver, ) for _, factory := range factories { diff --git a/pkg/authserver/server/provider_test.go b/pkg/authserver/server/provider_test.go index a05d64940b..15b4f90450 100644 --- a/pkg/authserver/server/provider_test.go +++ b/pkg/authserver/server/provider_test.go @@ -877,6 +877,11 @@ func TestNewAuthorizationServer_InstallsSPIFFEClientAuthenticationStrategy(t *te require.NotNil(t, config.ClientAuthenticationStrategy) require.NotNil(t, providerConfig.ClientAuthenticationStrategy) + // A request with the SPIFFE JWT assertion type but no assertion or + // client_id reaches the JWT arm and is rejected there for malformed + // fields, confirming NewAuthorizationServer wires the issuer and JWT + // bundle source through to newSPIFFEClientAuthenticationStrategy rather + // than leaving the JWT arm unreachable. request := httptest.NewRequest("POST", "/oauth/token", nil) originalClient, err := config.ClientAuthenticationStrategy(request.Context(), request, url.Values{ "client_assertion_type": {spiffeauth.SPIFFEJWTAssertionType}, @@ -889,16 +894,7 @@ func TestNewAuthorizationServer_InstallsSPIFFEClientAuthenticationStrategy(t *te _, err = providerConfig.ClientAuthenticationStrategy(request.Context(), request, url.Values{ "client_assertion_type": {spiffeauth.SPIFFEJWTAssertionType}, }) - require.Error(t, err) - var rfcErr *fosite.RFC6749Error - require.ErrorAs(t, err, &rfcErr) - assert.Equal(t, "SPIFFE JWT client authentication is not implemented", rfcErr.HintField) - assert.False(t, fallbackCalled) - - client, err := providerConfig.ClientAuthenticationStrategy(request.Context(), request, url.Values{}) - require.NoError(t, err) - assert.Same(t, fallbackClient, client) - assert.True(t, fallbackCalled) + require.ErrorIs(t, err, fosite.ErrInvalidRequest) } func TestNewAuthorizationServer_DoesNotShareAuthenticationStrategy(t *testing.T) { diff --git a/pkg/authserver/server/registration/spiffe_client.go b/pkg/authserver/server/registration/spiffe_client.go index 91e2339ebf..0040b8e342 100644 --- a/pkg/authserver/server/registration/spiffe_client.go +++ b/pkg/authserver/server/registration/spiffe_client.go @@ -13,10 +13,10 @@ import ( ) // SPIFFEClient is the immutable OAuth client representation of a configured -// SPIFFE principal association. It is neither public nor secret-bearing. A -// future credential-validation implementation will authenticate its SPIFFE -// credentials; this configuration-only implementation does not authenticate -// any credentials. +// SPIFFE principal association. It is neither public nor secret-bearing. +// JWT-SVID authentication of configured associations is implemented but not +// yet wired to a trust bundle source (see docs/arch/18-spiffe-association-declarations.md); +// X.509-SVID credential validation remains pending. type SPIFFEClient struct { BackChannelOnlyMarker id string @@ -50,8 +50,9 @@ func NewSPIFFEClient(id string, scopes, audiences, resources []string) (*SPIFFEC // GetID returns the configured association client ID. func (c *SPIFFEClient) GetID() string { return c.id } -// GetHashedSecret returns nil because no OAuth client secret is assigned. Future -// SPIFFE credential validation is outside this configuration-only implementation. +// GetHashedSecret returns nil because no OAuth client secret is assigned. +// JWT-SVID authentication does not use a client secret, and X.509-SVID +// credential validation remains pending. func (*SPIFFEClient) GetHashedSecret() []byte { return nil } // GetRedirectURIs returns nil because SPIFFE clients do not use authorization redirects. @@ -79,7 +80,8 @@ func (c *SPIFFEClient) Resources() []string { return slices.Clone(c.resources) } func (c *SPIFFEClient) GetAudience() fosite.Arguments { return slices.Clone(c.audiences) } // IsPublic returns false so Fosite does not treat unauthenticated requests as -// public-client requests. Future SPIFFE credential validation remains separate. +// public-client requests. JWT-SVID authentication remains separate from the +// still-pending X.509-SVID credential validation. func (*SPIFFEClient) IsPublic() bool { return false } var _ fosite.Client = (*SPIFFEClient)(nil) diff --git a/pkg/authserver/server/spiffe_client_auth.go b/pkg/authserver/server/spiffe_client_auth.go index aae3550d35..db20b2b57d 100644 --- a/pkg/authserver/server/spiffe_client_auth.go +++ b/pkg/authserver/server/spiffe_client_auth.go @@ -8,16 +8,26 @@ import ( "net/http" "net/url" "slices" + "strings" + "time" "github.com/ory/fosite" + "github.com/spiffe/go-spiffe/v2/bundle/jwtbundle" + "github.com/spiffe/go-spiffe/v2/svid/jwtsvid" spiffeauth "github.com/stacklok/toolhive/pkg/authserver/spiffe" ) +// maxSPIFFEJWTAssertionRemainingValidity permits the recommended five-minute +// JWT-SVID lifetime plus the one-minute clock skew tolerated by go-spiffe. +const maxSPIFFEJWTAssertionRemainingValidity = 6 * time.Minute + // SPIFFEClientResolver resolves a verified SPIFFE identity to its configured // OAuth client. It is the seam that lets the client-authentication strategy // here reach the association registry and storage constructed in package // authserver, which this package cannot import (authserver imports server). +// clientID is an optional exact selector; an empty value asks the resolver to +// derive the configured client from the verified SPIFFE identity. // One signature covers both X.509 and JWT credentials, with method as an // explicit discriminator, so the two arms share a single resolution path // instead of each inventing its own. spiffeID is passed explicitly rather @@ -29,20 +39,25 @@ type SPIFFEClientResolver func( func newSPIFFEClientAuthenticationStrategy( defaultStrategy fosite.ClientAuthenticationStrategy, + issuer string, + jwtBundleSource jwtbundle.Source, resolver SPIFFEClientResolver, ) fosite.ClientAuthenticationStrategy { return func(ctx context.Context, r *http.Request, form url.Values) (fosite.Client, error) { - // No SPIFFE trust configured: this server genuinely does not do SPIFFE, - // so neither arm applies and every request goes to the default strategy. + // Without SPIFFE trust, every request goes to the default strategy + // untouched — this arm must not change shared-dispatcher behavior for + // entirely non-SPIFFE requests (e.g. RFC 7523 private-key JWT). if resolver == nil { return defaultStrategy(ctx, r, form) } // An explicit assertion type takes precedence over an ambient mTLS identity. // A repeated form key must be checked in full: form.Get would only see the // first value, letting a SPIFFE assertion type hidden behind an earlier - // value slip through to the default strategy. + // value slip through to the default strategy. A duplicated + // client_assertion_type is rejected as ambiguous by + // authenticateSPIFFEJWTClient itself, once SPIFFE is actually selected. if slices.Contains(form["client_assertion_type"], spiffeauth.SPIFFEJWTAssertionType) { - return nil, fosite.ErrInvalidClient.WithHint("SPIFFE JWT client authentication is not implemented") + return authenticateSPIFFEJWTClient(ctx, r, form, issuer, jwtBundleSource, resolver) } if _, ok := spiffeauth.SPIFFEIDFromContext(ctx); ok { return nil, fosite.ErrInvalidClient.WithHint("SPIFFE X.509 client authentication is not implemented") @@ -50,3 +65,102 @@ func newSPIFFEClientAuthenticationStrategy( return defaultStrategy(ctx, r, form) } } + +// authenticateSPIFFEJWTClient validates a SPIFFE JWT-SVID client assertion and +// resolves it to its configured OAuth client. It fails closed on any +// malformed request field, verification failure, or resolver rejection, and +// never logs the assertion or its claims. +func authenticateSPIFFEJWTClient( + ctx context.Context, + r *http.Request, + form url.Values, + issuer string, + jwtBundleSource jwtbundle.Source, + resolver SPIFFEClientResolver, +) (fosite.Client, error) { + assertionType, ok := exactNonEmptyFormValue(form, "client_assertion_type") + if !ok || assertionType != spiffeauth.SPIFFEJWTAssertionType { + return nil, fosite.ErrInvalidRequest + } + assertion, ok := exactNonEmptyFormValue(form, "client_assertion") + if !ok { + return nil, fosite.ErrInvalidRequest + } + clientID, ok := optionalExactNonEmptyFormValue(form, "client_id") + if !ok { + return nil, fosite.ErrInvalidRequest + } + if rejectedSPIFFEJWTRequest(r, form, assertion) { + return nil, fosite.ErrInvalidClient + } + if jwtBundleSource == nil { + return nil, fosite.ErrInvalidClient + } + + svid, err := jwtsvid.ParseAndValidate(assertion, jwtBundleSource, []string{issuer}) + if err != nil || !validSPIFFEJWTIdentityClaims(svid, issuer) { + return nil, fosite.ErrInvalidClient + } + if svid.Expiry.After(time.Now().Add(maxSPIFFEJWTAssertionRemainingValidity)) { + return nil, fosite.ErrInvalidClient + } + client, err := resolver(ctx, svid.ID.String(), clientID, spiffeauth.SPIFFEAuthenticationMethodJWT) + if !validResolvedSPIFFEClient(client, err, clientID) { + return nil, fosite.ErrInvalidClient + } + return client, nil +} + +// validSPIFFEJWTIdentityClaims reports whether svid carries exactly the AS as +// its sole audience. It does not require an `iss` claim: RFC 7519 states iss +// is OPTIONAL, and the SPIFFE JWT-SVID spec does not mandate it either — a +// conformant SVID signed by a bundle-trusted key can omit it entirely. +func validSPIFFEJWTIdentityClaims(svid *jwtsvid.SVID, audience string) bool { + return svid != nil && len(svid.Audience) == 1 && svid.Audience[0] == audience +} + +func validResolvedSPIFFEClient(client fosite.Client, err error, requestedClientID string) bool { + return err == nil && client != nil && (requestedClientID == "" || client.GetID() == requestedClientID) +} + +// exactNonEmptyFormValue returns the sole value for key, rejecting an absent, +// duplicated, or whitespace-only field. Duplicated fields are rejected rather +// than taking the first or last value, since either choice could be steered +// by an attacker who controls only one of the duplicates. +func exactNonEmptyFormValue(form url.Values, key string) (string, bool) { + values, ok := form[key] + if !ok || len(values) != 1 || strings.TrimSpace(values[0]) == "" { + return "", false + } + return values[0], true +} + +// optionalExactNonEmptyFormValue returns an absent field as an empty value. +// A supplied field must contain exactly one non-whitespace value, which is +// returned without normalization. +func optionalExactNonEmptyFormValue(form url.Values, key string) (string, bool) { + if !form.Has(key) { + return "", true + } + return exactNonEmptyFormValue(form, key) +} + +// rejectedSPIFFEJWTRequest reports whether a request mixes SPIFFE JWT +// authentication with another client credential (HTTP Basic or a client +// secret form field), or carries an oversized assertion. SPIFFE JWT +// authentication must be the sole credential in the request; allowing a +// second one to also be present would let an attacker probe which the +// server accepts. +func rejectedSPIFFEJWTRequest(r *http.Request, form url.Values, assertion string) bool { + return hasBasicAuthorization(r) || form.Has("client_secret") || len(assertion) > 16*1024 +} + +func hasBasicAuthorization(r *http.Request) bool { + for _, authorization := range r.Header.Values("Authorization") { + fields := strings.Fields(authorization) + if len(fields) > 0 && strings.EqualFold(fields[0], "Basic") { + return true + } + } + return false +} diff --git a/pkg/authserver/server/spiffe_client_auth_test.go b/pkg/authserver/server/spiffe_client_auth_test.go index 82ef8806c6..6306cf22e5 100644 --- a/pkg/authserver/server/spiffe_client_auth_test.go +++ b/pkg/authserver/server/spiffe_client_auth_test.go @@ -5,13 +5,23 @@ package server import ( "context" + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" "errors" "net/http" "net/http/httptest" "net/url" + "strings" "testing" + "time" + "github.com/go-jose/go-jose/v4" + "github.com/go-jose/go-jose/v4/jwt" "github.com/ory/fosite" + "github.com/spiffe/go-spiffe/v2/bundle/jwtbundle" "github.com/spiffe/go-spiffe/v2/spiffeid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -19,6 +29,8 @@ import ( spiffeauth "github.com/stacklok/toolhive/pkg/authserver/spiffe" ) +const testIssuer = "https://auth.example.com" + // stubResolver is a SPIFFEClientResolver that is never called by these tests; // it exists only to make "resolver configured" distinguishable from "resolver // nil" for the client-authentication strategy under test. @@ -41,16 +53,6 @@ func TestSPIFFEClientAuthenticationStrategy(t *testing.T) { wantErr string wantDefaultCall bool }{ - { - name: "SPIFFE JWT assertion takes precedence over X.509 identity", - ctx: spiffeauth.ContextWithSPIFFEID(context.Background(), spiffeID), - resolver: stubResolver, - form: url.Values{ - "client_assertion_type": {spiffeauth.SPIFFEJWTAssertionType}, - "client_assertion": {"sensitive-assertion"}, - }, - wantErr: "SPIFFE JWT client authentication is not implemented", - }, { name: "SPIFFE X.509 identity does not fall through", ctx: spiffeauth.ContextWithSPIFFEID(context.Background(), spiffeID), @@ -61,7 +63,7 @@ func TestSPIFFEClientAuthenticationStrategy(t *testing.T) { wantErr: "SPIFFE X.509 client authentication is not implemented", }, { - name: "SPIFFE JWT assertion is detected when not the first value", + name: "SPIFFE JWT assertion is detected when not the first value, then rejected as duplicated", ctx: context.Background(), resolver: stubResolver, form: url.Values{ @@ -70,7 +72,7 @@ func TestSPIFFEClientAuthenticationStrategy(t *testing.T) { spiffeauth.SPIFFEJWTAssertionType, }, }, - wantErr: "SPIFFE JWT client authentication is not implemented", + wantErr: fosite.ErrInvalidRequest.HintField, }, { name: "RFC 7523 assertion delegates to default strategy", @@ -107,7 +109,7 @@ func TestSPIFFEClientAuthenticationStrategy(t *testing.T) { strategy := newSPIFFEClientAuthenticationStrategy(func(_ context.Context, _ *http.Request, _ url.Values) (fosite.Client, error) { defaultCalled = true return defaultClient, defaultErr - }, tt.resolver) + }, testIssuer, nil, tt.resolver) req := httptest.NewRequest("POST", "/oauth/token", nil).WithContext(tt.ctx) client, err := strategy(tt.ctx, req, tt.form) @@ -130,3 +132,296 @@ func TestSPIFFEClientAuthenticationStrategy(t *testing.T) { }) } } + +func TestSPIFFEJWTClientAuthentication(t *testing.T) { + t.Parallel() + + id := spiffeid.RequireFromString("spiffe://example.org/workload/my-service") + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + validToken := signedJWT(t, jose.RS256, key, "key-1", "JWT", standardClaims(id, []string{testIssuer})) + source := jwtbundle.NewSet(jwtbundle.FromJWTAuthorities(id.TrustDomain(), map[string]crypto.PublicKey{"key-1": key.Public()})) + client := &fosite.DefaultClient{ID: "client"} + + tests := []struct { + name string + form url.Values + authorize string + source jwtbundle.Source + resolverErr error + resolverClient fosite.Client + wantErr error + wantCall bool + }{ + {name: "valid JWT-SVID without jti", form: jwtForm(validToken), source: source, wantCall: true}, + {name: "valid JWT-SVID with omitted client ID", form: url.Values{"client_assertion_type": {spiffeauth.SPIFFEJWTAssertionType}, "client_assertion": {validToken}}, source: source, wantCall: true}, + {name: "missing assertion", form: url.Values{"client_assertion_type": {spiffeauth.SPIFFEJWTAssertionType}, "client_id": {"client"}}, source: source, wantErr: fosite.ErrInvalidRequest}, + {name: "empty assertion", form: url.Values{"client_assertion_type": {spiffeauth.SPIFFEJWTAssertionType}, "client_assertion": {""}, "client_id": {"client"}}, source: source, wantErr: fosite.ErrInvalidRequest}, + {name: "duplicate assertion type", form: url.Values{"client_assertion_type": {spiffeauth.SPIFFEJWTAssertionType, spiffeauth.SPIFFEJWTAssertionType}, "client_assertion": {validToken}, "client_id": {"client"}}, source: source, wantErr: fosite.ErrInvalidRequest}, + {name: "empty client ID", form: url.Values{"client_assertion_type": {spiffeauth.SPIFFEJWTAssertionType}, "client_assertion": {validToken}, "client_id": {""}}, source: source, wantErr: fosite.ErrInvalidRequest}, + {name: "whitespace client ID", form: url.Values{"client_assertion_type": {spiffeauth.SPIFFEJWTAssertionType}, "client_assertion": {validToken}, "client_id": {" \t"}}, source: source, wantErr: fosite.ErrInvalidRequest}, + {name: "whitespace assertion", form: url.Values{"client_assertion_type": {spiffeauth.SPIFFEJWTAssertionType}, "client_assertion": {" \t"}, "client_id": {"client"}}, source: source, wantErr: fosite.ErrInvalidRequest}, + {name: "whitespace assertion type", form: url.Values{"client_assertion_type": {spiffeauth.SPIFFEJWTAssertionType, " \t"}, "client_assertion": {validToken}, "client_id": {"client"}}, source: source, wantErr: fosite.ErrInvalidRequest}, + {name: "duplicate client ID", form: url.Values{"client_assertion_type": {spiffeauth.SPIFFEJWTAssertionType}, "client_assertion": {validToken}, "client_id": {"client", "client"}}, source: source, wantErr: fosite.ErrInvalidRequest}, + // A client_id padded with whitespace is passed to the resolver exactly + // as received (no implicit normalization); it is then rejected because + // it does not exactly match the resolved client's ID. Silently + // trimming it before the match would let a whitespace-decorated + // client_id impersonate the canonical one. + {name: "client ID is not normalized", form: url.Values{"client_assertion_type": {spiffeauth.SPIFFEJWTAssertionType}, "client_assertion": {validToken}, "client_id": {" client "}}, source: source, wantErr: fosite.ErrInvalidClient, wantCall: true}, + {name: "Basic authorization with empty credentials", form: jwtForm(validToken), authorize: "Basic", source: source, wantErr: fosite.ErrInvalidClient}, + {name: "malformed Basic authorization", form: jwtForm(validToken), authorize: "Basic not-base64", source: source, wantErr: fosite.ErrInvalidClient}, + {name: "case-insensitive Basic authorization", form: jwtForm(validToken), authorize: "basic credentials", source: source, wantErr: fosite.ErrInvalidClient}, + {name: "client secret key", form: func() url.Values { f := jwtForm(validToken); f["client_secret"] = []string{""}; return f }(), source: source, wantErr: fosite.ErrInvalidClient}, + {name: "maximum-sized assertion", form: func() url.Values { + f := jwtForm(validToken) + f["client_assertion"] = []string{strings.Repeat("a", 16*1024)} + return f + }(), source: source, wantErr: fosite.ErrInvalidClient}, + {name: "oversized assertion", form: func() url.Values { + f := jwtForm(validToken) + f["client_assertion"] = []string{strings.Repeat("a", 16*1024+1)} + return f + }(), source: source, wantErr: fosite.ErrInvalidClient}, + {name: "malformed token", form: jwtForm("not-a-jwt"), source: source, wantErr: fosite.ErrInvalidClient}, + {name: "wrong signature", form: jwtForm(signedJWT(t, jose.RS256, mustRSAKey(t), "key-1", "JWT", standardClaims(id, []string{testIssuer}))), source: source, wantErr: fosite.ErrInvalidClient}, + {name: "missing key ID", form: jwtForm(signedJWT(t, jose.RS256, key, "", "JWT", standardClaims(id, []string{testIssuer}))), source: source, wantErr: fosite.ErrInvalidClient}, + {name: "invalid type", form: jwtForm(signedJWT(t, jose.RS256, key, "key-1", "not-jwt", standardClaims(id, []string{testIssuer}))), source: source, wantErr: fosite.ErrInvalidClient}, + {name: "wrong audience", form: jwtForm(signedJWT(t, jose.RS256, key, "key-1", "JWT", standardClaims(id, []string{"wrong"}))), source: source, wantErr: fosite.ErrInvalidClient}, + {name: "iss claim is not required and a mismatched one is ignored", form: jwtForm(signedJWT(t, jose.RS256, key, "key-1", "JWT", func() jwt.Claims { + claims := standardClaims(id, []string{testIssuer}) + claims.Issuer = "example.org" + return claims + }())), source: source, wantCall: true}, + {name: "multiple audience", form: jwtForm(signedJWT(t, jose.RS256, key, "key-1", "JWT", standardClaims(id, []string{testIssuer, "other"}))), source: source, wantErr: fosite.ErrInvalidClient}, + {name: "validity comfortably within six minutes", form: jwtForm(signedJWT(t, jose.RS256, key, "key-1", "JWT", claimsExpiringIn(id, 5*time.Minute))), source: source, wantCall: true}, + {name: "validity comfortably beyond six minutes", form: jwtForm(signedJWT(t, jose.RS256, key, "key-1", "JWT", claimsExpiringIn(id, 7*time.Minute))), source: source, wantErr: fosite.ErrInvalidClient}, + {name: "expired token", form: jwtForm(signedJWT(t, jose.RS256, key, "key-1", "JWT", expiredClaims(id))), source: source, wantErr: fosite.ErrInvalidClient}, + {name: "wrong trust domain", form: jwtForm(signedJWT(t, jose.RS256, key, "key-1", "JWT", standardClaims(spiffeid.RequireFromString("spiffe://other.org/workload"), []string{testIssuer}))), source: source, wantErr: fosite.ErrInvalidClient}, + {name: "resolver rejection", form: jwtForm(validToken), source: source, resolverErr: errors.New("association denied"), wantErr: fosite.ErrInvalidClient, wantCall: true}, + {name: "resolver returns different client", form: jwtForm(validToken), source: source, resolverClient: &fosite.DefaultClient{ID: "other-client"}, wantErr: fosite.ErrInvalidClient, wantCall: true}, + {name: "no JWT bundle source configured", form: jwtForm(validToken), source: nil, wantErr: fosite.ErrInvalidClient}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + called := false + strategy := newSPIFFEClientAuthenticationStrategy( + func(context.Context, *http.Request, url.Values) (fosite.Client, error) { + t.Fatal("default strategy called") + return nil, nil + }, + testIssuer, tt.source, + func(_ context.Context, gotSPIFFEID, clientID string, method spiffeauth.SPIFFEAuthenticationMethod) (fosite.Client, error) { + called = true + assert.Equal(t, id.String(), gotSPIFFEID) + assert.Equal(t, spiffeauth.SPIFFEAuthenticationMethodJWT, method) + assert.Equal(t, tt.form.Get("client_id"), clientID) + var resolvedClient fosite.Client = client + if tt.resolverClient != nil { + resolvedClient = tt.resolverClient + } + return resolvedClient, tt.resolverErr + }, + ) + req := httptest.NewRequest(http.MethodPost, "/oauth/token", nil) + if tt.authorize != "" { + req.Header.Set("Authorization", tt.authorize) + } + got, err := strategy(context.Background(), req, tt.form) + assert.Equal(t, tt.wantCall, called) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + assert.Nil(t, got) + if assertion := tt.form.Get("client_assertion"); assertion != "" { + assert.NotContains(t, err.Error(), assertion) + } + return + } + require.NoError(t, err) + assert.Same(t, client, got) + }) + } +} + +func TestSPIFFEJWTAssertionSizeLimit(t *testing.T) { + t.Parallel() + + request := httptest.NewRequest(http.MethodPost, "/oauth/token", nil) + form := url.Values{} + assert.False(t, rejectedSPIFFEJWTRequest(request, form, strings.Repeat("a", 16*1024))) + assert.True(t, rejectedSPIFFEJWTRequest(request, form, strings.Repeat("a", 16*1024+1))) +} + +func TestSPIFFEJWTClientAuthenticationAlgorithmsAndPrecedence(t *testing.T) { + t.Parallel() + + id := spiffeid.RequireFromString("spiffe://example.org/workload/my-service") + keys := []struct { + name string + alg jose.SignatureAlgorithm + key crypto.Signer + }{ + {"RS256", jose.RS256, mustRSAKey(t)}, + {"ES256", jose.ES256, mustECDSAKey(t)}, + {"PS256", jose.PS256, mustRSAKey(t)}, + } + for _, tt := range keys { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + token := signedJWT(t, tt.alg, tt.key, "key", "JWT", standardClaims(id, []string{testIssuer})) + source := jwtbundle.NewSet(jwtbundle.FromJWTAuthorities(id.TrustDomain(), map[string]crypto.PublicKey{"key": tt.key.Public()})) + defaultCalled := false + strategy := newSPIFFEClientAuthenticationStrategy(func(context.Context, *http.Request, url.Values) (fosite.Client, error) { + defaultCalled = true + return nil, nil + }, testIssuer, source, func(_ context.Context, gotSPIFFEID, _ string, method spiffeauth.SPIFFEAuthenticationMethod) (fosite.Client, error) { + assert.Equal(t, id.String(), gotSPIFFEID) + assert.Equal(t, spiffeauth.SPIFFEAuthenticationMethodJWT, method) + return &fosite.DefaultClient{ID: "client"}, nil + }) + ambient := spiffeauth.ContextWithSPIFFEID(context.Background(), spiffeid.RequireFromString("spiffe://example.org/ambient")) + got, err := strategy(ambient, httptest.NewRequest(http.MethodPost, "/", nil), jwtForm(token)) + require.NoError(t, err) + assert.NotNil(t, got) + assert.False(t, defaultCalled) + }) + } +} + +func TestSPIFFEJWTDispatchRejectsDuplicateAssertionType(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + form url.Values + wantDelegate bool + }{ + { + name: "SPIFFE then non-SPIFFE", + form: url.Values{"client_assertion_type": { + spiffeauth.SPIFFEJWTAssertionType, + "urn:ietf:params:oauth:client-assertion-type:jwt-bearer", + }}, + }, + { + name: "non-SPIFFE then SPIFFE", + form: url.Values{"client_assertion_type": { + "urn:ietf:params:oauth:client-assertion-type:jwt-bearer", + spiffeauth.SPIFFEJWTAssertionType, + }}, + }, + { + name: "identical non-SPIFFE duplicates", + form: url.Values{"client_assertion_type": { + "urn:ietf:params:oauth:client-assertion-type:jwt-bearer", + "urn:ietf:params:oauth:client-assertion-type:jwt-bearer", + }}, + wantDelegate: true, + }, + { + name: "one non-SPIFFE assertion type", + form: url.Values{"client_assertion_type": {"urn:ietf:params:oauth:client-assertion-type:jwt-bearer"}}, + wantDelegate: true, + }, + { + name: "absent assertion type", + form: url.Values{"client_id": {"client"}}, + wantDelegate: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + called := false + strategy := newSPIFFEClientAuthenticationStrategy(func(context.Context, *http.Request, url.Values) (fosite.Client, error) { + called = true + return nil, errors.New("default") + }, testIssuer, nil, stubResolver) + _, err := strategy(context.Background(), httptest.NewRequest(http.MethodPost, "/", nil), tt.form) + assert.Equal(t, tt.wantDelegate, called) + if tt.wantDelegate { + assert.EqualError(t, err, "default") + return + } + require.ErrorIs(t, err, fosite.ErrInvalidRequest) + }) + } +} + +func TestSPIFFEJWTDispatchDelegatesWithoutResolverEvenWithDuplicateAssertionType(t *testing.T) { + t.Parallel() + + defaultCalled := false + strategy := newSPIFFEClientAuthenticationStrategy(func(context.Context, *http.Request, url.Values) (fosite.Client, error) { + defaultCalled = true + return nil, nil + }, testIssuer, nil, nil) + _, err := strategy(context.Background(), httptest.NewRequest(http.MethodPost, "/", nil), url.Values{ + "client_assertion_type": {spiffeauth.SPIFFEJWTAssertionType, spiffeauth.SPIFFEJWTAssertionType}, + }) + require.NoError(t, err) + assert.True(t, defaultCalled) +} + +func TestSPIFFEX509ClientAuthenticationDoesNotFallThrough(t *testing.T) { + t.Parallel() + + strategy := newSPIFFEClientAuthenticationStrategy(func(context.Context, *http.Request, url.Values) (fosite.Client, error) { + t.Fatal("default strategy called") + return nil, nil + }, testIssuer, nil, stubResolver) + ctx := spiffeauth.ContextWithSPIFFEID(context.Background(), spiffeid.RequireFromString("spiffe://example.org/workload")) + _, err := strategy(ctx, httptest.NewRequest(http.MethodPost, "/", nil), url.Values{"client_id": {"client"}}) + require.ErrorIs(t, err, fosite.ErrInvalidClient) +} + +func jwtForm(token string) url.Values { + return url.Values{"client_assertion_type": {spiffeauth.SPIFFEJWTAssertionType}, "client_assertion": {token}, "client_id": {"client"}} +} +func standardClaims(id spiffeid.ID, audience []string) jwt.Claims { + return jwt.Claims{ + Issuer: id.TrustDomain().IDString(), + Subject: id.String(), + Audience: audience, + Expiry: jwt.NewNumericDate(time.Now().Add(2 * time.Minute)), + NotBefore: jwt.NewNumericDate(time.Now().Add(-time.Minute)), + IssuedAt: jwt.NewNumericDate(time.Now()), + } +} +func claimsExpiringIn(id spiffeid.ID, validity time.Duration) jwt.Claims { + claims := standardClaims(id, []string{testIssuer}) + claims.Expiry = jwt.NewNumericDate(time.Now().Add(validity)) + return claims +} +func expiredClaims(id spiffeid.ID) jwt.Claims { + c := standardClaims(id, []string{testIssuer}) + c.Expiry = jwt.NewNumericDate(time.Now().Add(-2 * time.Minute)) + return c +} +func mustRSAKey(t *testing.T) *rsa.PrivateKey { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + return key +} +func mustECDSAKey(t *testing.T) *ecdsa.PrivateKey { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + return key +} +func signedJWT(t *testing.T, alg jose.SignatureAlgorithm, key crypto.Signer, kid, typ string, claims jwt.Claims) string { + t.Helper() + options := (&jose.SignerOptions{}).WithType(jose.ContentType(typ)) + signer, err := jose.NewSigner(jose.SigningKey{Algorithm: alg, Key: jose.JSONWebKey{Key: key, KeyID: kid}}, options) + require.NoError(t, err) + token, err := jwt.Signed(signer).Claims(claims).Serialize() + require.NoError(t, err) + return token +} diff --git a/pkg/authserver/spiffe_association_registry_test.go b/pkg/authserver/spiffe_association_registry_test.go index 6e54b01562..6b10da9db2 100644 --- a/pkg/authserver/spiffe_association_registry_test.go +++ b/pkg/authserver/spiffe_association_registry_test.go @@ -20,11 +20,11 @@ func TestSPIFFEAssociationRegistryResolve(t *testing.T) { wildcard.PrincipalPattern = "spiffe://example.org/ns/workloads/*" other := testSPIFFEAssociation("other-client", "profile") other.PrincipalPattern = "spiffe://example.org/ns/other/agent" - registry := newTestSPIFFEAssociationRegistry(t, []SPIFFEClientAuthRunConfig{exact, wildcard, other}) tests := []struct { name string - registry *SPIFFEAssociationRegistry + associations []SPIFFEClientAuthRunConfig + nilRegistry bool spiffeID string clientID string method SPIFFEAuthenticationMethod @@ -33,53 +33,63 @@ func TestSPIFFEAssociationRegistryResolve(t *testing.T) { wantErr string }{ { - name: "exact identity", registry: registry, - spiffeID: "spiffe://example.org/ns/default/agent", clientID: "exact-client", + name: "exact identity", + associations: []SPIFFEClientAuthRunConfig{exact, wildcard, other}, + spiffeID: "spiffe://example.org/ns/default/agent", clientID: "exact-client", method: SPIFFEAuthenticationMethodX509, wantScope: "openid", }, { - name: "empty client ID derives from association", registry: registry, - spiffeID: "spiffe://example.org/ns/default/agent", clientID: "", + name: "empty client ID derives from association", + associations: []SPIFFEClientAuthRunConfig{exact, wildcard, other}, + spiffeID: "spiffe://example.org/ns/default/agent", clientID: "", method: SPIFFEAuthenticationMethodX509, wantScope: "openid", wantClientID: "exact-client", }, { - name: "wildcard identity", registry: registry, - spiffeID: "spiffe://example.org/ns/workloads/agent", clientID: "wildcard-client", + name: "wildcard identity", + associations: []SPIFFEClientAuthRunConfig{exact, wildcard, other}, + spiffeID: "spiffe://example.org/ns/workloads/agent", clientID: "wildcard-client", method: SPIFFEAuthenticationMethodX509, wantScope: "profile", }, { - name: "nil registry", registry: nil, - spiffeID: "spiffe://example.org/ns/default/agent", clientID: "exact-client", + name: "nil registry", + nilRegistry: true, + spiffeID: "spiffe://example.org/ns/default/agent", clientID: "exact-client", method: SPIFFEAuthenticationMethodX509, wantErr: "no SPIFFE associations", }, { - name: "malformed identity", registry: registry, - spiffeID: "not-a-spiffe-id", clientID: "exact-client", + name: "malformed identity", + associations: []SPIFFEClientAuthRunConfig{exact}, + spiffeID: "not-a-spiffe-id", clientID: "exact-client", method: SPIFFEAuthenticationMethodX509, wantErr: "invalid SPIFFE ID", }, { - name: "unknown identity", registry: registry, - spiffeID: "spiffe://example.org/ns/missing/agent", clientID: "exact-client", + name: "unknown identity", + associations: []SPIFFEClientAuthRunConfig{exact}, + spiffeID: "spiffe://example.org/ns/missing/agent", clientID: "exact-client", method: SPIFFEAuthenticationMethodX509, wantErr: "no SPIFFE association for ID", }, { - name: "identity client mismatch", registry: registry, - spiffeID: "spiffe://example.org/ns/default/agent", clientID: "other-client", + name: "identity client mismatch", + associations: []SPIFFEClientAuthRunConfig{exact, other}, + spiffeID: "spiffe://example.org/ns/default/agent", clientID: "other-client", method: SPIFFEAuthenticationMethodX509, wantErr: "not associated with client ID", }, { - name: "unknown client", registry: registry, - spiffeID: "spiffe://example.org/ns/default/agent", clientID: "missing-client", + name: "unknown client", + associations: []SPIFFEClientAuthRunConfig{exact}, + spiffeID: "spiffe://example.org/ns/default/agent", clientID: "missing-client", method: SPIFFEAuthenticationMethodX509, wantErr: "not associated with client ID", }, { - name: "disabled method", registry: registry, - spiffeID: "spiffe://example.org/ns/default/agent", clientID: "exact-client", + name: "disabled method", + associations: []SPIFFEClientAuthRunConfig{exact}, + spiffeID: "spiffe://example.org/ns/default/agent", clientID: "exact-client", method: SPIFFEAuthenticationMethodJWT, wantErr: "is not enabled", }, { - name: "unknown method", registry: registry, - spiffeID: "spiffe://example.org/ns/default/agent", clientID: "exact-client", + name: "unknown method", + associations: []SPIFFEClientAuthRunConfig{exact}, + spiffeID: "spiffe://example.org/ns/default/agent", clientID: "exact-client", method: SPIFFEAuthenticationMethod("unknown"), wantErr: "is not enabled", }, } @@ -88,7 +98,11 @@ func TestSPIFFEAssociationRegistryResolve(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - principal, err := tt.registry.Resolve(tt.spiffeID, tt.clientID, tt.method) + var registry *SPIFFEAssociationRegistry + if !tt.nilRegistry { + registry = newTestSPIFFEAssociationRegistry(t, tt.associations) + } + principal, err := registry.Resolve(tt.spiffeID, tt.clientID, tt.method) if tt.wantErr != "" { require.ErrorContains(t, err, tt.wantErr) assert.Equal(t, NormalizedSPIFFEPrincipal{}, principal) diff --git a/pkg/authserver/spiffe_client_resolver_test.go b/pkg/authserver/spiffe_client_resolver_test.go new file mode 100644 index 0000000000..7f518ab7c6 --- /dev/null +++ b/pkg/authserver/spiffe_client_resolver_test.go @@ -0,0 +1,65 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package authserver + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + spiffeauth "github.com/stacklok/toolhive/pkg/authserver/spiffe" + "github.com/stacklok/toolhive/pkg/authserver/storage" +) + +func TestNewSPIFFEClientResolverUsesResolvedClientID(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + clientID string + pattern string + spiffeID string + }{ + { + name: "exact association", + clientID: "exact-client", + pattern: "spiffe://example.org/ns/default/agent", + spiffeID: "spiffe://example.org/ns/default/agent", + }, + { + name: "wildcard association", + clientID: "wildcard-client", + pattern: "spiffe://example.org/ns/default/*", + spiffeID: "spiffe://example.org/ns/default/agent", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + association := testSPIFFEAssociation(tt.clientID, "openid") + association.PrincipalPattern = tt.pattern + association.Methods = []SPIFFEAuthenticationMethod{SPIFFEAuthenticationMethodJWT} + registry := newTestSPIFFEAssociationRegistry(t, []SPIFFEClientAuthRunConfig{association}) + clients, err := registry.staticClients() + require.NoError(t, err) + + base := storage.NewMemoryStorage() + t.Cleanup(func() { _ = base.Close() }) + stor, err := storage.NewSPIFFEStorageDecorator(context.Background(), base, clients) + require.NoError(t, err) + + resolver := newSPIFFEClientResolver(registry, stor) + require.NotNil(t, resolver) + client, err := resolver( + context.Background(), tt.spiffeID, "", spiffeauth.SPIFFEAuthenticationMethodJWT, + ) + require.NoError(t, err) + assert.Equal(t, tt.clientID, client.GetID()) + }) + } +}