webauthn: pass unhashed PRF inputs to iCloud Keychain.

Despite being a platform API, iCloud Keychain expects unhashed WebAuthn
PRF inputs. Also, the `prf_inputs_hashed` field was used for hybrid
authenticator support and we've deleted that code in Chromium in favour
of the implementation in Play Services.

Change-Id: I12cf3f111ac29dab9b2b4e90e3d4b6fbdc48067c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5955569
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Ken Buchanan <kenrb@chromium.org>
Auto-Submit: Adam Langley <agl@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1373302}

components/webauthn/android/java/src/org/chromium/components/webauthn/Fido2Api.java

@@ -436,7 +436,7 @@ public final class Fido2Api {
                 // Two bytestrings for a single PRF input: the null credential ID and then the
                 // hashed salts, concatenated.
                 parcel.writeInt(2);
-                writePrfInput(options.prfInput, /* prfInputsHashed= */ false, parcel);
+                writePrfInput(options.prfInput, parcel);
             } else {
                 // No PRF inputs.
                 parcel.writeInt(0);
@@ -586,7 +586,7 @@ public final class Fido2Api {
             final int d = writeHeader(1, parcel);
             parcel.writeInt(2 * options.extensions.prfInputs.length);
             for (PrfValues input : options.extensions.prfInputs) {
-                writePrfInput(input, options.extensions.prfInputsHashed, parcel);
+                writePrfInput(input, parcel);
             }
             writeLength(d, parcel);
             writeLength(c, parcel);
@@ -606,17 +606,9 @@ public final class Fido2Api {
         writeLength(a, parcel);
     }
 
-    private static void writePrfInput(PrfValues input, boolean prfInputsHashed, Parcel parcel) {
+    private static void writePrfInput(PrfValues input, Parcel parcel) {
         parcel.writeByteArray(input.id);
-        if (prfInputsHashed) {
-            if (input.second == null) {
-                parcel.writeByteArray(input.first);
-            } else {
-                parcel.writeByteArray(concat(input.first, input.second));
-            }
-        } else {
-            parcel.writeByteArray(hashPrfInputs(input));
-        }
+        parcel.writeByteArray(hashPrfInputs(input));
     }
 
     /**

components/webauthn/android/java/src/org/chromium/components/webauthn/Fido2CredentialRequest.java

@@ -479,12 +479,9 @@ public class Fido2CredentialRequest
             return;
         }
 
-        // Payments should still go through Google Play Services. Also, if the request has
-        // pre-hashed PRF inputs then we cannot represent that in JSON and so can only forward to
-        // Play Services.
+        // Payments should still go through Google Play Services.
         final byte[] finalClientDataHash = clientDataHash;
         if (payment == null
-                && !options.extensions.prfInputsHashed
                 && getBarrierMode() == Barrier.Mode.ONLY_CRED_MAN) {
             if (options.isConditional) {
                 mBarrier.resetAndSetWaitStatus(Barrier.Mode.ONLY_CRED_MAN);

components/webauthn/json/value_conversions.cc

@@ -534,11 +534,6 @@ base::Value ToValue(
   }
 
   if (!options->extensions->prf_inputs.empty()) {
-    // Hashed PRF inputs are only used when Chrome is acting as a caBLE
-    // authenticator on Android. We can't convert the request to JSON in that
-    // context and should never try.
-    CHECK(!options->extensions->prf_inputs_hashed);
-
     base::Value::Dict prf_value;
     base::Value::Dict eval_by_cred;
     bool is_first = true;

components/webauthn/json/value_conversions_unittest.cc

@@ -184,7 +184,6 @@ TEST(WebAuthenticationJSONConversionTest,
           /*user_verification_methods=*/false,
 #endif
           /*prf=*/true, std::move(prf_values),
-          /*prf_inputs_hashed=*/false,
           /*large_blob_read=*/true,
           /*large_blob_write=*/std::vector<uint8_t>{8, 9, 10},
           /*get_cred_blob=*/true,

content/browser/webauth/authenticator_common_impl.cc

@@ -432,8 +432,10 @@ std::optional<device::PRFInput> ParsePRFInputForMakeCredential(
   }
 
   device::PRFInput prf_input;
+  prf_input.input1 = prf_input_from_renderer->first;
   prf_input.salt1 = HashPRFValue(prf_input_from_renderer->first);
   if (prf_input_from_renderer->second) {
+    prf_input.input2 = prf_input_from_renderer->second;
     prf_input.salt2 = HashPRFValue(*prf_input_from_renderer->second);
   }
 
@@ -1622,9 +1624,8 @@ void AuthenticatorCommonImpl::ContinueGetAssertionAfterRpIdCheck(
         ParsePRFInputsForGetAssertion(options->extensions->prf_inputs);
 
     // This should never happen for inputs from the renderer, which should sort
-    // the values itself. Additionally, `prf_inputs_hashed` is for hybrid
-    // authenticator support on Android.
-    if (!prf_inputs || options->extensions->prf_inputs_hashed) {
+    // the values itself.
+    if (!prf_inputs) {
       mojo::ReportBadMessage("invalid PRF inputs");
       req_state_->request_outcome = GetAssertionOutcome::kOtherFailure;
       CompleteGetAssertionRequest(

device/fido/cable/v2_authenticator.cc

@@ -908,11 +908,6 @@ class CTAP2Processor : public Transaction {
           }
         }
 
-        // PRF inputs are already hashed when coming via CTAP so, if there are
-        // any PRF inputs, they're hashed.
-        params->extensions->prf_inputs_hashed =
-            !params->extensions->prf_inputs.empty();
-
         transaction_received_ = true;
         const bool empty_allowlist = params->allow_credentials.empty();
         platform_->GetAssertion(

device/fido/cable/v2_test_util.cc

@@ -441,11 +441,6 @@ class TestPlatform : public authenticator::Platform {
     memcpy(request.client_data_hash.data(), params->challenge.data(),
            params->challenge.size());
     if (params->extensions) {
-      // The PRF inputs are hashed when they are sent over CTAP. So the
-      // `prf_inputs_hashed` flag should be set iff `prf_inputs` is non-empty.
-      CHECK(params->extensions->prf_inputs.empty() !=
-            params->extensions->prf_inputs_hashed);
-
       for (const auto& prf_input_from_request :
            params->extensions->prf_inputs) {
         PRFInput prf_input_to_authenticator;

device/fido/mac/icloud_keychain_sys.mm

@@ -25,10 +25,10 @@ NSData* ToNSData(base::span<const uint8_t> data) {
 API_AVAILABLE(macos(15.0))
 ASAuthorizationPublicKeyCredentialPRFAssertionInputValues* ToInputValues(
     const device::PRFInput& input) {
-  NSData* first = ToNSData(input.salt1);
+  NSData* first = ToNSData(input.input1);
   NSData* second = nil;
-  if (input.salt2) {
-    second = ToNSData(*input.salt2);
+  if (input.input2) {
+    second = ToNSData(*input.input2);
   }
   return [[ASAuthorizationPublicKeyCredentialPRFAssertionInputValues alloc]
       initWithSaltInput1:first

device/fido/prf_input.h

@@ -30,7 +30,12 @@ struct COMPONENT_EXPORT(DEVICE_FIDO) PRFInput {
   cbor::Value::MapValue ToCBOR() const;
 
   std::optional<std::vector<uint8_t>> credential_id;
+  // Input values are provided both unhashed (as `input1` and `input2`) and
+  // hashed (as `salt1` and `salt2`). Security keys use the hashed values but,
+  // e.g., iCloud Keychain needs unhashed values.
+  std::vector<uint8_t> input1;
   std::array<uint8_t, 32> salt1;
+  std::optional<std::vector<uint8_t>> input2;
   std::optional<std::array<uint8_t, 32>> salt2;
 };
 

third_party/blink/public/mojom/webauthn/authenticator.mojom

@@ -306,8 +306,7 @@ struct PRFValues {
   // IDs match. In the other direction, id will always be null.
   array<uint8>? id;
   // The WebAuthn spec says that inputs are hashed with a "WebAuthn PRF\x00"
-  // prefix before being used. These values may, or may not be, hashed when used
-  // as inputs. See the `prf_inputs_hashed` flag.
+  // prefix before being used. These are unhashed values when used as inputs.
   array<uint8> first;
   array<uint8>? second;
 };
@@ -416,10 +415,6 @@ struct AuthenticationExtensionsClientInputs {
   // must be sorted lexicographically by it.
   array<PRFValues> prf_inputs;
 
-  // prf_inputs_hashed is true if the `first` and `second` members of
-  // `prf_inputs` have already been hashed.
-  bool prf_inputs_hashed;
-
   // If true, attempt to read the large blob associated to the credential.
   // https://w3c.github.io/webauthn/#dom-authenticationextensionslargeblobinputs-read
   bool large_blob_read;