Reland "device/fido/mac: encrypt credential metadata in the macOS keychain"

This is a reland of 7acb4716 with a fix.

In the original CL, CredentialMetadata::HmacForStorage wrapped
HMAC::Init(base::StringPiece) in a DCHECK. In non-debug this resulted in
HMAC::Sign getting called on an unintialized HMAC instance.

Change-Id: Ibf6e34dc381812254aaed5c20a83eb6a28d43246
Reviewed-on: https://chromium-review.googlesource.com/1087315Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564864}

device/BUILD.gn

@@ -78,6 +78,7 @@ test("device_unittests") {
     "fido/fido_request_handler_unittest.cc",
     "fido/get_assertion_handler_unittest.cc",
     "fido/get_assertion_task_unittest.cc",
+    "fido/mac/credential_metadata_unittest.cc",
     "fido/mac/get_assertion_operation_unittest_mac.mm",
     "fido/mac/make_credential_operation_unittest_mac.mm",
     "fido/make_credential_handler_unittest.cc",

device/fido/BUILD.gn

@@ -156,6 +156,8 @@ component("fido") {
     sources += [
       "mac/authenticator.h",
       "mac/authenticator.mm",
+      "mac/credential_metadata.cc",
+      "mac/credential_metadata.h",
       "mac/get_assertion_operation.h",
       "mac/get_assertion_operation.mm",
       "mac/keychain.h",

device/fido/mac/authenticator.mm

@@ -76,7 +76,7 @@ TouchIdAuthenticator::TouchIdAuthenticator() = default;
 
 base::StringPiece TouchIdAuthenticator::GetOrInitializeProfileId() {
   // TODO(martinkr): Implement.
-  return "TODO";
+  return "12345678901234567890123456789012";
 }
 
 }  // namespace mac

device/fido/mac/credential_metadata.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "device/fido/mac/credential_metadata.h"
+
+#include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
+#include "components/cbor/cbor_reader.h"
+#include "components/cbor/cbor_values.h"
+#include "components/cbor/cbor_writer.h"
+#include "third_party/boringssl/src/include/openssl/digest.h"
+#include "third_party/boringssl/src/include/openssl/hkdf.h"
+#include "third_party/boringssl/src/include/openssl/rand.h"
+
+namespace device {
+namespace fido {
+namespace mac {
+
+using cbor::CBORWriter;
+using cbor::CBORReader;
+using cbor::CBORValue;
+
+namespace {
+
+enum Algorithm : uint8_t {
+  kAes256Gcm = 0,
+  kHmacSha256 = 1,
+};
+
+// Derive keys from the caller-provided key to avoid using the same key for
+// both algorithms.
+std::string DeriveKey(base::StringPiece in, Algorithm alg) {
+  static constexpr size_t kKeyLength = 32u;
+  std::string key;
+  const uint8_t info = static_cast<uint8_t>(alg);
+  const bool hkdf_init = ::HKDF(
+      reinterpret_cast<uint8_t*>(base::WriteInto(&key, kKeyLength + 1)),
+      kKeyLength, EVP_sha256(), reinterpret_cast<const uint8_t*>(in.data()),
+      in.size(), nullptr /* salt */, 0, &info, 1);
+  DCHECK(hkdf_init);
+  return key;
+}
+
+}  // namespace
+
+CredentialMetadata::CredentialMetadata(const std::string& profile_id)
+    : profile_id_(profile_id) {}
+CredentialMetadata::~CredentialMetadata() = default;
+
+CredentialMetadata::UserEntity::UserEntity(std::vector<uint8_t> id_,
+                                           std::string name_,
+                                           std::string display_name_)
+    : id(std::move(id_)),
+      name(std::move(name_)),
+      display_name(std::move(display_name_)) {}
+CredentialMetadata::UserEntity::UserEntity(
+    const CredentialMetadata::UserEntity&) = default;
+CredentialMetadata::UserEntity::UserEntity(CredentialMetadata::UserEntity&&) =
+    default;
+CredentialMetadata::UserEntity& CredentialMetadata::UserEntity::operator=(
+    CredentialMetadata::UserEntity&&) = default;
+CredentialMetadata::UserEntity::~UserEntity() = default;
+
+static constexpr size_t kNonceLength = 12;
+
+// static
+base::Optional<std::vector<uint8_t>> CredentialMetadata::SealCredentialId(
+    const std::string& profile_id,
+    const std::string& rp_id,
+    const UserEntity& user) {
+  CredentialMetadata cryptor(profile_id);
+
+  // The first 13 bytes are the version and nonce.
+  std::vector<uint8_t> result(1 + kNonceLength);
+  result[0] = kVersion;
+  // Pick a random nonce. N.B. the nonce is similar to an IV. It needs to be
+  // distinct (but not necessarily random). Nonce reuse breaks confidentiality
+  // (in particular, it leaks the XOR of the plaintexts encrypted under the
+  // same nonce and key).
+  base::span<uint8_t> nonce(result.data() + 1, kNonceLength);
+  RAND_bytes(nonce.data(), nonce.size());  // RAND_bytes always returns 1.
+
+  // The remaining bytes are the CBOR-encoded UserEntity, encrypted with
+  // AES-256-GCM and authenticated with the version and RP ID.
+  CBORValue::ArrayValue cbor_user;
+  cbor_user.emplace_back(CBORValue(user.id));
+  cbor_user.emplace_back(CBORValue(user.name, CBORValue::Type::BYTE_STRING));
+  cbor_user.emplace_back(
+      CBORValue(user.display_name, CBORValue::Type::BYTE_STRING));
+  base::Optional<std::vector<uint8_t>> pt =
+      CBORWriter::Write(CBORValue(std::move(cbor_user)));
+  if (!pt) {
+    return base::nullopt;
+  }
+  base::Optional<std::string> ciphertext =
+      cryptor.Seal(nonce, *pt, MakeAad(rp_id));
+  if (!ciphertext) {
+    return base::nullopt;
+  }
+  base::span<const char> cts(reinterpret_cast<const char*>(ciphertext->data()),
+                             ciphertext->size());
+  result.insert(result.end(), cts.begin(), cts.end());
+  return result;
+}
+
+// static
+base::Optional<CredentialMetadata::UserEntity>
+CredentialMetadata::UnsealCredentialId(
+    const std::string& profile_id,
+    const std::string& rp_id,
+    base::span<const uint8_t> credential_id) {
+  CredentialMetadata cryptor(profile_id);
+
+  // Recover the nonce and check for the correct version byte. Then try to
+  // decrypt the remaining bytes.
+  if (credential_id.size() <= 1 + kNonceLength ||
+      credential_id[0] != kVersion) {
+    return base::nullopt;
+  }
+
+  base::Optional<std::string> plaintext =
+      cryptor.Unseal(credential_id.subspan(1, kNonceLength),
+                     credential_id.subspan(1 + kNonceLength), MakeAad(rp_id));
+  if (!plaintext) {
+    return base::nullopt;
+  }
+
+  // The recovered plaintext should decode into the UserEntity struct.
+  base::Optional<CBORValue> maybe_array = CBORReader::Read(base::make_span(
+      reinterpret_cast<const uint8_t*>(plaintext->data()), plaintext->size()));
+  if (!maybe_array || !maybe_array->is_array()) {
+    return base::nullopt;
+  }
+  const CBORValue::ArrayValue& array = maybe_array->GetArray();
+  if (array.size() != 3 || !array[0].is_bytestring() ||
+      !array[1].is_bytestring() || !array[2].is_bytestring()) {
+    return base::nullopt;
+  }
+  return UserEntity(array[0].GetBytestring(),
+                    array[1].GetBytestringAsString().as_string(),
+                    array[2].GetBytestringAsString().as_string());
+}
+
+// static
+base::Optional<std::string> CredentialMetadata::EncodeRpIdAndUserId(
+    const std::string& profile_id,
+    const std::string& rp_id,
+    base::span<const uint8_t> user_id) {
+  // Encoding RP ID along with the user ID hides whether the same user ID was
+  // reused on different RPs.
+  const auto* user_id_data = reinterpret_cast<const char*>(user_id.data());
+  return CredentialMetadata(profile_id)
+      .HmacForStorage(rp_id + "/" +
+                      std::string(user_id_data, user_id_data + user_id.size()));
+}
+
+// static
+base::Optional<std::string> CredentialMetadata::EncodeRpId(
+    const std::string& profile_id,
+    const std::string& rp_id) {
+  return CredentialMetadata(profile_id).HmacForStorage(rp_id);
+}
+
+// static
+std::string CredentialMetadata::MakeAad(const std::string& rp_id) {
+  return std::string(1, kVersion) + rp_id;
+}
+
+base::Optional<std::string> CredentialMetadata::Seal(
+    base::span<const uint8_t> nonce,
+    base::span<const uint8_t> plaintext,
+    base::StringPiece authenticated_data) const {
+  const std::string key = DeriveKey(profile_id_, Algorithm::kAes256Gcm);
+  crypto::Aead aead(crypto::Aead::AES_256_GCM);
+  aead.Init(&key);
+  std::string ciphertext;
+  if (!aead.Seal(
+          base::StringPiece(reinterpret_cast<const char*>(plaintext.data()),
+                            plaintext.size()),
+          base::StringPiece(reinterpret_cast<const char*>(nonce.data()),
+                            nonce.size()),
+          authenticated_data, &ciphertext)) {
+    return base::nullopt;
+  }
+  return ciphertext;
+}
+
+base::Optional<std::string> CredentialMetadata::Unseal(
+    base::span<const uint8_t> nonce,
+    base::span<const uint8_t> ciphertext,
+    base::StringPiece authenticated_data) const {
+  const std::string key = DeriveKey(profile_id_, Algorithm::kAes256Gcm);
+  crypto::Aead aead(crypto::Aead::AES_256_GCM);
+  aead.Init(&key);
+  std::string plaintext;
+  if (!aead.Open(
+          base::StringPiece(reinterpret_cast<const char*>(ciphertext.data()),
+                            ciphertext.size()),
+          base::StringPiece(reinterpret_cast<const char*>(nonce.data()),
+                            nonce.size()),
+          authenticated_data, &plaintext)) {
+    return base::nullopt;
+  }
+  return plaintext;
+}
+
+base::Optional<std::string> CredentialMetadata::HmacForStorage(
+    base::StringPiece data) const {
+  crypto::HMAC hmac(crypto::HMAC::SHA256);
+  const std::string key = DeriveKey(profile_id_, Algorithm::kHmacSha256);
+  std::vector<uint8_t> digest(hmac.DigestLength());
+  if (!hmac.Init(key) || !hmac.Sign(data, digest.data(), hmac.DigestLength())) {
+    return base::nullopt;
+  }
+  // The keychain fields that store RP ID and User ID seem to only accept
+  // NSString (not NSData), so we HexEncode to ensure the result to be
+  // UTF-8-decodable.
+  return base::HexEncode(digest.data(), digest.size());
+}
+
+}  // namespace mac
+}  // namespace fido
+}  // namespace device

device/fido/mac/credential_metadata.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef DEVICE_FIDO_MAC_CREDENTIAL_METADATA_H_
+#define DEVICE_FIDO_MAC_CREDENTIAL_METADATA_H_
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/component_export.h"
+#include "base/containers/span.h"
+#include "base/macros.h"
+#include "base/optional.h"
+#include "base/strings/string_piece_forward.h"
+#include "crypto/aead.h"
+#include "crypto/hmac.h"
+#include "crypto/symmetric_key.h"
+
+namespace device {
+namespace fido {
+namespace mac {
+
+// CredentialMetadata generates credential IDs from the associated user entity
+// (user ID, name and display name) by encrypting them under a key tied to the
+// current Chrome profile. This gives us separation of credentials per Chrome
+// profile. It also guarantees that account metadata in the OS keychain is
+// rendered unusable after the Chrome profile and the associated encryption key
+// have been deleted, in order to limit leakage of account metadata, such as
+// the list of RPs with registered credentials, into the OS keychain.
+//
+// Credential IDs have following format
+//    | version  |    nonce   | AEAD(pt=CBOR(user_entity), |
+//    | (1 byte) | (12 bytes) |      nonce=nonce,          |
+//    |          |            |      ad=(version, rpID))   |
+// with version as 0x00, a random 12-byte nonce, and using AES-256-GCM as the
+// AEAD.
+//
+// CredentialMetadata also encodes the user ID and RP ID for storage in the OS
+// keychain by computing their HMAC.
+//
+// TODO(martinkr): We currently do not store profile icon URLs.
+class COMPONENT_EXPORT(DEVICE_FIDO) CredentialMetadata {
+ public:
+  // UserEntity loosely corresponds to a PublicKeyCredentialUserEntity
+  // (https://www.w3.org/TR/webauthn/#sctn-user-credential-params). Values of
+  // this type should be moved whenever possible.
+  struct UserEntity {
+   public:
+    UserEntity(std::vector<uint8_t> id_,
+               std::string name_,
+               std::string display_);
+    UserEntity(const UserEntity&);
+    UserEntity(UserEntity&&);
+    UserEntity& operator=(UserEntity&&);
+    ~UserEntity();
+
+    std::vector<uint8_t> id;
+    std::string name;
+    std::string display_name;
+  };
+
+  // SealCredentialId encrypts the given UserEntity into a credential id.
+  static base::Optional<std::vector<uint8_t>> SealCredentialId(
+      const std::string& profile_id,
+      const std::string& rp_id,
+      const UserEntity& user);
+
+  // UnsealCredentialId attempts to decrypt a UserEntity from a given credential
+  // id.
+  static base::Optional<UserEntity> UnsealCredentialId(
+      const std::string& profile_id,
+      const std::string& rp_id,
+      base::span<const uint8_t> credential_id);
+
+  // EncodeRpIdAndUserId encodes the concatenation of RP ID and user ID for
+  // storage in the macOS keychain.
+  static base::Optional<std::string> EncodeRpIdAndUserId(
+      const std::string& profile_id,
+      const std::string& rp_id,
+      base::span<const uint8_t> user_id);
+
+  // EncodeRpId encodes the given RP ID for storage in the macOS keychain.
+  static base::Optional<std::string> EncodeRpId(const std::string& profile_id,
+                                                const std::string& rp_id);
+
+ private:
+  static constexpr uint8_t kVersion = 0x00;
+
+  // MakeAad returns the concatenation of |kVersion| and |rp_id|,
+  // which is used as the additional authenticated data (AAD) input to the AEAD.
+  static std::string MakeAad(const std::string& rp_id);
+
+  CredentialMetadata(const std::string& profile_id);
+  ~CredentialMetadata();
+
+  base::Optional<std::string> Seal(base::span<const uint8_t> nonce,
+                                   base::span<const uint8_t> plaintext,
+                                   base::StringPiece authenticated_data) const;
+  base::Optional<std::string> Unseal(
+      base::span<const uint8_t> nonce,
+      base::span<const uint8_t> ciphertext,
+      base::StringPiece authenticated_data) const;
+  base::Optional<std::string> HmacForStorage(base::StringPiece data) const;
+
+  // profile_id_ is a randomly generated string identifying the current Chrome
+  // user profile. It is used to derive the key material for AEAD and HMAC.
+  // This ensures that credentials are logically tied to the Chrome user
+  // profile under which they were created.
+  const std::string& profile_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(CredentialMetadata);
+};
+
+}  // namespace mac
+}  // namespace fido
+}  // namespace device
+
+#endif  // DEVICE_FIDO_MAC_CREDENTIAL_METADATA_H_

device/fido/mac/credential_metadata_unittest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "device/fido/mac/credential_metadata.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace device {
+namespace fido {
+namespace mac {
+namespace {
+
+bool UserEqual(const CredentialMetadata::UserEntity& lhs,
+               const CredentialMetadata::UserEntity& rhs) {
+  return lhs.id == rhs.id && lhs.name == rhs.name &&
+         lhs.display_name == rhs.display_name;
+}
+
+base::span<const uint8_t> to_bytes(base::StringPiece in) {
+  return base::make_span(reinterpret_cast<const uint8_t*>(in.data()),
+                         in.size());
+}
+
+class CredentialMetadataTest : public ::testing::Test {
+ protected:
+  CredentialMetadata::UserEntity DefaultUser() {
+    return CredentialMetadata::UserEntity(default_id_, "user", "user@acme.com");
+  }
+
+  std::vector<uint8_t> SealCredentialId(CredentialMetadata::UserEntity user) {
+    return *CredentialMetadata::SealCredentialId(key_, rp_id_, std::move(user));
+  }
+
+  CredentialMetadata::UserEntity UnsealCredentialId(
+      base::span<const uint8_t> credential_id) {
+    return *CredentialMetadata::UnsealCredentialId(key_, rp_id_, credential_id);
+  }
+
+  std::string EncodeRpIdAndUserId(base::StringPiece user_id) {
+    return *CredentialMetadata::EncodeRpIdAndUserId(key_, rp_id_,
+                                                    to_bytes(user_id));
+  }
+  std::string EncodeRpId() {
+    return *CredentialMetadata::EncodeRpId(key_, rp_id_);
+  }
+
+  std::vector<uint8_t> default_id_ = {0, 1, 2, 3};
+  std::string rp_id_ = "acme.com";
+  std::string key_ = "supersecretsupersecretsupersecre";
+  std::string wrong_key_ = "SUPERsecretsupersecretsupersecre";
+};
+
+TEST_F(CredentialMetadataTest, CredentialId) {
+  std::vector<uint8_t> id = SealCredentialId(DefaultUser());
+  EXPECT_EQ(0, (id)[0]);
+  EXPECT_EQ(54u, id.size());
+  EXPECT_TRUE(UserEqual(UnsealCredentialId(id), DefaultUser()));
+}
+
+TEST_F(CredentialMetadataTest, CredentialId_IsRandom) {
+  EXPECT_NE(SealCredentialId(DefaultUser()), SealCredentialId(DefaultUser()));
+}
+
+TEST_F(CredentialMetadataTest, CredentialId_FailDecrypt) {
+  const auto id = SealCredentialId(DefaultUser());
+  // Flipping a bit in version, nonce, or ciphertext will fail credential
+  // decryption.
+  for (size_t i = 0; i < id.size(); i++) {
+    std::vector<uint8_t> new_id(id);
+    new_id[i] = new_id[i] ^ 0x01;
+    EXPECT_FALSE(CredentialMetadata::UnsealCredentialId(key_, rp_id_, new_id));
+  }
+}
+
+TEST_F(CredentialMetadataTest, CredentialId_InvalidRp) {
+  std::vector<uint8_t> id = SealCredentialId(DefaultUser());
+  // The credential id is authenticated with the RP, thus decryption under a
+  // different RP fails.
+  EXPECT_FALSE(CredentialMetadata::UnsealCredentialId(key_, "notacme.com", id));
+}
+
+TEST_F(CredentialMetadataTest, EncodeRpIdAndUserId) {
+  EXPECT_EQ(64u, EncodeRpIdAndUserId("user@acme.com").size())
+      << EncodeRpIdAndUserId("user@acme.com");
+
+  EXPECT_EQ(EncodeRpIdAndUserId("user"), EncodeRpIdAndUserId("user"));
+  EXPECT_NE(EncodeRpIdAndUserId("userA"), EncodeRpIdAndUserId("userB"));
+  EXPECT_NE(EncodeRpIdAndUserId("user"),
+            *CredentialMetadata::EncodeRpIdAndUserId(key_, "notacme.com",
+                                                     to_bytes("user")));
+  EXPECT_NE(EncodeRpIdAndUserId("user"),
+            *CredentialMetadata::EncodeRpIdAndUserId(wrong_key_, rp_id_,
+                                                     to_bytes("user")));
+}
+
+TEST_F(CredentialMetadataTest, EncodeRpId) {
+  EXPECT_EQ(64u, EncodeRpId().size());
+
+  EXPECT_EQ(EncodeRpId(), EncodeRpId());
+  EXPECT_NE(EncodeRpId(), *CredentialMetadata::EncodeRpId(key_, "notacme.com"));
+  EXPECT_NE(EncodeRpId(), *CredentialMetadata::EncodeRpId(wrong_key_, rp_id_));
+}
+
+}  // namespace
+}  // namespace mac
+}  // namespace fido
+}  // namespace device

device/fido/mac/get_assertion_operation.mm

@@ -39,6 +39,12 @@ const std::string& GetAssertionOperation::RpId() const {
 }
 
 void GetAssertionOperation::Run() {
+  if (!Init()) {
+    std::move(callback())
+        .Run(CtapDeviceResponseCode::kCtap2ErrOther, base::nullopt);
+    return;
+  }
+
   // Prompt the user for consent.
   // TODO(martinkr): Localize reason strings.
   PromptTouchId("sign in to " + RpId());

device/fido/mac/make_credential_operation.h

@@ -56,8 +56,11 @@ class API_AVAILABLE(macosx(10.12.2))
   void Run() override;
 
  private:
+  // OperationBase:
   const std::string& RpId() const override;
   void PromptTouchIdDone(bool success, NSError* err) override;
+
+  base::Optional<std::vector<uint8_t>> GenerateCredentialIdForRequest() const;
 };
 
 }  // namespace mac

device/fido/mac/make_credential_operation.mm

@@ -14,6 +14,7 @@
 #include "device/fido/fido_attestation_statement.h"
 #include "device/fido/fido_constants.h"
 #include "device/fido/fido_parsing_utils.h"
+#include "device/fido/mac/credential_metadata.h"
 #include "device/fido/mac/keychain.h"
 #include "device/fido/mac/util.h"
 
@@ -41,6 +42,12 @@ const std::string& MakeCredentialOperation::RpId() const {
 }
 
 void MakeCredentialOperation::Run() {
+  if (!Init()) {
+    std::move(callback())
+        .Run(CtapDeviceResponseCode::kCtap2ErrOther, base::nullopt);
+    return;
+  }
+
   // Verify pubKeyCredParams contains ES-256, which is the only algorithm we
   // support.
   auto is_es256 =
@@ -102,13 +109,19 @@ 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(RpId(), request().user().user_id());
+  base::Optional<std::string> encoded_rp_id_user_id =
+      CredentialMetadata::EncodeRpIdAndUserId(profile_id(), RpId(),
+                                              request().user().user_id());
+  if (!encoded_rp_id_user_id) {
+    // Internal error.
+    std::move(callback())
+        .Run(CtapDeviceResponseCode::kCtap2ErrOther, base::nullopt);
+    return;
+  }
   {
     ScopedCFTypeRef<CFMutableDictionaryRef> query = DefaultKeychainQuery();
-    CFDictionarySetValue(query, kSecAttrApplicationLabel,
-                         [NSData dataWithBytes:keychain_item_id.data()
-                                        length:keychain_item_id.size()]);
+    CFDictionarySetValue(query, kSecAttrApplicationTag,
+                         base::SysUTF8ToNSString(*encoded_rp_id_user_id));
     OSStatus status = Keychain::GetInstance().ItemDelete(query);
     if (status != errSecSuccess && status != errSecItemNotFound) {
       // Internal keychain error.
@@ -120,6 +133,15 @@ void MakeCredentialOperation::PromptTouchIdDone(bool success, NSError* err) {
   }
 
   // Generate the new key pair.
+  base::Optional<std::vector<uint8_t>> credential_id =
+      GenerateCredentialIdForRequest();
+  if (!credential_id) {
+    DLOG(ERROR) << "GenerateCredentialIdForRequest failed";
+    std::move(callback())
+        .Run(CtapDeviceResponseCode::kCtap2ErrOther, base::nullopt);
+    return;
+  }
+
   ScopedCFTypeRef<CFMutableDictionaryRef> params(
       CFDictionaryCreateMutable(kCFAllocatorDefault, 0, nullptr, nullptr));
   CFDictionarySetValue(params, kSecAttrKeyType,
@@ -136,9 +158,11 @@ void MakeCredentialOperation::PromptTouchIdDone(bool success, NSError* err) {
                        access_control());
   CFDictionarySetValue(private_key_params, kSecUseAuthenticationContext,
                        authentication_context());
+  CFDictionarySetValue(private_key_params, kSecAttrApplicationTag,
+                       base::SysUTF8ToNSString(*encoded_rp_id_user_id));
   CFDictionarySetValue(private_key_params, kSecAttrApplicationLabel,
-                       [NSData dataWithBytes:keychain_item_id.data()
-                                      length:keychain_item_id.size()]);
+                       [NSData dataWithBytes:credential_id->data()
+                                      length:credential_id->size()]);
 
   ScopedCFTypeRef<CFErrorRef> cferr;
   ScopedCFTypeRef<SecKeyRef> private_key(
@@ -162,7 +186,7 @@ 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(RpId(), keychain_item_id, public_key);
+      MakeAuthenticatorData(RpId(), *credential_id, public_key);
   if (!authenticator_data) {
     DLOG(ERROR) << "MakeAuthenticatorData failed";
     std::move(callback())
@@ -187,6 +211,15 @@ void MakeCredentialOperation::PromptTouchIdDone(bool success, NSError* err) {
       .Run(CtapDeviceResponseCode::kSuccess, std::move(response));
 }
 
+base::Optional<std::vector<uint8_t>>
+MakeCredentialOperation::GenerateCredentialIdForRequest() const {
+  return CredentialMetadata::SealCredentialId(
+      profile_id(), RpId(),
+      CredentialMetadata::UserEntity(
+          request().user().user_id(), request().user().user_name().value_or(""),
+          request().user().user_display_name().value_or("")));
+}
+
 }  // namespace mac
 }  // namespace fido
 }  // namespace device

device/fido/mac/operation_base.h

@@ -13,6 +13,7 @@
 #include "base/mac/scoped_cftyperef.h"
 #include "base/macros.h"
 #include "base/strings/sys_string_conversions.h"
+#include "device/fido/mac/credential_metadata.h"
 #include "device/fido/mac/operation.h"
 #include "device/fido/mac/touch_id_context.h"
 
@@ -37,9 +38,21 @@ class API_AVAILABLE(macosx(10.12.2)) OperationBase : public Operation {
         keychain_access_group_(std::move(keychain_access_group)),
         callback_(std::move(callback)),
         touch_id_context_(std::make_unique<TouchIdContext>()) {}
+
   ~OperationBase() override = default;
 
  protected:
+  // Subclasses must call Init() at the beginning of Run().
+  bool Init() {
+    base::Optional<std::string> rp_id =
+        CredentialMetadata::EncodeRpId(profile_id(), RpId());
+    if (!rp_id)
+      return false;
+
+    encoded_rp_id_ = std::move(*rp_id);
+    return true;
+  }
+
   // PromptTouchId triggers a Touch ID consent dialog with the given reason
   // string. Subclasses implement the PromptTouchIdDone callback to receive the
   // result.
@@ -76,12 +89,14 @@ class API_AVAILABLE(macosx(10.12.2)) OperationBase : public Operation {
     CFDictionarySetValue(query, kSecClass, kSecClassKey);
     CFDictionarySetValue(query, kSecAttrAccessGroup,
                          base::SysUTF8ToNSString(keychain_access_group_));
-    CFDictionarySetValue(query, kSecAttrLabel, base::SysUTF8ToNSString(RpId()));
-    CFDictionarySetValue(query, kSecAttrApplicationTag,
-                         base::SysUTF8ToNSString(profile_id_));
+    DCHECK(!encoded_rp_id_.empty());
+    CFDictionarySetValue(query, kSecAttrLabel,
+                         base::SysUTF8ToNSString(encoded_rp_id_));
     return query;
   }
 
+  const std::string& profile_id() const { return profile_id_; }
+
   const Request& request() const { return request_; }
   Callback& callback() { return callback_; }
 
@@ -89,6 +104,7 @@ class API_AVAILABLE(macosx(10.12.2)) OperationBase : public Operation {
   Request request_;
   std::string profile_id_;
   std::string keychain_access_group_;
+  std::string encoded_rp_id_ = "";
   Callback callback_;
 
   std::unique_ptr<TouchIdContext> touch_id_context_;

device/fido/mac/util.h

@@ -19,12 +19,6 @@ namespace device {
 namespace fido {
 namespace mac {
 
-// KeychainItemIdentifier returns the unique identifier for a key pair, derived
-// from an RP ID and user handle. It is stored in the keychain items
-// kSecAttrApplicationLabel attribute and can be used for lookup.
-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 Relying Party ID, credential ID and public key.
 // It returns |base::nullopt| on failure.

device/fido/mac/util.mm

@@ -69,19 +69,6 @@ std::unique_ptr<ECPublicKey> MakeECPublicKey(SecKeyRef public_key_ref)
 
 }  // namespace
 
-// KeychainItemIdentifier returns the unique identifier for a given RP ID
-// and user handle. It is stored in the keychain items Application Label and
-// used for later lookup.
-std::vector<uint8_t> KeychainItemIdentifier(std::string rp_id,
-                                            std::vector<uint8_t> user_id) {
-  std::vector<CBORValue> array;
-  array.emplace_back(CBORValue(rp_id));
-  array.emplace_back(CBORValue(user_id));
-  auto value = CBORWriter::Write(CBORValue(std::move(array)));
-  CHECK(value);
-  return *value;
-}
-
 base::Optional<AuthenticatorData> MakeAuthenticatorData(
     const std::string& rp_id,
     std::vector<uint8_t> credential_id,