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

This adds a CredentialMetadata class to encrypt any account metadata
associated with a WebAuthn credential (user ID, user name, user display
name, and RP ID) before writing it to the macOS keychain.

The key will be generated and stored in the Chrome profile under which
the credential was created. (It is currently hardcoded but I'm changing
that in a follow-up CL.) Deletion of the profile or key will therefore
render the data unreadable.

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

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);
+  DCHECK(hmac.Init(DeriveKey(profile_id_, Algorithm::kHmacSha256)));
+  std::vector<uint8_t> digest(hmac.DigestLength());
+  if (!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,