fix(protocol): credential public key match limited to ECDSA (#732)

§8.4 and §8.8 only require the attestation certificate public key to match the credentialPublicKey, but the check refused anything but ECDSA on either side, rejecting the RSA keys Android Keystore attests. It now converts the parsed COSE key to its standard library equivalent and compares with Equal, so a differing key type is a mismatch rather than a refusal.

The android-key handler's second signature verification now goes through webauthncose.VerifySignature. It isn't redundant with the certificate check above it, as each takes its digest from the algorithm it carries, so passing both requires the two to agree.

Author: james-d-elliott

protocol/attestation_androidkey.go

@@ -88,14 +88,21 @@ func attestationFormatValidationHandlerAndroidKey(att AttestationObject, clientD
 	}
 
 	// Verify that the public key in the first certificate in x5c matches the credentialPublicKey in the attestedCredentialData in authenticatorData.
-	var attPublicKeyData webauthncose.EC2PublicKeyData
-	if attPublicKeyData, err = verifyAttestationECDSAPublicKeyMatch(att, credCert); err != nil {
+	var credentialPublicKey any
+
+	if credentialPublicKey, err = verifyAttestationPublicKeyMatch(att, credCert); err != nil {
 		return "", nil, err
 	}
 
+	// The signature was verified above with the certificate public key and the algorithm of the attestation
+	// statement. Repeating it with the credential public key additionally requires the two to agree on the algorithm,
+	// as each selects the digest from the algorithm it carries.
 	var valid bool
-	if valid, err = attPublicKeyData.Verify(signatureData, sig); err != nil || !valid {
-		return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error parsing public key: %+v", err)).WithError(err)
+
+	if valid, err = webauthncose.VerifySignature(credentialPublicKey, signatureData, sig); err != nil {
+		return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error verifying the signature with the credential public key: %+v", err)).WithError(err)
+	} else if !valid {
+		return "", nil, ErrInvalidAttestation.WithDetails("Signature is not valid for the credential public key")
 	}
 
 	// §8.4.3. Verify that the attestationChallenge field in the attestation certificate extension data is identical to clientDataHash.

protocol/attestation_apple.go

@@ -81,7 +81,7 @@ func attestationFormatValidationHandlerAppleAnonymous(att AttestationObject, cli
 	}
 
 	// Step 5. Verify that the credential public key equals the Subject Public Key of credCert.
-	if _, err = verifyAttestationECDSAPublicKeyMatch(att, credCert); err != nil {
+	if _, err = verifyAttestationPublicKeyMatch(att, credCert); err != nil {
 		return "", nil, err
 	}
 

protocol/attestation_spec_test.go

@@ -155,9 +155,8 @@ func TestSpecVectors_AppleES256(t *testing.T) {
 // §16.16 FIDO U2F Attestation - ES256
 // See: https://www.w3.org/TR/webauthn-3/#sctn-test-vectors-fido-u2f-es256
 //
-// The spec test vector uses a non-zero AAGUID (afb3c2ef...) which is correctly rejected by
-// the FIDO U2F handler per §8.6 which requires AAGUID to be all zeros. This test validates
-// CBOR parsing, authData, rpIdHash, and credential algorithm matching.
+// The spec test vector carries a non-zero AAGUID (afb3c2ef...). §8.6 places no constraint on the
+// AAGUID, so the format handler verifies the attestation in full.
 func TestSpecVectors_FIDOU2FES256(t *testing.T) {
 	attObjHex := "a363666d74686669646f2d7532666761747453746d74a26373696758473045022100f41887a20063bb26867cb9751978accea5b81791a68f4f4dd6ea1fb6a5c086c302204e5e00aa3895777e6608f1f375f95450045da3da57a0e4fd451df35a31d2d98a637835638159022530820221308201c7a003020102021004f66dc6542ea7719dea416d325a2401300a06082a8648ce3d0403023062311e301c06035504030c15576562417574686e207465737420766563746f7273310c300a060355040a0c0357334331253023060355040b0c1c41757468656e74696361746f72204174746573746174696f6e204341310b30090603550406130241413020170d3234303130313030303030305a180f33303234303130313030303030305a305f311e301c06035504030c15576562417574686e207465737420766563746f7273310c300a060355040a0c0357334331223020060355040b0c1941757468656e74696361746f72204174746573746174696f6e310b30090603550406130241413059301306072a8648ce3d020106082a8648ce3d0301070342000456fffa7093dede46aefeefb6e520c7ccc78967636e2f92582ba71455f64e93932dff3be4e0d4ef68e3e3b73aa087e26a0a0a30b02dc2aa2309db4c3a2fc936dea360305e300c0603551d130101ff04023000300e0603551d0f0101ff040403020780301d0603551d0e04160414420822eb1908b5cd3911017fbcad4641c05e05a3301f0603551d2304183016801445aff715b0dd786741fee996ebc16547a3931b1e300a06082a8648ce3d040302034800304502200d0b777f0a0b181ad2830275acc3150fd6092430bcd034fd77beb7bdf8c2d546022100d4864edd95daa3927080855df199f1717299b24a5eecefbd017455a9b934d8f668617574684461746158a4bfabc37432958b063360d3ad6461c9c4735ae7f8edd46592a5e0f01452b2e4b54100000000afb3c2efc054df425013d5c88e79c3c10020a4ba6e2d2cfec43648d7d25c5ed5659bc18f2b781538527ebd492de03256bdf4a5010203262001215820b0d62de6b30f86f0bac7a9016951391c2e31849e2e64661cbd2b13cd7d5508ad225820503b0bda2a357a9a4b34475a28e65b660b4898a9e3e9bbf0820d43494297edd0"
 	clientDataJSONHex := "7b2274797065223a22776562617574686e2e637265617465222c226368616c6c656e6765223a22344851334b5a4335797155486f696666786e73414e344445557955344452715177672d4237583049444159222c226f726967696e223a2268747470733a2f2f6578616d706c652e6f7267222c2263726f73734f726967696e223a66616c73657d"

protocol/utils.go

@@ -1,12 +1,15 @@
 package protocol
 
 import (
-	"crypto/ecdsa"
+	"crypto"
+	"crypto/ed25519"
+	"crypto/rsa"
 	"crypto/x509"
 	"encoding/asn1"
 	"encoding/pem"
 	"errors"
 	"fmt"
+	"math/big"
 	"net"
 	"net/url"
 	"strings"
@@ -187,35 +190,58 @@ func certInsecureConditionalNotAfterMangle(cert *x509.Certificate, mangle bool,
 	return out
 }
 
-func verifyAttestationECDSAPublicKeyMatch(att AttestationObject, cert *x509.Certificate) (attPublicKeyData webauthncose.EC2PublicKeyData, err error) {
-	var (
-		key any
-		ok  bool
+// verifyAttestationPublicKeyMatch verifies the credentialPublicKey of the attested credential data is the public key
+// of the given attestation certificate, and returns the credential public key parsed from its COSE encoding so a
+// signature made with it can be verified.
+//
+// The attestation statement formats which perform this step place no restriction on the key type, so every type the
+// COSE parser produces is accepted rather than ECDSA alone.
+func verifyAttestationPublicKeyMatch(att AttestationObject, cert *x509.Certificate) (credentialPublicKey any, err error) {
+	if credentialPublicKey, err = webauthncose.ParsePublicKey(att.AuthData.AttData.CredentialPublicKey); err != nil {
+		return nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error parsing public key: %+v", err)).WithError(err)
+	}
 
-		publicKey, attPublicKey *ecdsa.PublicKey
-	)
+	var public crypto.PublicKey
 
-	if key, err = webauthncose.ParsePublicKey(att.AuthData.AttData.CredentialPublicKey); err != nil {
-		return attPublicKeyData, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error parsing public key: %+v", err)).WithError(err)
+	if public, err = attestationCredentialPublicKey(credentialPublicKey); err != nil {
+		return nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error converting public key: %+v", err)).WithError(err)
 	}
 
-	if attPublicKeyData, ok = key.(webauthncose.EC2PublicKeyData); !ok {
-		return attPublicKeyData, ErrInvalidAttestation.WithDetails("Attestation public key is not ECDSA")
+	// Each standard library public key type carries an Equal method which reports false for a key of another type, so
+	// a credential public key and a certificate public key of differing types are a mismatch rather than an error.
+	equatable, ok := public.(interface{ Equal(x crypto.PublicKey) bool })
+	if !ok {
+		return nil, ErrInvalidAttestation.WithDetails("Public key does not support comparison")
 	}
 
-	if publicKey, ok = cert.PublicKey.(*ecdsa.PublicKey); !ok {
-		return attPublicKeyData, ErrInvalidAttestation.WithDetails("Credential public key is not ECDSA")
+	if !equatable.Equal(cert.PublicKey) {
+		return nil, ErrInvalidAttestation.WithDetails("Certificate public key does not match public key in authData")
 	}
 
-	if attPublicKey, err = attPublicKeyData.ToECDSA(); err != nil {
-		return attPublicKeyData, ErrInvalidAttestation.WithDetails("Error converting public key to ECDSA").WithError(err)
-	}
+	return credentialPublicKey, nil
+}
 
-	if !attPublicKey.Equal(publicKey) {
-		return attPublicKeyData, ErrInvalidAttestation.WithDetails("Certificate public key does not match public key in authData")
-	}
+// attestationCredentialPublicKey converts a credential public key parsed from its COSE encoding into the equivalent
+// standard library type.
+func attestationCredentialPublicKey(credentialPublicKey any) (public crypto.PublicKey, err error) {
+	switch k := credentialPublicKey.(type) {
+	case webauthncose.EC2PublicKeyData:
+		return k.ToECDSA()
+	case webauthncose.RSAPublicKeyData:
+		var exponent int
+
+		if exponent, err = webauthncose.ParseRSAPublicKeyDataExponent(&k); err != nil {
+			return nil, err
+		}
 
-	return attPublicKeyData, nil
+		return &rsa.PublicKey{N: new(big.Int).SetBytes(k.Modulus), E: exponent}, nil
+	case webauthncose.OKPPublicKeyData:
+		// The coordinate is of the length ed25519 requires as webauthncose.ParsePublicKey rejects any other, so no
+		// length is asserted here.
+		return ed25519.PublicKey(k.XCoord), nil
+	default:
+		return nil, fmt.Errorf("unsupported public key type %T", credentialPublicKey)
+	}
 }
 
 // ValidateRPID performs non-exhaustive checks to ensure the string is most likely a domain string as