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.

Sign up for GitHub

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

Jump to bottom

Store/update credential record last in RP ops #2215

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Store/update credential record last in RP ops #2215

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2b9d8af
Store credential record last in RP registration ops
emlun Nov 27, 2024
4d7da39
Update credential record last in RP authentication ops
emlun Nov 27, 2024
91cd386
Equalize wording of last step of RP assertion ops with registration ops
emlun Nov 27, 2024
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
54 changes: 28 additions & 26 deletions index.bs
View file
Open in desktop
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 •
edited
Loading

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 Expand Up @@ -6244,20 +6246,6 @@ a numbered step. If outdented, it (today) is rendered as a bullet in the midst o
</dd>
</dl>

<li id='authn-ceremony-update-credential-record'>
Update |credentialRecord| with new state values:

1. Update <code>|credentialRecord|.[$credential record/signCount$]</code> to the value of |authData|.<code>[=authData/signCount=]</code>.
1. Update <code>|credentialRecord|.[$credential record/backupState$]</code> to the value of |currentBs|.
1. If <code>|credentialRecord|.[$credential record/uvInitialized$]</code> is [FALSE],
update it to the value of the [=authData/flags/UV=] bit in the [=flags=] in |authData|.
This change SHOULD require authorization by an additional [=authentication factor=] equivalent to WebAuthn [=user verification=];
if not authorized, skip this step.

If the [=[RP]=] performs additional security checks beyond these WebAuthn [=authentication ceremony=] steps,
the above state updates SHOULD be deferred to after those additional checks are completed successfully.
</li>

<!-- Note: this next step is actually a top-level step, but bikeshed wanted it indented this much in order to render it as a numbered step. If outdented, it (today) is rendered as a bullet in the midst of a numbered list -->
<li id='authn-ceremony-verify-extension-outputs'>
Process the [=client extension outputs=] in |clientExtensionResults|
Expand All @@ -6279,7 +6267,21 @@ 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 all the above steps are successful, continue with the [=authentication ceremony=] as appropriate. Otherwise, fail the
<li id='authn-ceremony-update-credential-record'>
Update |credentialRecord| with new state values:

1. Update <code>|credentialRecord|.[$credential record/signCount$]</code> to the value of |authData|.<code>[=authData/signCount=]</code>.
1. Update <code>|credentialRecord|.[$credential record/backupState$]</code> to the value of |currentBs|.
1. If <code>|credentialRecord|.[$credential record/uvInitialized$]</code> is [FALSE],
update it to the value of the [=authData/flags/UV=] bit in the [=flags=] in |authData|.
This change SHOULD require authorization by an additional [=authentication factor=] equivalent to WebAuthn [=user verification=];
if not authorized, skip this step.

If the [=[RP]=] performs additional security checks beyond these WebAuthn [=authentication ceremony=] steps,
the above state updates SHOULD be deferred to after those additional checks are completed successfully.
</li>

1. If all the above steps are successful, continue the [=authentication ceremony=] as appropriate. Otherwise, fail the
[=authentication ceremony=].


Expand Down