fido2-net-lib icon indicating copy to clipboard operation
fido2-net-lib copied to clipboard

attestationType/Attestation confusion

Open Regenhardt opened this issue 11 months ago • 0 comments

Separate discussion about point 4. from #426 as requested.

RequestNewCredential takes an AttestationConveyancePreference, but the extensions passed to it include AuthenticationExtensionsDevicePublicKeyInputs which also includes the AttestationConveyancePreference but as a string? Do I have that right? Seems redundant and it just being a string makes me think I should put something else there that is not the enum.

This is new territory with devicePublicKey, I don't see AttestationConveyancePreference in AuthenticationExtensionsDevicePublicKeyInputs, all I see is attestation and attestationFormats, no?

RequestNewCredential takes a mandatory parameter of type AttestationConveyancePreference (with a default value of None), so if there is a preference conveyed by the client, it's definitely there. This property is just added to the CredentialCreateOptions. It also takes extensions, which optionally include AuthenticationExtensionsDevicePublicKeyInputs, which have a property called Attestation spec'd like this:

The Relying Party MAY use this OPTIONAL member to specify a preference regarding attestation conveyance. Its value SHOULD be a member of AttestationConveyancePreference.

The extensions are also currently only used to add them to the CredentialCreateOptions.

We could be restrictive and just remove the redundancy, or we could keep the redundancy because it fits the spec and technically one of them is optional so maybe different use cases. In both cases, documentation would help to understand what's going on.

Also we could make the AuthenticationExtensionsDevicePublicKeyInputs.Attestation be of type AttestationConveyancePreference too, which would be restrictive on the SHOULD part of the spec again (it may technically be something else, in which case the client is supposed to ignore it).

Regenhardt avatar Mar 06 '24 19:03 Regenhardt