//device/fido: fix Touch ID AuthenticatorGetAssertionResponse generation

This makes GetAssertionOperation decrypt the matching credential id
id into its UserEntity value and carry those values as well as the
credential id into the returned response. It was previously returning
incomplete responses.

Also fix the signature counter to 1, rather than 0, because webauthndemo
insists that the number be greater than 0.

Bug: 803842
Change-Id: I42e22ca9fb571ad6dea924bd765aa276d8c62d1a
Reviewed-on: https://chromium-review.googlesource.com/1103518
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568720}

device/fido/mac/credential_metadata.cc

@@ -10,6 +10,7 @@
 #include "components/cbor/cbor_reader.h"
 #include "components/cbor/cbor_values.h"
 #include "components/cbor/cbor_writer.h"
+#include "device/fido/public_key_credential_user_entity.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"
@@ -59,6 +60,27 @@ CredentialMetadata::CredentialMetadata(const std::string& secret)
     : secret_(secret) {}
 CredentialMetadata::~CredentialMetadata() = default;
 
+// static
+CredentialMetadata::UserEntity
+CredentialMetadata::UserEntity::FromPublicKeyCredentialUserEntity(
+    const PublicKeyCredentialUserEntity& user) {
+  return CredentialMetadata::UserEntity(user.user_id(),
+                                        user.user_name().value_or(""),
+                                        user.user_display_name().value_or(""));
+}
+
+PublicKeyCredentialUserEntity
+CredentialMetadata::UserEntity::ToPublicKeyCredentialUserEntity() {
+  auto user_entity = PublicKeyCredentialUserEntity(id);
+  if (!name.empty()) {
+    user_entity.SetUserName(name);
+  }
+  if (!display_name.empty()) {
+    user_entity.SetDisplayName(display_name);
+  }
+  return user_entity;
+}
+
 CredentialMetadata::UserEntity::UserEntity(std::vector<uint8_t> id_,
                                            std::string name_,
                                            std::string display_name_)

device/fido/mac/credential_metadata.h

@@ -19,6 +19,9 @@
 #include "crypto/symmetric_key.h"
 
 namespace device {
+
+class PublicKeyCredentialUserEntity;
+
 namespace fido {
 namespace mac {
 
@@ -52,6 +55,9 @@ class COMPONENT_EXPORT(DEVICE_FIDO) CredentialMetadata {
   // this type should be moved whenever possible.
   struct UserEntity {
    public:
+    static UserEntity FromPublicKeyCredentialUserEntity(
+        const PublicKeyCredentialUserEntity&);
+
     UserEntity(std::vector<uint8_t> id_,
                std::string name_,
                std::string display_);
@@ -60,6 +66,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) CredentialMetadata {
     UserEntity& operator=(UserEntity&&);
     ~UserEntity();
 
+    PublicKeyCredentialUserEntity ToPublicKeyCredentialUserEntity();
+
     std::vector<uint8_t> id;
     std::string name;
     std::string display_name;

device/fido/mac/credential_metadata_unittest.cc

@@ -4,7 +4,9 @@
 
 #include "device/fido/mac/credential_metadata.h"
 
+#include "device/fido/public_key_credential_user_entity.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
 
 namespace device {
 namespace fido {
@@ -110,6 +112,30 @@ TEST(CredentialMetadata, GenerateRandomSecret) {
   EXPECT_NE(s1, s2);
 }
 
+TEST(CredentialMetadata, FromPublicKeyCredentialUserEntity) {
+  std::vector<uint8_t> user_id = {{1, 2, 3}};
+  PublicKeyCredentialUserEntity in(user_id);
+  in.SetUserName("username");
+  in.SetDisplayName("display name");
+  in.SetIconUrl(GURL("http://rp.foo/user.png"));
+  CredentialMetadata::UserEntity out =
+      CredentialMetadata::UserEntity::FromPublicKeyCredentialUserEntity(
+          std::move(in));
+  EXPECT_EQ(user_id, out.id);
+  EXPECT_EQ("username", out.name);
+  EXPECT_EQ("display name", out.display_name);
+}
+
+TEST(CredentialMetadata, ToPublicKeyCredentialUserEntity) {
+  std::vector<uint8_t> user_id = {{1, 2, 3}};
+  CredentialMetadata::UserEntity in(user_id, "username", "display name");
+  PublicKeyCredentialUserEntity out = in.ToPublicKeyCredentialUserEntity();
+  EXPECT_EQ(user_id, out.user_id());
+  EXPECT_EQ("username", out.user_name().value());
+  EXPECT_EQ("display name", out.user_display_name().value());
+  EXPECT_FALSE(out.user_icon_url().has_value());
+}
+
 }  // namespace
 }  // namespace mac
 }  // namespace fido

device/fido/mac/get_assertion_operation.h

@@ -31,7 +31,7 @@ class API_AVAILABLE(macosx(10.12.2))
                            AuthenticatorGetAssertionResponse> {
  public:
   GetAssertionOperation(CtapGetAssertionRequest request,
-                        std::string profile_id,
+                        std::string metadata_secret,
                         std::string keychain_access_group,
                         Callback callback);
   ~GetAssertionOperation() override;

device/fido/mac/get_assertion_operation.mm

@@ -16,6 +16,8 @@
 #include "device/fido/fido_constants.h"
 #include "device/fido/mac/keychain.h"
 #include "device/fido/mac/util.h"
+#include "device/fido/public_key_credential_descriptor.h"
+#include "device/fido/public_key_credential_user_entity.h"
 
 namespace device {
 namespace fido {
@@ -126,6 +128,21 @@ void GetAssertionOperation::PromptTouchIdDone(bool success, NSError* err) {
         .Run(CtapDeviceResponseCode::kCtap2ErrNoCredentials, base::nullopt);
     return;
   }
+
+  // Decrypt the user entity from the credential ID.
+  base::Optional<CredentialMetadata::UserEntity> credential_user =
+      CredentialMetadata::UnsealCredentialId(metadata_secret(), RpId(),
+                                             credential_id);
+  if (!credential_user) {
+    // The keychain query already filtered for the RP ID encoded under this
+    // operation's metadata secret, so the credential id really should have
+    // been decryptable.
+    DVLOG(1) << "UnsealCredentialId failed";
+    std::move(callback())
+        .Run(CtapDeviceResponseCode::kCtap2ErrNoCredentials, base::nullopt);
+    return;
+  }
+
   base::ScopedCFTypeRef<SecKeyRef> public_key(
       Keychain::GetInstance().KeyCopyPublicKey(private_key));
   if (!public_key) {
@@ -137,7 +154,7 @@ void GetAssertionOperation::PromptTouchIdDone(bool success, NSError* err) {
   }
 
   base::Optional<AuthenticatorData> authenticator_data =
-      MakeAuthenticatorData(RpId(), std::move(credential_id), public_key);
+      MakeAuthenticatorData(RpId(), credential_id, public_key);
   if (!authenticator_data) {
     DLOG(ERROR) << "MakeAuthenticatorData failed";
     std::move(callback())
@@ -152,10 +169,14 @@ void GetAssertionOperation::PromptTouchIdDone(bool success, NSError* err) {
         .Run(CtapDeviceResponseCode::kCtap2ErrOther, base::nullopt);
     return;
   }
+  auto response = AuthenticatorGetAssertionResponse(
+      std::move(*authenticator_data), std::move(*signature));
+  response.SetCredential(PublicKeyCredentialDescriptor(
+      CredentialType::kPublicKey, std::move(credential_id)));
+  response.SetUserEntity(credential_user->ToPublicKeyCredentialUserEntity());
+
   std::move(callback())
-      .Run(CtapDeviceResponseCode::kSuccess,
-           AuthenticatorGetAssertionResponse(std::move(*authenticator_data),
-                                             std::move(*signature)));
+      .Run(CtapDeviceResponseCode::kSuccess, std::move(response));
 }
 
 }  // namespace mac

device/fido/mac/get_assertion_operation_unittest_mac.mm

@@ -78,6 +78,8 @@ API_AVAILABLE(macos(10.12.2)) {
   EXPECT_EQ(CtapDeviceResponseCode::kSuccess, error);
   auto opt_response = std::move(std::get<1>(result));
   ASSERT_TRUE(opt_response);
+  ASSERT_TRUE(opt_response->credential());
+  EXPECT_FALSE(opt_response->credential()->id().empty());
 };
 }
 }  // namespace mac

device/fido/mac/make_credential_operation.mm

@@ -215,9 +215,8 @@ base::Optional<std::vector<uint8_t>>
 MakeCredentialOperation::GenerateCredentialIdForRequest() const {
   return CredentialMetadata::SealCredentialId(
       metadata_secret(), RpId(),
-      CredentialMetadata::UserEntity(
-          request().user().user_id(), request().user().user_name().value_or(""),
-          request().user().user_display_name().value_or("")));
+      CredentialMetadata::UserEntity::FromPublicKeyCredentialUserEntity(
+          request().user()));
 }
 
 }  // namespace mac

device/fido/mac/util.mm

@@ -83,7 +83,8 @@ base::Optional<AuthenticatorData> MakeAuthenticatorData(
   constexpr uint8_t flags =
       static_cast<uint8_t>(AuthenticatorData::Flag::kTestOfUserVerification) |
       static_cast<uint8_t>(AuthenticatorData::Flag::kAttestation);
-  std::array<uint8_t, 4> counter = {0, 0, 0, 0};  // implement
+  // TODO(martinkr): Implement signature counters.
+  std::array<uint8_t, 4> counter = {0, 0, 0, 1};
   auto ec_public_key = MakeECPublicKey(public_key);
   if (!ec_public_key) {
     LOG(ERROR) << "MakeECPublicKey failed";