Implement FidoCableDevice

Implement device abstraction for pairing-less BLE device as specified
in FIDO cloud assisted BLE protocol. All incoming/outgoing messages must
be encrypted using session key that is passed on from the relying party.

Bug: 839632
Change-Id: Id3649c9b28d83c9691123289fcbc756a0e0dbfd4
Reviewed-on: https://chromium-review.googlesource.com/1054636
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558988}

device/BUILD.gn

@@ -70,6 +70,7 @@ test("device_unittests") {
     "fido/fido_ble_connection_unittest.cc",
     "fido/fido_ble_device_unittest.cc",
     "fido/fido_ble_frames_unittest.cc",
+    "fido/fido_cable_device_unittest.cc",
     "fido/fido_cable_discovery_unittest.cc",
     "fido/fido_discovery_unittest.cc",
     "fido/fido_hid_message_unittest.cc",

device/fido/BUILD.gn

@@ -55,6 +55,8 @@ component("fido") {
     "fido_ble_transaction.h",
     "fido_ble_uuids.cc",
     "fido_ble_uuids.h",
+    "fido_cable_device.cc",
+    "fido_cable_device.h",
     "fido_cable_discovery.cc",
     "fido_cable_discovery.h",
     "fido_constants.cc",

device/fido/fido_ble_device.cc

@@ -84,6 +84,23 @@ base::WeakPtr<FidoDevice> FidoBleDevice::GetWeakPtr() {
   return weak_factory_.GetWeakPtr();
 }
 
+void FidoBleDevice::OnResponseFrame(FrameCallback callback,
+                                    base::Optional<FidoBleFrame> frame) {
+  // The request is done, time to reset |transaction_|.
+  ResetTransaction();
+
+  state_ = frame ? State::kReady : State::kDeviceError;
+  auto self = GetWeakPtr();
+  std::move(callback).Run(std::move(frame));
+  // Executing callbacks may free |this|. Check |self| first.
+  if (self)
+    Transition();
+}
+
+void FidoBleDevice::ResetTransaction() {
+  transaction_.reset();
+}
+
 void FidoBleDevice::Transition() {
   switch (state_) {
     case State::kInit:
@@ -165,19 +182,6 @@ void FidoBleDevice::SendRequestFrame(FidoBleFrame frame,
                      std::move(callback)));
 }
 
-void FidoBleDevice::OnResponseFrame(FrameCallback callback,
-                                    base::Optional<FidoBleFrame> frame) {
-  // The request is done, time to reset |transaction_|.
-  transaction_.reset();
-
-  state_ = frame ? State::kReady : State::kDeviceError;
-  auto self = GetWeakPtr();
-  std::move(callback).Run(std::move(frame));
-  // Executing callbacks may free |this|. Check |self| first.
-  if (self)
-    Transition();
-}
-
 void FidoBleDevice::StartTimeout() {
   timer_.Start(FROM_HERE, kDeviceTimeout, this, &FidoBleDevice::OnTimeout);
 }

device/fido/fido_ble_device.h

@@ -52,12 +52,15 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoBleDevice : public FidoDevice {
                       DeviceCallback callback) override;
   base::WeakPtr<FidoDevice> GetWeakPtr() override;
 
- private:
+  virtual void OnResponseFrame(FrameCallback callback,
+                               base::Optional<FidoBleFrame> frame);
   void Transition();
   void AddToPendingFrames(FidoBleDeviceCommand cmd,
                           std::vector<uint8_t> request,
                           DeviceCallback callback);
+  void ResetTransaction();
 
+ private:
   void OnConnectionStatus(bool success);
   void OnStatusMessage(std::vector<uint8_t> data);
 
@@ -66,8 +69,6 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoBleDevice : public FidoDevice {
 
   void SendPendingRequestFrame();
   void SendRequestFrame(FidoBleFrame frame, FrameCallback callback);
-  void OnResponseFrame(FrameCallback callback,
-                       base::Optional<FidoBleFrame> frame);
 
   void StartTimeout();
   void StopTimeout();

device/fido/fido_cable_device.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/fido_cable_device.h"
+
+#include <utility>
+
+#include "base/command_line.h"
+#include "base/strings/string_piece.h"
+#include "device/fido/fido_ble_connection.h"
+#include "device/fido/fido_ble_frames.h"
+#include "device/fido/fido_constants.h"
+#include "device/fido/fido_parsing_utils.h"
+
+namespace device {
+
+namespace switches {
+constexpr char kEnableCableEncryption[] = "enable-cable-encryption";
+}  // namespace switches
+
+namespace {
+
+// Maximum size of EncryptionData::read_sequence_num or
+// EncryptionData::write_sequence_num allowed. If we encounter
+// counter larger than |kMaxCounter| FidoCableDevice should error out.
+constexpr size_t kMaxCounter = (1 << 24) - 1;
+
+base::StringPiece ConvertToStringPiece(const std::vector<uint8_t>& data) {
+  return base::StringPiece(reinterpret_cast<const char*>(data.data()),
+                           data.size());
+}
+
+// static
+bool IsEncryptionEnabled() {
+  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+  return command_line->HasSwitch(switches::kEnableCableEncryption);
+}
+
+base::Optional<std::vector<uint8_t>> ConstructEncryptionNonce(
+    base::span<const uint8_t> nonce,
+    bool is_sender_client,
+    uint32_t counter) {
+  if (counter > kMaxCounter)
+    return base::nullopt;
+
+  auto constructed_nonce = fido_parsing_utils::Materialize(nonce);
+  constructed_nonce.push_back(is_sender_client ? 0x00 : 0x01);
+  constructed_nonce.push_back(counter >> 16 & 0xFF);
+  constructed_nonce.push_back(counter >> 8 & 0xFF);
+  constructed_nonce.push_back(counter & 0xFF);
+  return constructed_nonce;
+}
+
+bool EncryptOutgoingMessage(
+    const FidoCableDevice::EncryptionData& encryption_data,
+    std::vector<uint8_t>* message_to_encrypt) {
+  const auto nonce = ConstructEncryptionNonce(
+      encryption_data.nonce, true /* is_sender_client */,
+      encryption_data.write_sequence_num);
+  if (!nonce)
+    return false;
+
+  DCHECK_EQ(nonce->size(), encryption_data.aes_key.NonceLength());
+  std::string ciphertext;
+  bool encryption_success = encryption_data.aes_key.Seal(
+      ConvertToStringPiece(*message_to_encrypt), ConvertToStringPiece(*nonce),
+      nullptr /* additional_data */, &ciphertext);
+  if (!encryption_success)
+    return false;
+
+  message_to_encrypt->assign(ciphertext.begin(), ciphertext.end());
+  return true;
+}
+
+bool DecryptIncomingMessage(
+    const FidoCableDevice::EncryptionData& encryption_data,
+    FidoBleFrame* incoming_frame) {
+  const auto nonce = ConstructEncryptionNonce(
+      encryption_data.nonce, false /* is_sender_client */,
+      encryption_data.read_sequence_num);
+  if (!nonce)
+    return false;
+
+  DCHECK_EQ(nonce->size(), encryption_data.aes_key.NonceLength());
+  std::string ciphertext;
+
+  bool decryption_success = encryption_data.aes_key.Open(
+      ConvertToStringPiece(incoming_frame->data()),
+      ConvertToStringPiece(*nonce), nullptr /* additional_data */, &ciphertext);
+  if (!decryption_success)
+    return false;
+
+  incoming_frame->data().assign(ciphertext.begin(), ciphertext.end());
+  return true;
+}
+
+}  // namespace
+
+// FidoCableDevice::EncryptionData ----------------------------------------
+
+FidoCableDevice::EncryptionData::EncryptionData(
+    std::string session_key,
+    const std::array<uint8_t, 8>& nonce)
+    : encryption_key(std::move(session_key)), nonce(nonce) {
+  DCHECK_EQ(encryption_key.size(), aes_key.KeyLength());
+  aes_key.Init(&encryption_key);
+}
+
+FidoCableDevice::EncryptionData::EncryptionData(EncryptionData&& data) =
+    default;
+
+FidoCableDevice::EncryptionData& FidoCableDevice::EncryptionData::operator=(
+    EncryptionData&& other) = default;
+
+FidoCableDevice::EncryptionData::~EncryptionData() = default;
+
+// FidoCableDevice::EncryptionData ----------------------------------------
+
+FidoCableDevice::FidoCableDevice(std::string address,
+                                 std::string session_key,
+                                 const std::array<uint8_t, 8>& nonce)
+    : FidoBleDevice(std::move(address)),
+      encryption_data_(std::move(session_key), nonce),
+      weak_factory_(this) {}
+
+FidoCableDevice::FidoCableDevice(std::unique_ptr<FidoBleConnection> connection,
+                                 std::string session_key,
+                                 const std::array<uint8_t, 8>& nonce)
+    : FidoBleDevice(std::move(connection)),
+      encryption_data_(std::move(session_key), nonce),
+      weak_factory_(this) {}
+
+FidoCableDevice::~FidoCableDevice() = default;
+
+void FidoCableDevice::DeviceTransact(std::vector<uint8_t> command,
+                                     DeviceCallback callback) {
+  if (IsEncryptionEnabled()) {
+    if (!EncryptOutgoingMessage(encryption_data_, &command)) {
+      state_ = State::kDeviceError;
+      return;
+    }
+
+    ++encryption_data_.write_sequence_num;
+  }
+
+  AddToPendingFrames(FidoBleDeviceCommand::kMsg, std::move(command),
+                     std::move(callback));
+}
+
+void FidoCableDevice::OnResponseFrame(FrameCallback callback,
+                                      base::Optional<FidoBleFrame> frame) {
+  // The request is done, time to reset |transaction_|.
+  ResetTransaction();
+  state_ = frame ? State::kReady : State::kDeviceError;
+
+  if (frame && IsEncryptionEnabled()) {
+    if (!DecryptIncomingMessage(encryption_data_, &frame.value())) {
+      state_ = State::kDeviceError;
+      frame = base::nullopt;
+    }
+
+    ++encryption_data_.read_sequence_num;
+  }
+
+  auto self = GetWeakPtr();
+  std::move(callback).Run(std::move(frame));
+
+  // Executing callbacks may free |this|. Check |self| first.
+  if (self)
+    Transition();
+}
+
+base::WeakPtr<FidoDevice> FidoCableDevice::GetWeakPtr() {
+  return weak_factory_.GetWeakPtr();
+}
+
+}  // namespace device

device/fido/fido_cable_device.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_FIDO_CABLE_DEVICE_H_
+#define DEVICE_FIDO_FIDO_CABLE_DEVICE_H_
+
+#include <array>
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/component_export.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/optional.h"
+#include "crypto/aead.h"
+#include "device/fido/fido_ble_device.h"
+
+namespace device {
+
+class FidoBleFrame;
+class FidoBleConnection;
+
+class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDevice : public FidoBleDevice {
+ public:
+  // Encapsulates state FidoCableDevice maintains to encrypt and decrypt
+  // data within FidoBleFrame.
+  struct COMPONENT_EXPORT(DEVICE_FIDO) EncryptionData {
+    EncryptionData() = delete;
+    EncryptionData(std::string session_key,
+                   const std::array<uint8_t, 8>& nonce);
+    EncryptionData(EncryptionData&& data);
+    EncryptionData& operator=(EncryptionData&& other);
+    ~EncryptionData();
+
+    std::string encryption_key;
+    std::array<uint8_t, 8> nonce;
+    crypto::Aead aes_key{crypto::Aead::AES_256_GCM};
+    uint32_t write_sequence_num = 0;
+    uint32_t read_sequence_num = 0;
+
+    DISALLOW_COPY_AND_ASSIGN(EncryptionData);
+  };
+
+  using FrameCallback = FidoBleTransaction::FrameCallback;
+
+  FidoCableDevice(std::string address,
+                  std::string session_key,
+                  const std::array<uint8_t, 8>& nonce);
+  // Constructor used for testing purposes.
+  FidoCableDevice(std::unique_ptr<FidoBleConnection> connection,
+                  std::string session_key,
+                  const std::array<uint8_t, 8>& nonce);
+  ~FidoCableDevice() override;
+
+  // FidoBleDevice:
+  void DeviceTransact(std::vector<uint8_t> command,
+                      DeviceCallback callback) override;
+  void OnResponseFrame(FrameCallback callback,
+                       base::Optional<FidoBleFrame> frame) override;
+  base::WeakPtr<FidoDevice> GetWeakPtr() override;
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(FidoCableDeviceTest,
+                           TestCableDeviceSendMultipleRequests);
+  FRIEND_TEST_ALL_PREFIXES(FidoCableDeviceTest,
+                           TestCableDeviceErrorOnMaxCounter);
+
+  EncryptionData encryption_data_;
+  base::WeakPtrFactory<FidoCableDevice> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(FidoCableDevice);
+};
+
+}  // namespace device
+
+#endif  // DEVICE_FIDO_FIDO_CABLE_DEVICE_H_

device/fido/fido_cable_device_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/fido_cable_device.h"
+
+#include <array>
+#include <limits>
+#include <memory>
+#include <string>
+#include <utility>
+
+#include "base/command_line.h"
+#include "base/optional.h"
+#include "base/test/scoped_task_environment.h"
+#include "base/threading/sequenced_task_runner_handle.h"
+#include "crypto/aead.h"
+#include "device/bluetooth/test/bluetooth_test.h"
+#include "device/fido/fido_parsing_utils.h"
+#include "device/fido/mock_fido_ble_connection.h"
+#include "device/fido/test_callback_receiver.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace device {
+
+namespace {
+
+using ::testing::_;
+using ::testing::Invoke;
+using ::testing::Test;
+using TestDeviceCallbackReceiver =
+    test::ValueCallbackReceiver<base::Optional<std::vector<uint8_t>>>;
+
+// Sufficiently large test control point length as we are not interested
+// in testing fragmentations of BLE messages. All Cable messages are encrypted
+// and decrypted per request frame, not fragment.
+constexpr auto kControlPointLength = std::numeric_limits<uint16_t>::max();
+// Counter value that is larger than FidoCableDevice::kMaxCounter.
+constexpr uint32_t kInvalidCounter = 1 << 24;
+constexpr char kTestSessionKey[] = "00000000000000000000000000000000";
+constexpr std::array<uint8_t, 8> kTestEncryptionNonce = {
+    {1, 1, 1, 1, 1, 1, 1, 1}};
+constexpr uint8_t kTestData[] = {'T', 'E', 'S', 'T'};
+
+std::vector<uint8_t> ConstructSerializedOutgoingFragment(
+    base::span<const uint8_t> data) {
+  FidoBleFrame response_frame(FidoBleDeviceCommand::kMsg,
+                              fido_parsing_utils::Materialize(data));
+  const auto response_fragment =
+      std::get<0>(response_frame.ToFragments(kControlPointLength));
+
+  std::vector<uint8_t> outgoing_message;
+  response_fragment.Serialize(&outgoing_message);
+  return outgoing_message;
+}
+
+class FakeCableAuthenticator {
+ public:
+  // Returns encrypted message of the ciphertext received from the client.
+  std::vector<uint8_t> ReplyWithSameMessage(base::span<const uint8_t> message) {
+    auto decrypted_message = DecryptMessage(message);
+    auto message_to_send = EncryptMessage(std::move(decrypted_message));
+    return std::vector<uint8_t>(message_to_send.begin(), message_to_send.end());
+  }
+
+  void SetSessionKey(const std::string& session_key) {
+    session_key_ = session_key;
+  }
+
+  void SetAuthenticatorCounter(uint32_t authenticator_counter) {
+    authenticator_counter_ = authenticator_counter;
+  }
+
+ private:
+  std::string EncryptMessage(std::string message) {
+    crypto::Aead aead(crypto::Aead::AES_256_GCM);
+    DCHECK_EQ(session_key_.size(), aead.KeyLength());
+    aead.Init(&session_key_);
+
+    auto encryption_nonce = fido_parsing_utils::Materialize(nonce_);
+    encryption_nonce.push_back(0x01);
+    encryption_nonce.push_back(authenticator_counter_ >> 16 & 0xFF);
+    encryption_nonce.push_back(authenticator_counter_ >> 8 & 0xFF);
+    encryption_nonce.push_back(authenticator_counter_ & 0xFF);
+    DCHECK(encryption_nonce.size() == aead.NonceLength());
+
+    std::string ciphertext;
+    aead.Seal(message,
+              base::StringPiece(
+                  reinterpret_cast<const char*>(encryption_nonce.data()),
+                  encryption_nonce.size()),
+              nullptr /* additional_data */, &ciphertext);
+    authenticator_counter_++;
+    return ciphertext;
+  }
+
+  std::string DecryptMessage(base::span<const uint8_t> message) {
+    crypto::Aead aead(crypto::Aead::AES_256_GCM);
+    DCHECK_EQ(session_key_.size(), aead.KeyLength());
+    aead.Init(&session_key_);
+
+    auto encryption_nonce = fido_parsing_utils::Materialize(nonce_);
+    encryption_nonce.push_back(0x00);
+    encryption_nonce.push_back(expected_client_counter_ >> 16 & 0xFF);
+    encryption_nonce.push_back(expected_client_counter_ >> 8 & 0xFF);
+    encryption_nonce.push_back(expected_client_counter_ & 0xFF);
+    DCHECK(encryption_nonce.size() == aead.NonceLength());
+
+    std::string ciphertext;
+    aead.Open(base::StringPiece(reinterpret_cast<const char*>(message.data()),
+                                message.size()),
+              base::StringPiece(
+                  reinterpret_cast<const char*>(encryption_nonce.data()),
+                  encryption_nonce.size()),
+              nullptr /* additional_data */, &ciphertext);
+    expected_client_counter_++;
+    return ciphertext;
+  }
+
+  std::array<uint8_t, 8> nonce_ = kTestEncryptionNonce;
+  std::string session_key_ = kTestSessionKey;
+  uint32_t expected_client_counter_ = 0;
+  uint32_t authenticator_counter_ = 0;
+};
+
+}  // namespace
+
+class FidoCableDeviceTest : public Test {
+ public:
+  FidoCableDeviceTest() {
+    auto connection = std::make_unique<MockFidoBleConnection>(
+        BluetoothTestBase::kTestDeviceAddress1);
+    connection_ = connection.get();
+    device_ = std::make_unique<FidoCableDevice>(
+        std::move(connection), kTestSessionKey, kTestEncryptionNonce);
+
+    connection_->connection_status_callback() =
+        device_->GetConnectionStatusCallbackForTesting();
+    connection_->read_callback() = device_->GetReadCallbackForTesting();
+  }
+
+  void ConnectWithLength(uint16_t length) {
+    EXPECT_CALL(*connection(), Connect()).WillOnce(Invoke([this] {
+      connection()->connection_status_callback().Run(true);
+    }));
+
+    EXPECT_CALL(*connection(), ReadControlPointLengthPtr(_))
+        .WillOnce(Invoke([length](auto* cb) { std::move(*cb).Run(length); }));
+
+    device()->Connect();
+  }
+
+  void SetUpEncryptionSwitch() {
+    base::CommandLine::ForCurrentProcess()->AppendSwitch(
+        "enable-cable-encryption");
+  }
+
+  FidoCableDevice* device() { return device_.get(); }
+  MockFidoBleConnection* connection() { return connection_; }
+  FakeCableAuthenticator* authenticator() { return &authenticator_; }
+
+ protected:
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+
+ private:
+  FakeCableAuthenticator authenticator_;
+  MockFidoBleConnection* connection_;
+  std::unique_ptr<FidoCableDevice> device_;
+};
+
+TEST_F(FidoCableDeviceTest, TestCaBleDeviceSendData) {
+  SetUpEncryptionSwitch();
+  ConnectWithLength(kControlPointLength);
+
+  EXPECT_CALL(*connection(), WriteControlPointPtr(_, _))
+      .WillOnce(Invoke([this](const auto& data, auto* cb) {
+        base::SequencedTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE, base::BindOnce(std::move(*cb), true));
+
+        const auto authenticator_reply = authenticator()->ReplyWithSameMessage(
+            base::make_span(data).subspan(3));
+        base::SequencedTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE, base::BindOnce(connection()->read_callback(),
+                                      ConstructSerializedOutgoingFragment(
+                                          authenticator_reply)));
+      }));
+
+  TestDeviceCallbackReceiver callback_receiver;
+  device()->DeviceTransact(fido_parsing_utils::Materialize(kTestData),
+                           callback_receiver.callback());
+
+  callback_receiver.WaitForCallback();
+  const auto& value = callback_receiver.value();
+  ASSERT_TRUE(value);
+  EXPECT_THAT(*value, ::testing::ElementsAreArray(kTestData));
+}
+
+// Test that FidoCableDevice properly updates counters when sending/receiving
+// multiple requests.
+TEST_F(FidoCableDeviceTest, TestCableDeviceSendMultipleRequests) {
+  SetUpEncryptionSwitch();
+  ConnectWithLength(kControlPointLength);
+  EXPECT_CALL(*connection(), WriteControlPointPtr(_, _))
+      .Times(2)
+      .WillRepeatedly(Invoke([this](const auto& data, auto* cb) {
+        base::SequencedTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE, base::BindOnce(std::move(*cb), true));
+
+        const auto authenticator_reply = authenticator()->ReplyWithSameMessage(
+            base::make_span(data).subspan(3));
+        base::SequencedTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE, base::BindOnce(connection()->read_callback(),
+                                      ConstructSerializedOutgoingFragment(
+                                          authenticator_reply)));
+      }));
+
+  EXPECT_EQ(0u, device()->encryption_data_.write_sequence_num);
+  EXPECT_EQ(0u, device()->encryption_data_.read_sequence_num);
+
+  TestDeviceCallbackReceiver callback_receiver1;
+  device()->DeviceTransact(fido_parsing_utils::Materialize(kTestData),
+                           callback_receiver1.callback());
+  callback_receiver1.WaitForCallback();
+  const auto& value1 = callback_receiver1.value();
+  ASSERT_TRUE(value1);
+  EXPECT_THAT(*value1, ::testing::ElementsAreArray(kTestData));
+  EXPECT_EQ(1u, device()->encryption_data_.write_sequence_num);
+  EXPECT_EQ(1u, device()->encryption_data_.read_sequence_num);
+
+  constexpr uint8_t kTestData2[] = {'T', 'E', 'S', 'T', '2'};
+  TestDeviceCallbackReceiver callback_receiver2;
+  device()->DeviceTransact(fido_parsing_utils::Materialize(kTestData2),
+                           callback_receiver2.callback());
+  callback_receiver2.WaitForCallback();
+  const auto& value2 = callback_receiver2.value();
+  ASSERT_TRUE(value2);
+  EXPECT_THAT(*value2, ::testing::ElementsAreArray(kTestData2));
+  EXPECT_EQ(2u, device()->encryption_data_.write_sequence_num);
+  EXPECT_EQ(2u, device()->encryption_data_.read_sequence_num);
+}
+
+TEST_F(FidoCableDeviceTest, TestCableDeviceFailOnIncorrectSessionKey) {
+  constexpr char kIncorrectSessionKey[] = "11111111111111111111111111111111";
+  SetUpEncryptionSwitch();
+  ConnectWithLength(kControlPointLength);
+
+  EXPECT_CALL(*connection(), WriteControlPointPtr(_, _))
+      .WillOnce(Invoke([this, &kIncorrectSessionKey](const auto& data,
+                                                     auto* cb) {
+        base::SequencedTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE, base::BindOnce(std::move(*cb), true));
+
+        authenticator()->SetSessionKey(kIncorrectSessionKey);
+        const auto authenticator_reply = authenticator()->ReplyWithSameMessage(
+            base::make_span(data).subspan(3));
+
+        base::SequencedTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE, base::BindOnce(connection()->read_callback(),
+                                      ConstructSerializedOutgoingFragment(
+                                          authenticator_reply)));
+      }));
+
+  TestDeviceCallbackReceiver callback_receiver;
+  device()->DeviceTransact(fido_parsing_utils::Materialize(kTestData),
+                           callback_receiver.callback());
+
+  callback_receiver.WaitForCallback();
+  const auto& value = callback_receiver.value();
+  EXPECT_FALSE(value);
+}
+
+TEST_F(FidoCableDeviceTest, TestCableDeviceFailOnUnexpectedCounter) {
+  constexpr uint32_t kIncorrectAuthenticatorCounter = 1;
+  SetUpEncryptionSwitch();
+  ConnectWithLength(kControlPointLength);
+
+  EXPECT_CALL(*connection(), WriteControlPointPtr(_, _))
+      .WillOnce(Invoke([this, kIncorrectAuthenticatorCounter](const auto& data,
+                                                              auto* cb) {
+        base::SequencedTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE, base::BindOnce(std::move(*cb), true));
+
+        authenticator()->SetAuthenticatorCounter(
+            kIncorrectAuthenticatorCounter);
+        const auto authenticator_reply = authenticator()->ReplyWithSameMessage(
+            base::make_span(data).subspan(3));
+
+        base::SequencedTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE, base::BindOnce(connection()->read_callback(),
+                                      ConstructSerializedOutgoingFragment(
+                                          authenticator_reply)));
+      }));
+
+  TestDeviceCallbackReceiver callback_receiver;
+  device()->DeviceTransact(fido_parsing_utils::Materialize(kTestData),
+                           callback_receiver.callback());
+
+  callback_receiver.WaitForCallback();
+  const auto& value = callback_receiver.value();
+  EXPECT_FALSE(value);
+}
+
+// Test the unlikely event that the authenticator and client has sent/received
+// requests more than FidoCableDevice::kMaxCounter amount of times. As we are
+// only using 3 bytes to encapsulate counter during encryption, any counter
+// value that is above FidoCableDevice::kMaxCounter -- even though it may be
+// the expected counter value -- should return an error.
+TEST_F(FidoCableDeviceTest, TestCableDeviceErrorOnMaxCounter) {
+  ConnectWithLength(kControlPointLength);
+  SetUpEncryptionSwitch();
+
+  EXPECT_CALL(*connection(), WriteControlPointPtr(_, _))
+      .WillOnce(Invoke([this](const auto& data, auto* cb) {
+        base::SequencedTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE, base::BindOnce(std::move(*cb), true));
+
+        authenticator()->SetAuthenticatorCounter(kInvalidCounter);
+        const auto authenticator_reply = authenticator()->ReplyWithSameMessage(
+            base::make_span(data).subspan(3));
+
+        base::SequencedTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE, base::BindOnce(connection()->read_callback(),
+                                      ConstructSerializedOutgoingFragment(
+                                          authenticator_reply)));
+      }));
+
+  TestDeviceCallbackReceiver callback_receiver;
+  device()->encryption_data_.read_sequence_num = kInvalidCounter;
+  device()->DeviceTransact(fido_parsing_utils::Materialize(kTestData),
+                           callback_receiver.callback());
+
+  callback_receiver.WaitForCallback();
+  const auto& value = callback_receiver.value();
+  EXPECT_FALSE(value);
+}
+
+TEST_F(FidoCableDeviceTest, TestEncryptionDisabledWithoutCommandLineSwitch) {
+  ConnectWithLength(kControlPointLength);
+
+  EXPECT_CALL(*connection(), WriteControlPointPtr(_, _))
+      .WillOnce(Invoke([this](const auto& data, auto* cb) {
+        base::SequencedTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE, base::BindOnce(std::move(*cb), true));
+
+        base::SequencedTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE, base::BindOnce(connection()->read_callback(), data));
+      }));
+
+  TestDeviceCallbackReceiver callback_receiver;
+  device()->DeviceTransact(fido_parsing_utils::Materialize(kTestData),
+                           callback_receiver.callback());
+
+  callback_receiver.WaitForCallback();
+  const auto& value = callback_receiver.value();
+  ASSERT_TRUE(value);
+  EXPECT_THAT(*value, ::testing::ElementsAreArray(kTestData));
+}
+
+}  // namespace device

device/fido/fido_cable_discovery.cc

@@ -14,8 +14,8 @@
 #include "device/bluetooth/bluetooth_advertisement.h"
 #include "device/bluetooth/bluetooth_discovery_session.h"
 #include "device/bluetooth/bluetooth_uuid.h"
-#include "device/fido/fido_ble_device.h"
 #include "device/fido/fido_ble_uuids.h"
+#include "device/fido/fido_cable_device.h"
 #include "device/fido/fido_parsing_utils.h"
 
 namespace device {
@@ -79,8 +79,11 @@ std::unique_ptr<BluetoothAdvertisement::Data> ConstructAdvertisementData(
 
 FidoCableDiscovery::CableDiscoveryData::CableDiscoveryData(
     const EidArray& client_eid,
-    const EidArray& authenticator_eid)
-    : client_eid(client_eid), authenticator_eid(authenticator_eid) {}
+    const EidArray& authenticator_eid,
+    const SessionKeyArray& session_key)
+    : client_eid(client_eid),
+      authenticator_eid(authenticator_eid),
+      session_key(session_key) {}
 
 FidoCableDiscovery::CableDiscoveryData::CableDiscoveryData(
     const CableDiscoveryData& data) = default;
@@ -120,7 +123,7 @@ void FidoCableDiscovery::DeviceChanged(BluetoothAdapter* adapter,
 
 void FidoCableDiscovery::DeviceRemoved(BluetoothAdapter* adapter,
                                        BluetoothDevice* device) {
-  if (IsCableDevice(device) && IsExpectedCaBleDevice(device)) {
+  if (IsCableDevice(device) && GetFoundCableDiscoveryData(device)) {
     const auto& device_address = device->GetAddress();
     VLOG(2) << "Cable device removed: " << device_address;
     RemoveDevice(FidoBleDevice::GetId(device_address));
@@ -175,14 +178,27 @@ void FidoCableDiscovery::OnAdvertisementRegisterError(
 
 void FidoCableDiscovery::CableDeviceFound(BluetoothAdapter* adapter,
                                           BluetoothDevice* device) {
-  if (!IsExpectedCaBleDevice(device))
+  const auto* found_cable_device_data = GetFoundCableDiscoveryData(device);
+  if (!found_cable_device_data)
     return;
 
   DVLOG(2) << "Found new Cable device.";
-  AddDevice(std::make_unique<FidoBleDevice>(device->GetAddress()));
+  // Nonce is embedded as first 8 bytes of client EID.
+  std::array<uint8_t, 8> nonce;
+  bool extract_success = fido_parsing_utils::ExtractArray(
+      found_cable_device_data->client_eid, 0, &nonce);
+  if (!extract_success)
+    return;
+
+  AddDevice(std::make_unique<FidoCableDevice>(
+      device->GetAddress(),
+      std::string(found_cable_device_data->session_key.begin(),
+                  found_cable_device_data->session_key.end()),
+      nonce));
 }
 
-bool FidoCableDiscovery::IsExpectedCaBleDevice(
+const FidoCableDiscovery::CableDiscoveryData*
+FidoCableDiscovery::GetFoundCableDiscoveryData(
     const BluetoothDevice* device) const {
   const auto* service_data =
       device->GetServiceDataForUUID(CableAdvertisementUUID());
@@ -192,13 +208,17 @@ bool FidoCableDiscovery::IsExpectedCaBleDevice(
   bool extract_success = fido_parsing_utils::ExtractArray(
       *service_data, 8, &received_authenticator_eid);
   if (!extract_success)
-    return false;
+    return nullptr;
+
+  auto discovery_data_iterator = std::find_if(
+      discovery_data_.begin(), discovery_data_.end(),
+      [&received_authenticator_eid](const auto& data) {
+        return received_authenticator_eid == data.authenticator_eid;
+      });
 
-  return std::any_of(discovery_data_.cbegin(), discovery_data_.cend(),
-                     [&received_authenticator_eid](const auto& data) {
-                       return received_authenticator_eid ==
-                              data.authenticator_eid;
-                     });
+  return discovery_data_iterator != discovery_data_.end()
+             ? &(*discovery_data_iterator)
+             : nullptr;
 }
 
 }  // namespace device

device/fido/fido_cable_discovery.h

@@ -28,7 +28,9 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDiscovery
     : public FidoBleDiscoveryBase {
  public:
   static constexpr size_t kEphemeralIdSize = 16;
+  static constexpr size_t kSessionKeySize = 32;
   using EidArray = std::array<uint8_t, kEphemeralIdSize>;
+  using SessionKeyArray = std::array<uint8_t, kSessionKeySize>;
 
   // Encapsulates information required to discover Cable device per single
   // credential. When multiple credentials are enrolled to a single account
@@ -37,13 +39,15 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDiscovery
   // EID received from the relying party.
   struct COMPONENT_EXPORT(DEVICE_FIDO) CableDiscoveryData {
     CableDiscoveryData(const EidArray& client_eid,
-                       const EidArray& authenticator_eid);
+                       const EidArray& authenticator_eid,
+                       const SessionKeyArray& session_key);
     CableDiscoveryData(const CableDiscoveryData& data);
     CableDiscoveryData& operator=(const CableDiscoveryData& other);
     ~CableDiscoveryData();
 
     EidArray client_eid;
     EidArray authenticator_eid;
+    SessionKeyArray session_key;
   };
 
   FidoCableDiscovery(std::vector<CableDiscoveryData> discovery_data);
@@ -67,7 +71,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDiscovery
   void OnAdvertisementRegisterError(
       BluetoothAdvertisement::ErrorCode error_code);
   void CableDeviceFound(BluetoothAdapter* adapter, BluetoothDevice* device);
-  bool IsExpectedCaBleDevice(const BluetoothDevice* device) const;
+  const CableDiscoveryData* GetFoundCableDiscoveryData(
+      const BluetoothDevice* device) const;
 
   std::vector<CableDiscoveryData> discovery_data_;
   std::map<EidArray, scoped_refptr<BluetoothAdvertisement>> advertisements_;

device/fido/fido_cable_discovery_unittest.cc

@@ -40,6 +40,11 @@ constexpr FidoCableDiscovery::EidArray kInvalidAuthenticatorEid = {
     {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
      0x00, 0x00, 0x00, 0x00}};
 
+constexpr FidoCableDiscovery::SessionKeyArray kTestSessionKey = {
+    {0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+     0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+     0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}};
+
 // Below constants are used to construct MockBluetoothDevice for testing.
 constexpr char kTestBleDeviceAddress[] = "11:12:13:14:15:16";
 
@@ -110,7 +115,7 @@ class FidoCableDiscoveryTest : public ::testing::Test {
  public:
   std::unique_ptr<FidoCableDiscovery> CreateDiscovery() {
     std::vector<FidoCableDiscovery::CableDiscoveryData> discovery_data;
-    discovery_data.emplace_back(kClientEid, kAuthenticatorEid);
+    discovery_data.emplace_back(kClientEid, kAuthenticatorEid, kTestSessionKey);
     return std::make_unique<FidoCableDiscovery>(std::move(discovery_data));
   }
 
@@ -177,9 +182,15 @@ TEST_F(FidoCableDiscoveryTest, TestDiscoveryWithMultipleEids) {
       {0xee, 0xee, 0xee, 0xee, 0xee, 0xee, 0xee, 0xee, 0xee, 0xee, 0xee, 0xee,
        0xee, 0xee, 0xee, 0xee}};
 
+  constexpr FidoCableDiscovery::SessionKeyArray kSecondarySessionKey = {
+      {0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd,
+       0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd,
+       0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd}};
+
   std::vector<FidoCableDiscovery::CableDiscoveryData> discovery_data;
-  discovery_data.emplace_back(kClientEid, kAuthenticatorEid);
-  discovery_data.emplace_back(kSecondaryClientEid, kSecondaryAuthenticatorEid);
+  discovery_data.emplace_back(kClientEid, kAuthenticatorEid, kTestSessionKey);
+  discovery_data.emplace_back(kSecondaryClientEid, kSecondaryAuthenticatorEid,
+                              kSecondarySessionKey);
   auto cable_discovery =
       std::make_unique<FidoCableDiscovery>(std::move(discovery_data));