Repository navigation
fix(federation)!: federation fixes from the 2026-10-18 review - #688
Merged
Merged
Conversation
…red automatic registration Concurrent requests naming one unknown federation client_id share a single Trust Chain resolution, which ran under the first caller's context. Cancelling that one request (an attacker can do so at will) aborted the resolution, and every other request waiting on it failed with the cancellation: a targeted denial of service against one RP. The shared resolution now runs detached from any caller's cancellation (context.WithoutCancel keeps the values), bounded by the fetcher's own limits. Each caller, the first included, waits for the result or for its own context, and gets its own context's error if that ends first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…l policy A superior can mark a Relying Party's metadata parameter "essential" (OpenID Federation 1.0 §6.1.3.1.7), for instance to require ID token encryption. A parameter the RP published as null, or as "", counted as present and met the policy, and automatic registration then read both as "no encryption", so the requirement was bypassed. Applying a policy now treats a null value as absent, so it doesn't meet essential and is left out of the resolved metadata. Automatic registration refuses an id_token_ or userinfo_encrypted_response_alg or _enc that is present but empty, instead of reading it as unset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Config.Federation says the federation entity signing key is never reused as a protocol key, but nothing checked it. The entity key signs the Entity Configuration, which vouches for the server's protocol keys; one of those reused for it lets anything that key signs (an ID token, a JARM response) be presented as a statement of the entity itself. server.New now compares the keys.FederationEntitySigning key with every signing key the server publishes (each one a rotating key manager reports, too), and refuses a match. UPGRADING.md has a section. BREAKING CHANGE: with Config.Federation.EntityID set, server.New refuses a Dependencies.Keys whose federation entity signing key is also one of the server's protocol signing keys. Give keys.FederationEntitySigning its own key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t's jwks A federation statement's jwks claim, and a trust mark owner's, are published as given. One built from a private JWK (a key manager's export, a test fixture copied into production) published the private key in the signed Entity Configuration or Subordinate Statement for anyone to fetch. Building a statement now refuses a key carrying any private or secret member (d, p, q, dp, dq, qi, oth, or k) with ErrMalformedJWKS. Parsing other parties' statements is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…esolver doesn't trust ResolveRequest.TrustAnchor's doc says it must be one of the Resolver's own Config.TrustAnchors, but ResolveViaEndpoint never checked: it asked the resolve endpoint to resolve under any Trust Anchor named, and a caller could take the answer as resolved under one it trusts. ResolveViaEndpoint now refuses an unconfigured Trust Anchor before fetching anything. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…may not OpenID Federation 1.0 §3.2 has a validator refuse an Entity Statement carrying a claim its kind may not (steps 14 to 23): "If the metadata_policy Claim is present, the Entity Statement MUST be a Subordinate Statement", and likewise for metadata_policy_crit, constraints and source_endpoint, while authority_hints, trust_marks, trust_mark_issuers and trust_mark_owners belong only in an Entity Configuration. Create already refused to build such a statement, but Parse accepted one from another party, so a superior's Entity Configuration carrying metadata_policy, say, was read as given. Parse now refuses it with ErrMalformedClaims. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… validate it against OpenID Federation 1.0 §7.3 step 9: "If there is a delegation Claim in the instance, the value of that Claim MUST be validated", and §7.2.2 step 9 takes the Trust Mark Owner's keys from the Trust Anchor's trust_mark_owners. VerifyTrustMark validated a delegation only when the mark's type had an owner there, and ignored one otherwise, even a malformed one. A mark carrying a delegation for a type with no owner is now refused, since there is nothing to validate it against. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…ad keys from the manager directly checkFederationKeyDistinct read each key set back through a JWK Set's JSON, whose error branches couldn't be reached: checkSigningKeys has already resolved every one of those keys. It now reads them from the key manager (each key a keys.RotatingKeyManager still publishes, or the one PublicKey returns). A test covers a rotation whose outgoing protocol key is the federation key, and the trust mark owner JWKS cases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the federation findings from the 2026-10-18 review (batch G): L2, L3, L9 and L15, plus the Trust Anchor check, trust-mark delegation, and claim placement on parse. There's one commit per finding, and only L9 is breaking; UPGRADING.md has a section for it under
## v0.58.0.Security
context.WithoutCancel), bounded by the fetcher's own limits, and each caller waits on its own context.nullor""meets anessentialpolicyid_token_encrypted_response_algwas met bynullor"", both read as "no encryption". Applying a policy now treatsnullas absent; automatic registration refuses an encryption alg/enc that is present but empty.[]still meets essential, as §6.1.3.1.8's Table 1 says.server.Newnow refuses a federation entity signing key that equals any published protocol signing key (each one a rotating manager reports).Config.Federationalready said it is never reused.jwks(or a trust mark owner's) carryingd,p,q,dp,dq,qi,othork. Parsing others' statements is unchanged.Compliance
ResolveViaEndpointrefuses aTrustAnchorthat isn't one of the Resolver'sConfig.TrustAnchors, which its doc already required.Parserefuses a statement carrying a claim its kind may not (metadata_policyand the like in an Entity Configuration;authority_hintsand the like in a Subordinate Statement), asCreatealready did.delegationfor a type with no owner intrust_mark_ownersis refused. Before, it was ignored, even when malformed.Testing
context canceled) and L9.go vet,go test ./..., andgo test -raceon Go 1.26.9 for the affected packages;GOOS=linux GOARCH=arm go vet ./...;🤖 Generated with Claude Code