Metadata validation is performed during login on credentials with no attestation

[eduar-hte]

Version

0.12.1

Description

Metadata validation (through the ValidateMedata method) is skipped during the registration ceremony when no attestation information has been provided by the authenticator.

You can see that the call to ValidateMetadata in VerifyAttestation will not be reached because the method will exit previously at the start of the function with this code:

webauthn/protocol/attestation.go

Lines 152 to 158 in 086658d

	if AttestationFormat(a.Format) == AttestationFormatNone {
	if len(a.AttStatement) != 0 {
	return ErrAttestationFormat.WithInfo("Attestation format none with attestation present")
	}
	return nil
	}

However, a similar check is not performed during validateLogin when the a Metadata provider has been configured, and thus the call ValidateMetadata will always be executed.

webauthn/webauthn/login.go

Lines 342 to 355 in 086658d

	// Ensure authenticators with a bad status is not used.
	if webauthn.Config.MDS != nil {
	var aaguid uuid.UUID
	if aaguid, err = uuid.FromBytes(credential.Authenticator.AAGUID); err != nil {
	return nil, protocol.ErrBadRequest.WithDetails("Failed to decode AAGUID").WithInfo(fmt.Sprintf("Error occurred decoding AAGUID from the credential record: %s", err)).WithError(err)
	}
	var protoErr *protocol.Error
	if protoErr = protocol.ValidateMetadata(context.Background(), webauthn.Config.MDS, aaguid, "", nil); protoErr != nil {
	return nil, protocol.ErrBadRequest.WithDetails("Failed to validate credential record metadata").WithInfo(protoErr.DevInfo).WithError(protoErr)
	}
	}

NOTE: I was not sure whether to report this as an issue or start a discussion, because I'm not sure whether the behaviour is intended, though I think it'd make sense to add a similar check to that in VerifyAttestation in this last section of validateLogin.

Reproduction

1. Create a server where:
   • AttestationFormat has been configured to AttestationFormatNone.
   • Set up the Metadata provider
2. Register a credential (which will not provide attestation)
   • This works because the ValidateMetadata call is skipped.
3. Try to login with the credential created in the previous step.
   • Login fails in the call to ValidateMetadata in validateLogin with the error: Failed to validate credential record metadata.

Expectations

Login in step 3 to be successful.

[eduar-hte]

After further analysis, I understand that the Aaguid when no attestation has been provided will be the null guid (00000000-0000-0000-0000-000000000000) and the MDS provider can be configured to allow them with GetValidateEntryPermitZeroAAGUID.

[james-d-elliott]

If this errors it's unexpected behavior which we should fix. Especially if it errors during login but not registration.

[eduar-hte]

I initially thought so, because I saw that VerifyAttestation explicitly skips validating the metadata is no attestation was provided.

However, as I mentioned in my follow-up comment, after further analysis I found out about GetValidateEntryPermitZeroAAGUID & WithValidateEntryPermitZeroAAGUID and thought that it could be 'by design', as the library currently provides a mechanism to handle the scenario (as no attestation results in the 'zero AAGUID').

NOTE: My rationale for thinking this is 'by design' is that if ValidateMetadata is skipped during login ceremonies, I wouldn't know what would be the use of GetValidateEntryPermitZeroAAGUID (I can think of an authenticator identifying with 'zero AAGUID' when attestation has been requested, but I'm not sure if that is a real scenario).

I'm currently using cached.WithNew to override defaultNew and allowing zero AAGUIDs to handle this scenario.

[james-d-elliott]

Makes sense. I'll double check the logic and ensure it makes sense in all situations. But it should pass in both registration or login scenarios and generally should fail more in registration than login