fix(protocol): androidkey union generated (#729)

This adds union generated validation on the androidkey attestation which is more permissive until we add TEE-enforcement.

Author: james-d-elliott

protocol/attestation_androidkey.go

@@ -139,7 +139,25 @@ func androidKeyValidateAuthorizationLists(decoded *keyDescription) *Error {
 
 	// For the following, use only the teeEnforced authorization list if the RP wants to accept only keys from a trusted execution environment, otherwise use the union of teeEnforced and softwareEnforced.
 	// The value in the AuthorizationList.origin field is equal to KM_ORIGIN_GENERATED (which == 0).
-	if decoded.SoftwareEnforced.Origin != KM_ORIGIN_GENERATED || decoded.TeeEnforced.Origin != KM_ORIGIN_GENERATED {
+	var (
+		originTee, originSoftware   int
+		presentTee, presentSoftware bool
+		err                         error
+	)
+
+	if originTee, presentTee, err = authorizationListOrigin(&decoded.TeeEnforced); err != nil {
+		return ErrAttestationFormat.WithDetails("Unable to parse the origin of the teeEnforced authorization list").WithError(err)
+	}
+
+	if originSoftware, presentSoftware, err = authorizationListOrigin(&decoded.SoftwareEnforced); err != nil {
+		return ErrAttestationFormat.WithDetails("Unable to parse the origin of the softwareEnforced authorization list").WithError(err)
+	}
+
+	// The union is satisfied when either list carries an origin equal to KM_ORIGIN_GENERATED. An absent origin
+	// satisfies nothing as there is no value to compare against, which mirrors the purpose check below.
+	generated := (presentTee && originTee == KM_ORIGIN_GENERATED) || (presentSoftware && originSoftware == KM_ORIGIN_GENERATED)
+
+	if !generated {
 		return ErrAttestationFormat.WithDetails("Attestation certificate extensions contains authorization list with origin not equal KM_ORIGIN_GENERATED")
 	}
 
@@ -151,6 +169,29 @@ func androidKeyValidateAuthorizationLists(decoded *keyDescription) *Error {
 	return nil
 }
 
+// authorizationListOrigin returns the origin of an authorization list and reports whether the field was present. The
+// value is decoded from the raw element because encoding/asn1 leaves an absent optional integer at its zero value,
+// which is indistinguishable from a present origin of KM_ORIGIN_GENERATED.
+func authorizationListOrigin(list *authorizationList) (origin int, present bool, err error) {
+	// An explicit tag which is present always carries a child as encoding/asn1 rejects one which doesn't while
+	// decoding the key description, so an empty raw value means the field was absent rather than empty.
+	if len(list.Origin.FullBytes) == 0 {
+		return 0, false, nil
+	}
+
+	var rest []byte
+
+	if rest, err = asn1.Unmarshal(list.Origin.Bytes, &origin); err != nil {
+		return 0, false, err
+	}
+
+	if len(rest) != 0 {
+		return 0, false, fmt.Errorf("origin has %d bytes of trailing data", len(rest))
+	}
+
+	return origin, true, nil
+}
+
 func contains(s []int, e int) bool {
 	for _, a := range s {
 		if a == e {
@@ -194,21 +235,25 @@ type authorizationList struct {
 	AllApplications             asn1.RawValue `asn1:"tag:600,explicit,optional"`
 	ApplicationID               asn1.RawValue `asn1:"tag:601,explicit,optional"`
 	CreationDateTime            int           `asn1:"tag:701,explicit,optional"`
-	Origin                      int           `asn1:"tag:702,explicit,optional"`
-	RootOfTrust                 rootOfTrust   `asn1:"tag:704,explicit,optional"`
-	OsVersion                   int           `asn1:"tag:705,explicit,optional"`
-	OsPatchLevel                int           `asn1:"tag:706,explicit,optional"`
-	AttestationApplicationID    []byte        `asn1:"tag:709,explicit,optional"`
-	AttestationIDBrand          []byte        `asn1:"tag:710,explicit,optional"`
-	AttestationIDDevice         []byte        `asn1:"tag:711,explicit,optional"`
-	AttestationIDProduct        []byte        `asn1:"tag:712,explicit,optional"`
-	AttestationIDSerial         []byte        `asn1:"tag:713,explicit,optional"`
-	AttestationIDImei           []byte        `asn1:"tag:714,explicit,optional"`
-	AttestationIDMeid           []byte        `asn1:"tag:715,explicit,optional"`
-	AttestationIDManufacturer   []byte        `asn1:"tag:716,explicit,optional"`
-	AttestationIDModel          []byte        `asn1:"tag:717,explicit,optional"`
-	VendorPatchLevel            int           `asn1:"tag:718,explicit,optional"`
-	BootPatchLevel              int           `asn1:"tag:719,explicit,optional"`
+	// Origin is decoded as a raw element rather than an integer. encoding/asn1 leaves an absent optional integer at
+	// its zero value, which is indistinguishable from a present origin of KM_ORIGIN_GENERATED.
+	Origin asn1.RawValue `asn1:"tag:702,explicit,optional"`
+
+	RootOfTrust rootOfTrust `asn1:"tag:704,explicit,optional"`
+
+	OsVersion                 int    `asn1:"tag:705,explicit,optional"`
+	OsPatchLevel              int    `asn1:"tag:706,explicit,optional"`
+	AttestationApplicationID  []byte `asn1:"tag:709,explicit,optional"`
+	AttestationIDBrand        []byte `asn1:"tag:710,explicit,optional"`
+	AttestationIDDevice       []byte `asn1:"tag:711,explicit,optional"`
+	AttestationIDProduct      []byte `asn1:"tag:712,explicit,optional"`
+	AttestationIDSerial       []byte `asn1:"tag:713,explicit,optional"`
+	AttestationIDImei         []byte `asn1:"tag:714,explicit,optional"`
+	AttestationIDMeid         []byte `asn1:"tag:715,explicit,optional"`
+	AttestationIDManufacturer []byte `asn1:"tag:716,explicit,optional"`
+	AttestationIDModel        []byte `asn1:"tag:717,explicit,optional"`
+	VendorPatchLevel          int    `asn1:"tag:718,explicit,optional"`
+	BootPatchLevel            int    `asn1:"tag:719,explicit,optional"`
 }
 
 type rootOfTrust struct {

protocol/attestation_androidkey_test.go

@@ -8,6 +8,7 @@ import (
 	"crypto/x509"
 	"encoding/asn1"
 	"encoding/hex"
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -355,12 +356,192 @@ func TestAndroidKeyAuthorizationListDecoding(t *testing.T) {
 	assert.Len(t, decoded.TeeEnforced.RootOfTrust.VerifiedBootHash, 32)
 
 	assert.Equal(t, []int{KM_PURPOSE_SIGN}, decoded.TeeEnforced.Purpose)
-	assert.Equal(t, KM_ORIGIN_GENERATED, decoded.TeeEnforced.Origin)
+
+	origin, present, err := authorizationListOrigin(&decoded.TeeEnforced)
+	require.NoError(t, err)
+	assert.True(t, present)
+	assert.Equal(t, KM_ORIGIN_GENERATED, origin)
+
+	// A real attestation carries origin on one list only, which is why an absent origin decoding to the zero value
+	// went unnoticed.
+	_, present, err = authorizationListOrigin(&decoded.SoftwareEnforced)
+	require.NoError(t, err)
+	assert.False(t, present)
 
 	assert.Empty(t, decoded.TeeEnforced.AllApplications.FullBytes)
 	assert.Empty(t, decoded.SoftwareEnforced.AllApplications.FullBytes)
 }
 
+// TestAndroidKeyAuthorizationListOrigin asserts the §8.4 origin requirement is evaluated against the union of the two
+// authorization lists, matching the adjacent purpose requirement, and that an absent origin does not satisfy it. An
+// optional integer decodes to zero when absent, which is the value of KM_ORIGIN_GENERATED.
+func TestAndroidKeyAuthorizationListOrigin(t *testing.T) {
+	// SEQUENCE { [1] EXPLICIT SET { INTEGER 2 } [, [702] EXPLICIT INTEGER n] } where the purpose is KM_PURPOSE_SIGN.
+	list := func(t *testing.T, origin string) authorizationList {
+		t.Helper()
+
+		body := "a1053103020102"
+
+		if origin != "" {
+			body += "bf853e030201" + origin
+		}
+
+		der, err := hex.DecodeString(fmt.Sprintf("30%02x", len(body)/2) + body)
+		require.NoError(t, err)
+
+		var decoded authorizationList
+
+		rest, err := asn1.Unmarshal(der, &decoded)
+		require.NoError(t, err)
+		require.Empty(t, rest)
+
+		return decoded
+	}
+
+	generated := list(t, "00") // KM_ORIGIN_GENERATED.
+	imported := list(t, "02")  // KM_ORIGIN_IMPORTED.
+	missing := list(t, "")
+
+	testCases := []struct {
+		name     string
+		tee      authorizationList
+		software authorizationList
+		err      string
+	}{
+		{
+			name: "ShouldAcceptWhenGeneratedInTeeAndAbsentFromSoftware",
+			tee:  generated, software: missing,
+		},
+		{
+			name: "ShouldAcceptWhenGeneratedInSoftwareAndAbsentFromTee",
+			tee:  missing, software: generated,
+		},
+		{
+			name: "ShouldAcceptWhenGeneratedInBoth",
+			tee:  generated, software: generated,
+		},
+		{
+			// The union is satisfied by the teeEnforced list. The previous implementation required both lists to
+			// agree, which rejected this.
+			name: "ShouldAcceptWhenGeneratedInTeeAndImportedInSoftware",
+			tee:  generated, software: imported,
+		},
+		{
+			name: "ShouldRejectWhenImportedInBoth",
+			tee:  imported, software: imported,
+			err: "Attestation certificate extensions contains authorization list with origin not equal KM_ORIGIN_GENERATED",
+		},
+		{
+			// Neither list states an origin, so there is no value equal to KM_ORIGIN_GENERATED to verify.
+			name: "ShouldRejectWhenAbsentFromBothLists",
+			tee:  missing, software: missing,
+			err: "Attestation certificate extensions contains authorization list with origin not equal KM_ORIGIN_GENERATED",
+		},
+		{
+			name: "ShouldRejectWhenImportedInTeeAndAbsentFromSoftware",
+			tee:  imported, software: missing,
+			err: "Attestation certificate extensions contains authorization list with origin not equal KM_ORIGIN_GENERATED",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			decoded := keyDescription{TeeEnforced: tc.tee, SoftwareEnforced: tc.software}
+
+			protoErr := androidKeyValidateAuthorizationLists(&decoded)
+
+			if tc.err != "" {
+				require.NotNil(t, protoErr)
+				assert.EqualError(t, protoErr, tc.err)
+			} else {
+				assert.Nil(t, protoErr)
+			}
+		})
+	}
+}
+
+func TestAuthorizationListOriginDER(t *testing.T) {
+	const purpose = "a1053103020102" // [1] EXPLICIT SET { INTEGER 2 }.
+
+	sequence := func(t *testing.T, body string) []byte {
+		t.Helper()
+
+		der, err := hex.DecodeString(fmt.Sprintf("30%02x", len(body)/2) + body)
+		require.NoError(t, err)
+
+		return der
+	}
+
+	t.Run("ShouldRejectPresentButEmptyOriginWhileDecoding", func(t *testing.T) {
+		var list authorizationList
+
+		_, err := asn1.Unmarshal(sequence(t, purpose+"bf853e00"), &list)
+		require.EqualError(t, err, "asn1: structure error: explicit tag has no child")
+	})
+
+	testCases := []struct {
+		name    string
+		origin  string
+		origins int
+		present bool
+		err     string
+	}{
+		{
+			name:   "ShouldReportAbsentWhenOriginMissing",
+			origin: "",
+		},
+		{
+			name:    "ShouldDecodeGenerated",
+			origin:  "bf853e03020100",
+			origins: KM_ORIGIN_GENERATED,
+			present: true,
+		},
+		{
+			name:    "ShouldDecodeImported",
+			origin:  "bf853e03020102",
+			origins: KM_ORIGIN_IMPORTED,
+			present: true,
+		},
+		{
+			// [702] EXPLICIT { INTEGER 0, NULL }. The value decodes but a NULL follows it inside the wrapper, which
+			// was previously discarded along with the remainder returned by asn1.Unmarshal.
+			name:   "ShouldRejectTrailingDataAfterOrigin",
+			origin: "bf853e05" + "020100" + "0500",
+			err:    "origin has 2 bytes of trailing data",
+		},
+		{
+			// [702] EXPLICIT { INTEGER 0, INTEGER 2 }, so the trailing data is itself a valid origin value.
+			name:   "ShouldRejectTrailingOriginValue",
+			origin: "bf853e06" + "020100" + "020102",
+			err:    "origin has 3 bytes of trailing data",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			var list authorizationList
+
+			rest, err := asn1.Unmarshal(sequence(t, purpose+tc.origin), &list)
+			require.NoError(t, err)
+			require.Empty(t, rest)
+
+			origin, present, err := authorizationListOrigin(&list)
+
+			if tc.err != "" {
+				require.EqualError(t, err, tc.err)
+				assert.False(t, present)
+				assert.Equal(t, 0, origin)
+
+				return
+			}
+
+			require.NoError(t, err)
+			assert.Equal(t, tc.present, present)
+			assert.Equal(t, tc.origins, origin)
+		})
+	}
+}
+
 func TestAndroidKeyAuthorizationListAllApplicationsDER(t *testing.T) {
 	// SEQUENCE { [1] EXPLICIT SET { INTEGER 2 }, [702] EXPLICIT INTEGER 0 }
 	// purpose = KM_PURPOSE_SIGN, origin = KM_ORIGIN_GENERATED.
@@ -386,8 +567,13 @@ func TestAndroidKeyAuthorizationListAllApplicationsDER(t *testing.T) {
 	// Both lists must otherwise be identical, so a failure below is attributable to allApplications alone.
 	require.Equal(t, []int{KM_PURPOSE_SIGN}, absent.Purpose)
 	require.Equal(t, []int{KM_PURPOSE_SIGN}, present.Purpose)
-	require.Equal(t, KM_ORIGIN_GENERATED, absent.Origin)
-	require.Equal(t, KM_ORIGIN_GENERATED, present.Origin)
+
+	for _, list := range []*authorizationList{&absent, &present} {
+		origin, ok, err := authorizationListOrigin(list)
+		require.NoError(t, err)
+		require.True(t, ok)
+		require.Equal(t, KM_ORIGIN_GENERATED, origin)
+	}
 
 	assert.Empty(t, absent.AllApplications.FullBytes)
 	assert.NotEmpty(t, present.AllApplications.FullBytes, "allApplications must be detectable in the decoded list")

protocol/specification_vectors_e2e_test.go

@@ -235,7 +235,9 @@ func TestSpecVectors_Registration_E2E(t *testing.T) {
 			format:                      "android-key",
 			credParams:                  []CredentialParameter{{Type: PublicKeyCredentialType, Algorithm: webauthncose.AlgES256}},
 			rpTopOriginVerificationMode: TopOriginExplicitVerificationMode,
-			err:                         "Attestation certificate extensions contains authorization list with purpose not equal KM_PURPOSE_SIGN",
+			// Both authorization lists of this vector are empty sequences, so it satisfies neither the origin nor the
+			// purpose requirement of §8.4. The origin is reported as it is the first of the two in the specification.
+			err: "Attestation certificate extensions contains authorization list with origin not equal KM_ORIGIN_GENERATED",
 		},
 		{
 			// §16.15 Apple Anonymous Attestation - ES256 - Top Origin