From aca8350a3475a581d12ef620a557dc68bd4911d6 Mon Sep 17 00:00:00 2001 From: Daichi Shinozaki Date: Thu, 19 Jun 2025 18:36:54 +0900 Subject: [PATCH] fix(protocol): conditional create fail This fixes conditional create would fail. In this case, UP and UV flags will always false. According to WebAuthn L3 spec, UP flag will be verified when options.mediation is not set to "conditional". Step 15: https://www.w3.org/TR/webauthn-3/#sctn-registering-a-new-credential Related to #361 --- protocol/assertion.go | 4 ++-- protocol/assertion_test.go | 2 +- protocol/attestation.go | 4 ++-- protocol/attestation_test.go | 2 +- protocol/authenticator.go | 10 +++++----- protocol/authenticator_test.go | 9 ++++++--- protocol/credential.go | 4 ++-- protocol/credential_test.go | 2 +- webauthn/login.go | 3 ++- webauthn/registration.go | 4 +++- webauthn/types.go | 7 ++++--- 11 files changed, 29 insertions(+), 22 deletions(-) diff --git a/protocol/assertion.go b/protocol/assertion.go index f3caa91..ddd003f 100644 --- a/protocol/assertion.go +++ b/protocol/assertion.go @@ -139,7 +139,7 @@ func (car CredentialAssertionResponse) Parse() (par *ParsedCredentialAssertionDa // documentation. // // Specification: §7.2 Verifying an Authentication Assertion (https://www.w3.org/TR/webauthn/#sctn-verifying-assertion) -func (p *ParsedCredentialAssertionData) Verify(storedChallenge string, relyingPartyID string, rpOrigins, rpTopOrigins []string, rpTopOriginsVerify TopOriginVerificationMode, appID string, verifyUser bool, credentialBytes []byte) error { +func (p *ParsedCredentialAssertionData) Verify(storedChallenge string, relyingPartyID string, rpOrigins, rpTopOrigins []string, rpTopOriginsVerify TopOriginVerificationMode, appID string, verifyUser bool, verifyUserPresence bool, credentialBytes []byte) error { // Steps 4 through 6 in verifying the assertion data (https://www.w3.org/TR/webauthn/#verifying-assertion) are // "assertive" steps, i.e. "Let JSONtext be the result of running UTF-8 decode on the value of cData." // We handle these steps in part as we verify but also beforehand @@ -160,7 +160,7 @@ func (p *ParsedCredentialAssertionData) Verify(storedChallenge string, relyingPa } // Handle steps 11 through 14, verifying the authenticator data. - validError = p.Response.AuthenticatorData.Verify(rpIDHash[:], appIDHash[:], verifyUser) + validError = p.Response.AuthenticatorData.Verify(rpIDHash[:], appIDHash[:], verifyUser, verifyUserPresence) if validError != nil { return validError } diff --git a/protocol/assertion_test.go b/protocol/assertion_test.go index c93f467..cb101d2 100644 --- a/protocol/assertion_test.go +++ b/protocol/assertion_test.go @@ -180,7 +180,7 @@ func TestParsedCredentialAssertionData_Verify(t *testing.T) { Raw: tt.fields.Raw, } - if err := p.Verify(tt.args.storedChallenge.String(), tt.args.relyingPartyID, tt.args.relyingPartyOrigin, nil, TopOriginIgnoreVerificationMode, "", tt.args.verifyUser, tt.args.credentialBytes); (err != nil) != tt.wantErr { + if err := p.Verify(tt.args.storedChallenge.String(), tt.args.relyingPartyID, tt.args.relyingPartyOrigin, nil, TopOriginIgnoreVerificationMode, "", tt.args.verifyUser, false, tt.args.credentialBytes); (err != nil) != tt.wantErr { t.Errorf("ParsedCredentialAssertionData.Verify() error = %v, wantErr %v", err, tt.wantErr) } }) diff --git a/protocol/attestation.go b/protocol/attestation.go index 2d7d3af..0bf8aae 100644 --- a/protocol/attestation.go +++ b/protocol/attestation.go @@ -128,11 +128,11 @@ func (ccr *AuthenticatorAttestationResponse) Parse() (p *ParsedAttestationRespon // // Steps 13 through 15 are verified against the auth data. These steps are identical to 15 through 18 for assertion so we // handle them with AuthData. -func (a *AttestationObject) Verify(relyingPartyID string, clientDataHash []byte, userVerificationRequired bool, mds metadata.Provider, credParams []CredentialParameter) (err error) { +func (a *AttestationObject) Verify(relyingPartyID string, clientDataHash []byte, userVerificationRequired bool, userPresenceRequired bool, mds metadata.Provider, credParams []CredentialParameter) (err error) { rpIDHash := sha256.Sum256([]byte(relyingPartyID)) // Begin Step 13 through 15. Verify that the rpIdHash in authData is the SHA-256 hash of the RP ID expected by the RP. - if err = a.AuthData.Verify(rpIDHash[:], nil, userVerificationRequired); err != nil { + if err = a.AuthData.Verify(rpIDHash[:], nil, userVerificationRequired, userPresenceRequired); err != nil { return err } diff --git a/protocol/attestation_test.go b/protocol/attestation_test.go index 8ccb1f6..d3be883 100644 --- a/protocol/attestation_test.go +++ b/protocol/attestation_test.go @@ -29,7 +29,7 @@ func TestAttestationVerify(t *testing.T) { pcc.Response = *parsedAttestationResponse - _, err = pcc.Verify(options.Response.Challenge.String(), false, options.Response.RelyingParty.ID, []string{options.Response.RelyingParty.Name}, nil, TopOriginIgnoreVerificationMode, nil, options.Response.Parameters) + _, err = pcc.Verify(options.Response.Challenge.String(), false, false, options.Response.RelyingParty.ID, []string{options.Response.RelyingParty.Name}, nil, TopOriginIgnoreVerificationMode, nil, options.Response.Parameters) require.NoError(t, err) }) diff --git a/protocol/authenticator.go b/protocol/authenticator.go index d3c351a..ac75b15 100644 --- a/protocol/authenticator.go +++ b/protocol/authenticator.go @@ -385,7 +385,7 @@ func ResidentKeyNotRequired() *bool { // Verify on AuthenticatorData handles Steps 13 through 15 & 17 for Registration // and Steps 15 through 18 for Assertion. -func (a *AuthenticatorData) Verify(rpIdHash []byte, appIDHash []byte, userVerificationRequired bool) error { +func (a *AuthenticatorData) Verify(rpIdHash []byte, appIDHash []byte, userVerificationRequired bool, userPresenceRequired bool) error { // Registration Step 13 & Assertion Step 15 // Verify that the RP ID hash in authData is indeed the SHA-256 @@ -394,17 +394,17 @@ func (a *AuthenticatorData) Verify(rpIdHash []byte, appIDHash []byte, userVerifi return ErrVerification.WithInfo(fmt.Sprintf("RP Hash mismatch. Expected %x and Received %x", a.RPIDHash, rpIdHash)) } - // Registration Step 14 & Assertion Step 16 + // Registration Step 15 & Assertion Step 16 // Verify that the User Present bit of the flags in authData is set. - if !a.Flags.UserPresent() { - return ErrVerification.WithInfo(fmt.Sprintf("User presence flag not set by authenticator")) + if userPresenceRequired && !a.Flags.UserPresent() { + return ErrVerification.WithInfo("User presence required but 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.Sprintf("User verification required but flag not set by authenticator")) + return ErrVerification.WithInfo("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 2765dcf..3710741 100644 --- a/protocol/authenticator_test.go +++ b/protocol/authenticator_test.go @@ -431,6 +431,7 @@ func TestAuthenticatorData_Verify(t *testing.T) { type args struct { rpIdHash []byte userVerificationRequired bool + userPresenceRequired bool } tests := []struct { @@ -473,12 +474,13 @@ func TestAuthenticatorData_Verify(t *testing.T) { Flags: AuthenticatorFlags(0x04), }, args: args{ - rpIdHash: []byte{1, 2, 3}, + rpIdHash: []byte{1, 2, 3}, + userPresenceRequired: true, }, errString: "Error validating the authenticator response", errType: "verification_error", errDetails: "Error validating the authenticator response", - errInfo: "User presence flag not set by authenticator", + errInfo: "User presence required but flag not set by authenticator", }, { name: "User verification required", @@ -489,6 +491,7 @@ func TestAuthenticatorData_Verify(t *testing.T) { args: args{ rpIdHash: []byte{1, 2, 3}, userVerificationRequired: true, + userPresenceRequired: true, }, errString: "Error validating the authenticator response", errType: "verification_error", @@ -506,7 +509,7 @@ func TestAuthenticatorData_Verify(t *testing.T) { AttData: tt.fields.AttData, ExtData: tt.fields.ExtData, } - err := a.Verify(tt.args.rpIdHash, nil, tt.args.userVerificationRequired) + err := a.Verify(tt.args.rpIdHash, nil, tt.args.userVerificationRequired, tt.args.userPresenceRequired) if tt.errString != "" { assert.EqualError(t, err, tt.errString) diff --git a/protocol/credential.go b/protocol/credential.go index 66f1537..153403e 100644 --- a/protocol/credential.go +++ b/protocol/credential.go @@ -145,7 +145,7 @@ func (ccr CredentialCreationResponse) Parse() (pcc *ParsedCredentialCreationData // Verify the Client and Attestation data. // // Specification: §7.1. Registering a New Credential (https://www.w3.org/TR/webauthn/#sctn-registering-a-new-credential) -func (pcc *ParsedCredentialCreationData) Verify(storedChallenge string, verifyUser bool, relyingPartyID string, rpOrigins, rpTopOrigins []string, rpTopOriginsVerify TopOriginVerificationMode, mds metadata.Provider, credParams []CredentialParameter) (clientDataHash []byte, err error) { +func (pcc *ParsedCredentialCreationData) Verify(storedChallenge string, verifyUser bool, verifyUserPresence bool, relyingPartyID string, rpOrigins, rpTopOrigins []string, rpTopOriginsVerify TopOriginVerificationMode, mds metadata.Provider, credParams []CredentialParameter) (clientDataHash []byte, err error) { // Handles steps 3 through 6 - Verifying the Client Data against the Relying Party's stored data if err = pcc.Response.CollectedClientData.Verify(storedChallenge, CreateCeremony, rpOrigins, rpTopOrigins, rpTopOriginsVerify); err != nil { return nil, err @@ -161,7 +161,7 @@ func (pcc *ParsedCredentialCreationData) Verify(storedChallenge string, verifyUs // We do the above step while parsing and decoding the CredentialCreationResponse // Handle steps 9 through 14 - This verifies the attestation object. - if err = pcc.Response.AttestationObject.Verify(relyingPartyID, clientDataHash, verifyUser, mds, credParams); err != nil { + if err = pcc.Response.AttestationObject.Verify(relyingPartyID, clientDataHash, verifyUser, verifyUserPresence, mds, credParams); err != nil { return clientDataHash, err } diff --git a/protocol/credential_test.go b/protocol/credential_test.go index 4068e51..35689ef 100644 --- a/protocol/credential_test.go +++ b/protocol/credential_test.go @@ -243,7 +243,7 @@ func TestParsedCredentialCreationData_Verify(t *testing.T) { Response: tt.fields.Response, Raw: tt.fields.Raw, } - if _, err := pcc.Verify(tt.args.storedChallenge.String(), tt.args.verifyUser, tt.args.relyingPartyID, tt.args.relyingPartyOrigin, nil, TopOriginIgnoreVerificationMode, nil, tt.args.credParams); (err != nil) != tt.wantErr { + if _, err := pcc.Verify(tt.args.storedChallenge.String(), tt.args.verifyUser, false, tt.args.relyingPartyID, tt.args.relyingPartyOrigin, nil, TopOriginIgnoreVerificationMode, nil, tt.args.credParams); (err != nil) != tt.wantErr { t.Errorf("ParsedCredentialCreationData.Verify() error = %+v, wantErr %v", err, tt.wantErr) } }) diff --git a/webauthn/login.go b/webauthn/login.go index aa6b53e..352a4bd 100644 --- a/webauthn/login.go +++ b/webauthn/login.go @@ -357,6 +357,7 @@ func (webauthn *WebAuthn) validateLogin(user User, session SessionData, parsedRe } shouldVerifyUser := session.UserVerification == protocol.VerificationRequired + shouldVerifyUserPresence := true rpID := webauthn.Config.RPID rpOrigins := webauthn.Config.RPOrigins @@ -367,7 +368,7 @@ func (webauthn *WebAuthn) validateLogin(user User, session SessionData, parsedRe } // Handle steps 4 through 16. - if err = parsedResponse.Verify(session.Challenge, rpID, rpOrigins, rpTopOrigins, webauthn.Config.RPTopOriginVerificationMode, appID, shouldVerifyUser, credential.PublicKey); err != nil { + if err = parsedResponse.Verify(session.Challenge, rpID, rpOrigins, rpTopOrigins, webauthn.Config.RPTopOriginVerificationMode, appID, shouldVerifyUser, shouldVerifyUserPresence, credential.PublicKey); err != nil { return nil, err } diff --git a/webauthn/registration.go b/webauthn/registration.go index bd16b3d..aeaeba6 100644 --- a/webauthn/registration.go +++ b/webauthn/registration.go @@ -101,6 +101,7 @@ func (webauthn *WebAuthn) BeginMediatedRegistration(user User, mediation protoco UserID: user.WebAuthnID(), UserVerification: creation.Response.AuthenticatorSelection.UserVerification, CredParams: creation.Response.Parameters, + Mediation: creation.Mediation, } if webauthn.Config.Timeouts.Registration.Enforce { @@ -231,10 +232,11 @@ func (webauthn *WebAuthn) CreateCredential(user User, session SessionData, parse } shouldVerifyUser := session.UserVerification == protocol.VerificationRequired + shouldVerifyUserPresence := session.Mediation != protocol.MediationConditional var clientDataHash []byte - if clientDataHash, err = parsedResponse.Verify(session.Challenge, shouldVerifyUser, webauthn.Config.RPID, webauthn.Config.RPOrigins, webauthn.Config.RPTopOrigins, webauthn.Config.RPTopOriginVerificationMode, webauthn.Config.MDS, session.CredParams); err != nil { + if clientDataHash, err = parsedResponse.Verify(session.Challenge, shouldVerifyUser, shouldVerifyUserPresence, webauthn.Config.RPID, webauthn.Config.RPOrigins, webauthn.Config.RPTopOrigins, webauthn.Config.RPTopOriginVerificationMode, webauthn.Config.MDS, session.CredParams); err != nil { return nil, err } diff --git a/webauthn/types.go b/webauthn/types.go index 9e8ee8e..5af251d 100644 --- a/webauthn/types.go +++ b/webauthn/types.go @@ -209,7 +209,8 @@ type SessionData struct { AllowedCredentialIDs [][]byte `json:"allowed_credentials,omitempty"` Expires time.Time `json:"expires"` - UserVerification protocol.UserVerificationRequirement `json:"userVerification"` - Extensions protocol.AuthenticationExtensions `json:"extensions,omitempty"` - CredParams []protocol.CredentialParameter `json:"credParams,omitempty"` + UserVerification protocol.UserVerificationRequirement `json:"userVerification"` + Extensions protocol.AuthenticationExtensions `json:"extensions,omitempty"` + CredParams []protocol.CredentialParameter `json:"credParams,omitempty"` + Mediation protocol.CredentialMediationRequirement `json:"mediation,omitempty"` }