Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AttestationFormats may have duplicate entries #2202

Open
Kieun opened this issue Nov 14, 2024 · 2 comments · May be fixed by #2214
Open

AttestationFormats may have duplicate entries #2202

Kieun opened this issue Nov 14, 2024 · 2 comments · May be fixed by #2214

Comments

@Kieun
Copy link
Member

Kieun commented Nov 14, 2024

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
Copy link
Member

emlun commented Nov 14, 2024

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
Copy link
Member Author

Kieun commented Nov 21, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants