Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions cmd/conformance-as/client_credentials_smoke_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
}
Expand All @@ -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,
})
Expand Down Expand Up @@ -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,
})
Expand Down
9 changes: 8 additions & 1 deletion conformance/server/oidf-config/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 34 additions & 1 deletion server/backchannel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
7 changes: 5 additions & 2 deletions server/backchannel_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
// token when the granted scope included "openid" and a refresh token
// when it included "offline_access") — exactly once, mirroring
// ExchangeAuthorizationCode's single-issuance guarantee.
func (s *Server) ExchangeBackchannelAuthentication(ctx context.Context, req BackchannelTokenExchangeRequest) (TokenResult, error) {

Check failure on line 55 in server/backchannel_token.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 23 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=IDFoundry_FAPIgo&issues=AaDODFZyr4kOMVtlTUbO&open=AaDODFZyr4kOMVtlTUbO&pullRequest=370
params, err := formParametersToMap(req.HTTP.Parameters)
if err != nil {
return s.tokenFail(ctx, AuditEventExchangeBackchannelAuthentication, "", newError(ErrorInvalidRequest, 400, "the request contains a duplicated parameter", err))
Expand All @@ -64,8 +64,11 @@
// 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})
Expand Down
23 changes: 10 additions & 13 deletions server/mtls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}

Expand Down
48 changes: 28 additions & 20 deletions server/par.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
32 changes: 17 additions & 15 deletions server/par_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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)
}
}

Expand All @@ -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{
Expand Down
9 changes: 4 additions & 5 deletions server/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading