fix(server)!: attestation x5c key bounds, HAIP anchor interop, resource and embedder-binding fixes

UPGRADING.md

@@ -97,6 +97,73 @@ example letters and digits:
 req.BindingMessage = "W4SCT"
 ```
 
+### `X5CAttesterChain` limits the RSA keys of `x5c` certificates (server)
+
+**Affects:** a server whose `Dependencies.AttesterTrust` is
+`server.X5CAttesterChain`, when a wallet's attestation `x5c` carries a
+certificate — the signing certificate or an intermediate — whose
+public key is RSA with a modulus over 4096 bits or a public exponent
+other than 65537. Anchors you configure (`TrustAnchors`, `Anchors`)
+aren't limited.
+
+**Why:** the `x5c` certificates are the caller's, and `crypto/x509`
+bounds neither an RSA modulus nor its exponent. Path building spent an
+RSA operation on whatever key an unauthenticated request carried, so a
+31,744-bit modulus with exponent 2^31-1 cost the server about 60 ms of
+CPU per request. v0.63.0 already limited every other RSA key a caller
+supplies (a DPoP `jwk`, an attestation's `cnf.jwk`, a client's JWK Set)
+to 4096 bits and exponent 65537; `x5c` now follows the same rule,
+checked before any signature.
+
+**What fails:** nothing at startup. Such an attestation is refused
+with `invalid_client`. There is no option to keep the old behaviour.
+
+**What to change:** have the attester's CA issue its signing and
+intermediate certificates for 2048-, 3072- or 4096-bit RSA keys with
+exponent 65537, or for EC keys:
+
+```sh
+openssl genpkey -algorithm RSA -pkeyopt rsa_keygen_bits:4096 -out attester-ca.pem
+```
+
+### `IssueAccessToken` and `IssueRefreshToken` refuse an edited binding or attestation expiry (server)
+
+**Affects:** an embedder serving its own grant
+(`Config.AdditionalGrantTypes`) that changes, after the server returned
+them, the `Thumbprint` of a `server.TokenBinding` from
+`VerifyTokenRequestBinding` before passing it to `IssueAccessToken`, or
+the `AttestationExpiresAt` of a `server.AttestedClient` from
+`AuthenticateAttestedClient` before passing it to `IssueAccessToken` or
+`IssueRefreshToken`.
+
+**Why:** both documented that only what the server verified is
+accepted, but the check covered only the client the value was verified
+for. An edited `Thumbprint` bound the access token to a key that never
+proved possession, and an extended `AttestationExpiresAt` let a
+`Client` kept from an earlier request pass the attestation expiry
+check. Each now keeps its own copy of what was verified.
+
+**What fails:** nothing at startup. Such a call is refused with a
+`server_error` `*server.Error`.
+
+**What to change:** pass both exactly as the server returned them, for
+the current request:
+
+```go
+attested, err := srv.AuthenticateAttestedClient(ctx, req.AttestedClientAuthentication())
+if err != nil {
+	return err
+}
+binding, err := srv.VerifyTokenRequestBinding(ctx, attested.Client, req)
+if err != nil {
+	return err
+}
+result, err := srv.IssueAccessToken(ctx, server.IssueAccessTokenRequest{
+	GrantType: grantType, Client: attested, Binding: binding,
+	Subject: subject, Scope: scope,
+})
+```
+
 ## v0.68.0
 
 ### `DefaultAttesterLeafPolicy` refuses more signing roles (server)
@@ -171,6 +238,12 @@ anchor, including cross-signed copies:
 x5c := [][]byte{leaf.Raw, intermediate.Raw}
 ```
 
+*Relaxed in v0.69.0:* a configured anchor in `x5c` that isn't
+self-signed is accepted when a verified chain also ends at an anchor
+`x5c` doesn't carry, as `[leaf, intermediate]` does with both the root
+and the intermediate configured as anchors. It's refused only when
+every verified chain ends at an anchor the wallet sent.
+
 ## v0.66.0
 
 ### `X5CAttesterChain` refuses other roles' signing certificates (server)
@@ -274,6 +347,12 @@ anchor:
 x5c := [][]byte{leaf.Raw, intermediate.Raw}
 ```
 
+*Relaxed in v0.69.0:* a configured anchor in `x5c` that isn't
+self-signed is accepted when a verified chain also ends at an anchor
+`x5c` doesn't carry, as `[leaf, intermediate]` does with both the root
+and the intermediate configured as anchors. It's refused only when
+every verified chain ends at an anchor the wallet sent.
+
 ## v0.65.0
 
 ### `Limits.MaxAccessTokenClaimsBytes` is required (server)

docs/guides/native-wallet.md

@@ -36,17 +36,27 @@ A native client may use a private-use scheme in reverse-domain form,
 written with a single slash (`com.example.wallet:/callback`), loopback
 `http` to `127.0.0.1` or `[::1]` (never `localhost`, RFC 8252 §8.3),
 matched on any port, or a claimed `https` URI, in production as well;
-`NewRegisteredClient` refuses any other form. The server also
+`NewRegisteredClient` refuses any other form. The attestation is the
+client's authentication method, not an extra signal beside another
+one: the server doesn't support the attestation-based client
+authentication draft's use alongside `private_key_jwt` (draft-12
+§7.6), and refuses a request carrying both an attestation and a
+`client_assertion` with `invalid_request`. The server also
 needs `Config.AttestationBasedClientAuthentication` and an
 `AttesterTrust`: `server.RegisteredAttesterKeys` with the attesters'
 own keys, by issuer (a `keys.AttesterKeySource` such as
 `keys.StaticAttesterKeys` — never the clients' keys), or
-`server.X5CAttesterChain` to verify the attestation's `x5c` chain. That
-`x5c` carries the signing certificate and its intermediates only, as
-HAIP 1.0 requires: an attestation whose `x5c` also includes a trust
-anchor (any self-signed certificate, or a certificate with a
-configured anchor's subject and public key, such as a cross-signed
-copy of it) is `invalid_client`. With
+`server.X5CAttesterChain` to verify the attestation's `x5c` chain.
+HAIP 1.0 §4.4.1 has the wallet send the signing certificate and its
+intermediates only, leaving out the trust anchor. The server refuses
+an `x5c` that includes any self-signed certificate, or whose every
+verified chain ends at an anchor `x5c` carries itself (a certificate
+with a configured anchor's subject and public key, such as a
+cross-signed copy of it), with `invalid_client`. An intermediate the
+server also lists as an anchor is accepted when the chain verifies on
+to another anchor, since the wallet can't know which of its CAs the
+server lists. Each RSA key in `x5c` must be at most 4096 bits, with
+exponent 65537. With
 a trust list shared by several attesters, use its
 `AttesterIssuerBoundToAnchor` binding, so each anchor vouches only for
 the attesters it's bound to. Its `LeafPolicy`, by default

internal/claimvalues/claimvalues.go

@@ -12,7 +12,11 @@ package claimvalues
 import (
 	"bytes"
 	"encoding/json"
-	"reflect"
+	"io"
+	"maps"
+	"math/big"
+	"slices"
+	"strings"
 )
 
 // Acceptable is, for a claim requested with "value", "values" or both,
@@ -53,14 +57,91 @@ func FromRequest(request json.RawMessage) ([]json.RawMessage, bool) {
 }
 
 // Equal reports whether a and b are the same JSON value: equal once
-// decoded, so "1" equals "1.0" and member order doesn't matter. Text
-// that doesn't decode equals nothing.
+// decoded, so "1" equals "1.0" and member order doesn't matter. Numbers
+// compare exactly, by the value their text denotes, not as float64,
+// which would make integers above 2^53 that differ equal. Text that
+// doesn't decode equals nothing.
 func Equal(a, b json.RawMessage) bool {
-	var av, bv any
-	if json.Unmarshal(a, &av) != nil || json.Unmarshal(b, &bv) != nil {
-		return false
+	av, aok := decode(a)
+	bv, bok := decode(b)
+	return aok && bok && equalValues(av, bv)
+}
+
+// decode decodes raw as one JSON value, keeping numbers as their text.
+func decode(raw json.RawMessage) (any, bool) {
+	d := json.NewDecoder(bytes.NewReader(raw))
+	d.UseNumber()
+	var v any
+	if d.Decode(&v) != nil {
+		return nil, false
+	}
+	// Like json.Unmarshal, refuse anything after the value.
+	if _, err := d.Token(); err != io.EOF {
+		return nil, false
+	}
+	return v, true
+}
+
+// equalValues compares two values decode returned.
+func equalValues(a, b any) bool {
+	switch a := a.(type) {
+	case json.Number:
+		b, ok := b.(json.Number)
+		return ok && equalNumbers(a, b)
+	case []any:
+		b, ok := b.([]any)
+		return ok && slices.EqualFunc(a, b, equalValues)
+	case map[string]any:
+		b, ok := b.(map[string]any)
+		return ok && maps.EqualFunc(a, b, equalValues)
+	default:
+		// string, bool or nil.
+		return a == b
+	}
+}
+
+// equalNumbers reports whether two JSON number texts denote the same
+// value, exactly: each is reduced to a sign, its significant digits and
+// a decimal exponent, so "100", "1e2" and "100.0" are equal. The
+// exponent is a big.Int, so neither overflow nor a huge exponent's
+// expansion (as big.Rat or big.Float would compute) is possible.
+func equalNumbers(a, b json.Number) bool {
+	an, aok := canonicalNumber(string(a))
+	bn, bok := canonicalNumber(string(b))
+	return aok && bok && an.negative == bn.negative && an.digits == bn.digits && an.exponent.Cmp(bn.exponent) == 0
+}
+
+// number is a JSON number as ±digits × 10^exponent, with digits
+// free of leading and trailing zeros; zero has no digits, no sign and
+// exponent 0.
+type number struct {
+	negative bool
+	digits   string
+	exponent *big.Int
+}
+
+// canonicalNumber reduces text, a JSON number (RFC 8259 §6) as
+// encoding/json validated it, to a number.
+func canonicalNumber(text string) (number, bool) {
+	negative := strings.HasPrefix(text, "-")
+	text = strings.TrimPrefix(text, "-")
+	mantissa, exp, hasExp := strings.Cut(strings.ToLower(text), "e")
+	exponent := new(big.Int)
+	if hasExp {
+		if _, ok := exponent.SetString(strings.TrimPrefix(exp, "+"), 10); !ok {
+			return number{}, false
+		}
+	}
+	integer, fraction, _ := strings.Cut(mantissa, ".")
+	digits := integer + fraction
+	exponent.Sub(exponent, big.NewInt(int64(len(fraction))))
+	trimmed := strings.TrimRight(digits, "0")
+	exponent.Add(exponent, big.NewInt(int64(len(digits)-len(trimmed))))
+	digits = strings.TrimLeft(trimmed, "0")
+	if digits == "" {
+		return number{exponent: new(big.Int)}, true
 	}
-	return reflect.DeepEqual(av, bv)
+	return number{negative: negative, digits: digits, exponent: exponent}, true
 }
 
 // Allows reports whether name may be released with value under a.

internal/claimvalues/claimvalues_test.go

@@ -38,3 +38,49 @@ func TestFromRequest(t *testing.T) {
 		}
 	}
 }
+
+// TestEqualNumbersExactly covers OIDC Core §5.5.1's equality comparison
+// for numbers: by the value the text denotes, exactly, so integers
+// above 2^53 that float64 would round together stay distinct, while
+// different spellings of one value are equal.
+func TestEqualNumbersExactly(t *testing.T) {
+	for _, tc := range []struct {
+		a, b string
+		want bool
+	}{
+		{`9007199254740993`, `9007199254740992`, false},
+		{`9007199254740993`, `9007199254740993`, true},
+		{`[9007199254740993]`, `[9007199254740992]`, false},
+		{`{"n":9007199254740993}`, `{"n":9007199254740992}`, false},
+		{`0.1000000000000000000001`, `0.1`, false},
+		{`1`, `1.0`, true},
+		{`100`, `1e2`, true},
+		{`100`, `1E+2`, true},
+		{`0.01`, `1e-2`, true},
+		{`1.50`, `15e-1`, true},
+		{`-1`, `1`, false},
+		{`0`, `-0.0`, true},
+		{`0`, `0e5`, true},
+		{`1e400`, `1e400`, true},
+		{`1e400`, `1e401`, false},
+		{`1e99999999999999999999`, `10e99999999999999999998`, true},
+		{`1`, `"1"`, false},
+		{`"A"`, `"A"`, true},
+		{`{"x":[1,{"y":2}]}`, `{"x":[1.0,{"y":2e0}]}`, true},
+		{`[1,2]`, `[1,2,3]`, false},
+		{`null`, `null`, true},
+		{`1 2`, `1`, false},
+		{`1x`, `1`, false},
+	} {
+		if got := Equal(json.RawMessage(tc.a), json.RawMessage(tc.b)); got != tc.want {
+			t.Errorf("Equal(%s, %s) = %v, want %v", tc.a, tc.b, got, tc.want)
+		}
+		if got := Equal(json.RawMessage(tc.b), json.RawMessage(tc.a)); got != tc.want {
+			t.Errorf("Equal(%s, %s) = %v, want %v", tc.b, tc.a, got, tc.want)
+		}
+	}
+	a := Acceptable{"account_no": {json.RawMessage(`9007199254740993`)}}
+	if a.Allows("account_no", json.RawMessage(`9007199254740992`)) {
+		t.Error("Allows released 9007199254740992 for a request of value 9007199254740993")
+	}
+}

internal/jose/callerkey.go

@@ -57,3 +57,16 @@ func checkCallerKeyMaterial(pub crypto.PublicKey) error {
 	}
 	return nil
 }
+
+// ValidateCallerCertificateKey checks pub, the public key of a
+// certificate a caller supplied — one in an attestation's "x5c" chain,
+// say — against the key-material rules ParseJWK applies to a caller's
+// key, whatever algorithm the certificate is used with: an RSA modulus
+// of at most 4096 bits (maxCallerRSAModulusBits) and exponent 65537.
+// Run it before the certificate's key verifies anything, path building
+// included: crypto/x509 accepts any modulus size and exponents up to
+// 2^31-1, so an unchecked certificate lets the caller choose how much
+// CPU each signature check costs.
+func ValidateCallerCertificateKey(pub crypto.PublicKey) error {
+	return checkCallerKeyMaterial(pub)
+}

server/attested_client.go

@@ -48,7 +48,9 @@ type AttestedClient struct {
 	Client storage.RegisteredClient
 
 	// AttestationExpiresAt is when the verified Client Attestation
-	// expires.
+	// expires. IssueRefreshToken and IssueAccessToken refuse a Client
+	// whose AttestationExpiresAt was changed since: they judge the
+	// attestation's expiry by their own copy of it.
 	AttestationExpiresAt time.Time
 
 	// instanceKey is the RFC 7638 thumbprint of the Client Instance Key
@@ -57,8 +59,11 @@ type AttestedClient struct {
 	// this server authenticated carries one. authenticatedID is the
 	// client it was verified for: IssueRefreshToken refuses a Client
 	// changed since, so the key can't be attached to another client.
-	instanceKey     string
-	authenticatedID fapi.ClientID
+	// verifiedExpiresAt is AttestationExpiresAt as verified, which they
+	// compare it with and judge expiry by.
+	instanceKey       string
+	authenticatedID   fapi.ClientID
+	verifiedExpiresAt time.Time
 }
 
 // AuthenticateAttestedClient authenticates a client with a Client
@@ -108,7 +113,7 @@ func (s *Server) AuthenticateAttestedClient(ctx context.Context, req AttestedCli
 	if req.malformed {
 		err = newError(ErrorInvalidRequest, 400, "the request's parameters are duplicated, too many, or too large", nil)
 	} else if req.otherClientAuth {
-		err = newError(ErrorInvalidRequest, 400, "the request uses more than one client authentication method", nil)
+		err = newError(ErrorInvalidRequest, 400, "a client attestation cannot be combined with another client authentication method", nil)
 	} else if err == nil && attestation == "" {
 		err = newError(ErrorInvalidClient, 401, "client authentication is required", nil)
 	}
@@ -122,5 +127,5 @@ func (s *Server) AuthenticateAttestedClient(ctx context.Context, req AttestedCli
 		return AttestedClient{}, err
 	}
 	s.audit(ctx, AuditEventAuthenticateAttestedClient, client.ID(), AuditOutcomeSuccess, "")
-	return AttestedClient{Client: client, AttestationExpiresAt: verified.ExpiresAt, instanceKey: verified.InstanceKey, authenticatedID: client.ID()}, nil
+	return AttestedClient{Client: client, AttestationExpiresAt: verified.ExpiresAt, instanceKey: verified.InstanceKey, authenticatedID: client.ID(), verifiedExpiresAt: verified.ExpiresAt}, nil
 }