w3c / webauthn

Web Authentication: An API for accessing Public Key Credentials
https://w3c.github.io/webauthn/
Other
1.19k stars 172 forks source link

AttestationFormats may have duplicate entries #2202

Open Kieun opened 1 week ago

Kieun commented 1 week ago

Proposed Change

In the spec, we introduce attestationFormats for RPs to indicate the attestation statement format preference for create options. The definition in the spec is as follows.

attestationFormats, of type sequence<DOMString>, defaulting to [] The Relying Party MAY use this OPTIONAL member to specify a preference regarding the attestation statement format used by the authenticator. Values SHOULD be taken from the IANA "WebAuthn Attestation Statement Format Identifiers" registry [IANA-WebAuthn-Registries] established by [RFC8809]. Values are ordered from most preferable to least preferable. This parameter is advisory and the authenticator MAY use an attestation statement not enumerated in this parameter.

Since the value itself is the list of ordered preferences, it implies that the element of the fields would be unique. The sequence<'T'> does not have any such constraints for uniqueness.

Thus, duplicated entries in the attestationFormats may be valid input as per the current spec. We may add some constraints around the field itself or we could describe the way for authenticator and client to handle such duplicated entries without throwing errors.

This issue is related to the #2145 and maybe we could resolve this issue with similar manner.

emlun commented 1 week ago

This problem would also apply to any other "preference-ordered sequence" parameters, namely pubKeyCredParams and hints. For hints it is explicitly specified that duplicates are ignored, but pubKeyCredParams and attestationFormats have no explicit statement like that.

I don't think this is a problem in practice, though, precisely because both pubKeyCredParams and attestationFormats are meant so that the authenticator simply iterates through them and picks the first option it supports. Thus if a supported option appears more than once, the authenticator will just pick the first occurrence and ignore the rest. If an unsupported option appears more than once, it will just be rejected each time it is encountered. In both cases, everything works as expected.

One could make the argument that duplicates are most likely unintended, so it's better to reject them so the RP finds out about the issue. This would, however, not be backwards compatible with existing RP implementations that (intentionally or accidentally) rely on duplicates being silently ignored.

Kieun commented 1 day ago

One could make the argument that duplicates are most likely unintended, so it's better to reject them so the RP finds out about the issue. This would, however, not be backwards compatible with existing RP implementations that (intentionally or accidentally) rely on duplicates being silently ignored.

I agree that throwing an error is a bad approach. But, adding some notes explicitly make the readers and developers to clearly understand how the underlying client and authenticator would work.