refactor: final adjustments

Author: james-d-elliott

protocol/attestation_androidkey.go

@@ -12,7 +12,7 @@ import (
 )
 
 func init() {
-	RegisterAttestationFormat(AttestationFormatAndroidKey, verifyAndroidKeyFormat)
+	RegisterAttestationFormat(AttestationFormatAndroidKey, attestationFormatValidationHandlerAndroidKey)
 }
 
 // The android-key attestation statement looks like:
@@ -30,7 +30,7 @@ func init() {
 //	  }
 //
 // Specification: §8.4. Android Key Attestation Statement Format (https://www.w3.org/TR/webauthn/#sctn-android-key-attestation)
-func verifyAndroidKeyFormat(att AttestationObject, clientDataHash []byte, _ metadata.Provider) (attestationType string, x5cs []any, err error) {
+func attestationFormatValidationHandlerAndroidKey(att AttestationObject, clientDataHash []byte, _ metadata.Provider) (attestationType string, x5cs []any, err error) {
 	// Given the verification procedure inputs attStmt, authenticatorData and clientDataHash, the verification procedure is as follows:
 	// §8.4.1. Verify that attStmt is valid CBOR conforming to the syntax defined above and perform CBOR decoding on it to extract
 	// the contained fields.
@@ -63,7 +63,7 @@ func verifyAndroidKeyFormat(att AttestationObject, clientDataHash []byte, _ meta
 		return "", nil, ErrAttestation.WithDetails("Error parsing x5c cert chain").WithError(err)
 	}
 
-	leaf := certs[0]
+	credCert := certs[0]
 
 	if _, err = certChainVerify(certs, attAndroidKeyHardwareRootsCertPool, true, time.Now().Add(time.Hour*8760).UTC()); err != nil {
 		return "", nil, ErrInvalidAttestation.WithDetails("Error validating x5c cert chain").WithError(err)
@@ -72,32 +72,27 @@ func verifyAndroidKeyFormat(att AttestationObject, clientDataHash []byte, _ meta
 	signatureData := append(att.RawAuthData, clientDataHash...) //nolint:gocritic // This is intentional.
 
 	coseAlg := webauthncose.COSEAlgorithmIdentifier(alg)
-	if err = leaf.CheckSignature(webauthncose.SigAlgFromCOSEAlg(coseAlg), signatureData, sig); err != nil {
+	if err = credCert.CheckSignature(webauthncose.SigAlgFromCOSEAlg(coseAlg), signatureData, sig); err != nil {
 		return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Signature validation error: %+v", err)).WithError(err)
 	}
 
 	// Verify that the public key in the first certificate in x5c matches the credentialPublicKey in the attestedCredentialData in authenticatorData.
-	pubKey, err := webauthncose.ParsePublicKey(att.AuthData.AttData.CredentialPublicKey)
-	if err != nil {
-		return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error parsing public key: %+v", err)).WithError(err)
-	}
-
-	e, ok := pubKey.(webauthncose.EC2PublicKeyData)
-	if !ok {
-		return "", nil, ErrInvalidAttestation.WithDetails("Attestation public key is not EC2")
+	var attPublicKeyData webauthncose.EC2PublicKeyData
+	if attPublicKeyData, err = verifyAttestationECDSAPublicKeyMatch(att, credCert); err != nil {
+		return "", nil, err
 	}
 
 	var valid bool
-	if valid, err = e.Verify(signatureData, sig); err != nil || !valid {
+	if valid, err = attPublicKeyData.Verify(signatureData, sig); err != nil || !valid {
 		return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error parsing public key: %+v", err)).WithError(err)
 	}
 
 	// §8.4.3. Verify that the attestationChallenge field in the attestation certificate extension data is identical to clientDataHash.
 	// attCert.Extensions.
 	var attExtBytes []byte
 
-	for _, ext := range leaf.Extensions {
-		if ext.Id.Equal([]int{1, 3, 6, 1, 4, 1, 11129, 2, 1, 17}) {
+	for _, ext := range credCert.Extensions {
+		if ext.Id.Equal(oidExtensionAndroidKeystore) {
 			attExtBytes = ext.Value
 		}
 	}

protocol/attestation_androidkey_const.go

@@ -89,13 +89,13 @@ func TestVerifyAndroidKeyFormat(t *testing.T) {
 					tc.setup(t, mds)
 				}
 
-				attestationType, x5cs, err = verifyAndroidKeyFormat(tc.args.att, tc.args.clientDataHash, mds)
+				attestationType, x5cs, err = attestationFormatValidationHandlerAndroidKey(tc.args.att, tc.args.clientDataHash, mds)
 			} else {
 				if tc.setup != nil {
 					tc.setup(t, nil)
 				}
 
-				attestationType, x5cs, err = verifyAndroidKeyFormat(tc.args.att, tc.args.clientDataHash, nil)
+				attestationType, x5cs, err = attestationFormatValidationHandlerAndroidKey(tc.args.att, tc.args.clientDataHash, nil)
 			}
 
 			if tc.err != "" {

protocol/attestation_androidkey_test.go

@@ -2,20 +2,16 @@ package protocol
 
 import (
 	"bytes"
-	"crypto/ecdsa"
-	"crypto/elliptic"
 	"crypto/sha256"
 	"crypto/x509"
 	"encoding/asn1"
-	"fmt"
-	"math/big"
+	"time"
 
 	"github.com/go-webauthn/webauthn/metadata"
-	"github.com/go-webauthn/webauthn/protocol/webauthncose"
 )
 
 func init() {
-	RegisterAttestationFormat(AttestationFormatApple, verifyAppleFormat)
+	RegisterAttestationFormat(AttestationFormatApple, attestationFormatValidationHandlerAppleAnonymous)
 }
 
 // The apple attestation statement looks like:
@@ -31,7 +27,7 @@ func init() {
 //	  }
 //
 // Specification: §8.8. Apple Anonymous Attestation Statement Format (https://www.w3.org/TR/webauthn/#sctn-apple-anonymous-attestation)
-func verifyAppleFormat(att AttestationObject, clientDataHash []byte, _ metadata.Provider) (attestationType string, x5cs []any, err error) {
+func attestationFormatValidationHandlerAppleAnonymous(att AttestationObject, clientDataHash []byte, _ metadata.Provider) (attestationType string, x5cs []any, err error) {
 	// Step 1. Verify that attStmt is valid CBOR conforming to the syntax defined
 	// above and perform CBOR decoding on it to extract the contained fields.
 	// If x5c is not present, return an error.
@@ -41,27 +37,29 @@ func verifyAppleFormat(att AttestationObject, clientDataHash []byte, _ metadata.
 		return "", nil, ErrAttestationFormat.WithDetails("Error retrieving x5c value")
 	}
 
-	credCertBytes, valid := x5c[0].([]byte)
-	if !valid {
-		return "", nil, ErrAttestation.WithDetails("Error getting certificate from x5c cert chain")
-	}
-
-	credCert, err := x509.ParseCertificate(credCertBytes)
-	if err != nil {
-		return "", nil, ErrAttestationFormat.WithDetails(fmt.Sprintf("Error parsing certificate from ASN.1 data: %+v", err)).WithError(err)
-	}
-
 	// Step 2. Concatenate authenticatorData and clientDataHash to form nonceToHash.
 	nonceToHash := append(att.RawAuthData, clientDataHash...) //nolint:gocritic // This is intentional.
 
 	// Step 3. Perform SHA-256 hash of nonceToHash to produce nonce.
 	nonce := sha256.Sum256(nonceToHash)
 
 	// Step 4. Verify that nonce equals the value of the extension with OID 1.2.840.113635.100.8.2 in credCert.
+	var certs []*x509.Certificate
+
+	if certs, err = parseX5C(x5c); err != nil {
+		return "", nil, ErrAttestation.WithDetails("Error parsing x5c cert chain").WithError(err)
+	}
+
+	credCert := certs[0]
+
+	if _, err = certChainVerify(certs, attAppleHardwareRootsCertPool, true, time.Now().Add(time.Hour*8760).UTC()); err != nil {
+		return "", nil, ErrInvalidAttestation.WithDetails("Error validating x5c cert chain").WithError(err)
+	}
+
 	var attExtBytes []byte
 
 	for _, ext := range credCert.Extensions {
-		if ext.Id.Equal([]int{1, 2, 840, 113635, 100, 8, 2}) {
+		if ext.Id.Equal(oidExtensionAppleAnonymousAttestation) {
 			attExtBytes = ext.Value
 		}
 	}
@@ -81,22 +79,8 @@ func verifyAppleFormat(att AttestationObject, clientDataHash []byte, _ metadata.
 	}
 
 	// Step 5. Verify that the credential public key equals the Subject Public Key of credCert.
-	// TODO: Probably move this part to webauthncose.go.
-	pubKey, err := webauthncose.ParsePublicKey(att.AuthData.AttData.CredentialPublicKey)
-	if err != nil {
-		return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error parsing public key: %+v", err)).WithError(err)
-	}
-
-	credPK := pubKey.(webauthncose.EC2PublicKeyData)
-	subjectPK := credCert.PublicKey.(*ecdsa.PublicKey)
-	credPKInfo := &ecdsa.PublicKey{
-		Curve: elliptic.P256(),
-		X:     big.NewInt(0).SetBytes(credPK.XCoord),
-		Y:     big.NewInt(0).SetBytes(credPK.YCoord),
-	}
-
-	if !credPKInfo.Equal(subjectPK) {
-		return "", nil, ErrInvalidAttestation.WithDetails("Certificate public key does not match public key in authData")
+	if _, err = verifyAttestationECDSAPublicKeyMatch(att, credCert); err != nil {
+		return "", nil, err
 	}
 
 	// Step 6. If successful, return implementation-specific values representing attestation type Anonymization CA and attestation trust path x5c.
@@ -108,3 +92,7 @@ func verifyAppleFormat(att AttestationObject, clientDataHash []byte, _ metadata.
 type AppleAnonymousAttestation struct {
 	Nonce []byte `asn1:"tag:1,explicit"`
 }
+
+var (
+	attAppleHardwareRootsCertPool *x509.CertPool
+)