Merge pull request #2215 from w3c/issue-2204-premature-store-credential

Store/update credential record last in RP ops

index.bs

@@ -6015,6 +6015,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=].
@@ -6023,11 +6030,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$]
@@ -6093,13 +6097,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.
+    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.
@@ -6251,20 +6253,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|
@@ -6286,7 +6274,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=].