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

Store/update credential record last in RP ops #2215

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions index.bs
Original file line number Diff line number Diff line change
Expand Up @@ -6008,6 +6008,13 @@ a numbered step. If outdented, it (today) is rendered as a bullet in the midst o
- Otherwise, use the X.509 certificates returned as the [=attestation trust path=] from the [=verification procedure=]
to verify that the attestation public key either correctly chains up to an acceptable root certificate, or is itself an acceptable certificate
(i.e., it and the root certificate obtained in [step 22](#reg-ceremony-attestation-trust-anchors) may be the same).

If the attestation statement is not deemed trustworthy, the [=[RP]=] SHOULD fail the [=registration ceremony=].

NOTE: However, if permitted by policy, the [=[RP]=] MAY register the [=credential ID=] and credential public key but treat the
credential as one with [=self attestation=] (see [[#sctn-attestation-types]]). If doing so, the [=[RP]=] is asserting there
is no cryptographic proof that the [=public key credential=] has been generated by a particular [=authenticator=] model.
See [[FIDOSecRef]] and [[UAFProtocol]] for a more detailed discussion.
</li>

1. Verify that the <code>[=credentialId=]</code> is &le; 1023 bytes. Credential IDs larger than this many bytes SHOULD cause the RP to fail this [=registration ceremony=].
Expand All @@ -6016,11 +6023,8 @@ a numbered step. If outdented, it (today) is rendered as a bullet in the midst o

NOTE: The rationale for [=[RPS]=] rejecting duplicate [=credential IDs=] is as follows: [=credential IDs=] contain sufficient entropy that accidental duplication is very unlikely. However, [=attestation types=] other than [=self attestation=] do not include a self-signature to explicitly prove possession of the [=credential private key=] at [=registration=] time. Thus an attacker who has managed to obtain a user's [=credential ID=] and [=credential public key=] for a site (this could be potentially accomplished in various ways), could attempt to register a victim's credential as their own at that site. If the [=[RP]=] accepts this new registration and replaces the victim's existing credential registration, and the [=discoverable credentials|credentials are discoverable=], then the victim could be forced to sign into the attacker's account at their next attempt. Data saved to the site by the victim in that state would then be available to the attacker.

<li id="reg-ceremony-store-credential-record">
If the attestation statement |attStmt| verified successfully and is found to be trustworthy,
then create and store a new [=credential record=] in the [=user account=]
that was denoted in <code>|pkOptions|.{{PublicKeyCredentialCreationOptions/user}}</code>,
with the following contents:
<li id="reg-ceremony-create-credential-record">
Let |credentialRecord| be a new [=credential record=] with the following contents:

<dl>
: [$credential record/type$]
Expand Down Expand Up @@ -6086,13 +6090,11 @@ a numbered step. If outdented, it (today) is rendered as a bullet in the midst o
prepared to handle cases where none or not all of the requested extensions were acted upon.
</li>

1. If the attestation statement |attStmt| successfully verified but is not trustworthy per [step 23](#reg-ceremony-assess-trust) above,
the [=[RP]=] SHOULD fail the [=registration ceremony=].
1. If all the above steps are successful,
store |credentialRecord| in the [=user account=] that was denoted in <code>|pkOptions|.{{PublicKeyCredentialCreationOptions/user}}</code>
and continue the [=registration ceremony=] as appropriate.
Copy link
Contributor

@zacknewman zacknewman Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "continue the registration ceremony" mean? The way I interpret that is that RPs may have specific criteria in addition to what's stated in the spec, and this line means for the RP to continue with said criteria. However if that is true, then it's possible the ceremony fails; but we already stated to store the credentialRecord. If my interpretation is correct, then this needs to be re-worded like:

If all the above steps are successful, continue the registration ceremony as appropriate. If the ceremony is successful, then store credentialRecord

Ditto for the authentication ceremony section.

In other words the last "portion"/clause of the last sentence of the last step should be about storing/updating the credential unless subsequent clauses or sentences do not describe processes that even have the possibility for causing the ceremony to fail.

Otherwise, fail the [=registration ceremony=].

NOTE: However, if permitted by policy, the [=[RP]=] MAY register the [=credential ID=] and credential public key but treat the
credential as one with [=self attestation=] (see [[#sctn-attestation-types]]). If doing so, the [=[RP]=] is asserting there
is no cryptographic proof that the [=public key credential=] has been generated by a particular [=authenticator=] model.
See [[FIDOSecRef]] and [[UAFProtocol]] for a more detailed discussion.

Verification of [=attestation objects=] requires that the [=[RP]=] has a trusted method of determining acceptable trust anchors
in [step 22](#reg-ceremony-attestation-trust-anchors) above.
Expand Down