webauthn: plumb caBLEv2 QR data through.

Previous changes have left cut points where the change in QR key structure
was split off. This change contains the remainder of those changes. It
doesn't quite make CableDiscoveryData V1-only, but it's a lot closer.

BUG=1002262

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

chrome/android/features/cablev2_authenticator/java/src/org/chromium/chrome/browser/webauth/authenticator/CableAuthenticator.java

@@ -36,7 +36,6 @@ import com.google.android.gms.fido.fido2.api.common.PublicKeyCredentialUserEntit
 import com.google.android.gms.tasks.Task;
 
 import org.chromium.base.Log;
-import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.NativeMethods;
 import org.chromium.base.task.PostTask;
 import org.chromium.base.task.SingleThreadTaskRunner;
@@ -168,7 +167,6 @@ class CableAuthenticator {
      * Called by C++ code to start advertising a given UUID, which is passed
      * as 16 bytes.
      */
-    @CalledByNative
     public void sendBLEAdvert(byte[] dataUuidBytes) {
         assert mTaskRunner.belongsToCurrentThread();
 
@@ -184,7 +182,6 @@ class CableAuthenticator {
     /**
      * Called by native code to store a new state blob.
      */
-    @CalledByNative
     public void setState(byte[] newState) {
         assert mTaskRunner.belongsToCurrentThread();
 
@@ -200,7 +197,6 @@ class CableAuthenticator {
     /**
      * Called by native code to send BLE data to a specified client.
      */
-    @CalledByNative
     public void sendNotification(long client, byte[][] fragments, boolean isTransactionEnd) {
         assert mTaskRunner.belongsToCurrentThread();
         assert mBleStarted;
@@ -208,7 +204,6 @@ class CableAuthenticator {
         mBleHandler.sendNotification(client, fragments, /*closeWhenDone=*/isTransactionEnd);
     }
 
-    @CalledByNative
     public void makeCredential(String origin, String rpId, byte[] challenge, byte[] userId,
             int[] algorithms, byte[][] excludedCredentialIds, boolean residentKeyRequired) {
         // TODO: handle concurrent requests
@@ -277,7 +272,6 @@ class CableAuthenticator {
         Log.i(TAG, "op done");
     }
 
-    @CalledByNative
     public void getAssertion(
             String origin, String rpId, byte[] challenge, byte[][] allowedCredentialIds) {
         // TODO: handle concurrent requests

chrome/browser/ui/views/webauthn/authenticator_qr_sheet_view.cc

@@ -9,7 +9,9 @@
 #include "base/strings/string_piece.h"
 #include "chrome/common/qr_code_generator/dino_image.h"
 #include "chrome/common/qr_code_generator/qr_code_generator.h"
-#include "device/fido/cable/cable_discovery_data.h"
+#include "third_party/boringssl/src/include/openssl/ec.h"
+#include "third_party/boringssl/src/include/openssl/ec_key.h"
+#include "third_party/boringssl/src/include/openssl/obj.h"
 #include "ui/gfx/canvas.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/view.h"
@@ -167,28 +169,56 @@ class QRView : public views::View {
   DISALLOW_COPY_AND_ASSIGN(QRView);
 };
 
+// kCompressedPublicKeySize is the size of an X9.62 compressed P-256 public key.
+constexpr size_t kCompressedPublicKeySize = 33;
+
+std::array<uint8_t, kCompressedPublicKeySize> SeedToCompressedPublicKey(
+    base::span<const uint8_t, 32> seed) {
+  bssl::UniquePtr<EC_GROUP> p256(
+      EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1));
+  bssl::UniquePtr<EC_KEY> key(
+      EC_KEY_derive_from_secret(p256.get(), seed.data(), seed.size()));
+  const EC_POINT* public_key = EC_KEY_get0_public_key(key.get());
+
+  std::array<uint8_t, kCompressedPublicKeySize> ret;
+  CHECK_EQ(ret.size(), EC_POINT_point2oct(
+                           p256.get(), public_key, POINT_CONVERSION_COMPRESSED,
+                           ret.data(), ret.size(), /*ctx=*/nullptr));
+  return ret;
+}
+
 // Base64EncodedSize returns the number of bytes required to base64 encode an
 // input of |input_length| bytes, without padding.
 constexpr size_t Base64EncodedSize(size_t input_length) {
   return ((input_length * 4) + 2) / 3;
 }
 
-// QRDataForCurrentTime writes a URL suitable for encoding as a QR to |out_buf|
+// BuildQRData writes a URL suitable for encoding as a QR to |out_buf|
 // and returns a span pointing into that buffer. The URL is generated based on
-// |qr_generator_key| and the current time such that the caBLE discovery code
-// can recognise the URL as valid.
-base::span<uint8_t> QRDataForCurrentTime(
+// |qr_generator_key|.
+base::span<uint8_t> BuildQRData(
     uint8_t out_buf[QRCode::V5::kInputBytes],
-    base::span<const uint8_t, 32> qr_generator_key) {
-  const int64_t current_tick = device::CableDiscoveryData::CurrentTimeTick();
-  // TODO(agl): fix this. Currently doing this in order to split up CLs.
-  device::QRGeneratorKey temp_key = {0};
-  const device::CableQRData qr_data =
-      device::CableDiscoveryData::DeriveQRData(temp_key, current_tick);
+    base::span<const uint8_t, device::cablev2::kQRKeySize> qr_generator_key) {
+  static_assert(device::cablev2::kQRSeedSize <= device::cablev2::kQRKeySize,
+                "");
+  const std::array<uint8_t, kCompressedPublicKeySize> compressed_public_key =
+      SeedToCompressedPublicKey(
+          base::span<const uint8_t, device::cablev2::kQRSeedSize>(
+              qr_generator_key.data(), device::cablev2::kQRSeedSize));
+
+  uint8_t
+      qr_data[EXTENT(compressed_public_key) + device::cablev2::kQRSecretSize];
+  memcpy(qr_data, compressed_public_key.data(), compressed_public_key.size());
+  static_assert(EXTENT(qr_generator_key) == device::cablev2::kQRSeedSize +
+                                                device::cablev2::kQRSecretSize,
+                "");
+  memcpy(qr_data + compressed_public_key.size(),
+         &qr_generator_key[device::cablev2::kQRSeedSize],
+         device::cablev2::kQRSecretSize);
 
   std::string base64_qr_data;
   base::Base64UrlEncode(
-      base::StringPiece(reinterpret_cast<const char*>(qr_data.data()),
+      base::StringPiece(reinterpret_cast<const char*>(qr_data),
                         sizeof(qr_data)),
       base::Base64UrlEncodePolicy::OMIT_PADDING, &base64_qr_data);
   static constexpr size_t kEncodedDataLength =
@@ -241,7 +271,7 @@ std::unique_ptr<views::View>
 AuthenticatorQRSheetView::BuildStepSpecificContent() {
   uint8_t qr_data_buf[QRCode::V5::kInputBytes];
   auto qr_view = std::make_unique<AuthenticatorQRViewCentered>(
-      QRDataForCurrentTime(qr_data_buf, qr_generator_key_));
+      BuildQRData(qr_data_buf, qr_generator_key_));
   qr_view_ = qr_view.get();
 
   timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(600), this,
@@ -251,5 +281,5 @@ AuthenticatorQRSheetView::BuildStepSpecificContent() {
 
 void AuthenticatorQRSheetView::Update() {
   uint8_t qr_data_buf[QRCode::V5::kInputBytes];
-  qr_view_->RefreshQRCode(QRDataForCurrentTime(qr_data_buf, qr_generator_key_));
+  qr_view_->RefreshQRCode(BuildQRData(qr_data_buf, qr_generator_key_));
 }

chrome/browser/ui/views/webauthn/authenticator_qr_sheet_view.h

@@ -12,6 +12,7 @@
 #include "base/timer/timer.h"
 #include "chrome/browser/ui/views/webauthn/authenticator_request_sheet_view.h"
 #include "chrome/browser/ui/webauthn/sheet_models.h"
+#include "device/fido/cable/v2_constants.h"
 
 class AuthenticatorQRViewCentered;
 
@@ -31,7 +32,7 @@ class AuthenticatorQRSheetView : public AuthenticatorRequestSheetView {
   void Update();
 
   AuthenticatorQRViewCentered* qr_view_ = nullptr;
-  base::span<const uint8_t, 32> qr_generator_key_;
+  base::span<const uint8_t, device::cablev2::kQRKeySize> qr_generator_key_;
   base::RepeatingTimer timer_;
 
   DISALLOW_COPY_AND_ASSIGN(AuthenticatorQRSheetView);

chrome/browser/ui/webauthn/authenticator_dialog_browsertest.cc

@@ -40,9 +40,9 @@ class AuthenticatorDialogTest : public DialogBrowserTest {
         AuthenticatorTransport::kUsbHumanInterfaceDevice,
         AuthenticatorTransport::kInternal,
         AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy};
+    std::array<uint8_t, device::cablev2::kQRKeySize> qr_key = {0};
     model->set_cable_transport_info(/*cable_extension_provided=*/true,
-                                    /*have_paired_phones=*/false,
-                                    device::CableDiscoveryData::NewQRKey());
+                                    /*have_paired_phones=*/false, qr_key);
     model->StartFlow(std::move(transport_availability), base::nullopt);
 
     // The dialog should immediately close as soon as it is displayed.

chrome/browser/webauthn/authenticator_request_dialog_model.cc

@@ -581,10 +581,11 @@ void AuthenticatorRequestDialogModel::RequestAttestationPermission(
 void AuthenticatorRequestDialogModel::set_cable_transport_info(
     bool cable_extension_provided,
     bool have_paired_phones,
-    base::Optional<device::QRGeneratorKey> qr_generator_key) {
+    const base::Optional<std::array<uint8_t, device::cablev2::kQRKeySize>>&
+        qr_generator_key) {
   cable_extension_provided_ = cable_extension_provided;
   have_paired_phones_ = have_paired_phones;
-  qr_generator_key_ = std::move(qr_generator_key);
+  qr_generator_key_ = qr_generator_key;
 }
 
 base::WeakPtr<AuthenticatorRequestDialogModel>

chrome/browser/webauthn/authenticator_request_dialog_model.h

@@ -21,6 +21,7 @@
 #include "chrome/browser/webauthn/authenticator_transport.h"
 #include "chrome/browser/webauthn/observable_authenticator_list.h"
 #include "device/fido/cable/cable_discovery_data.h"
+#include "device/fido/cable/v2_constants.h"
 #include "device/fido/fido_request_handler_base.h"
 #include "device/fido/fido_transport_protocol.h"
 
@@ -361,11 +362,9 @@ class AuthenticatorRequestDialogModel {
     return transport_availability_.available_transports;
   }
 
-  base::span<const uint8_t, 32> qr_generator_key() const {
-    // TODO(agl): return the true generator key. This is currently broken to
-    // allow CLs to be split up reasonably.
-    static_assert(EXTENT(*qr_generator_key_) >= 32, "");
-    return base::span<const uint8_t, 32>(qr_generator_key_->data(), 32);
+  base::span<const uint8_t, device::cablev2::kQRKeySize> qr_generator_key()
+      const {
+    return *qr_generator_key_;
   }
 
   void CollectPIN(base::Optional<int> attempts,
@@ -410,7 +409,8 @@ class AuthenticatorRequestDialogModel {
   void set_cable_transport_info(
       bool cable_extension_provided,
       bool has_paired_phones,
-      base::Optional<device::QRGeneratorKey> qr_generator_key);
+      const base::Optional<std::array<uint8_t, device::cablev2::kQRKeySize>>&
+          qr_generator_key);
 
   bool win_native_api_enabled() const {
     return transport_availability_.has_win_native_api_authenticator;
@@ -505,7 +505,8 @@ class AuthenticatorRequestDialogModel {
   // have_paired_phones_ indicates whether this profile knows of any paired
   // phones.
   bool have_paired_phones_ = false;
-  base::Optional<device::QRGeneratorKey> qr_generator_key_;
+  base::Optional<std::array<uint8_t, device::cablev2::kQRKeySize>>
+      qr_generator_key_;
   // win_native_api_already_tried_ is true if the Windows-native UI has been
   // displayed already and the user cancelled it. In this case, we shouldn't
   // jump straight to showing it again.

chrome/browser/webauthn/chrome_authenticator_request_delegate.cc

@@ -32,6 +32,7 @@
 #include "content/public/browser/device_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
+#include "crypto/random.h"
 #include "device/fido/features.h"
 #include "device/fido/fido_authenticator.h"
 #include "device/fido/fido_discovery_factory.h"
@@ -353,11 +354,13 @@ void ChromeAuthenticatorRequestDelegate::ConfigureCable(
   }
   const bool cable_extension_provided = !pairings.empty();
 
-  base::Optional<device::QRGeneratorKey> qr_generator_key;
+  base::Optional<std::array<uint8_t, device::cablev2::kQRKeySize>>
+      qr_generator_key;
   bool have_paired_phones = false;
   std::vector<std::unique_ptr<device::cablev2::Pairing>> paired_phones;
   if (base::FeatureList::IsEnabled(device::kWebAuthPhoneSupport)) {
-    qr_generator_key.emplace(device::CableDiscoveryData::NewQRKey());
+    qr_generator_key.emplace();
+    crypto::RandBytes(*qr_generator_key);
     paired_phones = GetCablePairings();
     have_paired_phones = !paired_phones.empty();
 

content/browser/webauth/authenticator_impl_unittest.cc

@@ -5767,9 +5767,11 @@ class CableV2AuthenticatorImplTest : public AuthenticatorImplTest {
 
   const std::array<uint8_t, device::cablev2::kRootSecretSize> root_secret_ = {
       0};
-  const device::QRGeneratorKey qr_generator_key_ = {0};
-  const std::array<uint8_t, 16> zero_qr_secret_ = {0};
-  const device::CableIdentityKeySeed zero_seed_ = {0};
+  const std::array<uint8_t, device::cablev2::kQRKeySize> qr_generator_key_ = {
+      0};
+  const std::array<uint8_t, device::cablev2::kQRSecretSize> zero_qr_secret_ = {
+      0};
+  const std::array<uint8_t, device::cablev2::kQRSeedSize> zero_seed_ = {0};
 
   std::unique_ptr<network::mojom::NetworkContext> network_context_;
   uint8_t peer_identity_x962_[device::kP256X962Length] = {0};

device/fido/cable/cable_discovery_data.cc

@@ -20,29 +20,6 @@
 
 namespace device {
 
-namespace {
-
-enum class QRValue : uint8_t {
-  QR_SECRET = 0,
-  IDENTITY_KEY_SEED = 1,
-};
-
-void DeriveQRValue(base::span<const uint8_t, kCableQRDataSize> qr_generator_key,
-                   const int64_t tick,
-                   QRValue type,
-                   base::span<uint8_t> out) {
-  uint8_t hkdf_input[sizeof(uint64_t) + 1];
-  memcpy(hkdf_input, &tick, sizeof(uint64_t));
-  hkdf_input[sizeof(uint64_t)] = base::strict_cast<uint8_t>(type);
-
-  bool ok = HKDF(out.data(), out.size(), EVP_sha256(), qr_generator_key.data(),
-                 qr_generator_key.size(),
-                 /*salt=*/nullptr, 0, hkdf_input, sizeof(hkdf_input));
-  DCHECK(ok);
-}
-
-}  // namespace
-
 CableDiscoveryData::CableDiscoveryData() = default;
 
 CableDiscoveryData::CableDiscoveryData(
@@ -58,39 +35,6 @@ CableDiscoveryData::CableDiscoveryData(
   v1->session_pre_key = session_pre_key;
 }
 
-CableDiscoveryData::CableDiscoveryData(
-    base::span<const uint8_t, kCableQRSecretSize> qr_secret,
-    base::span<const uint8_t, kCableIdentityKeySeedSize> identity_key_seed) {
-  InitFromQRSecret(qr_secret);
-  v2->local_identity_seed = fido_parsing_utils::Materialize(identity_key_seed);
-}
-
-// static
-base::Optional<CableDiscoveryData> CableDiscoveryData::FromQRData(
-    base::span<const uint8_t,
-               kCableCompressedPublicKeySize + kCableQRSecretSize> qr_data) {
-  auto qr_secret = qr_data.subspan(kCableCompressedPublicKeySize);
-  CableDiscoveryData discovery_data;
-  discovery_data.InitFromQRSecret(base::span<const uint8_t, kCableQRSecretSize>(
-      qr_secret.data(), qr_secret.size()));
-
-  bssl::UniquePtr<EC_GROUP> p256(
-      EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1));
-  bssl::UniquePtr<EC_POINT> point(EC_POINT_new(p256.get()));
-  if (!EC_POINT_oct2point(p256.get(), point.get(), qr_data.data(),
-                          kCableCompressedPublicKeySize, /*ctx=*/nullptr)) {
-    return base::nullopt;
-  }
-  CableAuthenticatorIdentityKey& identity_key =
-      discovery_data.v2->peer_identity.emplace();
-  CHECK_EQ(identity_key.size(),
-           EC_POINT_point2oct(
-               p256.get(), point.get(), POINT_CONVERSION_UNCOMPRESSED,
-               identity_key.data(), identity_key.size(), /*ctx=*/nullptr));
-
-  return discovery_data;
-}
-
 CableDiscoveryData::CableDiscoveryData(const CableDiscoveryData& data) =
     default;
 
@@ -110,12 +54,6 @@ bool CableDiscoveryData::operator==(const CableDiscoveryData& other) const {
              v1->authenticator_eid == other.v1->authenticator_eid &&
              v1->session_pre_key == other.v1->session_pre_key;
 
-    case CableDiscoveryData::Version::V2:
-      return v2->eid_gen_key == other.v2->eid_gen_key &&
-             v2->psk_gen_key == other.v2->psk_gen_key &&
-             v2->peer_identity == other.v2->peer_identity &&
-             v2->peer_name == other.v2->peer_name;
-
     case CableDiscoveryData::Version::INVALID:
       CHECK(false);
       return false;
@@ -127,109 +65,6 @@ bool CableDiscoveryData::MatchV1(const CableEidArray& eid) const {
   return eid == v1->authenticator_eid;
 }
 
-bool CableDiscoveryData::MatchV2(const CableEidArray& eid,
-                                 CableEidArray* out_eid) const {
-  DCHECK_EQ(version, Version::V2);
-
-  // Attempt to decrypt the EID with the EID generator key and check whether
-  // it has a valid structure.
-  AES_KEY key;
-  CableEidArray& out = *out_eid;
-  CHECK(AES_set_decrypt_key(v2->eid_gen_key.data(),
-                            /*bits=*/8 * v2->eid_gen_key.size(), &key) == 0);
-  static_assert(kCableEphemeralIdSize == AES_BLOCK_SIZE,
-                "EIDs are not AES blocks");
-  AES_decrypt(/*in=*/eid.data(), /*out=*/out.data(), &key);
-  return cablev2::eid::IsValid(out);
-}
-
-// static
-QRGeneratorKey CableDiscoveryData::NewQRKey() {
-  QRGeneratorKey key;
-  crypto::RandBytes(key.data(), key.size());
-  return key;
-}
-
-// static
-int64_t CableDiscoveryData::CurrentTimeTick() {
-  // The ticks are currently 256ms.
-  return base::TimeTicks::Now().since_origin().InMilliseconds() >> 8;
-}
-
-// static
-std::array<uint8_t, kCableQRSecretSize> CableDiscoveryData::DeriveQRSecret(
-    base::span<const uint8_t, kCableQRDataSize> qr_generator_key,
-    const int64_t tick) {
-  std::array<uint8_t, kCableQRSecretSize> ret;
-  DeriveQRValue(qr_generator_key, tick, QRValue::QR_SECRET, ret);
-  return ret;
-}
-
-// static
-CableIdentityKeySeed CableDiscoveryData::DeriveIdentityKeySeed(
-    base::span<const uint8_t, kCableQRDataSize> qr_generator_key,
-    const int64_t tick) {
-  std::array<uint8_t, kCableIdentityKeySeedSize> ret;
-  DeriveQRValue(qr_generator_key, tick, QRValue::IDENTITY_KEY_SEED, ret);
-  return ret;
-}
-
-// static
-CableQRData CableDiscoveryData::DeriveQRData(
-    base::span<const uint8_t, kCableQRDataSize> qr_generator_key,
-    const int64_t tick) {
-  auto identity_key_seed = DeriveIdentityKeySeed(qr_generator_key, tick);
-  bssl::UniquePtr<EC_GROUP> p256(
-      EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1));
-  bssl::UniquePtr<EC_KEY> identity_key(EC_KEY_derive_from_secret(
-      p256.get(), identity_key_seed.data(), identity_key_seed.size()));
-  const EC_POINT* public_key = EC_KEY_get0_public_key(identity_key.get());
-  CableQRData qr_data;
-  static_assert(
-      qr_data.size() == kCableCompressedPublicKeySize + kCableQRSecretSize,
-      "this code needs to be updated");
-  CHECK_EQ(kCableCompressedPublicKeySize,
-           EC_POINT_point2oct(p256.get(), public_key,
-                              POINT_CONVERSION_COMPRESSED, qr_data.data(),
-                              kCableCompressedPublicKeySize, /*ctx=*/nullptr));
-
-  auto qr_secret = CableDiscoveryData::DeriveQRSecret(qr_generator_key, tick);
-  memcpy(&qr_data.data()[kCableCompressedPublicKeySize], qr_secret.data(),
-         qr_secret.size());
-
-  return qr_data;
-}
-
-CableDiscoveryData::V2Data::V2Data() = default;
-CableDiscoveryData::V2Data::V2Data(const V2Data&) = default;
-CableDiscoveryData::V2Data::~V2Data() = default;
-
-void CableDiscoveryData::InitFromQRSecret(
-    base::span<const uint8_t, kCableQRSecretSize> qr_secret) {
-  version = Version::V2;
-  v2.emplace();
-
-  static const char kEIDGen[] = "caBLE QR to EID generator key";
-  bool ok =
-      HKDF(v2->eid_gen_key.data(), v2->eid_gen_key.size(), EVP_sha256(),
-           qr_secret.data(), qr_secret.size(), /*salt=*/nullptr, 0,
-           reinterpret_cast<const uint8_t*>(kEIDGen), sizeof(kEIDGen) - 1);
-  DCHECK(ok);
-
-  static const char kPSKGen[] = "caBLE QR to PSK generator key";
-  ok = HKDF(v2->psk_gen_key.data(), v2->psk_gen_key.size(), EVP_sha256(),
-            qr_secret.data(), qr_secret.size(), /*salt=*/nullptr, 0,
-            reinterpret_cast<const uint8_t*>(kPSKGen), sizeof(kPSKGen) - 1);
-  DCHECK(ok);
-
-  static const char kTunnelIDGen[] = "caBLE QR to tunnel ID generator key";
-  ok = HKDF(v2->tunnel_id_gen_key.data(), v2->tunnel_id_gen_key.size(),
-            EVP_sha256(), qr_secret.data(), qr_secret.size(), /*salt=*/nullptr,
-            0, reinterpret_cast<const uint8_t*>(kTunnelIDGen),
-            sizeof(kTunnelIDGen) - 1);
-  DCHECK(ok);
-}
-
 namespace cablev2 {
 
 Pairing::Pairing() = default;
@@ -239,7 +74,7 @@ Pairing::~Pairing() = default;
 base::Optional<std::unique_ptr<Pairing>> Pairing::Parse(
     const cbor::Value& cbor,
     uint32_t tunnel_server_domain,
-    base::span<const uint8_t, kCableIdentityKeySeedSize> local_identity_seed,
+    base::span<const uint8_t, kQRSeedSize> local_identity_seed,
     base::span<const uint8_t, 32> handshake_hash) {
   if (!cbor.is_map()) {
     return base::nullopt;

device/fido/cable/cable_discovery_data.h

@@ -11,6 +11,7 @@
 #include "base/component_export.h"
 #include "base/containers/span.h"
 #include "base/optional.h"
+#include "device/fido/cable/v2_constants.h"
 #include "device/fido/fido_constants.h"
 
 namespace cbor {
@@ -21,19 +22,12 @@ namespace device {
 
 constexpr size_t kCableEphemeralIdSize = 16;
 constexpr size_t kCableSessionPreKeySize = 32;
-constexpr size_t kCableQRSecretSize = 16;
 constexpr size_t kCableNonceSize = 8;
-constexpr size_t kCableIdentityKeySeedSize = 32;
 constexpr size_t kCableCompressedPublicKeySize =
     /* type byte */ 1 + /* field element */ (256 / 8);
-constexpr size_t kCableQRDataSize =
-    kCableCompressedPublicKeySize + kCableQRSecretSize;
 
 using CableEidArray = std::array<uint8_t, kCableEphemeralIdSize>;
 using CableSessionPreKeyArray = std::array<uint8_t, kCableSessionPreKeySize>;
-// QRGeneratorKey is a hang-over from old code that hasn't been renamed yet.
-// TODO(agl): remove
-using QRGeneratorKey = std::array<uint8_t, kCableQRDataSize>;
 // CableNonce is a nonce used in BLE handshaking.
 using CableNonce = std::array<uint8_t, 8>;
 // CableEidGeneratorKey is an AES-256 key that is used to encrypt a 64-bit nonce
@@ -46,8 +40,6 @@ using CableTunnelIDGeneratorKey = std::array<uint8_t, 32>;
 // CableAuthenticatorIdentityKey is a P-256 public value used to authenticate a
 // paired phone.
 using CableAuthenticatorIdentityKey = std::array<uint8_t, kP256X962Length>;
-using CableIdentityKeySeed = std::array<uint8_t, kCableIdentityKeySeedSize>;
-using CableQRData = std::array<uint8_t, kCableQRDataSize>;
 
 // Encapsulates information required to discover Cable device per single
 // credential. When multiple credentials are enrolled to a single account
@@ -60,32 +52,16 @@ struct COMPONENT_EXPORT(DEVICE_FIDO) CableDiscoveryData {
   enum class Version {
     INVALID,
     V1,
-    V2,
   };
 
   CableDiscoveryData(Version version,
                      const CableEidArray& client_eid,
                      const CableEidArray& authenticator_eid,
                      const CableSessionPreKeyArray& session_pre_key);
-  // Creates discovery data given a specific QR secret and identity key seed.
-  // This will be used on the QR-displaying-side of a QR handshake. See
-  // |DeriveQRSecret| and |DeriveIdentityKeySeed| for how to generate such
-  // secrets.
-  CableDiscoveryData(
-      base::span<const uint8_t, kCableQRSecretSize> qr_secret,
-      base::span<const uint8_t, kCableIdentityKeySeedSize> identity_key_seed);
   CableDiscoveryData();
   CableDiscoveryData(const CableDiscoveryData& data);
   ~CableDiscoveryData();
 
-  // Creates discovery data given QR data, which contains a compressed public
-  // key and the QR secret. This will be used by the QR-scanning-side of a QR
-  // handshake. Returns |nullopt| if the embedded elliptic-curve point is
-  // invalid.
-  static base::Optional<CableDiscoveryData> FromQRData(
-      base::span<const uint8_t,
-                 kCableCompressedPublicKeySize + kCableQRSecretSize> qr_data);
-
   CableDiscoveryData& operator=(const CableDiscoveryData& other);
   bool operator==(const CableDiscoveryData& other) const;
 
@@ -93,38 +69,6 @@ struct COMPONENT_EXPORT(DEVICE_FIDO) CableDiscoveryData {
   // instance, which must be version one.
   bool MatchV1(const CableEidArray& candidate_eid) const;
 
-  // MatchV2 returns true if |candidate_eid| matches this caBLE discovery
-  // instance, which must be version two. If so, |*out_eid| is set to the value
-  // of the decrypted EID.
-  bool MatchV2(const CableEidArray& candidate_eid,
-               CableEidArray* out_eid) const;
-
-  // NewQRKey returns a random key for QR generation.
-  static QRGeneratorKey NewQRKey();
-
-  // CurrentTimeTick returns the current time as used by QR generation. The size
-  // of these ticks is a purely local matter for Chromium.
-  static int64_t CurrentTimeTick();
-
-  // DeriveQRKeyMaterial returns a QR-secret given a generating key and a
-  // timestamp.
-  static std::array<uint8_t, kCableQRSecretSize> DeriveQRSecret(
-      base::span<const uint8_t, kCableQRDataSize> qr_generator_key,
-      const int64_t tick);
-
-  // DeriveIdentityKeySeed returns a seed that can be used to create a P-256
-  // identity key for a handshake using |EC_KEY_derive_from_secret|.
-  static CableIdentityKeySeed DeriveIdentityKeySeed(
-      base::span<const uint8_t, kCableQRDataSize> qr_generator_key,
-      const int64_t tick);
-
-  // DeriveQRData returns the QR data, a combination of QR secret and public
-  // identity key. This is base64url-encoded and placed in a caBLE v2 QR code
-  // with a prefix prepended.
-  static CableQRData DeriveQRData(
-      base::span<const uint8_t, kCableQRDataSize> qr_generator_key,
-      const int64_t tick);
-
   // version indicates whether v1 or v2 data is contained in this object.
   // |INVALID| is not a valid version but is set as the default to catch any
   // cases where the version hasn't been set explicitly.
@@ -136,28 +80,6 @@ struct COMPONENT_EXPORT(DEVICE_FIDO) CableDiscoveryData {
     CableSessionPreKeyArray session_pre_key;
   };
   base::Optional<V1Data> v1;
-
-  struct COMPONENT_EXPORT(DEVICE_FIDO) V2Data {
-    V2Data();
-    V2Data(const V2Data&);
-    ~V2Data();
-
-    CableEidGeneratorKey eid_gen_key;
-    CablePskGeneratorKey psk_gen_key;
-    CableTunnelIDGeneratorKey tunnel_id_gen_key;
-    base::Optional<CableAuthenticatorIdentityKey> peer_identity;
-    base::Optional<CableIdentityKeySeed> local_identity_seed;
-    // peer_name is an authenticator-controlled, UTF8-valid string containing
-    // the self-reported, human-friendly name of a v2 authenticator. This need
-    // not be filled in when handshaking but an authenticator may provide it
-    // when offering long-term pairing data.
-    base::Optional<std::string> peer_name;
-  };
-  base::Optional<V2Data> v2;
-
- private:
-  void InitFromQRSecret(
-      base::span<const uint8_t, kCableQRSecretSize> qr_secret);
 };
 
 namespace cablev2 {
@@ -176,7 +98,7 @@ struct COMPONENT_EXPORT(DEVICE_FIDO) Pairing {
   static base::Optional<std::unique_ptr<Pairing>> Parse(
       const cbor::Value& cbor,
       uint32_t tunnel_server_domain,
-      base::span<const uint8_t, kCableIdentityKeySeedSize> local_identity_seed,
+      base::span<const uint8_t, kQRSeedSize> local_identity_seed,
       base::span<const uint8_t, 32> handshake_hash);
 
   // tunnel_server_domain is known to be a valid hostname as it's constructed

device/fido/cable/fido_cable_discovery.cc

@@ -188,7 +188,6 @@ FidoCableDiscovery::CreateV1HandshakeHandler(
           device, nonce, discovery_data.v1->session_pre_key);
     }
 
-    case CableDiscoveryData::Version::V2:
     case CableDiscoveryData::Version::INVALID:
       CHECK(false);
       return nullptr;

device/fido/cable/fido_cable_discovery.h

@@ -21,6 +21,7 @@
 #include "device/bluetooth/bluetooth_adapter.h"
 #include "device/fido/cable/cable_discovery_data.h"
 #include "device/fido/cable/fido_cable_device.h"
+#include "device/fido/cable/v2_constants.h"
 #include "device/fido/fido_device_discovery.h"
 
 namespace device {
@@ -151,7 +152,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDiscovery
   // will be ignored. However, devices may rotate their BLE address at will so
   // this is not completely effective.
   std::set<std::string> active_devices_;
-  base::Optional<QRGeneratorKey> qr_generator_key_;
+  base::Optional<std::array<uint8_t, cablev2::kQRKeySize>> qr_generator_key_;
 
   // Note that on Windows, |advertisements_| is the only reference holder of
   // BluetoothAdvertisement.

device/fido/cable/fido_tunnel_device.cc

@@ -74,7 +74,7 @@ FidoTunnelDevice::FidoTunnelDevice(
     network::mojom::NetworkContext* network_context,
     base::OnceCallback<void(std::unique_ptr<Pairing>)> pairing_callback,
     base::span<const uint8_t> secret,
-    base::span<const uint8_t, kCableIdentityKeySeedSize> local_identity_seed,
+    base::span<const uint8_t, kQRSeedSize> local_identity_seed,
     const CableEidArray& eid,
     const CableEidArray& decrypted_eid)
     : info_(absl::in_place_type<QRInfo>), id_(RandomId()) {

device/fido/cable/fido_tunnel_device.h

@@ -34,7 +34,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoTunnelDevice : public FidoDevice {
       network::mojom::NetworkContext* network_context,
       base::OnceCallback<void(std::unique_ptr<Pairing>)> pairing_callback,
       base::span<const uint8_t> secret,
-      base::span<const uint8_t, kCableIdentityKeySeedSize> local_identity_seed,
+      base::span<const uint8_t, kQRSeedSize> local_identity_seed,
       const CableEidArray& eid,
       const CableEidArray& decrypted_eid);
 
@@ -76,7 +76,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoTunnelDevice : public FidoDevice {
     CableEidArray eid;
     std::array<uint8_t, 32> psk;
     base::OnceCallback<void(std::unique_ptr<Pairing>)> pairing_callback;
-    std::array<uint8_t, kCableIdentityKeySeedSize> local_identity_seed;
+    std::array<uint8_t, kQRSeedSize> local_identity_seed;
     uint32_t tunnel_server_domain;
     base::Optional<HandshakeHash> handshake_hash;
   };

device/fido/cable/v2_constants.h

@@ -25,6 +25,12 @@ constexpr size_t kEIDKeySize = 32;
 constexpr size_t kPSKSize = 32;
 // kRootSecretSize is the size of the main key maintained by authenticators.
 constexpr size_t kRootSecretSize = 32;
+// kQRKeySize is the size of the private key data that generates a QR code. It
+// consists of a 256-bit seed value that's used to genertate the P-256 private
+// key and a 128-bit secret.
+constexpr size_t kQRSecretSize = 16;
+constexpr size_t kQRSeedSize = 32;
+constexpr size_t kQRKeySize = kQRSeedSize + kQRSecretSize;
 
 }  // namespace cablev2
 }  // namespace device

device/fido/cable/v2_discovery.cc

@@ -7,10 +7,10 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/strings/string_number_conversions.h"
-#include "device/fido/cable/v2_handshake.h"
-#include "device/fido/fido_parsing_utils.h"
 #include "components/device_event_log/device_event_log.h"
 #include "device/fido/cable/fido_tunnel_device.h"
+#include "device/fido/cable/v2_handshake.h"
+#include "device/fido/fido_parsing_utils.h"
 #include "third_party/boringssl/src/include/openssl/aes.h"
 
 namespace device {
@@ -18,7 +18,7 @@ namespace cablev2 {
 
 Discovery::Discovery(
     network::mojom::NetworkContext* network_context,
-    QRGeneratorKey qr_generator_key,
+    base::span<const uint8_t, kQRKeySize> qr_generator_key,
     std::vector<std::unique_ptr<Pairing>> pairings,
     base::Optional<base::RepeatingCallback<void(std::unique_ptr<Pairing>)>>
         pairing_callback)
@@ -26,22 +26,18 @@ Discovery::Discovery(
           FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy),
       network_context_(network_context),
       local_identity_seed_(fido_parsing_utils::Materialize(
-          base::span<const uint8_t, kCableIdentityKeySeedSize>(
-              qr_generator_key.data(),
-              kCableIdentityKeySeedSize))),
+          base::span<const uint8_t, kQRSeedSize>(qr_generator_key.data(),
+                                                 kQRSeedSize))),
       qr_secret_(fido_parsing_utils::Materialize(
-          base::span<const uint8_t, kCableQRSecretSize>(
-              qr_generator_key.data() + kCableIdentityKeySeedSize,
-              kCableQRSecretSize))),
+          base::span<const uint8_t, kQRSecretSize>(
+              qr_generator_key.data() + kQRSeedSize,
+              kQRSecretSize))),
       eid_key_(Derive<EXTENT(eid_key_)>(qr_secret_,
                                         base::span<const uint8_t>(),
                                         DerivedValueType::kEIDKey)),
       pairings_(std::move(pairings)),
       pairing_callback_(std::move(pairing_callback)) {
-  // TODO(agl): disabled in order to separate out CLs. Re-enable.
-  // static_assert(EXTENT(qr_generator_key) ==
-  //                   kCableIdentityKeySeedSize + kCableQRSecretSize,
-  //              "");
+  static_assert(EXTENT(qr_generator_key) == kQRSecretSize + kQRSeedSize, "");
 }
 
 Discovery::~Discovery() = default;

device/fido/cable/v2_discovery.h

@@ -8,11 +8,12 @@
 #include <memory>
 #include <vector>
 
-#include "base/component_export.h"
 #include "base/callback.h"
+#include "base/component_export.h"
 #include "base/containers/flat_set.h"
-#include "base/optional.h"
+#include "base/containers/span.h"
 #include "base/memory/weak_ptr.h"
+#include "base/optional.h"
 #include "device/fido/cable/cable_discovery_data.h"
 #include "device/fido/cable/v2_constants.h"
 #include "device/fido/fido_device_discovery.h"
@@ -33,7 +34,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) Discovery
  public:
   Discovery(
       network::mojom::NetworkContext* network_context,
-      QRGeneratorKey qr_generator_key,
+      base::span<const uint8_t, kQRKeySize> qr_generator_key,
       std::vector<std::unique_ptr<Pairing>> pairings,
       // pairing_callback will be called when a QR-initiated connection receives
       // pairing information from the peer.
@@ -54,8 +55,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) Discovery
   void AddPairing(std::unique_ptr<Pairing> pairing);
 
   network::mojom::NetworkContext* const network_context_;
-  const std::array<uint8_t, kCableIdentityKeySeedSize> local_identity_seed_;
-  const std::array<uint8_t, kCableQRSecretSize> qr_secret_;
+  const std::array<uint8_t, kQRSeedSize> local_identity_seed_;
+  const std::array<uint8_t, kQRSecretSize> qr_secret_;
   const std::array<uint8_t, kEIDKeySize> eid_key_;
   std::vector<std::unique_ptr<Pairing>> pairings_;
   const base::Optional<base::RepeatingCallback<void(std::unique_ptr<Pairing>)>>

device/fido/cable/v2_handshake.cc

@@ -489,8 +489,7 @@ ResponderResult::ResponderResult(ResponderResult&&) = default;
 base::Optional<ResponderResult> RespondToHandshake(
     base::span<const uint8_t, 32> psk,
     base::span<const uint8_t, kCableEphemeralIdSize> eid,
-    base::Optional<base::span<const uint8_t, kCableIdentityKeySeedSize>>
-        identity_seed,
+    base::Optional<base::span<const uint8_t, kQRSeedSize>> identity_seed,
     base::Optional<base::span<const uint8_t, kP256X962Length>> peer_identity,
     base::span<const uint8_t> in,
     std::vector<uint8_t>* out_response) {
@@ -617,7 +616,7 @@ base::Optional<ResponderResult> RespondToHandshake(
 }
 
 bool VerifyPairingSignature(
-    base::span<const uint8_t, kCableIdentityKeySeedSize> identity_seed,
+    base::span<const uint8_t, kQRSeedSize> identity_seed,
     base::span<const uint8_t, kP256X962Length> peer_public_key_x962,
     base::span<const uint8_t, std::tuple_size<HandshakeHash>::value>
         handshake_hash,

device/fido/cable/v2_handshake.h

@@ -275,8 +275,7 @@ base::Optional<ResponderResult> RespondToHandshake(
     base::span<const uint8_t, kCableEphemeralIdSize> eid,
     // identity_seed, if not nullopt, specifies that this is a QR handshake and
     // contains the seed for QR key for this client.
-    base::Optional<base::span<const uint8_t, kCableIdentityKeySeedSize>>
-        identity_seed,
+    base::Optional<base::span<const uint8_t, kQRSeedSize>> identity_seed,
     // peer_identity, which must be non-nullopt iff |identity| is nullopt,
     // contains the peer's public key as taken from the pairing data.
     base::Optional<base::span<const uint8_t, kP256X962Length>> peer_identity,
@@ -291,7 +290,7 @@ base::Optional<ResponderResult> RespondToHandshake(
 // random values generated by the desktop and thus is a fresh value.
 COMPONENT_EXPORT(DEVICE_FIDO)
 bool VerifyPairingSignature(
-    base::span<const uint8_t, kCableIdentityKeySeedSize> identity_seed,
+    base::span<const uint8_t, kQRSeedSize> identity_seed,
     base::span<const uint8_t, kP256X962Length> peer_public_key_x962,
     base::span<const uint8_t, std::tuple_size<HandshakeHash>::value>
         handshake_hash,

device/fido/cable/v2_handshake_unittest.cc

@@ -79,8 +79,8 @@ std::array<uint8_t, kP256X962Length> PublicKeyOf(const EC_KEY* private_key) {
 }
 
 TEST(CableV2Encoding, HandshakeSignatures) {
-  static const uint8_t kSeed0[kCableIdentityKeySeedSize] = {0};
-  static const uint8_t kSeed1[kCableIdentityKeySeedSize] = {1};
+  static const uint8_t kSeed0[kQRSeedSize] = {0};
+  static const uint8_t kSeed1[kQRSeedSize] = {1};
 
   bssl::UniquePtr<EC_GROUP> group(
       EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1));
@@ -135,7 +135,7 @@ class CableV2HandshakeTest : public ::testing::Test {
   CableEidArray eid_;
   bssl::UniquePtr<EC_KEY> identity_key_;
   std::array<uint8_t, kP256X962Length> identity_public_;
-  std::array<uint8_t, kCableIdentityKeySeedSize> identity_seed_;
+  std::array<uint8_t, kQRSeedSize> identity_seed_;
 };
 
 TEST_F(CableV2HandshakeTest, MessageEncrytion) {

device/fido/fido_discovery_factory.cc

@@ -98,7 +98,8 @@ bool FidoDiscoveryFactory::IsTestOverride() {
 
 void FidoDiscoveryFactory::set_cable_data(
     std::vector<CableDiscoveryData> cable_data,
-    base::Optional<QRGeneratorKey> qr_generator_key,
+    const base::Optional<std::array<uint8_t, cablev2::kQRKeySize>>&
+        qr_generator_key,
     std::vector<std::unique_ptr<cablev2::Pairing>> v2_pairings) {
   cable_data_ = std::move(cable_data);
   qr_generator_key_ = std::move(qr_generator_key);

device/fido/fido_discovery_factory.h

@@ -12,6 +12,7 @@
 #include "base/optional.h"
 #include "build/build_config.h"
 #include "device/fido/cable/cable_discovery_data.h"
+#include "device/fido/cable/v2_constants.h"
 #include "device/fido/fido_device_discovery.h"
 #include "device/fido/fido_discovery_base.h"
 #include "device/fido/fido_request_handler_base.h"
@@ -51,7 +52,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDiscoveryFactory {
   // set_cable_data configures caBLE obtained via a WebAuthn extension.
   void set_cable_data(
       std::vector<CableDiscoveryData> cable_data,
-      base::Optional<QRGeneratorKey> qr_generator_key,
+      const base::Optional<std::array<uint8_t, cablev2::kQRKeySize>>&
+          qr_generator_key,
       std::vector<std::unique_ptr<cablev2::Pairing>> v2_pairings);
 
   void set_usb_device_manager(mojo::Remote<device::mojom::UsbDeviceManager>);
@@ -102,7 +104,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDiscoveryFactory {
       usb_device_manager_;
   network::mojom::NetworkContext* network_context_ = nullptr;
   base::Optional<std::vector<CableDiscoveryData>> cable_data_;
-  base::Optional<QRGeneratorKey> qr_generator_key_;
+  base::Optional<std::array<uint8_t, cablev2::kQRKeySize>> qr_generator_key_;
   std::vector<std::unique_ptr<cablev2::Pairing>> v2_pairings_;
   base::Optional<
       base::RepeatingCallback<void(std::unique_ptr<cablev2::Pairing>)>>