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

Make AuthenticatorAttestationResponseJSON.publicKeyAlgorithm a long #2065

Closed
zacknewman opened this issue May 1, 2024 · 2 comments · Fixed by #2071
Closed

Make AuthenticatorAttestationResponseJSON.publicKeyAlgorithm a long #2065

zacknewman opened this issue May 1, 2024 · 2 comments · Fixed by #2071
Assignees
Milestone

Comments

@zacknewman
Copy link
Contributor

COSEAlgorithmIdentifier is defined as a long, but AuthenticatorAttestationResponseJSON.publicKeyAlgorithm is a long long. While RPs are likely based on 64-bit platforms, it seems unnecessary to require 64-bit signed integers when a 32-bit signed integer is sufficient. What is the reason for this deviation? Is it based on "common" JSON libraries that model numbers as 64-bit signed integers?

@nadalin nadalin added this to the L3-WD-02 milestone May 15, 2024
@emlun
Copy link
Member

emlun commented May 15, 2024

Thanks for pointing this out!

2024-05-15 WG call: AuthenticatorAttestationResponseJSON was added in the L3 drafts, so we can easily change AuthenticatorAttestationResponseJSON.publicKeyAlgorithm to type long (or COSEAlgorithmIdentifier) since L3 isn't formally released yet. AuthenticatorAttestationResponseJSON.publicKeyAlgorithm is also in output (covariant) position, so changing its type to be more restrictive is even backwards compatible.

@selfissued
Copy link
Contributor

I support this change. As discussed in last weeks' call, ff you look at the COSE algorithm registration rules, it's the numbers outside the 16-bit spaces that are clearly outliers. There's no hint of ever going beyond 32-bit identifiers.

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.

4 participants