Bug 1813282 - Add the residentKey field to AuthenticatorSelectionCriteria. r=keeler,webidl,smaug

Differential Revision: https://phabricator.services.mozilla.com/D173085

dom/webauthn/CtapArgs.cpp

@@ -125,13 +125,12 @@ CtapRegisterArgs::GetHmacCreateSecret(bool* aHmacCreateSecret) {
 }
 
 NS_IMETHODIMP
-CtapRegisterArgs::GetRequireResidentKey(bool* aRequireResidentKey) {
+CtapRegisterArgs::GetResidentKey(nsAString& aResidentKey) {
   mozilla::ipc::AssertIsOnBackgroundThread();
   if (mInfo.Extra().isNothing()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
-  *aRequireResidentKey =
-      (*mInfo.Extra()).AuthenticatorSelection().requireResidentKey();
+  aResidentKey = (*mInfo.Extra()).AuthenticatorSelection().residentKey();
   return NS_OK;
 }
 

dom/webauthn/PWebAuthnTransaction.ipdl

@@ -22,7 +22,7 @@ namespace mozilla {
 namespace dom {
 
 struct WebAuthnAuthenticatorSelection {
-  bool requireResidentKey;
+  nsString residentKey;
   nsString userVerificationRequirement;
   nsString? authenticatorAttachment;
 };

dom/webauthn/U2FSoftTokenTransport.cpp

@@ -600,11 +600,12 @@ U2FSoftTokenTransport::MakeCredential(uint64_t aTransactionId,
     return NS_ERROR_FAILURE;
   }
 
-  bool requireResidentKey;
-  // Bug 1737205 will make this infallible
-  rv = args->GetRequireResidentKey(&requireResidentKey);
-  if (NS_FAILED(rv)) {
-    requireResidentKey = false;
+  bool requireResidentKey = false;
+  nsString residentKey;
+  rv = args->GetResidentKey(residentKey);
+  if (NS_SUCCEEDED(rv) && residentKey.EqualsLiteral(
+                              MOZ_WEBAUTHN_RESIDENT_KEY_REQUIREMENT_REQUIRED)) {
+    requireResidentKey = true;
   }
 
   bool requireUserVerification = false;

dom/webauthn/WebAuthnManager.cpp

@@ -394,10 +394,36 @@ already_AddRefed<Promise> WebAuthnManager::MakeCredential(
     authenticatorAttachment.emplace(attachment.Value());
   }
 
+  // The residentKey field was added in WebAuthn level 2. It takes precedent
+  // over the requireResidentKey field if and only if it is present and it is a
+  // member of the ResidentKeyRequirement enum.
+  static_assert(MOZ_WEBAUTHN_ENUM_STRINGS_VERSION == 2);
+  bool useResidentKeyValue =
+      selection.mResidentKey.WasPassed() &&
+      (selection.mResidentKey.Value().EqualsLiteral(
+           MOZ_WEBAUTHN_RESIDENT_KEY_REQUIREMENT_REQUIRED) ||
+       selection.mResidentKey.Value().EqualsLiteral(
+           MOZ_WEBAUTHN_RESIDENT_KEY_REQUIREMENT_PREFERRED) ||
+       selection.mResidentKey.Value().EqualsLiteral(
+           MOZ_WEBAUTHN_RESIDENT_KEY_REQUIREMENT_DISCOURAGED));
+
+  nsString residentKey;
+  if (useResidentKeyValue) {
+    residentKey = selection.mResidentKey.Value();
+  } else {
+    // "If no value is given then the effective value is required if
+    // requireResidentKey is true or discouraged if it is false or absent."
+    if (selection.mRequireResidentKey) {
+      residentKey.AssignLiteral(MOZ_WEBAUTHN_RESIDENT_KEY_REQUIREMENT_REQUIRED);
+    } else {
+      residentKey.AssignLiteral(
+          MOZ_WEBAUTHN_RESIDENT_KEY_REQUIREMENT_DISCOURAGED);
+    }
+  }
+
   // Create and forward authenticator selection criteria.
-  WebAuthnAuthenticatorSelection authSelection(selection.mRequireResidentKey,
-                                               selection.mUserVerification,
-                                               authenticatorAttachment);
+  WebAuthnAuthenticatorSelection authSelection(
+      residentKey, selection.mUserVerification, authenticatorAttachment);
 
   nsString rpIcon;
   if (aOptions.mRp.mIcon.WasPassed()) {

dom/webauthn/WinWebAuthnManager.cpp

@@ -210,6 +210,7 @@ void WinWebAuthnManager::Register(
 
   // Resident Key
   BOOL winRequireResidentKey = FALSE;
+  BOOL winPreferResidentKey = FALSE;
 
   // AttestationConveyance
   DWORD winAttestation = WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_ANY;
@@ -271,7 +272,26 @@ void WinWebAuthnManager::Register(
       }
     }
 
+    const nsString& residentKey = sel.residentKey();
+    // This mapping needs to be reviewed if values are added to the
+    // ResidentKeyRequirement enum.
+    static_assert(MOZ_WEBAUTHN_ENUM_STRINGS_VERSION == 2);
+    if (residentKey.EqualsLiteral(
+            MOZ_WEBAUTHN_RESIDENT_KEY_REQUIREMENT_REQUIRED)) {
+      winRequireResidentKey = TRUE;
+      winPreferResidentKey = TRUE;
+    } else if (residentKey.EqualsLiteral(
+                   MOZ_WEBAUTHN_RESIDENT_KEY_REQUIREMENT_PREFERRED)) {
+      winRequireResidentKey = FALSE;
+      winPreferResidentKey = TRUE;
+    } else {
+      // https://w3c.github.io/webauthn/#dictionary-authenticatorSelection
+      // "[The client MUST treat] an unknown value as if the member does not
+      // exist. If no value is given then the effective value is required if
+      // requireResidentKey is true or discouraged if it is false or absent."
       winRequireResidentKey = sel.requireResidentKey();
+      winPreferResidentKey = sel.requireResidentKey();
+    }
 
     // AttestationConveyance
     const nsString& attestation = extra.attestationConveyancePreference();
@@ -381,7 +401,7 @@ void WinWebAuthnManager::Register(
       pExcludeCredentialList,
       WEBAUTHN_ENTERPRISE_ATTESTATION_NONE,
       WEBAUTHN_LARGE_BLOB_SUPPORT_NONE,
-      FALSE,  // PreferResidentKey
+      winPreferResidentKey,  // PreferResidentKey
   };
 
   GUID cancellationId = {0};

dom/webauthn/authrs_bridge/src/lib.rs

@@ -480,8 +480,15 @@ impl AuthrsTransport {
             .map(|alg| PublicKeyCredentialParameters::try_from(*alg).unwrap())
             .collect();
 
-        let mut require_resident_key = false;
-        unsafe { args.GetRequireResidentKey(&mut require_resident_key) }.to_result()?;
+        let mut resident_key = nsString::new();
+        unsafe { args.GetResidentKey(&mut *resident_key) }.to_result()?;
+        let resident_key = if resident_key.eq("required") {
+            Some(true)
+        } else if resident_key.eq("discouraged") {
+            Some(false)
+        } else {
+            None
+        };
 
         let mut user_verification = nsString::new();
         unsafe { args.GetUserVerification(&mut *user_verification) }.to_result()?;
@@ -523,7 +530,7 @@ impl AuthrsTransport {
             pub_cred_params,
             exclude_list,
             options: MakeCredentialsOptions {
-                resident_key: require_resident_key.then_some(true),
+                resident_key,
                 user_verification,
             },
             extensions: Default::default(),

dom/webauthn/nsIWebAuthnController.idl

@@ -47,7 +47,7 @@ interface nsICtapRegisterArgs : nsISupports {
     [must_use] readonly attribute bool hmacCreateSecret;
 
     // Options.
-    [must_use] readonly attribute bool requireResidentKey;
+    [must_use] readonly attribute AString residentKey;
     [must_use] readonly attribute AString userVerification;
     [must_use] readonly attribute AString authenticatorAttachment;
 

dom/webidl/WebAuthentication.webidl

@@ -81,6 +81,7 @@ dictionary PublicKeyCredentialUserEntity : PublicKeyCredentialEntity {
 
 dictionary AuthenticatorSelectionCriteria {
     DOMString                    authenticatorAttachment;
+    DOMString                    residentKey;
     boolean                      requireResidentKey = false;
     DOMString                    userVerification = "preferred";
 };