diff --git a/UPGRADING.md b/UPGRADING.md index acbbcb68..d9e0a6ea 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -15,6 +15,90 @@ Most production-assurance changes only affect `Config.Assurance = AssuranceProduction`. A development-assurance setup built on `memstore` and `keys/ephemeral` needs only the steps not marked *production only*. +## v0.61.0 + +### A statically registered client authenticates with its static keys only (federation, server) + +**Affects:** a server with OpenID Federation automatic registration (or +a `federation.AutomaticClientKeySource` built by hand) where a client in +`Dependencies.Clients` has a `client_id` that is also an Entity +Identifier resolving under one of `AutomaticRegistration.TrustAnchors`, +and that client signs with a key `Dependencies.ClientKeys` doesn't +return for it. Also a client `Dependencies.Clients` doesn't have, for +which `Dependencies.ClientKeys` does return keys. + +**Why:** when `Dependencies.ClientKeys` had no matching key for a client, +its keys were looked up in its federation metadata instead, even for a +statically registered client. Any superior in a Trust Chain can set that +metadata's `jwks`, so a key it published authenticated as the static +client, with the static registration's scopes and grants. + +**What fails:** a statically registered client's keys now come from +`Dependencies.ClientKeys` alone; an assertion or request object signed +with a key only its federation metadata publishes fails with +`invalid_client`. A client resolved through federation gets its keys from +its federation metadata alone, never from `Dependencies.ClientKeys`. +Whether a client is statically registered is decided as `ResolveClient` +decides it: `Dependencies.Clients` resolves it. If that store fails with +`storage.ErrStoreUnavailable`, key resolution now fails with that error +too. + +**What to change:** give each statically registered client's keys to +`Dependencies.ClientKeys`, or remove its static registration so that it +registers automatically (with automatic registration's scopes and +grants). + +### Automatic registration holds RP Metadata Choices to the policy on the singular parameter (federation) + +**Affects:** a Relying Party registered automatically under a Trust +Anchor or Intermediate whose `metadata_policy` sets `one_of` or `value` on +`token_endpoint_auth_signing_alg` or `token_endpoint_auth_method`, and +which declares algorithms or methods outside it in +`token_endpoint_auth_signing_alg_values_supported` or +`token_endpoint_auth_methods_supported`, or names no algorithm at all. +Also a Relying Party whose `token_endpoint_auth_signing_alg_values_supported` +or `token_endpoint_auth_methods_supported` is an empty array. + +**Why:** a policy operator applies only to the parameter it names, and +`one_of` only when that parameter is present (OpenID Federation 1.0 +§6.1.3.1.4). A Relying Party could leave out +`token_endpoint_auth_signing_alg` and list a forbidden algorithm in its +RP Metadata Choices sibling, or name none and get the server's defaults, +and register an algorithm or method the policy rules out. An empty array +(which a superior's `subset_of` can leave) was read as not declared, so +the defaults applied there too. + +**What fails:** algorithms and methods outside the policy on the singular +parameter are no longer registered, from either parameter or from +`AutomaticRegistrationConfig.ClientAssertionAlgorithms`. A Relying Party +left with none, or declaring an empty array, isn't registered, and its +requests fail with `invalid_client` (`OnResolutionFailure` is told why). +A Relying Party whose `token_endpoint_auth_method` or +`token_endpoint_auth_signing_alg` isn't in its own empty array is refused +too, as RP Metadata Choices requires it to be in the list. + +**What to change:** nothing on the OP. A refused Relying Party lists an +algorithm or method its superiors' policy permits. + +### Automatic registration refuses empty-string metadata (federation) + +**Affects:** a Relying Party registered automatically whose +`openid_relying_party` metadata gives `""` for a string parameter +automatic registration reads, such as `subject_type`, `client_name`, +`logo_uri` or `request_object_signing_alg`. v0.58.0 already refused it +for the ID token and UserInfo encryption parameters. + +**Why:** a metadata policy counts `""` as present, so it met an +`essential` policy (OpenID Federation 1.0 §6.1.3.1.7), while automatic +registration read it as left out and applied the OP's default instead, +such as its default `subject_type`. + +**What fails:** the Relying Party isn't registered, and its requests fail +with `invalid_client` (`OnResolutionFailure` names the parameter). + +**What to change:** nothing on the OP. A refused Relying Party leaves the +parameter out, or gives it a value. + ## v0.58.0 ### Automatic registration skips `tls_client_auth` unless chains are verified (server) diff --git a/docs/guides/openid-federation.md b/docs/guides/openid-federation.md index 2afd24a0..bbb9bdde 100644 --- a/docs/guides/openid-federation.md +++ b/docs/guides/openid-federation.md @@ -149,7 +149,12 @@ each hop, and applies: - **Bounded fetching**: `Limits.MaxPathLength` and `Limits.MaxAuthorityHints` cap one resolution at a fixed number of outbound fetches, since automatic registration resolves a `client_id` - chosen by an unauthenticated caller. + chosen by an unauthenticated caller. What isn't bounded is where they + go: a chain is fetched before it can be verified, so the + `federation_fetch_endpoint` URLs of its not-yet-trusted superiors + (host, port, path and query) are the caller's choice. The fetcher's + address checks are what limit them; a host you list in + `fapihttp.Config.AllowedPrivateHosts` is reachable by anyone this way. - **Strict JSON**: entity metadata and keys are decoded case-sensitively, so a member that differs only in case (`REDIRECT_URIS`) can't slip past a policy written for the real one. diff --git a/fapihttp/client.go b/fapihttp/client.go index 1544e5a5..25e6edf8 100644 --- a/fapihttp/client.go +++ b/fapihttp/client.go @@ -108,6 +108,16 @@ type Config struct { // wouldn't work — never for arbitrary or end-user-supplied input. // Nil/empty (the zero value) exempts nothing, matching every other // optional field here. + // + // The exemption holds for every URL the Client is given, not only + // the ones an operator configured. A Client that fetches URLs other + // parties choose — an OpenID Federation resolver following a Trust + // Chain, whose federation_fetch_endpoint URLs are fetched before the + // chain is trusted, and so, through automatic registration, for any + // client_id an unauthenticated caller names — lets them send a GET + // to any path and query on an allowed host. List a host here only if + // it is safe for anyone to reach that way, or give such fetches a + // Client without it. AllowedPrivateHosts []string } diff --git a/federation/automatic_registration.go b/federation/automatic_registration.go index 59887447..c1a33219 100644 --- a/federation/automatic_registration.go +++ b/federation/automatic_registration.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "net/url" + "reflect" "slices" "strings" "sync" @@ -15,6 +16,7 @@ import ( fapi "github.com/idfoundry/fapigo" "github.com/idfoundry/fapigo/fapihttp" + intfed "github.com/idfoundry/fapigo/internal/federation" "github.com/idfoundry/fapigo/internal/jose" "github.com/idfoundry/fapigo/internal/mtls" "github.com/idfoundry/fapigo/internal/strictjson" @@ -139,7 +141,9 @@ type AutomaticRegistrationConfig struct { // any algorithm supported by the OP and the RP MAY be used". Typically // the server's own allowed client assertion algorithms (server.New // sets it so). Empty means an RP must name its algorithm to register - // for private_key_jwt. + // for private_key_jwt. A superior's metadata policy restricting + // token_endpoint_auth_signing_alg (one_of or value) restricts these + // defaults too, as it does token_endpoint_auth_signing_alg_values_supported. ClientAssertionAlgorithms []fapi.SignatureAlgorithm // SubjectTypes is the kinds of sub this OP gives an automatically @@ -253,13 +257,21 @@ func newRememberedFailure(err error) error { // Static registration always wins: Underlying is tried first, and its // result returned as-is when it succeeds — federation resolution is // only attempted when Underlying fails to resolve id AND id is itself -// a well-formed https Entity Identifier (ValidEntityID). Underlying's -// own interface has no way to distinguish "no such client" from a real -// backing-store failure (storage.ClientRepository's own contract), so -// a transient Underlying failure for an id that happens to look like -// an Entity Identifier is indistinguishable from "not statically -// registered" here — an inherent limitation of that interface, not -// something this type can work around. +// a well-formed https Entity Identifier (ValidEntityID). An Underlying +// failure wrapping storage.ErrStoreUnavailable is returned as it is; +// any other is read as "not statically registered". The same split +// decides where AutomaticClientKeySource takes a client's keys from, so +// a statically registered client is authenticated with its static keys +// only, never with ones its federation metadata publishes. +// +// A client_id is resolved before its client is authenticated — that is +// how automatic registration finds the keys to authenticate it with — +// so any caller can name one, and Resolver.Resolve then fetches the URLs +// that Entity Identifier's statements name, before any of them is +// trusted (see Resolver.Resolve and Dependencies.HTTP). The fetcher +// bounds which hosts that can reach; a host listed in its +// fapihttp.Config.AllowedPrivateHosts is reachable by unauthenticated +// callers this way. // // The client_credentials grant and CIBA are each gated by their own // config-level switch (AutomaticRegistrationConfig.AllowsClientCredentialsGrant/ @@ -391,6 +403,21 @@ func (a *AutomaticClientRepository) ResolveClient(ctx context.Context, id fapi.C return entry.client, nil } +// staticallyRegistered reports whether Underlying resolves id — the +// same test ResolveClient uses to decide that a client is statically +// registered rather than resolved through federation. An unavailable +// store is an error, not a "no". +func (a *AutomaticClientRepository) staticallyRegistered(ctx context.Context, id fapi.ClientID) (bool, error) { + _, err := a.underlying.ResolveClient(ctx, id) + if err == nil { + return true, nil + } + if errors.Is(err, storage.ErrStoreUnavailable) { + return false, err + } + return false, nil +} + // resolveVerificationKeys resolves id's own client authentication keys // from its cached (or freshly resolved) openid_relying_party metadata — // the shared implementation behind AutomaticClientKeySource, factored @@ -527,7 +554,11 @@ func (a *AutomaticClientRepository) register(ctx context.Context, id fapi.Client if !ok { return cachedClient{}, fmt.Errorf("federation: %q has no %s metadata", id, relyingPartyEntityType) } - clientCfg, jwks, err := a.registeredClientConfigFromMetadata(ctx, id, raw) + limits, err := choiceLimitsFromPolicy(resolved.policy[relyingPartyEntityType]) + if err != nil { + return cachedClient{}, fmt.Errorf("federation: %q: %w", id, err) + } + clientCfg, jwks, err := a.registeredClientConfigFromMetadata(ctx, id, raw, limits) if err != nil { return cachedClient{}, fmt.Errorf("federation: %q: %w", id, err) } @@ -578,24 +609,26 @@ func (a *AutomaticClientRepository) fetchJWKS(ctx context.Context, uri string) ( // Entity Configuration's own top-level federation signing key — a // different key, for a different purpose (federation/doc.go). // -// Underlying is tried first, exactly mirroring -// AutomaticClientRepository's own "static registration always wins" -// precedence — a statically registered client's real keys must never -// be shadowed by a federation lookup. Unlike ResolveClient's own -// single-value result, a keys.ClientKeySource legitimately returns an -// empty, error-free VerificationKeySet to mean "no matching key" (e.g. -// a real client mid-rotation, or simply a kid Underlying doesn't -// recognize) — so an empty result from Underlying falls through to -// federation resolution exactly like an error would, on the -// expectation that an Underlying implementation unaware of a given -// client ID at all is at least as likely to signal that with an empty -// result as with an error (keys.ClientKeySource's own interface -// documents no requirement either way). This mirrors -// AutomaticClientRepository's own documented "Underlying's interface -// has no way to distinguish these cases" limitation — a statically -// registered client legitimately missing a requested key triggers one -// wasted federation resolution attempt (which then also fails), not a -// silent wrong answer. +// Keys come from the same source as the registration they authenticate. +// A client that repo's own underlying client repository (the one passed +// to NewAutomaticClientRepository) resolves is statically registered, +// and its keys come from Underlying (this type's key source) alone: +// whatever Underlying returns, an error or an empty set included, is +// the answer, exactly as it would be without automatic registration. +// Only a client that repository doesn't have is resolved through its +// Trust Chain, and its keys then come from its openid_relying_party +// metadata alone. A +// statically registered client whose client_id is also an Entity +// Identifier resolving under one of the configured Trust Anchors is +// therefore never authenticated with a key its federation metadata +// publishes (which any superior in its Trust Chain can set): that would +// let a federation key act as the static client, with the static +// registration's privileges. An error from that repository other than +// "not registered" — storage.ErrStoreUnavailable — is returned too, +// rather than read as "not statically registered", for the same reason +// ResolveClient does. Each call asks the client repository first, so a +// key resolution costs one more lookup there than it would without +// automatic registration. type AutomaticClientKeySource struct { underlying keys.ClientKeySource repo *AutomaticClientRepository @@ -621,10 +654,10 @@ func NewAutomaticClientKeySource(underlying keys.ClientKeySource, repo *Automati // federation-derived resolution path is always hardened (it goes // through AutomaticClientRepository's Resolver, which requires a real // fapihttp.Client), but that alone doesn't make the whole type safe to -// declare LiveFetchHardened unconditionally: Underlying is tried first -// (see this type's own doc comment), so an Underlying that isn't itself -// hardened — or doesn't declare — must not be silently papered over by -// this type's other, safe path. +// declare LiveFetchHardened unconditionally: Underlying answers for every +// statically registered client (see this type's own doc comment), so an +// Underlying that isn't itself hardened — or doesn't declare — must not +// be silently papered over by this type's other, safe path. func (s *AutomaticClientKeySource) Capabilities() keys.KeySourceCapabilities { if asserter, ok := s.underlying.(keys.KeySourceAssurance); ok { return asserter.Capabilities() @@ -634,8 +667,12 @@ func (s *AutomaticClientKeySource) Capabilities() keys.KeySourceCapabilities { // ResolveVerificationKeys implements keys.ClientKeySource. func (s *AutomaticClientKeySource) ResolveVerificationKeys(ctx context.Context, req keys.ClientKeyRequest) (keys.VerificationKeySet, error) { - if set, err := s.underlying.ResolveVerificationKeys(ctx, req); err == nil && len(set.Keys) > 0 { - return set, nil + static, err := s.repo.staticallyRegistered(ctx, req.ClientID) + if err != nil { + return keys.VerificationKeySet{}, err + } + if static { + return s.underlying.ResolveVerificationKeys(ctx, req) } return s.repo.resolveVerificationKeys(ctx, req) } @@ -712,16 +749,24 @@ type relyingPartyMetadata struct { // can even validate the resulting config, so there is no later point at // which deferring the fetch would still work for every auth method. // +// limits holds the choices taken from token_endpoint_auth_methods_supported +// and token_endpoint_auth_signing_alg_values_supported to what the Trust +// Chain's metadata policy permits for their singular parameters (see +// choiceLimits). +// // a.cfg supplies the operator-level capability grants (AllowedScopes, // AllowsClientCredentialsGrant, AllowsCIBA) that must never be inferred // from raw itself, plus the AllowedClientAuthMethods restriction raw's // own token_endpoint_auth_method is checked against — see AutomaticRegistrationConfig's own doc comments // for why. -func (a *AutomaticClientRepository) registeredClientConfigFromMetadata(ctx context.Context, id fapi.ClientID, raw json.RawMessage) (storage.RegisteredClientConfig, json.RawMessage, error) { +func (a *AutomaticClientRepository) registeredClientConfigFromMetadata(ctx context.Context, id fapi.ClientID, raw json.RawMessage, limits choiceLimits) (storage.RegisteredClientConfig, json.RawMessage, error) { var m relyingPartyMetadata if err := strictjson.Unmarshal(raw, &m); err != nil { return storage.RegisteredClientConfig{}, nil, fmt.Errorf("parse %s metadata: %w", relyingPartyEntityType, err) } + if err := refuseEmptyStrings(raw); err != nil { + return storage.RegisteredClientConfig{}, nil, err + } if len(m.RedirectURIs) == 0 { return storage.RegisteredClientConfig{}, nil, fmt.Errorf("%s metadata has no redirect_uris", relyingPartyEntityType) } @@ -729,7 +774,7 @@ func (a *AutomaticClientRepository) registeredClientConfigFromMetadata(ctx conte return storage.RegisteredClientConfig{}, nil, fmt.Errorf("%s metadata must declare exactly one of jwks or jwks_uri", relyingPartyEntityType) } - declared, err := a.declaredClientAuthMethods(m) + declared, err := a.declaredClientAuthMethods(m, limits.authMethods) if err != nil { return storage.RegisteredClientConfig{}, nil, err } @@ -778,7 +823,7 @@ func (a *AutomaticClientRepository) registeredClientConfigFromMetadata(ctx conte // client's own published certificate (via jwks/jwks_uri's "x5c") for // self_signed_tls_client_auth, and a plain metadata string for // tls_client_auth and its four SAN-typed siblings. - if err := a.applyClientAuthMethods(&cfg, declared, m, jwks); err != nil { + if err := a.applyClientAuthMethods(&cfg, declared, m, jwks, limits.signingAlgs); err != nil { return storage.RegisteredClientConfig{}, nil, err } @@ -836,10 +881,10 @@ func clientDisplayFromMetadata(m relyingPartyMetadata) (storage.ClientDisplay, e // applyClientAuthMethodFields sets cfg's authMethod-specific field from // m/jwks — split out of registeredClientConfigFromMetadata purely to // keep that method's own cognitive complexity manageable. -func applyClientAuthMethodFields(cfg *storage.RegisteredClientConfig, authMethod storage.ClientAuthMethod, m relyingPartyMetadata, jwks json.RawMessage, defaultAlgs []fapi.SignatureAlgorithm) error { +func applyClientAuthMethodFields(cfg *storage.RegisteredClientConfig, authMethod storage.ClientAuthMethod, m relyingPartyMetadata, jwks json.RawMessage, defaultAlgs []fapi.SignatureAlgorithm, permittedAlgs *permittedValues) error { switch authMethod { case storage.ClientAuthMethodPrivateKeyJWT: - algs, err := assertionAlgorithms(m, defaultAlgs) + algs, err := assertionAlgorithms(m, defaultAlgs, permittedAlgs) if err != nil { return err } @@ -903,13 +948,23 @@ type declaredMethod struct { // package infers no other. Listed methods it doesn't implement are // skipped; a token_endpoint_auth_method it doesn't implement, or one // AllowedClientAuthMethods refuses, fails the registration. -func (a *AutomaticClientRepository) declaredClientAuthMethods(m relyingPartyMetadata) ([]declaredMethod, error) { +// +// Every method that remains is registered, not only the +// token_endpoint_auth_method: §12.1.4 has the OP accept "any client +// authentication method that is mutually supported", since automatic +// registration never tells it which one the RP will use. permitted, when +// set, is what the metadata policy allows token_endpoint_auth_method to +// be, and listed methods outside it are skipped as well: the policy +// names the singular parameter only, and a superior restricting how its +// subordinates authenticate means the methods they register, whichever +// parameter declares them. +func (a *AutomaticClientRepository) declaredClientAuthMethods(m relyingPartyMetadata, permitted *permittedValues) ([]declaredMethod, error) { single := m.TokenEndpointAuthMethod list := m.TokenEndpointAuthMethodsSupported if single == "" && len(list) == 0 { return nil, fmt.Errorf("%s metadata declares no client authentication method (token_endpoint_auth_method or token_endpoint_auth_methods_supported)", relyingPartyEntityType) } - if single != "" && len(list) > 0 && !slices.Contains(list, single) { + if single != "" && list != nil && !slices.Contains(list, single) { return nil, fmt.Errorf("token_endpoint_auth_method %q is not among token_endpoint_auth_methods_supported", single) } var declared []declaredMethod @@ -925,13 +980,13 @@ func (a *AutomaticClientRepository) declaredClientAuthMethods(m relyingPartyMeta } for _, name := range list { method, err := storage.ParseClientAuthMethod(name) - if err != nil || !a.allowsClientAuthMethod(method) || slices.ContainsFunc(declared, func(d declaredMethod) bool { return d.method == method }) { + if err != nil || !a.allowsClientAuthMethod(method) || !permitted.allows(name) || slices.ContainsFunc(declared, func(d declaredMethod) bool { return d.method == method }) { continue } declared = append(declared, declaredMethod{method: method}) } if len(declared) == 0 { - return nil, fmt.Errorf("none of token_endpoint_auth_methods_supported %v is supported and permitted by allowed_client_auth_methods", list) + return nil, fmt.Errorf("none of token_endpoint_auth_methods_supported %v is supported and permitted by allowed_client_auth_methods and the metadata policy", list) } return declared, nil } @@ -942,10 +997,10 @@ func (a *AutomaticClientRepository) declaredClientAuthMethods(m relyingPartyMeta // tls_client_auth_subject_dn) is dropped; the explicitly requested // token_endpoint_auth_method must be usable. At least one method must // remain. -func (a *AutomaticClientRepository) applyClientAuthMethods(cfg *storage.RegisteredClientConfig, declared []declaredMethod, m relyingPartyMetadata, jwks json.RawMessage) error { +func (a *AutomaticClientRepository) applyClientAuthMethods(cfg *storage.RegisteredClientConfig, declared []declaredMethod, m relyingPartyMetadata, jwks json.RawMessage, permittedAlgs *permittedValues) error { var lastErr error for _, d := range declared { - if err := applyClientAuthMethodFields(cfg, d.method, m, jwks, a.cfg.ClientAssertionAlgorithms); err != nil { + if err := applyClientAuthMethodFields(cfg, d.method, m, jwks, a.cfg.ClientAssertionAlgorithms, permittedAlgs); err != nil { if d.explicit { return err } @@ -967,19 +1022,34 @@ func (a *AutomaticClientRepository) applyClientAuthMethods(cfg *storage.Register // token_endpoint_auth_signing_alg_values_supported (skipping algorithms // this package doesn't implement), else defaultAlgs — "any algorithm // supported by the OP and the RP". -func assertionAlgorithms(m relyingPartyMetadata, defaultAlgs []fapi.SignatureAlgorithm) ([]fapi.SignatureAlgorithm, error) { +// +// permitted, when set, is what the metadata policy allows +// token_endpoint_auth_signing_alg to be. The policy already holds a +// declared token_endpoint_auth_signing_alg to it, but not the other two +// sources, so algorithms outside it are left out of those, and an RP left +// with none is refused (RP Metadata Choices 1.0 §4: use a choice that is +// supported, or reject the registration when none is). A superior's +// one_of on token_endpoint_auth_signing_alg then means the algorithms +// the RP registers, however it declares them. +func assertionAlgorithms(m relyingPartyMetadata, defaultAlgs []fapi.SignatureAlgorithm, permitted *permittedValues) ([]fapi.SignatureAlgorithm, error) { single := m.TokenEndpointAuthSigningAlg list := m.TokenEndpointAuthSigningAlgValuesSupported if single != "" { return declaredAssertionAlgorithm(single, list) } - if len(list) > 0 { - return supportedAssertionAlgorithms(list) + if list != nil { + return supportedAssertionAlgorithms(list, permitted) + } + var algs []fapi.SignatureAlgorithm + for _, alg := range defaultAlgs { + if permitted.allows(alg.String()) { + algs = append(algs, alg) + } } - if len(defaultAlgs) == 0 { - return nil, fmt.Errorf("token_endpoint_auth_signing_alg is required for private_key_jwt: no default algorithms are configured") + if len(algs) == 0 { + return nil, fmt.Errorf("token_endpoint_auth_signing_alg is required for private_key_jwt: no default algorithm is configured that the metadata policy permits") } - return slices.Clone(defaultAlgs), nil + return algs, nil } // declaredAssertionAlgorithm is assertionAlgorithms' answer when the RP @@ -987,7 +1057,7 @@ func assertionAlgorithms(m relyingPartyMetadata, defaultAlgs []fapi.SignatureAlg // must be among list, its declared supported algorithms, when it // declared any. func declaredAssertionAlgorithm(single string, list []string) ([]fapi.SignatureAlgorithm, error) { - if len(list) > 0 && !slices.Contains(list, single) { + if list != nil && !slices.Contains(list, single) { return nil, fmt.Errorf("token_endpoint_auth_signing_alg %q is not among token_endpoint_auth_signing_alg_values_supported", single) } alg, err := fapi.ParseSignatureAlgorithm(single) @@ -998,20 +1068,113 @@ func declaredAssertionAlgorithm(single string, list []string) ([]fapi.SignatureA } // supportedAssertionAlgorithms is list's algorithms this package -// implements, once each, in list's order. -func supportedAssertionAlgorithms(list []string) ([]fapi.SignatureAlgorithm, error) { +// implements and permitted allows, once each, in list's order. An empty +// list (which a superior's subset_of can leave) declares no algorithm, +// so it is refused like one with none usable, never read as absent. +func supportedAssertionAlgorithms(list []string, permitted *permittedValues) ([]fapi.SignatureAlgorithm, error) { var algs []fapi.SignatureAlgorithm for _, name := range list { + if !permitted.allows(name) { + continue + } if alg, err := fapi.ParseSignatureAlgorithm(name); err == nil && !slices.Contains(algs, alg) { algs = append(algs, alg) } } if len(algs) == 0 { - return nil, fmt.Errorf("none of token_endpoint_auth_signing_alg_values_supported %v is supported", list) + return nil, fmt.Errorf("none of token_endpoint_auth_signing_alg_values_supported %v is supported and permitted by the metadata policy", list) } return algs, nil } +// choiceLimits is what a Trust Chain's metadata policy permits for the +// singular parameters automatic registration also reads an OpenID +// Connect RP Metadata Choices 1.0 §2 plural sibling of. Policy +// operators act on the parameter they name only (OpenID Federation 1.0 +// §6.1.3.1), and one_of checks a value only "if the metadata parameter +// is present" (§6.1.3.1.4); so an RP leaving out +// token_endpoint_auth_signing_alg under a one_of on it, and declaring +// token_endpoint_auth_signing_alg_values_supported instead, would +// otherwise register a value the policy forbids. Each nil field means +// no policy restricts that parameter. +type choiceLimits struct { + authMethods *permittedValues // token_endpoint_auth_method + signingAlgs *permittedValues // token_endpoint_auth_signing_alg +} + +// permittedValues is the set of values a metadata policy allows a +// singular parameter; a nil *permittedValues allows any value. +type permittedValues struct{ values []string } + +// allows reports whether p permits v. +func (p *permittedValues) allows(v string) bool { + return p == nil || slices.Contains(p.values, v) +} + +// choiceLimitsFromPolicy reads choiceLimits from policy, the merged +// openid_relying_party metadata policy (nil when none applies). +func choiceLimitsFromPolicy(policy map[string]intfed.PolicyOperators) (choiceLimits, error) { + var limits choiceLimits + for _, field := range []struct { + param string + dst **permittedValues + }{ + {"token_endpoint_auth_method", &limits.authMethods}, + {"token_endpoint_auth_signing_alg", &limits.signingAlgs}, + } { + permitted, constrained, err := intfed.PermittedStrings(policy[field.param]) + if err != nil { + return choiceLimits{}, fmt.Errorf("metadata policy for %s: %w", field.param, err) + } + if constrained { + *field.dst = &permittedValues{values: permitted} + } + } + return limits, nil +} + +// refuseEmptyStrings refuses raw, an openid_relying_party metadata +// object, when a string parameter this package reads from it is the +// empty string. None of them has "" among its values, and each would +// otherwise read as left out while counting as present to a metadata +// policy (OpenID Federation 1.0 §6.1.3.1.7: essential requires only that +// the parameter "MUST be present"), so a superior's essential policy +// would be met by a value that registers the OP's default instead. +func refuseEmptyStrings(raw json.RawMessage) error { + var members map[string]json.RawMessage + if err := json.Unmarshal(raw, &members); err != nil { + return fmt.Errorf("parse %s metadata: %w", relyingPartyEntityType, err) + } + for _, name := range relyingPartyStringParameters { + // Unmarshalling null into a string leaves it "" too, so only a + // JSON string counts: null states nothing, and reads as left out. + var value string + if v, ok := members[name]; ok && len(v) > 0 && v[0] == '"' && json.Unmarshal(v, &value) == nil && value == "" { + return fmt.Errorf("%s must not be empty", name) + } + } + return nil +} + +// relyingPartyStringParameters is the name of every string parameter +// relyingPartyMetadata reads, from its json tags, so a parameter added +// there is covered by refuseEmptyStrings without being listed twice. +var relyingPartyStringParameters = func() []string { + var names []string + t := reflect.TypeFor[relyingPartyMetadata]() + for i := range t.NumField() { + f := t.Field(i) + kind := f.Type.Kind() + if kind == reflect.Pointer { + kind = f.Type.Elem().Kind() + } + if kind == reflect.String { + names = append(names, strings.Split(f.Tag.Get("json"), ",")[0]) + } + } + return names +}() + // allowsClientAuthMethod reports whether method is permitted by // AutomaticRegistrationConfig.AllowedClientAuthMethods — every method // when that list is empty. diff --git a/federation/automatic_registration_choices_policy_test.go b/federation/automatic_registration_choices_policy_test.go new file mode 100644 index 00000000..1f3f9945 --- /dev/null +++ b/federation/automatic_registration_choices_policy_test.go @@ -0,0 +1,180 @@ +package federation_test + +import ( + "context" + "slices" + "strings" + "testing" + + fapi "github.com/idfoundry/fapigo" + "github.com/idfoundry/fapigo/federation" + "github.com/idfoundry/fapigo/storage" +) + +// resolveUnderPolicy registers the fixture RP publishing members (plus +// its jwks) under policy, a Trust Anchor's openid_relying_party +// metadata_policy, with ES256 and PS256 as the OP's default assertion +// algorithms. +func resolveUnderPolicy(t *testing.T, policy string, members map[string]any) (storage.RegisteredClient, error) { + t.Helper() + f := setupAutomaticRegistrationFixtureWithPolicy(t, rpMetadataMembers(t, members), + mustPolicy(t, `{"openid_relying_party":`+policy+`}`)) + cfg := validAutomaticRegistrationConfig() + cfg.ClientAssertionAlgorithms = []fapi.SignatureAlgorithm{fapi.ES256, fapi.PS256} + repo, err := federation.NewAutomaticClientRepository(alwaysFailsRepository{}, f.newResolver(t), f.fetcher, cfg, fixedClock{now: f.now}) + if err != nil { + t.Fatalf("NewAutomaticClientRepository: %v", err) + } + return repo.ResolveClient(context.Background(), fapi.ClientID(f.rpID)) +} + +// TestAutomaticRegistrationHoldsAlgorithmChoicesToPolicy: a superior's +// one_of (or value) on token_endpoint_auth_signing_alg bounds the +// algorithms the RP registers however it declares them — with the +// singular parameter, its RP Metadata Choices plural sibling, or neither +// (the OP's defaults) — since the policy itself checks only the singular +// parameter, and only when it is present. +func TestAutomaticRegistrationHoldsAlgorithmChoicesToPolicy(t *testing.T) { + base := map[string]any{"redirect_uris": []string{"https://rp.example.org/cb"}, "token_endpoint_auth_method": "private_key_jwt"} + with := func(extra map[string]any) map[string]any { + m := map[string]any{} + for k, v := range base { + m[k] = v + } + for k, v := range extra { + m[k] = v + } + return m + } + oneOfPS256 := `{"token_endpoint_auth_signing_alg":{"one_of":["PS256"]}}` + for _, tc := range []struct { + name string + policy string + members map[string]any + want []fapi.SignatureAlgorithm // nil: refused + }{ + {"plural outside one_of", oneOfPS256, with(map[string]any{"token_endpoint_auth_signing_alg_values_supported": []string{"ES256"}}), nil}, + {"plural narrowed to one_of", oneOfPS256, with(map[string]any{"token_endpoint_auth_signing_alg_values_supported": []string{"ES256", "PS256"}}), []fapi.SignatureAlgorithm{fapi.PS256}}, + {"defaults narrowed to one_of", oneOfPS256, base, []fapi.SignatureAlgorithm{fapi.PS256}}, + {"singular within one_of", oneOfPS256, with(map[string]any{"token_endpoint_auth_signing_alg": "PS256"}), []fapi.SignatureAlgorithm{fapi.PS256}}, + {"plural outside value", `{"token_endpoint_auth_signing_alg":{"value":"PS256"}}`, with(map[string]any{"token_endpoint_auth_signing_alg_values_supported": []string{"ES256"}}), nil}, + {"defaults outside one_of", `{"token_endpoint_auth_signing_alg":{"one_of":["RS256"]}}`, base, nil}, + // subset_of on the plural can leave it empty: that declares no + // algorithm, never "none declared, use the defaults". + {"plural emptied by subset_of", `{"token_endpoint_auth_signing_alg_values_supported":{"subset_of":["RS256"]}}`, with(map[string]any{"token_endpoint_auth_signing_alg_values_supported": []string{"ES256"}}), nil}, + {"no policy", `{}`, with(map[string]any{"token_endpoint_auth_signing_alg_values_supported": []string{"ES256"}}), []fapi.SignatureAlgorithm{fapi.ES256}}, + } { + t.Run(tc.name, func(t *testing.T) { + client, err := resolveUnderPolicy(t, tc.policy, tc.members) + if tc.want == nil { + if err == nil { + t.Fatalf("ResolveClient registered %v, want the registration refused", client.ClientAssertionAlgorithms()) + } + return + } + if err != nil { + t.Fatalf("ResolveClient: %v", err) + } + if got := client.ClientAssertionAlgorithms(); !slices.Equal(got, tc.want) { + t.Errorf("ClientAssertionAlgorithms() = %v, want %v", got, tc.want) + } + }) + } +} + +// TestAutomaticRegistrationHoldsMethodChoicesToPolicy: likewise for +// token_endpoint_auth_method and token_endpoint_auth_methods_supported. +func TestAutomaticRegistrationHoldsMethodChoicesToPolicy(t *testing.T) { + members := func(extra map[string]any) map[string]any { + m := map[string]any{ + "redirect_uris": []string{"https://rp.example.org/cb"}, + "token_endpoint_auth_signing_alg": "ES256", + "tls_client_auth_subject_dn": "CN=rp.example.org", + } + for k, v := range extra { + m[k] = v + } + return m + } + both := []string{"tls_client_auth", "private_key_jwt"} + for _, tc := range []struct { + name string + policy string + members map[string]any + want []storage.ClientAuthMethod // nil: refused + }{ + {"plural narrowed to one_of", `{"token_endpoint_auth_method":{"one_of":["private_key_jwt"]}}`, + members(map[string]any{"token_endpoint_auth_methods_supported": both}), + []storage.ClientAuthMethod{storage.ClientAuthMethodPrivateKeyJWT}}, + {"plural outside one_of", `{"token_endpoint_auth_method":{"one_of":["private_key_jwt"]}}`, + members(map[string]any{"token_endpoint_auth_methods_supported": []string{"tls_client_auth"}}), nil}, + {"plural narrowed to value", `{"token_endpoint_auth_method":{"value":"private_key_jwt"}}`, + members(map[string]any{"token_endpoint_auth_methods_supported": both}), + []storage.ClientAuthMethod{storage.ClientAuthMethodPrivateKeyJWT}}, + // §12.1.4: with no policy on the method, every mutually supported + // one is registered. + {"no policy", `{}`, + members(map[string]any{"token_endpoint_auth_methods_supported": both}), + []storage.ClientAuthMethod{storage.ClientAuthMethodTLSClientAuth, storage.ClientAuthMethodPrivateKeyJWT}}, + } { + t.Run(tc.name, func(t *testing.T) { + client, err := resolveUnderPolicy(t, tc.policy, tc.members) + if tc.want == nil { + if err == nil { + t.Fatalf("ResolveClient registered %v, want the registration refused", client.ClientAuthMethods()) + } + return + } + if err != nil { + t.Fatalf("ResolveClient: %v", err) + } + if got := client.ClientAuthMethods(); !slices.Equal(got, tc.want) { + t.Errorf("ClientAuthMethods() = %v, want %v", got, tc.want) + } + }) + } +} + +// TestAutomaticRegistrationRefusesEmptyStringParameters: an empty +// string is present to a metadata policy, so it meets essential, but +// would read as left out; every string parameter registration reads +// refuses it. null, which a policy treats as absent, still reads as +// left out. +func TestAutomaticRegistrationRefusesEmptyStringParameters(t *testing.T) { + base := map[string]any{ + "redirect_uris": []string{"https://rp.example.org/cb"}, + "token_endpoint_auth_method": "private_key_jwt", + "token_endpoint_auth_signing_alg": "ES256", + } + with := func(name string, value any) map[string]any { + m := map[string]any{name: value} + for k, v := range base { + if k != name { + m[k] = v + } + } + return m + } + for _, name := range []string{"subject_type", "client_name", "logo_uri", "request_object_signing_alg", "token_endpoint_auth_signing_alg", "backchannel_token_delivery_mode"} { + t.Run(name, func(t *testing.T) { + _, err := resolveUnderPolicy(t, `{"`+name+`":{"essential":true}}`, with(name, "")) + if err == nil || !strings.Contains(err.Error(), name+" must not be empty") { + t.Errorf("ResolveClient(%s \"\") = %v, want an empty-value error", name, err) + } + }) + } + t.Run("null subject_type", func(t *testing.T) { + client, err := resolveUnderPolicy(t, `{}`, with("subject_type", nil)) + if err != nil { + t.Fatalf("ResolveClient: %v", err) + } + if client.SubjectType() != fapi.SubjectTypePublic { + t.Errorf("SubjectType() = %v, want the default", client.SubjectType()) + } + }) + t.Run("null subject_type under essential", func(t *testing.T) { + if _, err := resolveUnderPolicy(t, `{"subject_type":{"essential":true}}`, with("subject_type", nil)); err == nil { + t.Error("ResolveClient = nil error, want the essential policy unmet") + } + }) +} diff --git a/federation/automatic_registration_internal_test.go b/federation/automatic_registration_internal_test.go index 17134ad7..7fe718cb 100644 --- a/federation/automatic_registration_internal_test.go +++ b/federation/automatic_registration_internal_test.go @@ -53,7 +53,7 @@ func newTestAutomaticClientRepositoryWithFetcher(t *testing.T, automaticCfg Auto func call(t *testing.T, repo *AutomaticClientRepository, raw string) (storage.RegisteredClientConfig, json.RawMessage, error) { t.Helper() - return repo.registeredClientConfigFromMetadata(context.Background(), "https://rp.example.org", json.RawMessage(raw)) + return repo.registeredClientConfigFromMetadata(context.Background(), "https://rp.example.org", json.RawMessage(raw), choiceLimits{}) } // testSelfSignedCert generates a fresh, real self-signed X.509 diff --git a/federation/automatic_registration_test.go b/federation/automatic_registration_test.go index d89ca305..68888949 100644 --- a/federation/automatic_registration_test.go +++ b/federation/automatic_registration_test.go @@ -664,37 +664,149 @@ func TestAutomaticClientRepositoryResolveClientFailsOnJWKSURIFetchError(t *testi } } -func TestAutomaticClientKeySourcePrefersUnderlying(t *testing.T) { - f := setupAutomaticRegistrationFixture(t, rpMetadataBuilder(t)) - resolver := f.newResolver(t) - repo, err := federation.NewAutomaticClientRepository(alwaysFailsRepository{}, resolver, f.fetcher, validAutomaticRegistrationConfig(), fixedClock{now: f.now}) +// TestAutomaticClientKeySourceStaticClientUsesUnderlyingOnly: a client +// the underlying repository registers statically gets its keys from the +// underlying key source alone, even when its client_id is also an Entity +// Identifier resolving under a configured Trust Anchor. An empty set or +// an error from the underlying source is the answer, not a cue to try +// the key its federation metadata publishes (rp-oidc), which a superior +// in its Trust Chain can set; and federation isn't consulted at all. +func TestAutomaticClientKeySourceStaticClientUsesUnderlyingOnly(t *testing.T) { + staticKey := generateKey(t) + staticKeys := keys.VerificationKeySet{Keys: []keys.VerificationKey{ + {KeyID: "static-kid", Algorithm: fapi.ES256, PublicKey: &staticKey.PublicKey}, + }} + for _, tc := range []struct { + name string + underlying keys.ClientKeySource + kid string + wantKID string + wantErr bool + }{ + {name: "static key", kid: "static-kid", wantKID: "static-kid"}, + {name: "any key", wantKID: "static-kid"}, + {name: "federation-published key", kid: "rp-oidc"}, + {name: "underlying error", underlying: alwaysFailsKeySource{}, kid: "rp-oidc", wantErr: true}, + } { + t.Run(tc.name, func(t *testing.T) { + f := setupAutomaticRegistrationFixture(t, rpMetadataBuilder(t)) + underlying := tc.underlying + if underlying == nil { + underlying = staticKeySource{id: fapi.ClientID(f.rpID), keySet: staticKeys} + } + src := newStaticClientKeySource(t, f, underlying) + + set, err := src.ResolveVerificationKeys(context.Background(), keys.ClientKeyRequest{ + ClientID: fapi.ClientID(f.rpID), Purpose: keys.ClientAssertionVerification, Algorithm: fapi.ES256, KeyID: tc.kid, + }) + if (err != nil) != tc.wantErr { + t.Fatalf("ResolveVerificationKeys error = %v, want error %v", err, tc.wantErr) + } + checkOnlyStaticKeys(t, set, tc.wantKID) + if n := atomic.LoadInt32(f.rpConfigCalls); n != 0 { + t.Errorf("RP's own well-known endpoint was fetched %d times, want 0 for a statically registered client", n) + } + }) + } +} + +// newStaticClientKeySource returns an AutomaticClientKeySource over +// underlying, whose client repository registers the fixture RP's +// Entity Identifier statically. +func newStaticClientKeySource(t *testing.T, f *automaticRegistrationFixture, underlying keys.ClientKeySource) *federation.AutomaticClientKeySource { + t.Helper() + staticClient, err := storage.NewRegisteredClient(storage.RegisteredClientConfig{ + ID: fapi.ClientID(f.rpID), RedirectURIs: []fapi.RegisteredRedirectURI{fapi.RegisteredRedirectURI(f.rpID + "/static-cb")}, + ClientAuthMethod: storage.ClientAuthMethodPrivateKeyJWT, ClientAssertionAlgorithm: fapi.ES256, + SenderConstrain: storage.SenderConstrainDPoP, AllowedScopes: []string{"openid"}, + }) + if err != nil { + t.Fatalf("storage.NewRegisteredClient: %v", err) + } + repo, err := federation.NewAutomaticClientRepository( + staticRepository{id: fapi.ClientID(f.rpID), client: staticClient}, f.newResolver(t), f.fetcher, + validAutomaticRegistrationConfig(), fixedClock{now: f.now}) if err != nil { t.Fatalf("NewAutomaticClientRepository: %v", err) } + src, err := federation.NewAutomaticClientKeySource(underlying, repo) + if err != nil { + t.Fatalf("NewAutomaticClientKeySource: %v", err) + } + return src +} + +// checkOnlyStaticKeys fails t if set holds any key but the static +// client's own ("static-kid"), or, when wantKID is set, doesn't start +// with that key. staticKeySource returns its whole set whatever kid is +// asked for; what matters is that no federation-published key is in it. +func checkOnlyStaticKeys(t *testing.T, set keys.VerificationKeySet, wantKID string) { + t.Helper() + for _, k := range set.Keys { + if k.KeyID != "static-kid" { + t.Errorf("ResolveVerificationKeys returned key %q, want only the static client's own keys", k.KeyID) + } + } + if wantKID != "" && (len(set.Keys) == 0 || set.Keys[0].KeyID != wantKID) { + t.Errorf("ResolveVerificationKeys = %+v, want %q", set, wantKID) + } +} - underlyingKey := generateKey(t) +// TestAutomaticClientKeySourceFederationClientUsesFederationKeysOnly: a +// client the underlying repository doesn't register is an automatically +// registered one, and its keys come from its federation metadata — not +// from the underlying key source, even one that answers for its +// client_id. +func TestAutomaticClientKeySourceFederationClientUsesFederationKeysOnly(t *testing.T) { + f := setupAutomaticRegistrationFixture(t, rpMetadataBuilder(t)) + repo, err := federation.NewAutomaticClientRepository(alwaysFailsRepository{}, f.newResolver(t), f.fetcher, validAutomaticRegistrationConfig(), fixedClock{now: f.now}) + if err != nil { + t.Fatalf("NewAutomaticClientRepository: %v", err) + } + otherKey := generateKey(t) underlying := staticKeySource{ id: fapi.ClientID(f.rpID), keySet: keys.VerificationKeySet{Keys: []keys.VerificationKey{ - {KeyID: "static-kid", Algorithm: fapi.ES256, PublicKey: &underlyingKey.PublicKey}, + {KeyID: "static-kid", Algorithm: fapi.ES256, PublicKey: &otherKey.PublicKey}, }}, } src, err := federation.NewAutomaticClientKeySource(underlying, repo) if err != nil { t.Fatalf("NewAutomaticClientKeySource: %v", err) } - set, err := src.ResolveVerificationKeys(context.Background(), keys.ClientKeyRequest{ ClientID: fapi.ClientID(f.rpID), Purpose: keys.ClientAssertionVerification, Algorithm: fapi.ES256, }) if err != nil { t.Fatalf("ResolveVerificationKeys: %v", err) } - if len(set.Keys) != 1 || set.Keys[0].KeyID != "static-kid" { - t.Errorf("ResolveVerificationKeys = %+v, want the underlying static key, unshadowed by federation resolution", set) + if len(set.Keys) != 1 || set.Keys[0].KeyID != "rp-oidc" { + t.Errorf("ResolveVerificationKeys = %+v, want only the federation-published rp-oidc key", set) } - if atomic.LoadInt32(f.rpConfigCalls) != 0 { - t.Errorf("RP's own well-known endpoint was fetched, want underlying to have short-circuited federation resolution entirely") +} + +// TestAutomaticClientKeySourceDoesNotMaskAnUnavailableStore: when the +// client repository can't answer, whether the client is statically +// registered is unknown, so its error is returned rather than either +// source's keys. +func TestAutomaticClientKeySourceDoesNotMaskAnUnavailableStore(t *testing.T) { + f := setupAutomaticRegistrationFixture(t, rpMetadataBuilder(t)) + repo, err := federation.NewAutomaticClientRepository(unavailableRepository{}, f.newResolver(t), f.fetcher, validAutomaticRegistrationConfig(), fixedClock{now: f.now}) + if err != nil { + t.Fatalf("NewAutomaticClientRepository: %v", err) + } + src, err := federation.NewAutomaticClientKeySource(alwaysFailsKeySource{}, repo) + if err != nil { + t.Fatalf("NewAutomaticClientKeySource: %v", err) + } + _, err = src.ResolveVerificationKeys(context.Background(), keys.ClientKeyRequest{ + ClientID: fapi.ClientID(f.rpID), Purpose: keys.ClientAssertionVerification, Algorithm: fapi.ES256, + }) + if !errors.Is(err, storage.ErrStoreUnavailable) { + t.Errorf("ResolveVerificationKeys error = %v, want storage.ErrStoreUnavailable", err) + } + if n := atomic.LoadInt32(f.rpConfigCalls); n != 0 { + t.Errorf("RP's own well-known endpoint was fetched %d times, want 0", n) } } diff --git a/federation/federation_fuzz_test.go b/federation/federation_fuzz_test.go index 19b31380..fd308add 100644 --- a/federation/federation_fuzz_test.go +++ b/federation/federation_fuzz_test.go @@ -60,7 +60,7 @@ func FuzzRegisteredClientConfigFromMetadata(f *testing.F) { cfg := fuzzAutomaticRegistrationConfig(flags) repo := &AutomaticClientRepository{cfg: cfg} - got, jwks, err := repo.registeredClientConfigFromMetadata(context.Background(), id, json.RawMessage(raw)) + got, jwks, err := repo.registeredClientConfigFromMetadata(context.Background(), id, json.RawMessage(raw), choiceLimits{}) if err == nil { checkNoCaseVariantMember(t, members, names) @@ -359,14 +359,22 @@ func FuzzEntityIDHostAgreement(f *testing.F) { if wk.Hostname() != host { t.Fatalf("%q: constraints see host %q, the fetch targets %q", id, host, wk.Hostname()) } - again, err := url.Parse(wk.String()) - if err != nil || again.Hostname() != host || again.Scheme != "https" || again.User != nil { - t.Fatalf("%q: .well-known URL %q re-parses to host %q (err %v), not %q", id, wk.String(), again.Hostname(), err, host) - } - for i := 0; i < len(host); i++ { - if host[i] >= 0x80 { - t.Fatalf("%q: non-ASCII host %q accepted", id, host) - } - } + checkWellKnownReparse(t, id, wk, host) }) } + +// checkWellKnownReparse fails t unless the serialized .well-known URL wk +// for id parses again to the same https host, with no userinfo, and that +// host is ASCII. +func checkWellKnownReparse(t *testing.T, id string, wk *url.URL, host string) { + t.Helper() + again, err := url.Parse(wk.String()) + if err != nil || again.Hostname() != host || again.Scheme != "https" || again.User != nil { + t.Fatalf("%q: .well-known URL %q re-parses to host %q (err %v), not %q", id, wk.String(), again.Hostname(), err, host) + } + for i := 0; i < len(host); i++ { + if host[i] >= 0x80 { + t.Fatalf("%q: non-ASCII host %q accepted", id, host) + } + } +} diff --git a/federation/resolver.go b/federation/resolver.go index 08c10629..9b6dc375 100644 --- a/federation/resolver.go +++ b/federation/resolver.go @@ -120,6 +120,16 @@ type Dependencies struct { // rebinding) can point at an internal host: and the entities a // resolver fetches are named by the Trust Chain it's resolving, so // by whoever controls the subject. + // + // Those fetches happen before anything is trusted: Resolve fetches + // each authority hint's Entity Configuration, and then the + // Subordinate Statement from the federation_fetch_endpoint that + // configuration names, before any signature on the path has been + // checked against a Trust Anchor (see Resolve). So whoever controls + // the subject chooses those URLs — host, port, path and query, with + // only "sub" added — and HTTP gets a GET to each, whatever the chain + // later turns out to be. Every host HTTP may reach, including any in + // fapihttp.Config.AllowedPrivateHosts, is reachable that way. HTTP *fapihttp.Client Clock Clock @@ -247,6 +257,14 @@ type ResolvedEntity struct { // (OpenID Federation 1.0 §8.3.2); most callers have no reason to // read this directly. Tokens []string + + // policy is the metadata policy Metadata was resolved with: every + // superior's, merged (§6.1.4.1). Automatic registration reads it to + // hold the values it takes from a parameter's RP Metadata Choices + // sibling to the policy on the parameter itself (see + // intfed.PermittedStrings). Nil for a Trust Anchor resolved as + // itself, whose metadata no policy applies to. + policy MetadataPolicy } // chainWalkState is Resolve's own mutable state as it walks upward from @@ -343,6 +361,18 @@ func newChainWalkState(subjectID string, leafStmt intfed.Statement, leafToken st // for VerifyTrustMark; issuing statements and serving federation // endpoints is SelfIssuer's and SubordinateIssuer's job, not a // Resolver's. See doc.go for the package's scope. +// +// Trust is established from the top down, but the chain is discovered +// from the bottom up: the subject's Entity Configuration names its +// authority hints, each hint's own Entity Configuration names its +// federation_fetch_endpoint, and the Subordinate Statement fetched from +// there names the next keys — all fetched before the Trust Anchor's key +// verifies any of them. Until then they are claims of whoever controls +// the subject, so the URLs Resolve fetches, up to Limits.MaxPathLength × +// Limits.MaxAuthorityHints superiors' worth, are that party's choice +// (see Dependencies.HTTP), and a caller that resolves an Entity +// Identifier an unauthenticated party names (as automatic registration +// does with a client_id) lets that party send them. func (r *Resolver) Resolve(ctx context.Context, subjectID string) (ResolvedEntity, error) { if subjectID == "" { return ResolvedEntity{}, fmt.Errorf("federation: subject entity ID is empty") @@ -711,7 +741,7 @@ func (r *Resolver) finalizeTrustChain(st *chainWalkState, hop int, sup hopSuperi return ResolvedEntity{}, fmt.Errorf("federation: metadata %q's Immediate Superior sets for it: %w", st.subjectID, err) } leafMetadata := filterAllowedEntityTypes(st.subordinateConstraints, declared) - resolvedMetadata, err := r.resolveMetadata(st.subordinatePolicies, leafMetadata) + resolvedMetadata, policy, err := r.resolveMetadata(st.subordinatePolicies, leafMetadata) if err != nil { return ResolvedEntity{}, err } @@ -723,6 +753,7 @@ func (r *Resolver) finalizeTrustChain(st *chainWalkState, hop int, sup hopSuperi JWKS: st.subjectJWKS, TrustMarks: st.leafClaims.TrustMarks, TrustMarkOwners: st.leafClaims.TrustMarkOwners, TrustMarkIssuers: st.leafClaims.TrustMarkIssuers, Tokens: append(st.tokens, sup.token), + policy: policy, }, nil } @@ -859,16 +890,18 @@ func mergeCrit(a, b []string) []string { return out } -func (r *Resolver) resolveMetadata(policies []subordinatePolicy, leafMetadata map[string]json.RawMessage) (map[string]json.RawMessage, error) { +// resolveMetadata merges policies and applies the result to +// leafMetadata, returning the Resolved Metadata and the merged policy. +func (r *Resolver) resolveMetadata(policies []subordinatePolicy, leafMetadata map[string]json.RawMessage) (map[string]json.RawMessage, MetadataPolicy, error) { merged, crit, err := resolveMetadataPolicies(policies) if err != nil { - return nil, fmt.Errorf("federation: resolve metadata policy: %w", err) + return nil, nil, fmt.Errorf("federation: resolve metadata policy: %w", err) } resolved, err := intfed.ApplyPolicy(merged, crit, leafMetadata) if err != nil { - return nil, fmt.Errorf("federation: apply metadata policy: %w", err) + return nil, nil, fmt.Errorf("federation: apply metadata policy: %w", err) } - return resolved, nil + return resolved, merged, nil } // verifySelfSigned verifies stmt (expected to be entityID's own Entity diff --git a/internal/federation/policy.go b/internal/federation/policy.go index be52d6d0..e8b2a5a0 100644 --- a/internal/federation/policy.go +++ b/internal/federation/policy.go @@ -364,6 +364,48 @@ func isJSONNull(raw json.RawMessage) bool { return json.Unmarshal(raw, &v) == nil && v == nil } +// PermittedStrings is the set of string values ops — the merged policy +// for one single-valued metadata parameter — lets that parameter take: +// the value operator's own value when it sets one, else the one_of +// operator's values. constrained is false when ops has neither (value +// null removes the parameter rather than restricting it), so the +// parameter may take any value; an operand that isn't a string permits +// no string at all. ops must already have passed ValidatePolicy (as a +// policy ApplyPolicy accepted has). +// +// Policy operators act on one parameter only (§6.1.3.1), so a value +// can reach a Relying Party's registration through a parameter the +// policy never names: its OpenID Connect RP Metadata Choices 1.0 §2 +// plural sibling (token_endpoint_auth_signing_alg_values_supported for +// token_endpoint_auth_signing_alg). A caller choosing the effective +// value from that sibling uses this to keep its choice within what the +// policy permits for the singular parameter. +func PermittedStrings(ops PolicyOperators) (permitted []string, constrained bool, err error) { + if raw, ok := ops[opValue]; ok && !isJSONNull(raw) { + var v string + if json.Unmarshal(raw, &v) != nil { + return nil, true, nil + } + return []string{v}, true, nil + } + raw, ok := ops[opOneOf] + if !ok { + return nil, false, nil + } + options, err := decodeArray(raw) + if err != nil { + return nil, false, err + } + permitted = []string{} + for _, option := range options { + var v string + if json.Unmarshal(option, &v) == nil { + permitted = append(permitted, v) + } + } + return permitted, true, nil +} + // ValidatePolicy checks every metadata parameter policy in policy with // validateCombination. func ValidatePolicy(policy MetadataPolicy, crit []string) error { diff --git a/internal/federation/policy_permitted_test.go b/internal/federation/policy_permitted_test.go new file mode 100644 index 00000000..e01f1e20 --- /dev/null +++ b/internal/federation/policy_permitted_test.go @@ -0,0 +1,35 @@ +package federation + +import ( + "encoding/json" + "slices" + "testing" +) + +// TestPermittedStrings pins which policy shapes restrict a +// single-valued parameter's values, and to what. +func TestPermittedStrings(t *testing.T) { + for _, tc := range []struct { + ops string + want []string + wantConstrained bool + }{ + {`{}`, nil, false}, + {`{"default":"PS256"}`, nil, false}, + {`{"essential":true}`, nil, false}, + {`{"value":null}`, nil, false}, + {`{"value":"PS256"}`, []string{"PS256"}, true}, + {`{"value":"PS256","one_of":["PS256","ES256"]}`, []string{"PS256"}, true}, + {`{"one_of":["PS256","ES256"]}`, []string{"PS256", "ES256"}, true}, + {`{"value":1}`, nil, true}, + } { + var ops PolicyOperators + if err := json.Unmarshal([]byte(tc.ops), &ops); err != nil { + t.Fatal(err) + } + got, constrained, err := PermittedStrings(ops) + if err != nil || constrained != tc.wantConstrained || !slices.Equal(got, tc.want) { + t.Errorf("PermittedStrings(%s) = %v, %v, %v; want %v, %v", tc.ops, got, constrained, err, tc.want, tc.wantConstrained) + } + } +} diff --git a/server/automatic_registration_static_keys_test.go b/server/automatic_registration_static_keys_test.go new file mode 100644 index 00000000..6d047571 --- /dev/null +++ b/server/automatic_registration_static_keys_test.go @@ -0,0 +1,77 @@ +package server_test + +import ( + "context" + "crypto/ecdsa" + "testing" + "time" + + fapi "github.com/idfoundry/fapigo" + "github.com/idfoundry/fapigo/internal/clientassertion" + "github.com/idfoundry/fapigo/keys/ephemeral" + "github.com/idfoundry/fapigo/server" + "github.com/idfoundry/fapigo/storage" +) + +// TestAutomaticRegistrationStaticClientRefusesFederationKey: a +// statically registered client whose client_id is also an Entity +// Identifier resolving under one of the server's Automatic Registration +// Trust Anchors authenticates with its static keys only. Its static +// registration grants client_credentials and a scope automatic +// registration doesn't; a client assertion signed with the key its +// federation metadata publishes (rp-oidc — a key any superior in its +// Trust Chain can set) must not authenticate as it. +func TestAutomaticRegistrationStaticClientRefusesFederationKey(t *testing.T) { + f := setupAutomaticRegistrationFixture(t, testRedirectURI) + + staticKey := generateKey(t) + staticClient, err := storage.NewRegisteredClient(storage.RegisteredClientConfig{ + ID: fapi.ClientID(f.rpID), + ClientAuthMethod: storage.ClientAuthMethodPrivateKeyJWT, + ClientAssertionAlgorithm: fapi.ES256, + AllowedScopes: []string{"accounts", "payments:admin"}, + AllowsClientCredentialsGrant: true, + }) + if err != nil { + t.Fatalf("NewRegisteredClient: %v", err) + } + staticKeys, err := ephemeral.NewClientKeySource(nil, []ephemeral.ClientKeySpec{{ClientID: fapi.ClientID(f.rpID), JWKS: fedJWKS(t, "static-kid", staticKey)}}) + if err != nil { + t.Fatalf("ephemeral.NewClientKeySource: %v", err) + } + + srv := newAutomaticRegistrationTestServer(t, f, func(cfg *server.Config, deps *server.Dependencies) { + cfg.ClientCredentialsGrant = true + cfg.AutomaticRegistration.AllowsClientCredentialsGrant = false + deps.Clients = &fakeClientRepository{clients: map[fapi.ClientID]storage.RegisteredClient{fapi.ClientID(f.rpID): staticClient}} + deps.ClientKeys = staticKeys + }) + + request := func(signer *ecdsa.PrivateKey, kid string) error { + t.Helper() + assertion, err := clientassertion.CreateAssertion(clientassertion.AssertionRequest{ + Signer: signer, Algorithm: fapi.ES256, KeyID: kid, + ClientID: f.rpID, Audience: testIssuer, Now: f.now, Lifetime: 30 * time.Second, + }) + if err != nil { + t.Fatalf("CreateAssertion: %v", err) + } + _, err = srv.RequestClientCredentialsToken(context.Background(), server.ClientCredentialsTokenRequest{ + HTTP: server.FormRequest{Parameters: []server.FormParameter{ + formParam("grant_type", "client_credentials"), + formParam("client_assertion", assertion), + formParam("client_assertion_type", clientassertion.AssertionType), + formParam("scope", "payments:admin"), + }}, + DPoPProofs: []string{createDPoPProof(t, generateKey(t), f.now)}, + }) + return err + } + + if err := request(staticKey, "static-kid"); err != nil { + t.Fatalf("assertion signed with the static key: %v, want a token", err) + } + if err := request(f.rpOIDCKey, "rp-oidc"); err == nil { + t.Fatalf("assertion signed with the federation-published key authenticated as the statically registered client, want invalid_client") + } +} diff --git a/server/dependencies.go b/server/dependencies.go index 2a9df97f..36f95920 100644 --- a/server/dependencies.go +++ b/server/dependencies.go @@ -185,7 +185,12 @@ type Dependencies struct { // holds under DNS rebinding. Under AssuranceProduction, New refuses a // fetcher that isn't fapihttp.Client.HardenedForProduction (one over // any other HTTPClient, a transport changed since NewClient built it, - // or a loopback exception on either). + // or a loopback exception on either). Resolution runs for a client_id + // before the client is authenticated, and fetches URLs the named + // Entity Identifier's statements choose before any is trusted (see + // federation.Resolver.Resolve), so every host this fetcher can reach, + // any in its fapihttp.Config.AllowedPrivateHosts included, can be + // sent a GET by an unauthenticated caller. FederationHTTP *fapihttp.Client // ClientCertificateTrust decides how chain-of-trust for a client