cablev2: disconnect on the desktop and mobile.

This change disconnects caBLEv2 on the desktop and mobile so that
further changes can be made incrementally. Otherwise, in order to
keep everything working, large changes cannot be broken up.

BUG=1002262

Change-Id: If2e4c5c561fd2b57cd8f70dbe33c769109ca9c1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2332375
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Auto-Submit: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795534}

chrome/android/features/cablev2_authenticator/native/cablev2_authenticator_android.cc

@@ -300,11 +300,8 @@ class BLEClient {
                                  bool is_transaction_complete) = 0;
   };
 
-  BLEClient(uint64_t addr,
-            uint16_t mtu,
-            const AuthenticatorState* auth_state,
-            Delegate* delegate)
-      : addr_(addr), mtu_(mtu), auth_state_(auth_state), delegate_(delegate) {}
+  BLEClient(uint64_t addr, uint16_t mtu, Delegate* delegate)
+      : addr_(addr), mtu_(mtu), delegate_(delegate) {}
 
   bool Process(
       base::span<const uint8_t> fragment,
@@ -384,26 +381,6 @@ class BLEClient {
 
         base::Optional<std::unique_ptr<device::cablev2::Crypter>>
             handshake_result;
-        if (requested_eid == auth_state_->pairing_advert.second) {
-          handshake_result = device::cablev2::RespondToHandshake(
-              auth_state_->pairing_data.v2->psk_gen_key,
-              auth_state_->pairing_advert, auth_state_->identity_key.get(),
-              /*peer_identity=*/nullptr, /*pairing_data=*/nullptr,
-              message->second, &response);
-        } else if (auth_state_->qr_advert.has_value() &&
-                   requested_eid == auth_state_->qr_advert->second) {
-          // TODO: QR handshakes currently always send pairing data, but it's
-          // optional in the protocol.
-          handshake_result = device::cablev2::RespondToHandshake(
-              *auth_state_->qr_psk_gen_key, *auth_state_->qr_advert,
-              /*identity=*/nullptr, auth_state_->qr_peer_identity->get(),
-              &auth_state_->pairing_data, message->second, &response);
-        } else {
-          FIDO_LOG(ERROR) << "Peer is connecting to unknown EID "
-                          << base::HexEncode(requested_eid);
-          return false;
-        }
-
         if (!handshake_result) {
           FIDO_LOG(ERROR) << "Handshake failed";
           return false;
@@ -502,7 +479,6 @@ class BLEClient {
 
   const uint64_t addr_;
   const uint16_t mtu_;
-  const AuthenticatorState* const auth_state_;
   State state_ = State::kHandshake;
   Defragmenter defrag_;
   std::unique_ptr<device::cablev2::Crypter> crypter_;
@@ -595,8 +571,7 @@ class CableInterface : public BLEClient::Delegate {
     // only permissible client for the lifetime of this instance. The Java side
     // filters writes from all other clients.
     if (ble_client_ == nullptr) {
-      ble_client_ =
-          std::make_unique<BLEClient>(client_addr, mtu, &auth_state_, this);
+      ble_client_ = std::make_unique<BLEClient>(client_addr, mtu, this);
     } else if (ble_client_->addr() != static_cast<uint64_t>(client_addr)) {
       NOTREACHED() << "Write from unknown client " << ble_client_->addr();
       return nullptr;

device/fido/BUILD.gn

@@ -326,19 +326,6 @@ fuzzer_test("fido_cable_handshake_handler_fuzzer") {
   libfuzzer_options = [ "max_len=2048" ]
 }
 
-fuzzer_test("fido_cable_handshake_handler_v2_fuzzer") {
-  sources = [ "cable/fido_cable_handshake_handler_v2_fuzzer.cc" ]
-  deps = [
-    ":fido",
-    "//base",
-    "//base/test:test_support",
-    "//device/bluetooth:mocks",
-    "//testing/gmock",
-    "//testing/gtest",
-  ]
-  libfuzzer_options = [ "max_len=2048" ]
-}
-
 is_linux_without_udev = (is_linux || is_chromeos) && !use_udev
 
 source_set("test_support") {

device/fido/cable/fido_cable_device.cc

@@ -135,8 +135,7 @@ FidoTransportProtocol FidoCableDevice::DeviceTransport() const {
 FidoDevice::CancelToken FidoCableDevice::DeviceTransact(
     std::vector<uint8_t> command,
     DeviceCallback callback) {
-  if ((!encryption_data_ && !v2_crypter_) ||
-      !EncryptOutgoingMessage(&command)) {
+  if (!encryption_data_ || !EncryptOutgoingMessage(&command)) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::BindOnce(std::move(callback), base::nullopt));
     state_ = State::kDeviceError;
@@ -156,8 +155,7 @@ void FidoCableDevice::OnResponseFrame(FrameCallback callback,
   state_ = frame ? State::kReady : State::kDeviceError;
 
   if (frame && frame->command() != FidoBleDeviceCommand::kControl) {
-    if ((!encryption_data_ && !v2_crypter_) ||
-        !DecryptIncomingMessage(&frame.value())) {
+    if (!encryption_data_ || !DecryptIncomingMessage(&frame.value())) {
       state_ = State::kDeviceError;
       frame = base::nullopt;
     }
@@ -233,20 +231,12 @@ void FidoCableDevice::SetV1EncryptionData(
     base::span<const uint8_t, 8> nonce) {
   // Encryption data must be set at most once during Cable handshake protocol.
   DCHECK(!encryption_data_);
-  DCHECK(!v2_crypter_);
   encryption_data_.emplace();
   encryption_data_->read_key = fido_parsing_utils::Materialize(session_key);
   encryption_data_->write_key = fido_parsing_utils::Materialize(session_key);
   encryption_data_->nonce = fido_parsing_utils::Materialize(nonce);
 }
 
-void FidoCableDevice::SetV2EncryptionData(
-    std::unique_ptr<cablev2::Crypter> crypter) {
-  DCHECK(!encryption_data_);
-  DCHECK(!v2_crypter_);
-  v2_crypter_.emplace(std::move(crypter));
-}
-
 void FidoCableDevice::SetSequenceNumbersForTesting(uint32_t read_seq,
                                                    uint32_t write_seq) {
   encryption_data_->write_sequence_num = write_seq;
@@ -377,40 +367,14 @@ void FidoCableDevice::ProcessBleDeviceError(base::span<const uint8_t> data) {
 
 bool FidoCableDevice::EncryptOutgoingMessage(
     std::vector<uint8_t>* message_to_encrypt) {
-  if (v2_crypter_) {
-    return v2_crypter_.value()->Encrypt(message_to_encrypt);
-  }
-
-  return EncryptV1OutgoingMessage(&encryption_data_.value(),
-                                  message_to_encrypt);
-}
-
-bool FidoCableDevice::DecryptIncomingMessage(FidoBleFrame* incoming_frame) {
-  if (v2_crypter_) {
-    std::vector<uint8_t> plaintext;
-    if (!v2_crypter_.value()->Decrypt(incoming_frame->command(),
-                                      incoming_frame->data(), &plaintext)) {
-      return false;
-    }
-    incoming_frame->data().swap(plaintext);
-    return true;
-  }
-
-  return DecryptV1IncomingMessage(&encryption_data_.value(), incoming_frame);
-}
-
-// static
-bool FidoCableDevice::EncryptV1OutgoingMessage(
-    EncryptionData* encryption_data,
-    std::vector<uint8_t>* message_to_encrypt) {
   const auto nonce =
-      ConstructV1Nonce(encryption_data->nonce, /*is_sender_client=*/true,
-                       encryption_data->write_sequence_num++);
+      ConstructV1Nonce(encryption_data_->nonce, /*is_sender_client=*/true,
+                       encryption_data_->write_sequence_num++);
   if (!nonce)
     return false;
 
   crypto::Aead aes_key(crypto::Aead::AES_256_GCM);
-  aes_key.Init(encryption_data->write_key);
+  aes_key.Init(encryption_data_->write_key);
   DCHECK_EQ(nonce->size(), aes_key.NonceLength());
 
   const uint8_t additional_data[1] = {
@@ -421,17 +385,15 @@ bool FidoCableDevice::EncryptV1OutgoingMessage(
   return true;
 }
 
-// static
-bool FidoCableDevice::DecryptV1IncomingMessage(EncryptionData* encryption_data,
-                                               FidoBleFrame* incoming_frame) {
+bool FidoCableDevice::DecryptIncomingMessage(FidoBleFrame* incoming_frame) {
   const auto nonce =
-      ConstructV1Nonce(encryption_data->nonce, /*is_sender_client=*/false,
-                       encryption_data->read_sequence_num);
+      ConstructV1Nonce(encryption_data_->nonce, /*is_sender_client=*/false,
+                       encryption_data_->read_sequence_num);
   if (!nonce)
     return false;
 
   crypto::Aead aes_key(crypto::Aead::AES_256_GCM);
-  aes_key.Init(encryption_data->read_key);
+  aes_key.Init(encryption_data_->read_key);
   DCHECK_EQ(nonce->size(), aes_key.NonceLength());
 
   const uint8_t additional_data[1] = {
@@ -443,7 +405,7 @@ bool FidoCableDevice::DecryptV1IncomingMessage(EncryptionData* encryption_data,
     return false;
   }
 
-  encryption_data->read_sequence_num++;
+  encryption_data_->read_sequence_num++;
   incoming_frame->data().swap(*plaintext);
   return true;
 }

device/fido/cable/fido_cable_device.h

@@ -22,10 +22,6 @@
 
 namespace device {
 
-namespace cablev2 {
-class Crypter;
-}
-
 class BluetoothAdapter;
 class FidoBleFrame;
 
@@ -68,8 +64,6 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDevice : public FidoDevice {
   // Configure caBLE v1 keys.
   void SetV1EncryptionData(base::span<const uint8_t, 32> session_key,
                            base::span<const uint8_t, 8> nonce);
-  // Configure caBLE v2 keys.
-  void SetV2EncryptionData(std::unique_ptr<cablev2::Crypter> crypter);
 
   // SetCountersForTesting allows tests to set the message counters. Non-test
   // code must not call this function.
@@ -128,12 +122,6 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDevice : public FidoDevice {
   bool EncryptOutgoingMessage(std::vector<uint8_t>* message_to_encrypt);
   bool DecryptIncomingMessage(FidoBleFrame* incoming_frame);
 
-  static bool EncryptV1OutgoingMessage(
-      EncryptionData* encryption_data,
-      std::vector<uint8_t>* message_to_encrypt);
-  static bool DecryptV1IncomingMessage(EncryptionData* encryption_data,
-                                       FidoBleFrame* incoming_frame);
-
   base::OneShotTimer timer_;
 
   std::unique_ptr<FidoBleConnection> connection_;
@@ -149,7 +137,6 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDevice : public FidoDevice {
   Observer* observer_ = nullptr;
 
   base::Optional<EncryptionData> encryption_data_;
-  base::Optional<std::unique_ptr<cablev2::Crypter>> v2_crypter_;
   base::WeakPtrFactory<FidoCableDevice> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(FidoCableDevice);

device/fido/cable/fido_cable_discovery.cc

@@ -214,19 +214,11 @@ FidoCableDiscovery::CreateHandshakeHandler(
       if (!base::FeatureList::IsEnabled(device::kWebAuthPhoneSupport)) {
         return base::nullopt;
       }
-      if (!pairing_callback_) {
-        FIDO_LOG(DEBUG) << "Discarding caBLE v2 handshake because of missing "
-                           "pairing callback";
+      FIDO_LOG(DEBUG)
+          << "Discarding unsupported caBLEv2 advert during transition.";
       return base::nullopt;
     }
 
-      handler.reset(new FidoCableV2HandshakeHandler(
-          device, discovery_data.v2->psk_gen_key, nonce, eid,
-          discovery_data.v2->peer_identity,
-          discovery_data.v2->local_identity_seed, *pairing_callback_));
-      break;
-    }
-
     case CableDiscoveryData::Version::INVALID:
       CHECK(false);
       return base::nullopt;

device/fido/cable/fido_cable_handshake_handler.cc

@@ -178,43 +178,4 @@ FidoCableV1HandshakeHandler::GetEncryptionKeyAfterSuccessfulHandshake(
                             /*derived_key_length=*/32);
 }
 
-FidoCableV2HandshakeHandler::FidoCableV2HandshakeHandler(
-    FidoCableDevice* cable_device,
-    base::span<const uint8_t, 32> psk_gen_key,
-    base::span<const uint8_t, 8> nonce,
-    base::span<const uint8_t, kCableEphemeralIdSize> eid,
-    base::Optional<base::span<const uint8_t, kP256X962Length>> peer_identity,
-    base::Optional<base::span<const uint8_t, kCableIdentityKeySeedSize>>
-        local_seed,
-    base::RepeatingCallback<void(std::unique_ptr<CableDiscoveryData>)>
-        pairing_callback)
-    : cable_device_(cable_device),
-      pairing_callback_(std::move(pairing_callback)),
-      handshake_(psk_gen_key, nonce, eid, peer_identity, local_seed) {}
-
-FidoCableV2HandshakeHandler::~FidoCableV2HandshakeHandler() = default;
-
-void FidoCableV2HandshakeHandler::InitiateCableHandshake(
-    FidoDevice::DeviceCallback callback) {
-  std::vector<uint8_t> message = handshake_.BuildInitialMessage();
-  cable_device_->SendHandshakeMessage(std::move(message), std::move(callback));
-}
-
-bool FidoCableV2HandshakeHandler::ValidateAuthenticatorHandshakeMessage(
-    base::span<const uint8_t> response) {
-  base::Optional<std::pair<std::unique_ptr<cablev2::Crypter>,
-                           base::Optional<std::unique_ptr<CableDiscoveryData>>>>
-      result = handshake_.ProcessResponse(response);
-  if (!result) {
-    return false;
-  }
-
-  if (result->second.has_value()) {
-    pairing_callback_.Run(std::move(result->second.value()));
-  }
-
-  cable_device_->SetV2EncryptionData(std::move(result->first));
-  return true;
-}
-
 }  // namespace device

device/fido/cable/fido_cable_handshake_handler.h

@@ -27,8 +27,8 @@ namespace device {
 
 class FidoCableDevice;
 
-// FidoCableHandshakeHandler abstracts over the different versions of caBLE
-// handshakes.
+// FidoCableHandshakeHandler abstracts FidoCableV1HandshakeHandler to allow
+// tests to inject fake handshake handlers.
 class FidoCableHandshakeHandler {
  public:
   virtual ~FidoCableHandshakeHandler() = 0;
@@ -74,37 +74,6 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableV1HandshakeHandler
   DISALLOW_COPY_AND_ASSIGN(FidoCableV1HandshakeHandler);
 };
 
-// FidoCableV2HandshakeHandler implements an NNpsk0[1] handshake that provides
-// forward secrecy.
-//
-// [1] https://noiseexplorer.com/patterns/NNpsk0/
-class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableV2HandshakeHandler
-    : public FidoCableHandshakeHandler {
- public:
-  FidoCableV2HandshakeHandler(
-      FidoCableDevice* device,
-      base::span<const uint8_t, 32> psk_gen_key,
-      base::span<const uint8_t, 8> nonce,
-      base::span<const uint8_t, kCableEphemeralIdSize> eid,
-      base::Optional<base::span<const uint8_t, 65>> peer_identity,
-      base::Optional<base::span<const uint8_t, kCableIdentityKeySeedSize>>
-          local_seed,
-      base::RepeatingCallback<void(std::unique_ptr<CableDiscoveryData>)>
-          pairing_callback);
-  ~FidoCableV2HandshakeHandler() override;
-
-  // FidoCableHandshakeHandler:
-  void InitiateCableHandshake(FidoDevice::DeviceCallback callback) override;
-  bool ValidateAuthenticatorHandshakeMessage(
-      base::span<const uint8_t> response) override;
-
- private:
-  FidoCableDevice* const cable_device_;
-  base::RepeatingCallback<void(std::unique_ptr<CableDiscoveryData>)>
-      pairing_callback_;
-  cablev2::HandshakeInitiator handshake_;
-};
-
 }  // namespace device
 
 #endif  // DEVICE_FIDO_CABLE_FIDO_CABLE_HANDSHAKE_HANDLER_H_

device/fido/cable/fido_cable_handshake_handler_v2_fuzzer.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 <stddef.h>
-#include <stdint.h>
-
-#include <array>
-
-#include "base/bind_helpers.h"
-#include "base/containers/span.h"
-#include "base/memory/ref_counted.h"
-#include "device/bluetooth/test/mock_bluetooth_adapter.h"
-#include "device/fido/cable/fido_cable_device.h"
-#include "device/fido/cable/fido_cable_handshake_handler.h"
-#include "device/fido/fido_constants.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace {
-
-constexpr std::array<uint8_t, 32> kTestPSKGeneratorKey = {
-    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,
-};
-constexpr std::array<uint8_t, 8> kTestNonce = {1, 2, 3, 4, 5, 6, 7, 8};
-constexpr std::array<uint8_t, 16> kTestEphemeralID = {
-    1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16};
-constexpr std::array<uint8_t, 65> kTestPeerIdentity = {
-    0x04, 0x67, 0x80, 0xc5, 0xfc, 0x70, 0x27, 0x5e, 0x2c, 0x70, 0x61,
-    0xa0, 0xe7, 0x87, 0x7b, 0xb1, 0x74, 0xde, 0xad, 0xeb, 0x98, 0x87,
-    0x02, 0x7f, 0x3f, 0xa8, 0x36, 0x54, 0x15, 0x8b, 0xa7, 0xf5, 0x0c,
-    0x3c, 0xba, 0x8c, 0x34, 0xbc, 0x35, 0xd2, 0x0e, 0x81, 0xf7, 0x30,
-    0xac, 0x1c, 0x7b, 0xd6, 0xd6, 0x61, 0xa9, 0x42, 0xf9, 0x0c, 0x6a,
-    0x9c, 0xa5, 0x5c, 0x51, 0x2f, 0x9e, 0x4a, 0x00, 0x12, 0x66,
-};
-constexpr char kTestDeviceAddress[] = "Fake_Address";
-constexpr std::array<uint8_t, 32> kLocalSeed = {
-    0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05,
-    0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05,
-    0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05,
-};
-
-}  // namespace
-
-extern "C" int LLVMFuzzerTestOneInput(const uint8_t* raw_data, size_t size) {
-  auto input = base::make_span(raw_data, size);
-  auto adapter =
-      base::MakeRefCounted<::testing::NiceMock<device::MockBluetoothAdapter>>();
-  device::FidoCableDevice test_cable_device(adapter.get(), kTestDeviceAddress);
-  test_cable_device.SetStateForTesting(
-      device::FidoCableDevice::State::kDeviceError);
-
-  base::Optional<base::span<const uint8_t, 65>> peer_identity;
-  base::Optional<base::span<const uint8_t, 32>> local_seed;
-  if (!input.empty() && (input[0] & 1)) {
-    peer_identity = kTestPeerIdentity;
-    input = input.subspan(1);
-  } else {
-    local_seed = kLocalSeed;
-  }
-
-  device::FidoCableV2HandshakeHandler handshake_handler_v2(
-      &test_cable_device, kTestPSKGeneratorKey, kTestNonce, kTestEphemeralID,
-      peer_identity, local_seed, base::DoNothing());
-  handshake_handler_v2.InitiateCableHandshake(base::DoNothing());
-  handshake_handler_v2.ValidateAuthenticatorHandshakeMessage(input);
-  return 0;
-}