fix(protocol): compound returns incorrect type (#731)

This fixes an issue where compound attestation statement parsing returns "none" as the attestation type in all situations, instead we return the first validated type.

Author: james-d-elliott

protocol/attestation_compound.go

@@ -25,6 +25,10 @@ func init() {
 //
 // nonCompoundAttStmt = { $$attStmtType } .within { fmt: text .ne "compound", * any => any }
 //
+// §8.9 leaves the handling of a sub-statement which fails verification, and the number which must succeed, to Relying
+// Party policy. The policy applied here is the strictest available: every sub-statement must verify, and the first
+// failure rejects the attestation.
+//
 // Specification: §8.9. Compound Attestation Statement Forma
 //
 // See: https://www.w3.org/TR/webauthn-3/#sctn-compound-attestation
@@ -87,7 +91,7 @@ func attestationFormatValidationHandlerCompound(att AttestationObject, clientDat
 		}
 	}
 
-	for _, attStmt := range attStmts {
+	for i, attStmt := range attStmts {
 		object := AttestationObject{
 			Format:       attStmt.Format,
 			AttStatement: attStmt.AttStatement,
@@ -104,6 +108,12 @@ func attestationFormatValidationHandlerCompound(att AttestationObject, clientDat
 			return "", nil, err
 		}
 
+		// Every sub-statement attests the same credential, so the type conveyed by the first describes an attestation
+		// which was obtained and is the value recorded against the credential.
+		if i == 0 {
+			attestationType = subAttType
+		}
+
 		if mds == nil {
 			continue
 		}
@@ -113,5 +123,8 @@ func attestationFormatValidationHandlerCompound(att AttestationObject, clientDat
 		}
 	}
 
-	return stmtTypNone, nil, nil
+	// The trust paths are not conveyed to the caller. Each is validated against the Metadata Service above alongside
+	// the format and attestation type it belongs to, which a single chain can't describe for more than one
+	// sub-statement, and the paths of independent sub-statements joined together describe no real chain.
+	return attestationType, nil, nil
 }

protocol/attestation_compound_test.go

@@ -1,6 +1,7 @@
 package protocol
 
 import (
+	"context"
 	"errors"
 	"reflect"
 	"testing"
@@ -248,7 +249,9 @@ func TestAttestationFormatValidationHandlerCompound(t *testing.T) {
 		gotType, gotX5Cs, err := attestationFormatValidationHandlerCompound(att, []byte("hash"), nil)
 		require.NoError(t, err)
 
-		assert.Equal(t, stmtTypNone, gotType)
+		// §8.9 returns any combination of the outputs of the successful verification procedures. The type of the
+		// first sub-statement is conveyed so the credential isn't recorded as carrying no attestation.
+		assert.Equal(t, "packed-type", gotType)
 		assert.Nil(t, gotX5Cs)
 
 		require.Len(t, calls, 2)
@@ -364,10 +367,53 @@ func TestAttestationFormatValidationHandlerCompound(t *testing.T) {
 		gotType, gotX5Cs, err := attestationFormatValidationHandlerCompound(att, []byte("hash"), nil)
 		require.NoError(t, err)
 
-		assert.Equal(t, stmtTypNone, gotType)
+		assert.Equal(t, testAttTypeSome, gotType)
 		assert.Nil(t, gotX5Cs)
 		assert.Equal(t, 2, handlerCalls)
 	})
+
+	// A compound attestation which conveys stmtTypNone suppresses the attestation type validation performed by
+	// ValidateMetadata, which skips the check for that value, so the type of a sub-statement must reach it.
+	t.Run("ShouldValidateMetadataAgainstTheConveyedAttestationType", func(t *testing.T) {
+		withFreshAttestationRegistry(t)
+
+		attestationRegistry[AttestationFormatPacked] = func(att AttestationObject, clientDataHash []byte, mds metadata.Provider) (string, []any, error) {
+			return string(metadata.BasicFull), nil, nil
+		}
+
+		att := AttestationObject{
+			Format: string(AttestationFormatCompound),
+			AuthData: AuthenticatorData{
+				AttData: AttestedCredentialData{AAGUID: make([]byte, 0)},
+			},
+			AttStatement: map[string]any{
+				stmtAttStmt: []any{
+					map[string]any{stmtFmt: string(AttestationFormatPacked), stmtAttStmt: map[string]any{}},
+					map[string]any{stmtFmt: string(AttestationFormatPacked), stmtAttStmt: map[string]any{}},
+				},
+			},
+		}
+
+		gotType, _, err := attestationFormatValidationHandlerCompound(att, []byte("hash"), nil)
+		require.NoError(t, err)
+		require.NotEqual(t, stmtTypNone, gotType)
+
+		ctrl := gomock.NewController(t)
+		mds := mocks.NewMockMetadataProvider(ctrl)
+
+		entry := &metadata.Entry{
+			MetadataStatement: metadata.Statement{
+				AttestationTypes: metadata.AuthenticatorAttestationTypes{metadata.AttCA},
+			},
+		}
+
+		mds.EXPECT().GetEntry(gomock.Any(), gomock.Any()).Return(entry, nil)
+		mds.EXPECT().GetValidateAttestationTypes(gomock.Any()).Return(true)
+
+		protoErr := ValidateMetadata(context.Background(), mds, uuid.Nil, gotType, string(AttestationFormatCompound), nil)
+		require.NotNil(t, protoErr)
+		assert.Contains(t, protoErr.DevInfo, "is not known to be used by this authenticator")
+	})
 }
 
 // Supporting functions.