Enforce stricter restrictions of version response

Response to CTAP AuthenticatorGetInfo command contains supported versions.
Previously, the CTAP spec did not specify exactly which strings to expect
for versions, but with the new spec, only 2 -- "FIDO_2_0" and "U2F_V_2"
are expected to be present as versions.

Bug: 851057
Change-Id: Iac1b3c45205a956e927aa2be3e6064c479b84481
Reviewed-on: https://chromium-review.googlesource.com/1093542
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568180}

device/fido/authenticator_get_info_response.cc

@@ -9,7 +9,7 @@
 namespace device {
 
 AuthenticatorGetInfoResponse::AuthenticatorGetInfoResponse(
-    std::vector<std::string> versions,
+    base::flat_set<ProtocolVersion> versions,
     std::vector<uint8_t> aaguid)
     : versions_(std::move(versions)), aaguid_(std::move(aaguid)) {}
 
@@ -22,7 +22,7 @@ AuthenticatorGetInfoResponse& AuthenticatorGetInfoResponse::operator=(
 AuthenticatorGetInfoResponse::~AuthenticatorGetInfoResponse() = default;
 
 AuthenticatorGetInfoResponse& AuthenticatorGetInfoResponse::SetMaxMsgSize(
-    uint8_t max_msg_size) {
+    uint32_t max_msg_size) {
   max_msg_size_ = max_msg_size;
   return *this;
 }

device/fido/authenticator_get_info_response.h

@@ -11,6 +11,7 @@
 #include <vector>
 
 #include "base/component_export.h"
+#include "base/containers/flat_set.h"
 #include "base/macros.h"
 #include "base/optional.h"
 #include "device/fido/authenticator_supported_options.h"
@@ -24,13 +25,13 @@ namespace device {
 // https://fidoalliance.org/specs/fido-v2.0-rd-20170927/fido-client-to-authenticator-protocol-v2.0-rd-20170927.html#authenticatorGetInfo
 class COMPONENT_EXPORT(DEVICE_FIDO) AuthenticatorGetInfoResponse {
  public:
-  AuthenticatorGetInfoResponse(std::vector<std::string> versions,
+  AuthenticatorGetInfoResponse(base::flat_set<ProtocolVersion> versions,
                                std::vector<uint8_t> aaguid);
   AuthenticatorGetInfoResponse(AuthenticatorGetInfoResponse&& that);
   AuthenticatorGetInfoResponse& operator=(AuthenticatorGetInfoResponse&& other);
   ~AuthenticatorGetInfoResponse();
 
-  AuthenticatorGetInfoResponse& SetMaxMsgSize(uint8_t max_msg_size);
+  AuthenticatorGetInfoResponse& SetMaxMsgSize(uint32_t max_msg_size);
   AuthenticatorGetInfoResponse& SetPinProtocols(
       std::vector<uint8_t> pin_protocols);
   AuthenticatorGetInfoResponse& SetExtensions(
@@ -38,9 +39,9 @@ class COMPONENT_EXPORT(DEVICE_FIDO) AuthenticatorGetInfoResponse {
   AuthenticatorGetInfoResponse& SetOptions(
       AuthenticatorSupportedOptions options);
 
-  const std::vector<std::string>& versions() { return versions_; }
+  const base::flat_set<ProtocolVersion>& versions() { return versions_; }
   const std::vector<uint8_t>& aaguid() const { return aaguid_; }
-  const base::Optional<uint8_t>& max_msg_size() const { return max_msg_size_; }
+  const base::Optional<uint32_t>& max_msg_size() const { return max_msg_size_; }
   const base::Optional<std::vector<uint8_t>>& pin_protocol() const {
     return pin_protocols_;
   }
@@ -50,9 +51,9 @@ class COMPONENT_EXPORT(DEVICE_FIDO) AuthenticatorGetInfoResponse {
   const AuthenticatorSupportedOptions& options() const { return options_; }
 
  private:
-  std::vector<std::string> versions_;
+  base::flat_set<ProtocolVersion> versions_;
   std::vector<uint8_t> aaguid_;
-  base::Optional<uint8_t> max_msg_size_;
+  base::Optional<uint32_t> max_msg_size_;
   base::Optional<std::vector<uint8_t>> pin_protocols_;
   base::Optional<std::vector<std::string>> extensions_;
   AuthenticatorSupportedOptions options_;

device/fido/ctap_response_unittest.cc

@@ -20,6 +20,122 @@ namespace device {
 
 namespace {
 
+constexpr uint8_t kTestAuthenticatorGetInfoResponseWithNoVersion[] = {
+    // Success status byte
+    0x00,
+    // Map of 6 elements
+    0xA6,
+    // Key(01) - versions
+    0x01,
+    // Array(0)
+    0x80,
+    // Key(02) - extensions
+    0x02,
+    // Array(2)
+    0x82,
+    // "uvm"
+    0x63, 0x75, 0x76, 0x6D,
+    // "hmac-secret"
+    0x6B, 0x68, 0x6D, 0x61, 0x63, 0x2D, 0x73, 0x65, 0x63, 0x72, 0x65, 0x74,
+    // Key(03) - AAGUID
+    0x03,
+    // Bytes(16)
+    0x50, 0xF8, 0xA0, 0x11, 0xF3, 0x8C, 0x0A, 0x4D, 0x15, 0x80, 0x06, 0x17,
+    0x11, 0x1F, 0x9E, 0xDC, 0x7D,
+    // Key(04) - options
+    0x04,
+    // Map(05)
+    0xA5,
+    // Key - "rk"
+    0x62, 0x72, 0x6B,
+    // true
+    0xF5,
+    // Key - "up"
+    0x62, 0x75, 0x70,
+    // true
+    0xF5,
+    // Key - "uv"
+    0x62, 0x75, 0x76,
+    // true
+    0xF5,
+    // Key - "plat"
+    0x64, 0x70, 0x6C, 0x61, 0x74,
+    // true
+    0xF5,
+    // Key - "clientPin"
+    0x69, 0x63, 0x6C, 0x69, 0x65, 0x6E, 0x74, 0x50, 0x69, 0x6E,
+    // false
+    0xF4,
+    // Key(05) - Max message size
+    0x05,
+    // 1200
+    0x19, 0x04, 0xB0,
+    // Key(06) - Pin protocols
+    0x06,
+    // Array[1]
+    0x81, 0x01,
+};
+
+constexpr uint8_t kTestAuthenticatorGetInfoResponseWithDuplicateVersion[] = {
+    // Success status byte
+    0x00,
+    // Map of 6 elements
+    0xA6,
+    // Key(01) - versions
+    0x01,
+    // Array(02)
+    0x82,
+    // "U2F_V2"
+    0x66, 0x55, 0x32, 0x46, 0x5F, 0x56, 0x32,
+    // "U2F_V2"
+    0x66, 0x55, 0x32, 0x46, 0x5F, 0x56, 0x32,
+    // Key(02) - extensions
+    0x02,
+    // Array(2)
+    0x82,
+    // "uvm"
+    0x63, 0x75, 0x76, 0x6D,
+    // "hmac-secret"
+    0x6B, 0x68, 0x6D, 0x61, 0x63, 0x2D, 0x73, 0x65, 0x63, 0x72, 0x65, 0x74,
+    // Key(03) - AAGUID
+    0x03,
+    // Bytes(16)
+    0x50, 0xF8, 0xA0, 0x11, 0xF3, 0x8C, 0x0A, 0x4D, 0x15, 0x80, 0x06, 0x17,
+    0x11, 0x1F, 0x9E, 0xDC, 0x7D,
+    // Key(04) - options
+    0x04,
+    // Map(05)
+    0xA5,
+    // Key - "rk"
+    0x62, 0x72, 0x6B,
+    // true
+    0xF5,
+    // Key - "up"
+    0x62, 0x75, 0x70,
+    // true
+    0xF5,
+    // Key - "uv"
+    0x62, 0x75, 0x76,
+    // true
+    0xF5,
+    // Key - "plat"
+    0x64, 0x70, 0x6C, 0x61, 0x74,
+    // true
+    0xF5,
+    // Key - "clientPin"
+    0x69, 0x63, 0x6C, 0x69, 0x65, 0x6E, 0x74, 0x50, 0x69, 0x6E,
+    // false
+    0xF4,
+    // Key(05) - Max message size
+    0x05,
+    // 1200
+    0x19, 0x04, 0xB0,
+    // Key(06) - Pin protocols
+    0x06,
+    // Array[1]
+    0x81, 0x01,
+};
+
 // The attested credential data, excluding the public key bytes. Append
 // with kTestECPublicKeyCOSE to get the complete attestation data.
 constexpr uint8_t kTestAttestedCredentialDataPrefix[] = {
@@ -361,4 +477,32 @@ TEST(CTAPResponseTest, TestParseU2fSignWithNullCorruptedSignature) {
   EXPECT_FALSE(response);
 }
 
+TEST(CTAPResponseTest, TestReadGetInfoResponse) {
+  auto get_info_response =
+      ReadCTAPGetInfoResponse(test_data::kTestAuthenticatorGetInfoResponse);
+  ASSERT_TRUE(get_info_response);
+  ASSERT_TRUE(get_info_response->max_msg_size());
+  EXPECT_EQ(*get_info_response->max_msg_size(), 1200u);
+  EXPECT_TRUE(
+      base::ContainsKey(get_info_response->versions(), ProtocolVersion::kCtap));
+  EXPECT_TRUE(
+      base::ContainsKey(get_info_response->versions(), ProtocolVersion::kU2f));
+  EXPECT_TRUE(get_info_response->options().is_platform_device());
+  EXPECT_TRUE(get_info_response->options().supports_resident_key());
+  EXPECT_TRUE(get_info_response->options().user_presence_required());
+  EXPECT_EQ(AuthenticatorSupportedOptions::UserVerificationAvailability::
+                kSupportedAndConfigured,
+            get_info_response->options().user_verification_availability());
+  EXPECT_EQ(AuthenticatorSupportedOptions::ClientPinAvailability::
+                kSupportedButPinNotSet,
+            get_info_response->options().client_pin_availability());
+}
+
+TEST(CTAPResponseTest, TestReadGetInfoResponseWithIncorrectVersionFormat) {
+  EXPECT_FALSE(
+      ReadCTAPGetInfoResponse(kTestAuthenticatorGetInfoResponseWithNoVersion));
+  EXPECT_FALSE(ReadCTAPGetInfoResponse(
+      kTestAuthenticatorGetInfoResponseWithDuplicateVersion));
+}
+
 }  // namespace device

device/fido/device_response_converter.cc

@@ -21,9 +21,20 @@
 namespace device {
 
 namespace {
+
 constexpr size_t kResponseCodeLength = 1;
+
+ProtocolVersion ConvertStringToProtocolVersion(base::StringPiece version) {
+  if (version == kCtap2Version)
+    return ProtocolVersion::kCtap;
+  if (version == kU2fVersion)
+    return ProtocolVersion::kU2f;
+
+  return ProtocolVersion::kUnknown;
 }
 
+}  // namespace
+
 using CBOR = cbor::CBORValue;
 
 CtapDeviceResponseCode GetResponseCode(base::span<const uint8_t> buffer) {
@@ -146,22 +157,34 @@ base::Optional<AuthenticatorGetInfoResponse> ReadCTAPGetInfoResponse(
   const auto& response_map = decoded_response->GetMap();
 
   auto it = response_map.find(CBOR(1));
-  if (it == response_map.end() || !it->second.is_array())
+  if (it == response_map.end() || !it->second.is_array() ||
+      it->second.GetArray().size() > 2) {
     return base::nullopt;
+  }
 
-  std::vector<std::string> versions;
+  base::flat_set<ProtocolVersion> protocol_versions;
   for (const auto& version : it->second.GetArray()) {
     if (!version.is_string())
       return base::nullopt;
 
-    versions.push_back(version.GetString());
+    auto protocol = ConvertStringToProtocolVersion(version.GetString());
+    if (protocol == ProtocolVersion::kUnknown) {
+      DLOG(ERROR) << "Unexpected protocol version received.";
+      return base::nullopt;
+    }
+
+    if (!protocol_versions.insert(protocol).second)
+      return base::nullopt;
   }
 
+  if (protocol_versions.empty())
+    return base::nullopt;
+
   it = response_map.find(CBOR(3));
   if (it == response_map.end() || !it->second.is_bytestring())
     return base::nullopt;
 
-  AuthenticatorGetInfoResponse response(std::move(versions),
+  AuthenticatorGetInfoResponse response(std::move(protocol_versions),
                                         it->second.GetBytestring());
 
   it = response_map.find(CBOR(2));

device/fido/fido_constants.cc

@@ -19,7 +19,7 @@ const std::array<uint8_t, 32> kBogusChallenge = {
 const char kResidentKeyMapKey[] = "rk";
 const char kUserVerificationMapKey[] = "uv";
 const char kUserPresenceMapKey[] = "up";
-const char kClientPinMapKey[] = "client_pin";
+const char kClientPinMapKey[] = "clientPin";
 const char kPlatformDeviceMapKey[] = "plat";
 
 const size_t kHidPacketSize = 64;
@@ -68,4 +68,7 @@ const char kCableDeviceEncryptionKeyInfo[] = "FIDO caBLE v1 sessionKey";
 const char kCableAuthenticatorHelloMessage[] = "caBLE v1 authenticator hello";
 const char kCableClientHelloMessage[] = "caBLE v1 client hello";
 
+const char kCtap2Version[] = "FIDO_2_0";
+const char kU2fVersion[] = "U2F_V2";
+
 }  // namespace device

device/fido/fido_constants.h

@@ -351,6 +351,10 @@ COMPONENT_EXPORT(DEVICE_FIDO)
 extern const char kCableAuthenticatorHelloMessage[];
 COMPONENT_EXPORT(DEVICE_FIDO) extern const char kCableClientHelloMessage[];
 
+// TODO(hongjunchoi): Add url to the official spec once it's standardized.
+COMPONENT_EXPORT(DEVICE_FIDO) extern const char kCtap2Version[];
+COMPONENT_EXPORT(DEVICE_FIDO) extern const char kU2fVersion[];
+
 }  // namespace device
 
 #endif  // DEVICE_FIDO_FIDO_CONSTANTS_H_

device/fido/fido_task.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/stl_util.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "device/fido/ctap_empty_authenticator_request.h"
 #include "device/fido/device_response_converter.h"
@@ -49,7 +50,10 @@ void FidoTask::OnAuthenticatorInfoReceived(
   device()->set_state(FidoDevice::State::kReady);
 
   base::Optional<AuthenticatorGetInfoResponse> get_info_response;
-  if (!response || !(get_info_response = ReadCTAPGetInfoResponse(*response))) {
+  if (!response || !(get_info_response = ReadCTAPGetInfoResponse(*response)) ||
+      !base::ContainsKey(get_info_response->versions(),
+                         ProtocolVersion::kCtap)) {
+    device()->set_supported_protocol(ProtocolVersion::kU2f);
     std::move(u2f_callback).Run();
     return;
   }