fido/bio: add support for enrollment renaming

Expand bio enrollment support for CTAP2.1 §5.7.7, renaming enrollments.
According to the standard, if an enrollment doesn't exist the operation
will return INVALID_OPTION like enumeration. This CL also switches
from BioEnrollmentHandler's OnceClosure to
OnceCallback<CtapDeviceResponseCode> since the status from enumerating
and renaming must be checked.

Change-Id: I38fc7f063edd28538d8414ff7a4bb6054c779796
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1654882Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Commit-Queue: Matthew Webb <noviv@google.com>
Cr-Commit-Position: refs/heads/master@{#668998}

device/fido/bio/enrollment.cc

@@ -82,6 +82,30 @@ BioEnrollmentRequest BioEnrollmentRequest::ForEnumerate(
   return request;
 }
 
+// static
+BioEnrollmentRequest BioEnrollmentRequest::ForRename(
+    const pin::TokenResponse& response,
+    std::vector<uint8_t> id,
+    std::string name) {
+  BioEnrollmentRequest request;
+  request.pin_protocol = 1;
+  request.modality = BioEnrollmentModality::kFingerprint;
+  request.subcommand = BioEnrollmentSubCommand::kSetFriendlyName;
+  request.params = cbor::Value::MapValue();
+  request.params->emplace(
+      static_cast<int>(BioEnrollmentSubCommandParam::kTemplateId),
+      cbor::Value(std::move(id)));
+
+  std::vector<uint8_t> pin_auth =
+      *cbor::Writer::Write(cbor::Value(*request.params));
+  pin_auth.insert(pin_auth.begin(), static_cast<int>(*request.subcommand));
+  pin_auth.insert(pin_auth.begin(), static_cast<int>(*request.modality));
+
+  request.pin_auth = response.PinAuth(std::move(pin_auth));
+
+  return request;
+}
+
 BioEnrollmentRequest::BioEnrollmentRequest(BioEnrollmentRequest&&) = default;
 BioEnrollmentRequest& BioEnrollmentRequest::operator=(BioEnrollmentRequest&&) =
     default;

device/fido/bio/enrollment.h

@@ -108,7 +108,10 @@ struct BioEnrollmentRequest {
       const pin::TokenResponse& pin_token,
       std::vector<uint8_t> template_id);
   static BioEnrollmentRequest ForCancel();
-  static BioEnrollmentRequest ForEnumerate(const pin::TokenResponse&);
+  static BioEnrollmentRequest ForEnumerate(const pin::TokenResponse& token);
+  static BioEnrollmentRequest ForRename(const pin::TokenResponse& token,
+                                        std::vector<uint8_t> id,
+                                        std::string name);
 
   base::Optional<BioEnrollmentModality> modality;
   base::Optional<BioEnrollmentSubCommand> subcommand;

device/fido/bio/enrollment_handler.cc

@@ -68,7 +68,7 @@ void BioEnrollmentHandler::EnrollTemplate(ResponseCallback callback) {
                           weak_factory_.GetWeakPtr()));
 }
 
-void BioEnrollmentHandler::Cancel(base::OnceClosure callback) {
+void BioEnrollmentHandler::Cancel(StatusCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker);
 
   status_callback_ = std::move(callback);
@@ -86,6 +86,18 @@ void BioEnrollmentHandler::EnumerateTemplates(ResponseCallback callback) {
                      weak_factory_.GetWeakPtr()));
 }
 
+void BioEnrollmentHandler::RenameTemplate(std::vector<uint8_t> id,
+                                          std::string name,
+                                          StatusCallback callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker);
+
+  status_callback_ = std::move(callback);
+  authenticator_->BioEnrollRename(
+      *pin_token_response_, std::move(id), std::move(name),
+      base::BindOnce(&BioEnrollmentHandler::OnRenameTemplate,
+                     weak_factory_.GetWeakPtr()));
+}
+
 void BioEnrollmentHandler::DispatchRequest(FidoAuthenticator* authenticator) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker);
   authenticator->GetTouch(base::BindOnce(&BioEnrollmentHandler::OnTouch,
@@ -220,9 +232,9 @@ void BioEnrollmentHandler::OnEnrollTemplate(
   std::move(response_callback_).Run(code, std::move(response));
 }
 
-void BioEnrollmentHandler::OnCancel(CtapDeviceResponseCode,
+void BioEnrollmentHandler::OnCancel(CtapDeviceResponseCode code,
                                     base::Optional<BioEnrollmentResponse>) {
-  std::move(status_callback_).Run();
+  std::move(status_callback_).Run(code);
 }
 
 void BioEnrollmentHandler::OnEnumerateTemplates(
@@ -243,4 +255,10 @@ void BioEnrollmentHandler::OnEnumerateTemplates(
   std::move(response_callback_).Run(code, std::move(response));
 }
 
+void BioEnrollmentHandler::OnRenameTemplate(
+    CtapDeviceResponseCode code,
+    base::Optional<BioEnrollmentResponse> response) {
+  std::move(status_callback_).Run(code);
+}
+
 }  // namespace device

device/fido/bio/enrollment_handler.h

@@ -29,6 +29,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) BioEnrollmentHandler
   using ResponseCallback =
       base::OnceCallback<void(CtapDeviceResponseCode,
                               base::Optional<BioEnrollmentResponse>)>;
+  using StatusCallback = base::OnceCallback<void(CtapDeviceResponseCode)>;
 
   BioEnrollmentHandler(
       service_manager::Connector* connector,
@@ -43,8 +44,11 @@ class COMPONENT_EXPORT(DEVICE_FIDO) BioEnrollmentHandler
   void GetModality(ResponseCallback);
   void GetSensorInfo(ResponseCallback);
   void EnrollTemplate(ResponseCallback);
-  void Cancel(base::OnceClosure);
+  void Cancel(StatusCallback);
   void EnumerateTemplates(ResponseCallback);
+  void RenameTemplate(std::vector<uint8_t> id,
+                      std::string name,
+                      StatusCallback);
 
  private:
   // FidoRequestHandlerBase:
@@ -65,6 +69,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) BioEnrollmentHandler
   void OnCancel(CtapDeviceResponseCode, base::Optional<BioEnrollmentResponse>);
   void OnEnumerateTemplates(CtapDeviceResponseCode,
                             base::Optional<BioEnrollmentResponse>);
+  void OnRenameTemplate(CtapDeviceResponseCode,
+                        base::Optional<BioEnrollmentResponse>);
 
   SEQUENCE_CHECKER(sequence_checker);
 
@@ -73,7 +79,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) BioEnrollmentHandler
   ErrorCallback error_callback_;
   GetPINCallback get_pin_callback_;
   ResponseCallback response_callback_;
-  base::OnceClosure status_callback_;
+  StatusCallback status_callback_;
   base::Optional<pin::TokenResponse> pin_token_response_;
   base::WeakPtrFactory<BioEnrollmentHandler> weak_factory_;
 

device/fido/bio/enrollment_handler_unittest.cc

@@ -4,6 +4,7 @@
 
 #include <vector>
 
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #include "base/bind.h"
@@ -182,11 +183,14 @@ TEST_F(BioEnrollmentHandlerTest, CancelNoEnroll) {
   auto handler = MakeHandler();
   ready_callback_.WaitForCallback();
 
-  test::TestCallbackReceiver<> cb;
+  test::ValueCallbackReceiver<CtapDeviceResponseCode> cb;
   handler->Cancel(cb.callback());
   cb.WaitForCallback();
+
+  EXPECT_EQ(cb.value(), CtapDeviceResponseCode::kSuccess);
 }
 
+// Tests enumerating enrollments expecting a list of size 1.
 TEST_F(BioEnrollmentHandlerTest, Enumerate) {
   VirtualCtap2Device::Config config;
   config.pin_support = true;
@@ -215,5 +219,31 @@ TEST_F(BioEnrollmentHandlerTest, Enumerate) {
   EXPECT_EQ(v, expected);
 }
 
+// Tests renaming an enrollment (success and failure).
+TEST_F(BioEnrollmentHandlerTest, Rename) {
+  VirtualCtap2Device::Config config;
+  config.pin_support = true;
+  config.bio_enrollment_support = true;
+
+  virtual_device_factory_.SetCtap2Config(config);
+
+  auto handler = MakeHandler();
+  ready_callback_.WaitForCallback();
+
+  // Rename existing enrollment.
+  test::ValueCallbackReceiver<CtapDeviceResponseCode> cb0;
+  handler->RenameTemplate({0, 0, 0, 1}, "OtherFingerprint1", cb0.callback());
+
+  cb0.WaitForCallback();
+  EXPECT_EQ(cb0.value(), CtapDeviceResponseCode::kSuccess);
+
+  // Rename non-existent enrollment.
+  test::ValueCallbackReceiver<CtapDeviceResponseCode> cb1;
+  handler->RenameTemplate({0, 0, 0, 2}, "OtherFingerprint2", cb1.callback());
+
+  cb1.WaitForCallback();
+  EXPECT_EQ(cb1.value(), CtapDeviceResponseCode::kCtap2ErrInvalidOption);
+}
+
 }  // namespace
 }  // namespace device

device/fido/fido_authenticator.cc

@@ -105,6 +105,13 @@ void FidoAuthenticator::BioEnrollEnumerate(pin::TokenResponse,
   NOTREACHED();
 }
 
+void FidoAuthenticator::BioEnrollRename(pin::TokenResponse,
+                                        std::vector<uint8_t>,
+                                        std::string,
+                                        BioEnrollmentCallback) {
+  NOTREACHED();
+}
+
 void FidoAuthenticator::Reset(ResetCallback callback) {
   std::move(callback).Run(CtapDeviceResponseCode::kCtap1ErrInvalidCommand,
                           base::nullopt);

device/fido/fido_authenticator.h

@@ -174,6 +174,10 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoAuthenticator {
   virtual void BioEnrollFingerprint(pin::TokenResponse, BioEnrollmentCallback);
   virtual void BioEnrollCancel(BioEnrollmentCallback);
   virtual void BioEnrollEnumerate(pin::TokenResponse, BioEnrollmentCallback);
+  virtual void BioEnrollRename(pin::TokenResponse,
+                               std::vector<uint8_t>,
+                               std::string,
+                               BioEnrollmentCallback);
 
   // Reset triggers a reset operation on the authenticator. This erases all
   // stored resident keys and any configured PIN.

device/fido/fido_device_authenticator.cc

@@ -527,6 +527,23 @@ void FidoDeviceAuthenticator::BioEnrollFingerprint(
       base::BindOnce(&BioEnrollmentResponse::Parse));
 }
 
+void FidoDeviceAuthenticator::BioEnrollRename(pin::TokenResponse token,
+                                              std::vector<uint8_t> id,
+                                              std::string name,
+                                              BioEnrollmentCallback callback) {
+  DCHECK(
+      Options()->bio_enrollment_availability_preview !=
+      AuthenticatorSupportedOptions::BioEnrollmentAvailability::kNotSupported);
+
+  operation_ = std::make_unique<
+      Ctap2DeviceOperation<BioEnrollmentRequest, BioEnrollmentResponse>>(
+      device_.get(),
+      BioEnrollmentRequest::ForRename(token, std::move(id), std::move(name)),
+      std::move(callback), base::BindOnce(&BioEnrollmentResponse::Parse),
+      /*string_fixup_predicate=*/nullptr);
+  operation_->Start();
+}
+
 void FidoDeviceAuthenticator::OnBioEnroll(
     pin::TokenResponse token,
     BioEnrollmentCallback callback,

device/fido/fido_device_authenticator.h

@@ -84,6 +84,10 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDeviceAuthenticator
   void BioEnrollFingerprint(pin::TokenResponse, BioEnrollmentCallback) override;
   void BioEnrollCancel(BioEnrollmentCallback) override;
   void BioEnrollEnumerate(pin::TokenResponse, BioEnrollmentCallback) override;
+  void BioEnrollRename(pin::TokenResponse,
+                       std::vector<uint8_t>,
+                       std::string,
+                       BioEnrollmentCallback) override;
 
   void Reset(ResetCallback callback) override;
   void Cancel() override;

device/fido/virtual_ctap2_device.cc

@@ -1364,6 +1364,7 @@ CtapDeviceResponseCode VirtualCtap2Device::OnBioEnrollment(
     return CtapDeviceResponseCode::kCtap2ErrCBORUnexpectedType;
   }
 
+  // List of enrollments currently on test key.
   cbor::Value::MapValue id0;
   id0.emplace(cbor::Value(static_cast<int>(
                   BioEnrollmentTemplateInfoParam::kTemplateId)),
@@ -1375,6 +1376,20 @@ CtapDeviceResponseCode VirtualCtap2Device::OnBioEnrollment(
   cbor::Value::ArrayValue enumerated_ids;
   enumerated_ids.emplace_back(id0);
 
+  // Template id from subcommand parameters, if it exists.
+  base::Optional<std::vector<uint8_t>> template_id;
+
+  auto params_it = request_map.find(cbor::Value(
+      static_cast<int>(BioEnrollmentRequestKey::kSubCommandParams)));
+  if (params_it != request_map.end()) {
+    const auto& params = params_it->second.GetMap();
+    auto template_it = params.find(cbor::Value(
+        static_cast<int>(BioEnrollmentSubCommandParam::kTemplateId)));
+    if (template_it != params.end() && template_it->second.is_bytestring()) {
+      template_id = template_it->second.GetBytestring();
+    }
+  }
+
   switch (it->second.GetUnsigned()) {
     case static_cast<int>(SubCmd::kGetFingerprintSensorInfo):
       response_map.emplace(
@@ -1405,13 +1420,18 @@ CtapDeviceResponseCode VirtualCtap2Device::OnBioEnrollment(
       response_map.emplace(
           static_cast<int>(BioEnrollmentResponseKey::kRemainingSamples), 0);
       break;
-    case static_cast<int>(SubCmd::kCancelCurrentEnrollment):
-      return CtapDeviceResponseCode::kSuccess;
     case static_cast<int>(SubCmd::kEnumerateEnrollments):
       response_map.emplace(
           static_cast<int>(BioEnrollmentResponseKey::kTemplateInfos),
           enumerated_ids);
       break;
+    case static_cast<int>(SubCmd::kSetFriendlyName):
+      if (template_id && *template_id == std::vector<uint8_t>{0, 0, 0, 2}) {
+        return CtapDeviceResponseCode::kCtap2ErrInvalidOption;
+      }
+      [[fallthrough]];
+    case static_cast<int>(SubCmd::kCancelCurrentEnrollment):
+      return CtapDeviceResponseCode::kSuccess;
     default:
       // Handle all other commands as if they were unsupported (will change
       // when support is added).