//device/fido/mac: fix a bug in MakeAuthenticatorData

AuthenticatorData was previously created with the hash of the
clientDataJSON, rather than the hash of the RP ID. See
https://www.w3.org/TR/webauthn/#authenticator-data for reference.

Bug: 678128
Change-Id: I1628172212fadedd02a8ccf5daa46237bccd12bd
Reviewed-on: https://chromium-review.googlesource.com/1066486
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Adam Langley <agl@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561640}

device/fido/mac/get_assertion_operation.mm

@@ -41,7 +41,7 @@ const std::string& GetAssertionOperation::RpId() const {
 void GetAssertionOperation::Run() {
   // Prompt the user for consent.
   // TODO(martinkr): Localize reason strings.
-  PromptTouchId("sign in to " + request().rp_id());
+  PromptTouchId("sign in to " + RpId());
 }
 
 void GetAssertionOperation::PromptTouchIdDone(bool success, NSError* err) {
@@ -130,8 +130,8 @@ void GetAssertionOperation::PromptTouchIdDone(bool success, NSError* err) {
     return;
   }
 
-  base::Optional<AuthenticatorData> authenticator_data = MakeAuthenticatorData(
-      request().client_data_hash(), std::move(credential_id), public_key);
+  base::Optional<AuthenticatorData> authenticator_data =
+      MakeAuthenticatorData(RpId(), std::move(credential_id), public_key);
   if (!authenticator_data) {
     DLOG(ERROR) << "MakeAuthenticatorData failed";
     std::move(callback())

device/fido/mac/make_credential_operation.mm

@@ -13,6 +13,7 @@
 #include "base/mac/scoped_cftyperef.h"
 #include "device/fido/fido_attestation_statement.h"
 #include "device/fido/fido_constants.h"
+#include "device/fido/fido_parsing_utils.h"
 #include "device/fido/mac/keychain.h"
 #include "device/fido/mac/util.h"
 
@@ -59,7 +60,7 @@ void MakeCredentialOperation::Run() {
 
   // Prompt the user for consent.
   // TODO(martinkr): Localize reason strings.
-  PromptTouchId("register with " + request().rp().rp_id());
+  PromptTouchId("register with " + RpId());
 }
 
 void MakeCredentialOperation::PromptTouchIdDone(bool success, NSError* err) {
@@ -101,8 +102,8 @@ void MakeCredentialOperation::PromptTouchIdDone(bool success, NSError* err) {
   }
 
   // Delete the key pair for this RP + user handle if one already exists.
-  const std::vector<uint8_t> keychain_item_id = KeychainItemIdentifier(
-      request().rp().rp_id(), request().user().user_id());
+  const std::vector<uint8_t> keychain_item_id =
+      KeychainItemIdentifier(RpId(), request().user().user_id());
   {
     ScopedCFTypeRef<CFMutableDictionaryRef> query = DefaultKeychainQuery();
     CFDictionarySetValue(query, kSecAttrApplicationLabel,
@@ -160,8 +161,8 @@ void MakeCredentialOperation::PromptTouchIdDone(bool success, NSError* err) {
 
   // Create attestation object. There is no separate attestation key pair, so
   // we perform self-attestation.
-  base::Optional<AuthenticatorData> authenticator_data = MakeAuthenticatorData(
-      request().client_data_hash(), keychain_item_id, public_key);
+  base::Optional<AuthenticatorData> authenticator_data =
+      MakeAuthenticatorData(RpId(), keychain_item_id, public_key);
   if (!authenticator_data) {
     DLOG(ERROR) << "MakeAuthenticatorData failed";
     std::move(callback())

device/fido/mac/util.h

@@ -26,10 +26,10 @@ std::vector<uint8_t> KeychainItemIdentifier(std::string rp_id,
                                             std::vector<uint8_t> user_id);
 
 // MakeAuthenticatorData returns an AuthenticatorData instance for the Touch ID
-// authenticator with the given client data, credential id and public key. It
-// returns |base::nullopt| on failure.
+// authenticator with the given Relying Party ID, credential ID and public key.
+// It returns |base::nullopt| on failure.
 base::Optional<AuthenticatorData> MakeAuthenticatorData(
-    std::vector<uint8_t> client_data_hash,
+    const std::string& rp_id,
     std::vector<uint8_t> credential_id,
     SecKeyRef public_key) API_AVAILABLE(macosx(10.12.2));
 

device/fido/mac/util.mm

@@ -83,7 +83,7 @@ std::vector<uint8_t> KeychainItemIdentifier(std::string rp_id,
 }
 
 base::Optional<AuthenticatorData> MakeAuthenticatorData(
-    std::vector<uint8_t> client_data_hash,
+    const std::string& rp_id,
     std::vector<uint8_t> credential_id,
     SecKeyRef public_key) API_AVAILABLE(macosx(10.12.2)) {
   if (credential_id.size() > 255) {
@@ -103,7 +103,7 @@ base::Optional<AuthenticatorData> MakeAuthenticatorData(
     return base::nullopt;
   }
   return AuthenticatorData(
-      std::move(client_data_hash), flags, counter,
+      fido_parsing_utils::CreateSHA256Hash(rp_id), flags, counter,
       AttestedCredentialData(kAaguid, encoded_credential_id_length,
                              std::move(credential_id),
                              std::move(ec_public_key)));