feat(protocol): att format updates (#485)

This updates the Android Keystore (andoird-key) and Apple Anonymous (apple) attestation format handlers. Specifically these methods now share a validator which checks the x5c leaf is the same as the credential public key, they validate the attestation x5c's against known roots, and were generally refactored to be in a spec-friendly order making them easier to validate. In addition it cleans up several areas and adds additional validations and documentation to attestation format handlers.

Author: james-d-elliott

.codecov.yml

@@ -14,6 +14,7 @@ coverage:
 
 ignore:
   - "**/coverage.txt"
+  - "testing"
 
 parsers:
   gcov:

.github/workflows/dependabot.yml

@@ -2,12 +2,14 @@ name: 'Dependabot'
 on:
   pull_request: {}
 permissions:
-  contents: 'write'
-  pull-requests: 'write'
+  contents: 'read'
 jobs:
   automerge:
     name: 'Auto-Merge'
     runs-on: 'ubuntu-latest'
+    permissions:
+      contents: 'read'
+      pull-requests: 'write'
     if: github.event.pull_request.user.login == 'dependabot[bot]' && github.repository == 'go-webauthn/webauthn'
     steps:
       - name: 'Harden Runner'
@@ -23,5 +25,5 @@ jobs:
         run: |
           gh pr merge --auto --squash "$PR_URL"
         env:
-          PR_URL: ${{github.event.pull_request.html_url}}
-          GH_TOKEN: ${{secrets.GITHUB_TOKEN}}
+          PR_URL: ${{ github.event.pull_request.html_url }}
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

go.mod

@@ -12,6 +12,7 @@ require (
 	github.com/google/uuid v1.6.0
 	github.com/mitchellh/mapstructure v1.5.0
 	github.com/stretchr/testify v1.11.0
+	go.uber.org/mock v0.6.0
 )
 
 require (

go.sum

@@ -18,6 +18,8 @@ github.com/stretchr/testify v1.11.0 h1:ib4sjIrwZKxE5u/Japgo/7SJV3PvgjGiRNAvTVGqQ
 github.com/stretchr/testify v1.11.0/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
 github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM=
 github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg=
+go.uber.org/mock v0.6.0 h1:hyF9dfmbgIX5EfOdasqLsWD6xqpNZlXblLB/Dbnwv3Y=
+go.uber.org/mock v0.6.0/go.mod h1:KiVJ4BqZJaMj4svdfmHM0AUx4NJYO8ZNpPnZn1Z+BBU=
 golang.org/x/crypto v0.41.0 h1:WKYxWedPGCTVVl5+WHSSrOBT0O8lx32+zxmHxijgXp4=
 golang.org/x/crypto v0.41.0/go.mod h1:pO5AFd7FA68rFak7rOAGVuygIISepHftHnr8dr6+sUc=
 golang.org/x/sys v0.35.0 h1:vz1N37gP5bs89s7He8XuIYXpyY0+QlsKmzipCbUtyxI=

protocol/attestation_androidkey.go

@@ -5,93 +5,101 @@ import (
 	"crypto/x509"
 	"encoding/asn1"
 	"fmt"
+	"time"
 
 	"github.com/go-webauthn/webauthn/metadata"
 	"github.com/go-webauthn/webauthn/protocol/webauthncose"
 )
 
-func init() {
-	RegisterAttestationFormat(AttestationFormatAndroidKey, verifyAndroidKeyFormat)
-}
-
-// The android-key attestation statement looks like:
-// $$attStmtType //= (
+// attestationFormatValidationHandlerAndroidKey is the handler for the Android Key Attestation Statement Format.
 //
-//	fmt: "android-key",
-//	attStmt: androidStmtFormat
+// An Android key attestation statement consists simply of the Android attestation statement, which is a series of DER
+// encoded X.509 certificates. See the Android developer documentation. Its syntax is defined as follows:
 //
-// )
+// $$attStmtType //= (
+//                       fmt: "android-key",
+//                       attStmt: androidStmtFormat
+//                   )
+//
+// androidStmtFormat = {
+//                       alg: COSEAlgorithmIdentifier,
+//                       sig: bytes,
+//                       x5c: [ credCert: bytes, * (caCert: bytes) ]
+//                     }
 //
-//	androidStmtFormat = {
-//			alg: COSEAlgorithmIdentifier,
-//			sig: bytes,
-//			x5c: [ credCert: bytes, * (caCert: bytes) ]
-//	  }
+// Specification: §8.4. Android Key Attestation Statement Format
 //
-// 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) {
+// See: https://www.w3.org/TR/webauthn/#sctn-android-key-attestation
+func attestationFormatValidationHandlerAndroidKey(att AttestationObject, clientDataHash []byte, _ metadata.Provider) (attestationType string, x5cs []any, err error) {
+	var (
+		alg int64
+		sig []byte
+		ok  bool
+	)
+	
 	// 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.
-
 	// Get the alg value - A COSEAlgorithmIdentifier containing the identifier of the algorithm
 	// used to generate the attestation signature.
-	alg, present := att.AttStatement[stmtAlgorithm].(int64)
-	if !present {
+	if alg, ok = att.AttStatement[stmtAlgorithm].(int64); !ok {
 		return "", nil, ErrAttestationFormat.WithDetails("Error retrieving alg value")
 	}
 
 	// Get the sig value - A byte string containing the attestation signature.
-	sig, present := att.AttStatement[stmtSignature].([]byte)
-	if !present {
+	if sig, ok = att.AttStatement[stmtSignature].([]byte); !ok {
 		return "", nil, ErrAttestationFormat.WithDetails("Error retrieving sig value")
 	}
 
-	// If x5c is not present, return an error.
-	x5c, x509present := att.AttStatement[stmtX5C].([]any)
-	if !x509present {
-		// Handle Basic Attestation steps for the x509 Certificate.
-		return "", nil, ErrAttestationFormat.WithDetails("Error retrieving x5c value")
-	}
-
 	// §8.4.2. Verify that sig is a valid signature over the concatenation of authenticatorData and clientDataHash
 	// using the public key in the first certificate in x5c with the algorithm specified in alg.
-	attCertBytes, valid := x5c[0].([]byte)
-	if !valid {
-		return "", nil, ErrAttestation.WithDetails("Error getting certificate from x5c cert chain")
+	var (
+		x5c   []any
+		certs []*x509.Certificate
+	)
+
+	if x5c, certs, err = attStatementParseX5CS(att.AttStatement, stmtX5C); err != nil {
+		return "", nil, err
 	}
 
-	attCert, err := x509.ParseCertificate(attCertBytes)
-	if err != nil {
-		return "", nil, ErrAttestationFormat.WithDetails(fmt.Sprintf("Error parsing certificate from ASN.1 data: %+v", err)).WithError(err)
+	if len(certs) == 0 {
+		return "", nil, ErrInvalidAttestation.WithDetails("No certificates in x5c")
+	}
+
+	credCert := certs[0]
+
+	if _, err = attStatementCertChainVerify(certs, attAndroidKeyHardwareRootsCertPool, true, time.Now().Add(time.Hour*8760).UTC()); err != nil {
+		return "", nil, ErrInvalidAttestation.WithDetails("Error validating x5c cert chain").WithError(err)
 	}
 
 	signatureData := append(att.RawAuthData, clientDataHash...) //nolint:gocritic // This is intentional.
 
-	coseAlg := webauthncose.COSEAlgorithmIdentifier(alg)
-	if err = attCert.CheckSignature(webauthncose.SigAlgFromCOSEAlg(coseAlg), signatureData, sig); err != nil {
-		return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Signature validation error: %+v\n", err)).WithError(err)
+	if sigAlg := webauthncose.SigAlgFromCOSEAlg(webauthncose.COSEAlgorithmIdentifier(alg)); sigAlg == x509.UnknownSignatureAlgorithm {
+		return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Unsupported COSE alg: %d", alg))
+	} else if err = credCert.CheckSignature(sigAlg, 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\n", err)).WithError(err)
+	var attPublicKeyData webauthncose.EC2PublicKeyData
+	if attPublicKeyData, err = verifyAttestationECDSAPublicKeyMatch(att, credCert); err != nil {
+		return "", nil, err
 	}
 
-	e := pubKey.(webauthncose.EC2PublicKeyData)
-
-	valid, err = e.Verify(signatureData, sig)
-	if err != nil || !valid {
-		return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error parsing public key: %+v\n", err)).WithError(err)
+	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)
 	}
 
 	// §8.4.3. Verify that the attestationChallenge field in the attestation certificate extension data is identical to clientDataHash.
 	// attCert.Extensions.
+	// As noted in §8.4.1 (https://www.w3.org/TR/webauthn/#key-attstn-cert-requirements) the Android Key Attestation
+	// certificate's android key attestation certificate extension data is identified by the OID
+	// "1.3.6.1.4.1.11129.2.1.17".
 	var attExtBytes []byte
 
-	for _, ext := range attCert.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
 		}
 	}
@@ -100,8 +108,6 @@ func verifyAndroidKeyFormat(att AttestationObject, clientDataHash []byte, _ meta
 		return "", nil, ErrAttestationFormat.WithDetails("Attestation certificate extensions missing 1.3.6.1.4.1.11129.2.1.17")
 	}
 
-	// As noted in §8.4.1 (https://www.w3.org/TR/webauthn/#key-attstn-cert-requirements) the Android Key Attestation attestation certificate's
-	// android key attestation certificate extension data is identified by the OID "1.3.6.1.4.1.11129.2.1.17".
 	decoded := keyDescription{}
 
 	if _, err = asn1.Unmarshal(attExtBytes, &decoded); err != nil {
@@ -114,7 +120,7 @@ func verifyAndroidKeyFormat(att AttestationObject, clientDataHash []byte, _ meta
 	}
 
 	// The AuthorizationList.allApplications field is not present on either authorization list (softwareEnforced nor teeEnforced), since PublicKeyCredential MUST be scoped to the RP ID.
-	if nil != decoded.SoftwareEnforced.AllApplications || nil != decoded.TeeEnforced.AllApplications {
+	if decoded.SoftwareEnforced.AllApplications != nil || decoded.TeeEnforced.AllApplications != nil {
 		return "", nil, ErrAttestationFormat.WithDetails("Attestation certificate extensions contains all applications field")
 	}
 
@@ -208,34 +214,47 @@ const (
 	Failed
 )
 
-/**
- * The origin of a key (or pair), i.e. where it was generated.  Note that KM_TAG_ORIGIN can be found
- * in either the hardware-enforced or software-enforced list for a key, indicating whether the key
- * is hardware or software-based.  Specifically, a key with KM_ORIGIN_GENERATED in the
- * hardware-enforced list is guaranteed never to have existed outide the secure hardware.
- */
-type KM_KEY_ORIGIN int
-
 const (
-	KM_ORIGIN_GENERATED = iota /* Generated in keymaster.  Should not exist outside the TEE. */
-	KM_ORIGIN_DERIVED          /* Derived inside keymaster.  Likely exists off-device. */
-	KM_ORIGIN_IMPORTED         /* Imported into keymaster.  Existed as clear text in Android. */
-	KM_ORIGIN_UNKNOWN          /* Keymaster did not record origin.  This value can only be seen on
-	 * keys in a keymaster0 implementation.  The keymaster0 adapter uses
-	 * this value to document the fact that it is unknown whether the key
-	 * was generated inside or imported into keymaster. */
+	// KM_ORIGIN_GENERATED means generated in keymaster. Should not exist outside the TEE.
+	KM_ORIGIN_GENERATED = iota
+
+	// KM_ORIGIN_DERIVED means derived inside keymaster. Likely exists off-device.
+	KM_ORIGIN_DERIVED
+
+	// KM_ORIGIN_IMPORTED means imported into keymaster. Existed as clear text in Android.
+	KM_ORIGIN_IMPORTED
+
+	// KM_ORIGIN_UNKNOWN means keymaster did not record origin.  This value can only be seen on keys in a keymaster0
+	// implementation. The keymaster0 adapter uses this value to document the fact that it is unknown whether the key
+	// was generated inside or imported into keymaster.
+	KM_ORIGIN_UNKNOWN
 )
 
-/**
- * Possible purposes of a key (or pair).
- */
-type KM_PURPOSE int
 
 const (
-	KM_PURPOSE_ENCRYPT    = iota /* Usable with RSA, EC and AES keys. */
-	KM_PURPOSE_DECRYPT           /* Usable with RSA, EC and AES keys. */
-	KM_PURPOSE_SIGN              /* Usable with RSA, EC and HMAC keys. */
-	KM_PURPOSE_VERIFY            /* Usable with RSA, EC and HMAC keys. */
-	KM_PURPOSE_DERIVE_KEY        /* Usable with EC keys. */
-	KM_PURPOSE_WRAP              /* Usable with wrapped keys. */
+	// KM_PURPOSE_ENCRYPT is usable with RSA, EC and AES keys.
+	KM_PURPOSE_ENCRYPT = iota
+
+	// KM_PURPOSE_DECRYPT is usable with RSA, EC and AES keys.
+	KM_PURPOSE_DECRYPT
+
+	// KM_PURPOSE_SIGN is usable with RSA, EC and HMAC keys.
+	KM_PURPOSE_SIGN
+
+	// KM_PURPOSE_VERIFY is usable with RSA, EC and HMAC keys.
+	KM_PURPOSE_VERIFY
+
+	// KM_PURPOSE_DERIVE_KEY is usable with EC keys.
+	KM_PURPOSE_DERIVE_KEY
+
+	// KM_PURPOSE_WRAP is usable with wrapped keys.
+	KM_PURPOSE_WRAP
+)
+
+var (
+	attAndroidKeyHardwareRootsCertPool *x509.CertPool
 )
+
+func init() {
+	RegisterAttestationFormat(AttestationFormatAndroidKey, attestationFormatValidationHandlerAndroidKey)
+}