From 4075f9678e29b08f7db0949af07df5f90c486707 Mon Sep 17 00:00:00 2001 From: "yingran.xu" Date: Fri, 9 May 2025 13:46:02 -0700 Subject: [PATCH 1/2] test(protocol): add test cases to authenticator_test Change-Id: Ie3459891e5ee08ceffc8d0939cbe60e2ecde3333 --- protocol/authenticator_test.go | 287 +++++++++++++++++++++++++++++---- 1 file changed, 257 insertions(+), 30 deletions(-) diff --git a/protocol/authenticator_test.go b/protocol/authenticator_test.go index af03c040..2d951b04 100644 --- a/protocol/authenticator_test.go +++ b/protocol/authenticator_test.go @@ -2,8 +2,19 @@ package protocol import ( "encoding/base64" + "encoding/binary" + "encoding/hex" + "fmt" "reflect" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + noneAuthDataBase64 = "pkLSG3xtVeHOI8U5mCjSx0m/am7y/gPMnhDN9O1ttItBAAAAAAAAAAAAAAAAAAAAAAAAAAAAQMAxl6G32ykWaLrv/ouCs5HoGsvONqBtOb7ZmyMs8K8PccnwyyqPzWn/yZuyQmQBguvjYSvH6gDBlFG65quUDCSlAQIDJiABIVggyJGP+ra/u/eVjqN4OeYXUShRWxrEeC6Sb5/bZmJ9q8MiWCCHIkRdg5oRb1RHoFVYUpogcjlObCKFsV1ls1T+uUc6rA==" + attAuthDataBase64 = "lWkIjx7O4yMpVANdvRDXyuORMFonUbVZu4/Xy7IpvdRBAAAAAAAAAAAAAAAAAAAAAAAAAAAAQIniszxcGnhupdPFOHJIm6dscrWCC2h8xHicBMu91THD0kdOdB0QQtkaEn+6KfsfT1o3NmmFT8YfXrG734WfVSmlAQIDJiABIVggyoHHeiUw5aSbt8/GsL9zaqZGRzV26A4y3CnCGUhVXu4iWCBMnc8za5xgPzIygngAv9W+vZTMGJwwZcM4sjiqkcb/1g==" ) func TestAuthenticatorFlags_UserPresent(t *testing.T) { @@ -147,30 +158,104 @@ func TestAuthenticatorData_Unmarshal(t *testing.T) { rawAuthData []byte } - noneAuthData, _ := base64.StdEncoding.DecodeString("pkLSG3xtVeHOI8U5mCjSx0m/am7y/gPMnhDN9O1TCItBAAAAAAAAAAAAAAAAAAAAAAAAAAAAQMAxl6G32ykWaLrv/ouCs5HoGsvONqBtOb7ZmyMs8K8PccnwyyqPzWn/yZuyQmQBguvjYSvH6gDBlFG65quUDCSlAQIDJiABIVggyJGP+ra/u/eVjqN4OeYXUShRWxrEeC6Sb5/bZmJ9q8MiWCCHIkRdg5oRb1RHoFVYUpogcjlObCKFsV1ls1T+uUc6rA==") - attAuthData, _ := base64.StdEncoding.DecodeString("lWkIjx7O4yMpVANdvRDXyuORMFonUbVZu4/Xy7IpvdRBAAAAAAAAAAAAAAAAAAAAAAAAAAAAQIniszxcGnhupdPFOHJIm6dscrWCC2h8xHicBMu91THD0kdOdB0QQtkaEn+6KfsfT1o3NmmFT8YfXrG734WfVSmlAQIDJiABIVggyoHHeiUw5aSbt8/GsL9zaqZGRzV26A4y3CnCGUhVXu4iWCBMnc8za5xgPzIygngAv9W+vZTMGJwwZcM4sjiqkcb/1g==") + noneAuthData, _ := base64.StdEncoding.DecodeString(noneAuthDataBase64) + attAuthData, _ := base64.StdEncoding.DecodeString(attAuthDataBase64) + // Empty data + badAuthData1 := []byte{} + // Attested credential data missing + badAuthData2 := make([]byte, minAttestedAuthLength-1) + copy(badAuthData2, attAuthData) + // Flags not set but data exists + badAuthData3 := make([]byte, len(attAuthData)) + copy(badAuthData3, attAuthData) + badAuthData3[32] &= 0b0011_1111 + // Extensions data missing + badAuthData4 := make([]byte, len(attAuthData)) + copy(badAuthData4, attAuthData) + badAuthData4[32] |= 0b1000_0000 + // Leftover bytes + badAuthData5 := make([]byte, len(attAuthData)) + copy(badAuthData5, attAuthData) + badAuthData5 = append(badAuthData5, []byte("Hello World")...) tests := []struct { - name string - fields fields - args args - wantErr bool + name string + fields fields + args args + + errString string + errType string + errDetails string + errInfo string }{ { - "None Marshall Successfully", - fields{}, - args{ + name: "None Marshall Successfully", + fields: fields{}, + args: args{ noneAuthData, }, - false, }, { - "Att Data Marshall Successfully", - fields{}, - args{ + name: "Att Data Marshall Successfully", + fields: fields{}, + args: args{ attAuthData, }, - false, + }, + { + name: "Authenticator data too short", + fields: fields{}, + args: args{ + badAuthData1, + }, + errString: "Authenticator data length too short", + errType: "invalid_request", + errDetails: "Authenticator data length too short", + errInfo: fmt.Sprintf("Expected data greater than %d bytes. Got %d bytes", minAuthDataLength, len(badAuthData1)), + }, + { + name: "Attested credential missing", + fields: fields{}, + args: args{ + badAuthData2, + }, + errString: "Attested credential flag set but data is missing", + errType: "invalid_request", + errDetails: "Attested credential flag set but data is missing", + errInfo: "", + }, + { + name: "Attested credential missing", + fields: fields{}, + args: args{ + badAuthData3, + }, + errString: "Attested credential flag not set", + errType: "invalid_request", + errDetails: "Attested credential flag not set", + errInfo: "", + }, + { + name: "Extensions data missing", + fields: fields{}, + args: args{ + badAuthData4, + }, + errString: "Extensions flag set but extensions data is missing", + errType: "invalid_request", + errDetails: "Extensions flag set but extensions data is missing", + errInfo: "", + }, + { + name: "Leftover bytes", + fields: fields{}, + args: args{ + badAuthData5, + }, + errString: "Leftover bytes decoding AuthenticatorData", + errType: "invalid_request", + errDetails: "Leftover bytes decoding AuthenticatorData", + errInfo: "", }, } @@ -183,9 +268,17 @@ func TestAuthenticatorData_Unmarshal(t *testing.T) { AttData: tt.fields.AttData, ExtData: tt.fields.ExtData, } - if err := a.Unmarshal(tt.args.rawAuthData); (err != nil) != tt.wantErr { - t.Errorf("AuthenticatorData.Unmarshal() error = %v, wantErr %v", err, tt.wantErr) + + err := a.Unmarshal(tt.args.rawAuthData) + if tt.errString != "" { + assert.EqualError(t, err, tt.errString) + + AssertIsProtocolError(t, err, tt.errType, tt.errDetails, tt.errInfo) + + return } + + require.NoError(t, err) }) } } @@ -203,13 +296,78 @@ func TestAuthenticatorData_unmarshalAttestedData(t *testing.T) { rawAuthData []byte } + noneAuthData, _ := base64.StdEncoding.DecodeString(noneAuthDataBase64) + attAuthData, _ := base64.StdEncoding.DecodeString(attAuthDataBase64) + // Data length too short + badAuthData1 := make([]byte, len(attAuthData)) + copy(badAuthData1, attAuthData) + binary.BigEndian.PutUint16(badAuthData1[53:], 256) + // ID length too long + badAuthData2 := make([]byte, len(attAuthData)+maxCredentialIDLength+1) + copy(badAuthData2, attAuthData) + binary.BigEndian.PutUint16(badAuthData2[53:], maxCredentialIDLength+1) + // Malformed public key + badAuthData3 := make([]byte, 119) + copy(badAuthData3, attAuthData[:119]) + badData, _ := hex.DecodeString("83FF20030102") + badAuthData3 = append(badAuthData3, badData...) + tests := []struct { - name string - fields fields - args args - wantErr bool + name string + fields fields + args args + errString string + errType string + errDetails string + errInfo string }{ - // TODO: Add test cases. + { + name: "None Marshall Successfully", + fields: fields{}, + args: args{ + noneAuthData, + }, + }, + { + name: "Att Data Marshall Successfully", + fields: fields{}, + args: args{ + attAuthData, + }, + }, + { + name: "Data length too short", + fields: fields{}, + args: args{ + badAuthData1, + }, + errString: "Authenticator attestation data length too short", + errType: "invalid_request", + errDetails: "Authenticator attestation data length too short", + errInfo: "", + }, + { + name: "ID length too long", + fields: fields{}, + args: args{ + badAuthData2, + }, + errString: "Authenticator attestation data credential id length too long", + errType: "invalid_request", + errDetails: "Authenticator attestation data credential id length too long", + errInfo: "", + }, + { + name: "Could not unmarshal Credential Public Key", + fields: fields{}, + args: args{ + badAuthData3, + }, + errString: "Could not unmarshal Credential Public Key: cbor: unexpected \"break\" code", + errType: "invalid_request", + errDetails: "Could not unmarshal Credential Public Key: cbor: unexpected \"break\" code", + errInfo: "", + }, } for _, tt := range tests { @@ -221,9 +379,16 @@ func TestAuthenticatorData_unmarshalAttestedData(t *testing.T) { AttData: tt.fields.AttData, ExtData: tt.fields.ExtData, } - if err := a.unmarshalAttestedData(tt.args.rawAuthData); (err != nil) != tt.wantErr { - t.Errorf("AuthenticatorData.unmarshalAttestedData() error = %v, wantErr %v", err, tt.wantErr) + err := a.unmarshalAttestedData(tt.args.rawAuthData) + if tt.errString != "" { + assert.EqualError(t, err, tt.errString) + + AssertIsProtocolError(t, err, tt.errType, tt.errDetails, tt.errInfo) + + return } + + require.NoError(t, err) }) } } @@ -269,12 +434,67 @@ func TestAuthenticatorData_Verify(t *testing.T) { } tests := []struct { - name string - fields fields - args args - wantErr bool + name string + fields fields + args args + errString string + errType string + errDetails string + errInfo string }{ - // TODO: Add test cases. + { + name: "Success", + fields: fields{ + RPIDHash: []byte{1, 2, 3}, + Flags: AuthenticatorFlags(0x05), + }, + args: args{ + rpIdHash: []byte{1, 2, 3}, + }, + errString: "", + }, + { + name: "RP hash mismatch", + fields: fields{ + RPIDHash: []byte{0xff}, + }, + args: args{ + rpIdHash: []byte{0xaa}, + }, + errString: "Error validating the authenticator response", + errType: "verification_error", + errDetails: "Error validating the authenticator response", + errInfo: "RP Hash mismatch. Expected ff and Received aa", + }, + { + name: "UP flag not set", + fields: fields{ + RPIDHash: []byte{1, 2, 3}, + Flags: AuthenticatorFlags(0x04), + }, + args: args{ + rpIdHash: []byte{1, 2, 3}, + }, + errString: "Error validating the authenticator response", + errType: "verification_error", + errDetails: "Error validating the authenticator response", + errInfo: "User presence flag not set by authenticator\n", + }, + { + name: "User verification required", + fields: fields{ + RPIDHash: []byte{1, 2, 3}, + Flags: AuthenticatorFlags(0x01), + }, + args: args{ + rpIdHash: []byte{1, 2, 3}, + userVerificationRequired: true, + }, + errString: "Error validating the authenticator response", + errType: "verification_error", + errDetails: "Error validating the authenticator response", + errInfo: "User verification required but flag not set by authenticator\n", + }, } for _, tt := range tests { @@ -286,9 +506,16 @@ func TestAuthenticatorData_Verify(t *testing.T) { AttData: tt.fields.AttData, ExtData: tt.fields.ExtData, } - if err := a.Verify(tt.args.rpIdHash, nil, tt.args.userVerificationRequired); (err != nil) != tt.wantErr { - t.Errorf("AuthenticatorData.Verify() error = %v, wantErr %v", err, tt.wantErr) + err := a.Verify(tt.args.rpIdHash, nil, tt.args.userVerificationRequired) + if tt.errString != "" { + assert.EqualError(t, err, tt.errString) + + AssertIsProtocolError(t, err, tt.errType, tt.errDetails, tt.errInfo) + + return } + + require.NoError(t, err) }) } } From f0be60b337ea8e42b9c77ac81059da32177796ab Mon Sep 17 00:00:00 2001 From: "yingran.xu" Date: Fri, 9 May 2025 13:46:12 -0700 Subject: [PATCH 2/2] refactor(protocol): remove newline in some authenticator data verify errors Change-Id: I6a146c9114550fa6bad9e3376602f3fd06fa0270 --- protocol/authenticator.go | 4 ++-- protocol/authenticator_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/protocol/authenticator.go b/protocol/authenticator.go index c27726d2..d3c351af 100644 --- a/protocol/authenticator.go +++ b/protocol/authenticator.go @@ -397,14 +397,14 @@ func (a *AuthenticatorData) Verify(rpIdHash []byte, appIDHash []byte, userVerifi // Registration Step 14 & Assertion Step 16 // Verify that the User Present bit of the flags in authData is set. if !a.Flags.UserPresent() { - return ErrVerification.WithInfo(fmt.Sprintln("User presence flag not set by authenticator")) + return ErrVerification.WithInfo(fmt.Sprintf("User presence flag not set by authenticator")) } // Registration Step 15 & Assertion Step 17 // If user verification is required for this assertion, verify that // the User Verified bit of the flags in authData is set. if userVerificationRequired && !a.Flags.UserVerified() { - return ErrVerification.WithInfo(fmt.Sprintln("User verification required but flag not set by authenticator")) + return ErrVerification.WithInfo(fmt.Sprintf("User verification required but flag not set by authenticator")) } // Registration Step 17 & Assertion Step 18 diff --git a/protocol/authenticator_test.go b/protocol/authenticator_test.go index 2d951b04..2765dcf6 100644 --- a/protocol/authenticator_test.go +++ b/protocol/authenticator_test.go @@ -478,7 +478,7 @@ func TestAuthenticatorData_Verify(t *testing.T) { errString: "Error validating the authenticator response", errType: "verification_error", errDetails: "Error validating the authenticator response", - errInfo: "User presence flag not set by authenticator\n", + errInfo: "User presence flag not set by authenticator", }, { name: "User verification required", @@ -493,7 +493,7 @@ func TestAuthenticatorData_Verify(t *testing.T) { errString: "Error validating the authenticator response", errType: "verification_error", errDetails: "Error validating the authenticator response", - errInfo: "User verification required but flag not set by authenticator\n", + errInfo: "User verification required but flag not set by authenticator", }, }