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

credProps output directions contradict notes #2213

Open
zacknewman opened this issue Nov 23, 2024 · 1 comment
Open

credProps output directions contradict notes #2213

zacknewman opened this issue Nov 23, 2024 · 1 comment

Comments

@zacknewman
Copy link
Contributor

zacknewman commented Nov 23, 2024

credProps states the following for the output:

Client extension output
Set clientExtensionResults"[credProps"]["rk"] to the value of the requireResidentKey parameter that was used in the invocation of the authenticatorMakeCredential operation.

If this is true, then I don't see how rk is optional since per the above directive it will always be set since requireResidentKey is always set (it defaults to false when not present). Furthermore the notes about the extension later state that rk should only be set when the client platform knows definitively one way or the other; and if it does not know, then it should omit the property:

Note: some authenticators create discoverable credentials even when not requested by the client platform. Because of this, client platforms may be forced to omit the rk property because they lack the assurance to be able to set it to false. Relying Parties should assume that, if the credProps extension is supported, then client platforms will endeavour to populate the rk property. Therefore a missing rk indicates that the created credential is most likely a non-discoverable credential.

This justifies the reason rk is optional, but it clearly goes against the actual directive which is to always use requireResidentKey. I think the directive should be changed. I think it should state that rk should be set to true when requireResidentKey is true; however it should only set it to false when it knows for sure. This makes it align with the IDL which has rk as optional as well as the notes.

@zacknewman
Copy link
Contributor Author

Here is a real-world example that is potentially caused from this directive. When an iPhone is used as an authenticator, it only creates client-side/"resident" credentials. If one uses Chromium on a separate device to register a credential on the iPhone using false for AuthenticatorSelectionCriteria.requireResidentKey and "discouraged" for AuthenticatorSelectionCriteria.residentKey and passes the credProps extension, Chromium outputs false for CredentialPropertiesOutput.rk.

This is clearly incorrect. Perhaps if the directive aligned with the notes, then Chromium would instead omit the property entirely rather than assign an incorrect value.

I also understand that credProps is largely useless since it's unlikely a client or user agent could ever know definitively if a client-side credential were created unless requireResidentKey is true making the only possible outputs true or missing; however that's better than giving a directive that causes the wrong value to be assigned sometimes. Perhaps the extension should be removed altogether; but until then, it's better for the notes, IDL and ouput directions to align.

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

No branches or pull requests

2 participants