From e1c8f79bdc2e2b56d7ae765875079bde3768dccb Mon Sep 17 00:00:00 2001 From: Oscar Sanderson Date: Wed, 23 Sep 2026 19:29:19 +0800 Subject: [PATCH] fix: accept only the issuer as client assertion aud for non-CIBA clients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FAPI 2.0 Security Profile Final §5.3.2.1 requires the authorization server to "only accept its issuer identifier value ... as a string in the aud claim received in client authentication assertions". The token endpoint also accepted its own URL (and its mTLS alias), per RFC 7523 §3's older allowance. The OIDF suite's new fapi2-security-profile-final-ensure-invalid-client-assertions-fail module now checks this at the token endpoint for the client-credentials variant, failing the scheduled conformance run. Endpoint-URL audiences are now accepted only for clients registered for CIBA: CIBA Core 1.0 §7.1 requires the token and backchannel authentication endpoint URLs to be accepted, and the FAPI-CIBA suite signs every token-endpoint assertion (refresh included) with the token endpoint URL. PAR stays issuer-only for every client. Co-Authored-By: Claude Opus 5.5 --- ARCHITECTURE.md | 15 ++++++ .../client_credentials_smoke_test.go | 7 +-- conformance/server/oidf-config/README.md | 9 +++- server/backchannel_test.go | 35 +++++++++++++- server/backchannel_token.go | 7 ++- server/mtls_test.go | 23 ++++----- server/par.go | 48 +++++++++++-------- server/par_test.go | 32 +++++++------ server/token.go | 9 ++-- 9 files changed, 125 insertions(+), 60 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index d7839af..c0a06c0 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -800,6 +800,21 @@ URL only) briefly regressed the already-passing AS ciba-mtls/ciba-ping legs before this was caught by a second full local conformance run and corrected. Confirmed live: all nine test configurations clean after both fixes. +The token endpoint's own URL-audience allowance was narrowed further +later still, again caught by the daily `conformance.yml` run: FAPI 2.0 +Security Profile Final §5.3.2.1 says the authorization server "shall +only accept its issuer identifier value ... as a string in the aud +claim received in client authentication assertions", overriding RFC +7523 §3's token-endpoint-URL allowance, and the suite's +`ensure-invalid-client-assertions-fail` module began enforcing it at the +token endpoint for the client-credentials variant. Endpoint-URL +audiences are now accepted only for a client registered for CIBA +(`BackchannelAuthenticationRequestAlgorithm` set), which keeps CIBA Core +1.0 §7.1's widened set at the token and backchannel authentication +endpoints — the FAPI-CIBA-ID1 suite signs every token-endpoint +assertion, refresh included, with the token endpoint URL. Every other +client is issuer-only at every endpoint. + `client`'s own CIBA support (`BeginBackchannelAuthentication`/`PollBackchannelAuthentication`) was genuinely attempted against the OIDF suite's RP-side plan diff --git a/cmd/conformance-as/client_credentials_smoke_test.go b/cmd/conformance-as/client_credentials_smoke_test.go index 6a6fffb..e69e195 100644 --- a/cmd/conformance-as/client_credentials_smoke_test.go +++ b/cmd/conformance-as/client_credentials_smoke_test.go @@ -33,6 +33,7 @@ import ( // driving through, unlike TestSmokeAuthorizationCodeFlow/TestSmokeMTLSFlow. type clientCredentialsSmokeServer struct { httpClient *http.Client + issuer fapi.URL endpoints server.Endpoints accountsURL fapi.URL clientAuthPriv *ecdsa.PrivateKey @@ -133,7 +134,7 @@ func newClientCredentialsSmokeServer(t *testing.T) clientCredentialsSmokeServer httpClient := &http.Client{Transport: &http.Transport{TLSClientConfig: &tls.Config{RootCAs: pool}}} return clientCredentialsSmokeServer{ - httpClient: httpClient, endpoints: endpoints, accountsURL: accountsURL, + httpClient: httpClient, issuer: issuer, endpoints: endpoints, accountsURL: accountsURL, clientAuthPriv: clientAuthPriv, dpopPriv: dpopPriv, clientAuthKeyID: clientAuthKeyID, clientID: testClientID, } @@ -154,7 +155,7 @@ func TestSmokeClientCredentialsGrantFlow(t *testing.T) { Algorithm: fapi.ES256, KeyID: s.clientAuthKeyID, ClientID: string(s.clientID), - Audience: s.endpoints.Token.String(), + Audience: s.issuer.String(), // FAPI 2.0 SP Final §5.3.2.1: issuer only Now: now, Lifetime: time.Minute, }) @@ -292,7 +293,7 @@ func TestSmokeClientCredentialsGrantFlowWithAuthorizationDetails(t *testing.T) { Algorithm: fapi.ES256, KeyID: s.clientAuthKeyID, ClientID: string(s.clientID), - Audience: s.endpoints.Token.String(), + Audience: s.issuer.String(), // FAPI 2.0 SP Final §5.3.2.1: issuer only Now: now, Lifetime: time.Minute, }) diff --git a/conformance/server/oidf-config/README.md b/conformance/server/oidf-config/README.md index ab3a655..170d084 100644 --- a/conformance/server/oidf-config/README.md +++ b/conformance/server/oidf-config/README.md @@ -544,7 +544,14 @@ surfaced seven real, since-fixed library/harness gaps: client-authenticated endpoint URL this server exposes (Token, PAR, BackchannelAuthentication), plus their mTLS aliases for an mTLS-bound client. This is not mTLS-specific — a DPoP-bound client - gets the same widened set now too (`TestPushAuthorizationRequestAcceptsTokenEndpointURLAsClientAssertionAudience`). + gets the same widened set now too. **Since narrowed twice:** PAR + became issuer-only (FAPI2's `par-test-{par,token}-endpoint-url-as-audience-fails`), + then FAPI 2.0 SP Final §5.3.2.1's issuer-only rule was applied at + every endpoint for non-CIBA clients (FAPI2's + `ensure-invalid-client-assertions-fail`, client-credentials variant). + The widened set above now applies only to CIBA-registered clients — + which is all this CIBA plan's clients are, so this plan is + unaffected. See `acceptableClientAssertionAudiences`' doc comment. - **Fixed: FAPI-RW-8.5-1/8.5-2 ("Server accepted a cipher that is not on the list of permitted ciphers") — two distinct causes, both resolved.** First cause: `cmd/conformance-as/main.go`'s own cipher diff --git a/server/backchannel_test.go b/server/backchannel_test.go index 6013983..aea6176 100644 --- a/server/backchannel_test.go +++ b/server/backchannel_test.go @@ -584,7 +584,7 @@ func TestBeginBackchannelAuthenticationSuccess(t *testing.T) { } // TestBeginBackchannelAuthenticationAcceptsBackchannelAuthenticationEndpointURLAsClientAssertionAudience -// covers RFC 7523 §3's URL-audience carve-out as scoped to the +// covers CIBA Core 1.0 §7.1's URL-audience widening as scoped to the // backchannel authentication endpoint specifically — confirmed live // against the OIDF conformance suite's own CIBA client, which signs // "aud" as whichever endpoint URL it is actually calling. See @@ -1583,6 +1583,39 @@ func TestExchangeBackchannelAuthenticationBeforeDecisionIsPending(t *testing.T) } } +// TestExchangeBackchannelAuthenticationAcceptsTokenEndpointURLAsClientAssertionAudience +// confirms a CIBA-registered client keeps CIBA Core 1.0 §7.1's widened +// audiences at the token endpoint, even though FAPI 2.0 Final makes +// every other client issuer-only there: the OIDF FAPI-CIBA-ID1 suite +// signs every token-endpoint assertion (polling and refresh alike) with +// the token endpoint URL. Reaching authorization_pending proves client +// authentication succeeded. +func TestExchangeBackchannelAuthenticationAcceptsTokenEndpointURLAsClientAssertionAudience(t *testing.T) { + h, _ := newHarnessWithBackchannel(t) + required := beginBackchannel(t, h, standardBackchannelParams(t)) + assertion, err := clientassertion.CreateAssertion(clientassertion.AssertionRequest{ + Signer: h.key, Algorithm: fapi.ES256, + ClientID: testClientID.String(), Audience: testTokenEndpoint, + Now: h.now, Lifetime: 30 * time.Second, + }) + if err != nil { + t.Fatalf("CreateAssertion: %v", err) + } + + _, err = h.server.ExchangeBackchannelAuthentication(context.Background(), server.BackchannelTokenExchangeRequest{ + HTTP: server.FormRequest{Parameters: []server.FormParameter{ + formParam("client_assertion", assertion), + formParam("client_assertion_type", clientassertion.AssertionType), + formParam("grant_type", server.CIBAGrantType), + formParam("auth_req_id", required.AuthReqID.String()), + }}, + DPoPProofs: []string{createDPoPProof(t, generateKey(t), h.now)}, + }) + if code := serverErrorCode(t, err); code != server.ErrorAuthorizationPending { + t.Fatalf("error code = %q, want %q (client authentication should have succeeded)", code, server.ErrorAuthorizationPending) + } +} + func TestExchangeBackchannelAuthenticationRejectsUnknownAuthReqID(t *testing.T) { h, _ := newHarnessWithBackchannel(t) diff --git a/server/backchannel_token.go b/server/backchannel_token.go index d058415..07344ae 100644 --- a/server/backchannel_token.go +++ b/server/backchannel_token.go @@ -64,8 +64,11 @@ func (s *Server) ExchangeBackchannelAuthentication(ctx context.Context, req Back // CIBA's own token-polling step (CIBA §10.1) is a grant_type on the // physical token endpoint, not a separate endpoint of its own — see // CIBAGrantType's own doc comment — so this authenticates against - // the Token endpoint's own audience carve-out, the same as - // ExchangeAuthorizationCode/RefreshAccessToken. + // the Token endpoint's audiences, the same as + // ExchangeAuthorizationCode/RefreshAccessToken. Only a CIBA-registered + // client can hold an auth_req_id, and such a client keeps CIBA Core + // §7.1's endpoint-URL audiences — see + // acceptableClientAssertionAudiences. client, dpopProof, authErr := s.authenticateRequest(ctx, params, requestCredentials{ PeerCertificate: req.PeerCertificate, DPoPProofs: req.DPoPProofs, ClientAttestations: req.ClientAttestations, ClientAttestationPoPs: req.ClientAttestationPoPs, }, []fapi.URL{s.cfg.Endpoints.Token}, []fapi.URL{s.cfg.MTLSEndpoints.Token}) diff --git a/server/mtls_test.go b/server/mtls_test.go index 125a79a..9838b44 100644 --- a/server/mtls_test.go +++ b/server/mtls_test.go @@ -229,16 +229,12 @@ func newHarnessWithSenderConstrainMTLSAndAliases(t *testing.T) (harness, string) return harness{server: srv, key: key, serverKey: serverKey, now: now}, mtlsToken.String() } -// TestExchangeAuthorizationCodeAcceptsMTLSAliasAsClientAssertionAudience -// covers RFC 7523 §3's own looseness ("aud"... identifies the AS, -// not necessarily one fixed URL): an mTLS-bound client may call the -// token endpoint via its RFC 8705 §5 alias and sign its client -// assertion's "aud" against that alias URL rather than the issuer, -// since both identify the same authorization server. Scoped to the -// token endpoint specifically — see -// TestPushAuthorizationRequestRejectsMTLSAliasAsClientAssertionAudience -// for why PAR must reject this exact same value. -func TestExchangeAuthorizationCodeAcceptsMTLSAliasAsClientAssertionAudience(t *testing.T) { +// TestExchangeAuthorizationCodeRejectsMTLSAliasAsClientAssertionAudience +// confirms FAPI 2.0 Security Profile Final §5.3.2.1's issuer-only "aud" +// rule applies to the token endpoint's RFC 8705 §5 mTLS alias too — the +// alias identifies the same authorization server, but it isn't the +// issuer identifier. See acceptableClientAssertionAudiences. +func TestExchangeAuthorizationCodeRejectsMTLSAliasAsClientAssertionAudience(t *testing.T) { h, mtlsToken := newHarnessWithSenderConstrainMTLSAndAliases(t) code := completeSuccessfulAuthorization(t, h, []string{"openid", "accounts"}) cert := selfSignedTestClientCert(t) @@ -250,11 +246,12 @@ func TestExchangeAuthorizationCodeAcceptsMTLSAliasAsClientAssertionAudience(t *t if err != nil { t.Fatalf("CreateAssertion: %v", err) } - if _, err := h.server.ExchangeAuthorizationCode(context.Background(), server.AuthorizationCodeExchangeRequest{ + _, err = h.server.ExchangeAuthorizationCode(context.Background(), server.AuthorizationCodeExchangeRequest{ HTTP: server.FormRequest{Parameters: exchangeFormParams(assertion, code, testRedirectURI, testCodeVerifier)}, PeerCertificate: cert, - }); err != nil { - t.Fatalf("ExchangeAuthorizationCode: %v", err) + }) + if code := serverErrorCode(t, err); code != server.ErrorInvalidClient { + t.Fatalf("error code = %q, want %q", code, server.ErrorInvalidClient) } } diff --git a/server/par.go b/server/par.go index d601e73..bb1247a 100644 --- a/server/par.go +++ b/server/par.go @@ -403,31 +403,39 @@ func (s *Server) authenticateClientViaAssertion(ctx context.Context, params map[ // acceptableClientAssertionAudiences returns the "aud" values this // server accepts on a client assertion presented to one specific -// endpoint: always the issuer identifier, plus — only the endpoint URLs -// actually named in endpoints, appropriate for the endpoint actually -// authenticating this request — those URLs, and, for a client actually -// registered SenderConstrainMTLS, their RFC 8705 §5 mTLS aliases -// (mtlsEndpoints, matched by position to endpoints). RFC 7523 §3 -// sanctions "the token endpoint URL" as "aud" alongside the issuer -// identifier; CIBA Core 1.0 §7.1 separately, explicitly widens this for -// its own backchannel authentication endpoint: "the OP MUST accept its +// endpoint. Always the issuer identifier — and, for a client not +// registered for CIBA, only the issuer identifier: FAPI 2.0 Security +// Profile Final §5.3.2.1 requires the authorization server to "only +// accept its issuer identifier value ... as a string in the aud claim +// received in client authentication assertions", at every endpoint — +// narrowing RFC 7523 §3's older "the token endpoint URL ... MAY be used" +// allowance. Confirmed live via the OIDF suite's +// fapi2-security-profile-final-ensure-invalid-client-assertions-fail +// module, which (for the client_credentials variant, where the token +// endpoint is the first authenticated endpoint) requires a token +// endpoint URL "aud" to be rejected. +// +// A client registered for CIBA (BackchannelAuthenticationRequestAlgorithm +// set) instead follows FAPI-CIBA's rules at every endpoint that passes +// endpoints: CIBA Core 1.0 §7.1 requires that "the OP MUST accept its // Issuer Identifier, Token Endpoint URL, or Backchannel Authentication -// Endpoint URL as values that identify it as an intended audience" — -// confirmed live via the OIDF conformance suite's own -// fapi-ciba-id1/-refresh-token modules, each of which deliberately -// signs "aud" as the token endpoint's URL on a request sent to the -// backchannel authentication endpoint and requires it to succeed. +// Endpoint URL as values that identify it as an intended audience", and +// the OIDF FAPI-CIBA-ID1 suite signs every token-endpoint assertion — +// including refresh — with the token endpoint URL. For such a client, +// the result also includes the URLs actually named in endpoints (those +// appropriate for the endpoint authenticating this request) and, when +// it is registered SenderConstrainMTLS, their RFC 8705 §5 mTLS aliases +// (mtlsEndpoints, matched by position to endpoints). // -// Deliberately NOT a blanket "any of this server's own endpoint URLs, -// from any endpoint" — confirmed live via +// PAR never accepts an endpoint-URL audience, for any client — +// confirmed live via // fapi2-security-profile-final-par-test-{par,token}-endpoint-url-as-audience-fails -// that PAR must reject a client assertion whose "aud" is the PAR -// endpoint's own URL, or the token endpoint's URL: unlike Token and -// BackchannelAuthentication, PAR was never granted a URL-audience -// carve-out by RFC 7523 or CIBA, so PushAuthorizationRequest passes nil -// for both parameters, accepting only the issuer identifier. +// — so PushAuthorizationRequest passes nil for both parameters. func (s *Server) acceptableClientAssertionAudiences(client storage.RegisteredClient, endpoints, mtlsEndpoints []fapi.URL) []string { auds := []string{s.cfg.Issuer.String()} + if _, ciba := client.BackchannelAuthenticationRequestAlgorithm(); !ciba { + return auds + } for _, u := range endpoints { if !u.IsZero() { auds = append(auds, u.String()) diff --git a/server/par_test.go b/server/par_test.go index 23d1161..bc37897 100644 --- a/server/par_test.go +++ b/server/par_test.go @@ -715,14 +715,16 @@ func newHarnessWithClientKeys(t *testing.T, clientKeys keys.ClientKeySource) har return harness{server: srv, key: key, serverKey: serverKey, now: now} } -// TestExchangeAuthorizationCodeAcceptsTokenEndpointURLAsClientAssertionAudience -// covers RFC 7523 §3's own sanctioned alternative to the issuer -// identifier: "The token endpoint URL of the authorization server MAY -// be used as a value for an 'aud' element". This carve-out is scoped to -// the token endpoint specifically — see -// TestPushAuthorizationRequestRejectsTokenEndpointURLAsClientAssertionAudience -// for why PAR must reject this exact same value. -func TestExchangeAuthorizationCodeAcceptsTokenEndpointURLAsClientAssertionAudience(t *testing.T) { +// TestExchangeAuthorizationCodeRejectsTokenEndpointURLAsClientAssertionAudience +// covers FAPI 2.0 Security Profile Final §5.3.2.1: the authorization +// server "shall only accept its issuer identifier value ... as a string +// in the aud claim received in client authentication assertions" — +// including at the token endpoint, where RFC 7523 §3 alone would have +// permitted the token endpoint URL. Confirmed live via the OIDF suite's +// fapi2-security-profile-final-ensure-invalid-client-assertions-fail +// module. Only a CIBA-registered client keeps the URL audiences — see +// TestExchangeBackchannelAuthenticationAcceptsTokenEndpointURLAsClientAssertionAudience. +func TestExchangeAuthorizationCodeRejectsTokenEndpointURLAsClientAssertionAudience(t *testing.T) { h := newHarness(t, server.ProfileFAPISecurity, true) code := completeSuccessfulAuthorization(t, h, []string{"openid", "accounts"}) assertion, err := clientassertion.CreateAssertion(clientassertion.AssertionRequest{ @@ -734,11 +736,12 @@ func TestExchangeAuthorizationCodeAcceptsTokenEndpointURLAsClientAssertionAudien t.Fatalf("CreateAssertion: %v", err) } dpopKey := generateKey(t) - if _, err := h.server.ExchangeAuthorizationCode(context.Background(), server.AuthorizationCodeExchangeRequest{ + _, err = h.server.ExchangeAuthorizationCode(context.Background(), server.AuthorizationCodeExchangeRequest{ HTTP: server.FormRequest{Parameters: exchangeFormParams(assertion, code, testRedirectURI, testCodeVerifier)}, DPoPProofs: []string{createDPoPProof(t, dpopKey, h.now)}, - }); err != nil { - t.Fatalf("ExchangeAuthorizationCode: %v", err) + }) + if code := serverErrorCode(t, err); code != server.ErrorInvalidClient { + t.Fatalf("error code = %q, want %q", code, server.ErrorInvalidClient) } } @@ -747,10 +750,9 @@ func TestExchangeAuthorizationCodeAcceptsTokenEndpointURLAsClientAssertionAudien // cover PAR's own, narrower audience acceptance — confirmed live against // the OIDF conformance suite's own // fapi2-security-profile-final-par-test-{token,par}-endpoint-url-as-audience-fails -// modules: unlike Token and BackchannelAuthentication, PAR was never -// granted a URL-audience carve-out by RFC 7523 in the first place (see -// acceptableClientAssertionAudiences's own doc comment), so PAR accepts -// only the issuer identifier — not even its own endpoint URL. +// modules: PAR accepts only the issuer identifier for every client — +// not even its own endpoint URL, and not even for a CIBA-registered +// client (see acceptableClientAssertionAudiences's own doc comment). func TestPushAuthorizationRequestRejectsTokenEndpointURLAsClientAssertionAudience(t *testing.T) { h := newHarness(t, server.ProfileFAPISecurity, true) assertion, err := clientassertion.CreateAssertion(clientassertion.AssertionRequest{ diff --git a/server/token.go b/server/token.go index 7d1e34b..da62904 100644 --- a/server/token.go +++ b/server/token.go @@ -449,11 +449,10 @@ func (s *Server) verifyTokenRequestDPoP(ctx context.Context, proof string) (dpop // conformance suite's own FAPI2SPFinal driver does exactly this once it // discovers mtls_endpoint_aliases, for a client_auth_type=mtls client // that is otherwise plain sender_constrain=dpop. Both URLs -// unambiguously identify this same authorization server (the same -// reasoning acceptableClientAssertionAudiences documents for the -// analogous "aud" case), so accepting either is interoperability, not a -// security relaxation — and unconditional on which client, unlike that -// aud-widening: any client may legitimately reach either URL. +// unambiguously identify this same authorization server, so accepting +// either is interoperability, not a security relaxation — and +// unconditional on which client, unlike acceptableClientAssertionAudiences's +// CIBA-only "aud" widening: any client may legitimately reach either URL. func verifyDPoPAtEitherEndpoint(ctx context.Context, req dpop.VerifyRequest, primary, mtlsAlias fapi.URL) (dpop.VerifiedProof, error) { primaryURL := primary.URL() req.URL = &primaryURL