Revert "Revert "Give the desktop a public key in QR handshakes.""

This reverts commit aa1faf82, i.e. it relands fccf3a8f.

(After thinking about structure padding and writing a comment, I forgot
to actually handle it! Probably too complex anyway; do something dumber.)

> Revert "Give the desktop a public key in QR handshakes."
>
> This reverts commit fccf3a8f.
>
> Reason for revert:
> This CL breaks on Linux MSan bot
> FidoCableDiscoveryTest.TestDiscoveryFindsNewAppleDevice
> FidoCableDiscoveryTest.TestDiscoveryFindsIncorrectDevice
>
> First failure:
> https://ci.chromium.org/p/chromium/builders/ci/Linux%20MSan%20Tests/22909
>
> Original change's description:
> > Give the desktop a public key in QR handshakes.
> >
> > This change switches the QR-based handshake from NNpsk0 to KNpsk0. I.e.
> > it gives the desktop a public-key. That key is included in the QR code
> > and means that an attacker who sees the QR code cannot impersonate the
> > desktop and pair with the phone. (The other direction is still possible,
> > however, and seems fundamental without the user entering a confirmation
> > code on the desktop.)
> >
> > BUG=1002262
> >
> > Change-Id: I7254a8aa17df71d2bcabb4371a1c18914ed20641
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2109032
> > Commit-Queue: Adam Langley <agl@chromium.org>
> > Auto-Submit: Adam Langley <agl@chromium.org>
> > Reviewed-by: Martin Kreichgauer <martinkr@google.com>
> > Cr-Commit-Position: refs/heads/master@{#752196}
>
> TBR=agl@chromium.org,martinkr@google.com
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 1002262
> Change-Id: Id75ad0bab2862b3dafe73d20cf762883cc24faf9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2113771
> Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
> Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#752403}

BUG=10022628

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

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

@@ -175,6 +175,8 @@ struct AuthenticatorState {
   base::Optional<device::cablev2::NonceAndEID> qr_advert;
   // qr_psk_gen_key contains the PSK generating key derived from the QR secret.
   base::Optional<device::CablePskGeneratorKey> qr_psk_gen_key;
+  // peer_identity is the public-key of the desktop from the scanned QR code.
+  base::Optional<bssl::UniquePtr<EC_POINT>> qr_peer_identity;
 };
 
 // Client represents the state of a single BLE peer.
@@ -273,15 +275,16 @@ class Client {
           handshake_result = device::cablev2::RespondToHandshake(
               auth_state_->pairing_data.v2->psk_gen_key,
               auth_state_->pairing_advert, auth_state_->identity_key.get(),
-              /*pairing_data=*/nullptr, message->second, &response);
+              /*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_->pairing_data, message->second,
-              &response);
+              /*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);
@@ -487,21 +490,36 @@ class CableInterface : public Client::Delegate {
 
     base::StringPiece qr_url_base64(qr_url);
     qr_url_base64 = qr_url_base64.substr(sizeof(kPrefix) - 1);
-    std::string qr_secret_str;
+    std::string qr_data_str;
     if (!base::Base64UrlDecode(qr_url_base64,
                                base::Base64UrlDecodePolicy::DISALLOW_PADDING,
-                               &qr_secret_str) ||
-        qr_secret_str.size() != device::kCableQRSecretSize) {
+                               &qr_data_str) ||
+        qr_data_str.size() != device::kCableQRDataSize) {
       FIDO_LOG(ERROR) << "QR decoding failed: " << qr_url;
       return;
     }
 
-    uint8_t qr_secret[device::kCableQRSecretSize];
-    memcpy(qr_secret, qr_secret_str.data(), sizeof(qr_secret));
-    const device::CableDiscoveryData discovery_data(qr_secret);
-    auth_state_.qr_psk_gen_key.emplace(discovery_data.v2->psk_gen_key);
+    const base::Optional<device::CableDiscoveryData> discovery_data =
+        device::CableDiscoveryData::FromQRData(
+            base::span<const uint8_t, device::kCableQRDataSize>(
+                reinterpret_cast<const uint8_t*>(qr_data_str.data()),
+                qr_data_str.size()));
+    if (!discovery_data) {
+      FIDO_LOG(ERROR) << "Failed to decode QR data from: " << qr_url;
+      return;
+    }
+
+    auth_state_.qr_psk_gen_key.emplace(discovery_data->v2->psk_gen_key);
+
+    bssl::UniquePtr<EC_GROUP> p256(
+        EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1));
+    auth_state_.qr_peer_identity.emplace(EC_POINT_new(p256.get()));
+    CHECK(EC_POINT_oct2point(p256.get(), auth_state_.qr_peer_identity->get(),
+                             discovery_data->v2->peer_identity->data(),
+                             discovery_data->v2->peer_identity->size(),
+                             /*ctx=*/nullptr));
 
-    StartAdvertising(discovery_data.v2->eid_gen_key,
+    StartAdvertising(discovery_data->v2->eid_gen_key,
                      &auth_state_.qr_advert.emplace());
   }
 

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

@@ -172,27 +172,26 @@ base::span<uint8_t> QRDataForCurrentTime(
     uint8_t out_buf[QRCode::kInputBytes],
     base::span<const uint8_t, 32> qr_generator_key) {
   const int64_t current_tick = device::CableDiscoveryData::CurrentTimeTick();
-  auto qr_secret = device::CableDiscoveryData::DeriveQRSecret(qr_generator_key,
-                                                              current_tick);
+  const device::CableQRData qr_data =
+      device::CableDiscoveryData::DeriveQRData(qr_generator_key, current_tick);
 
-  std::string base64_qr_secret;
+  std::string base64_qr_data;
   base::Base64UrlEncode(
-      base::StringPiece(reinterpret_cast<const char*>(qr_secret.data()),
-                        qr_secret.size()),
-      base::Base64UrlEncodePolicy::OMIT_PADDING, &base64_qr_secret);
-  static constexpr size_t kEncodedSecretLength =
-      Base64EncodedSize(sizeof(qr_secret));
-  DCHECK_EQ(kEncodedSecretLength, base64_qr_secret.size());
+      base::StringPiece(reinterpret_cast<const char*>(qr_data.data()),
+                        sizeof(qr_data)),
+      base::Base64UrlEncodePolicy::OMIT_PADDING, &base64_qr_data);
+  static constexpr size_t kEncodedDataLength =
+      Base64EncodedSize(sizeof(qr_data));
+  DCHECK_EQ(kEncodedDataLength, base64_qr_data.size());
 
   static constexpr char kPrefix[] = "fido://c1/";
   static constexpr size_t kPrefixLength = sizeof(kPrefix) - 1;
 
-  static_assert(QRCode::kInputBytes >= kPrefixLength + kEncodedSecretLength,
+  static_assert(QRCode::kInputBytes >= kPrefixLength + kEncodedDataLength,
                 "unexpected QR input length");
   memcpy(out_buf, kPrefix, kPrefixLength);
-  memcpy(&out_buf[kPrefixLength], base64_qr_secret.data(),
-         kEncodedSecretLength);
-  return base::span<uint8_t>(out_buf, kPrefixLength + kEncodedSecretLength);
+  memcpy(&out_buf[kPrefixLength], base64_qr_data.data(), kEncodedDataLength);
+  return base::span<uint8_t>(out_buf, kPrefixLength + kEncodedDataLength);
 }
 
 }  // anonymous namespace

device/fido/cable/cable_discovery_data.h

@@ -17,6 +17,11 @@ 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>;
@@ -35,6 +40,8 @@ using CablePskGeneratorKey = std::array<uint8_t, 32>;
 // CableAuthenticatorIdentityKey is a P-256 public value used to authenticate a
 // paired phone.
 using CableAuthenticatorIdentityKey = std::array<uint8_t, 65>;
+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
@@ -54,14 +61,25 @@ struct COMPONENT_EXPORT(DEVICE_FIDO) CableDiscoveryData {
                      const CableEidArray& client_eid,
                      const CableEidArray& authenticator_eid,
                      const CableSessionPreKeyArray& session_pre_key);
-  // Creates discovery data given a specific QR secret. See |DeriveQRSecret| for
-  // how to generate such secrets.
-  explicit CableDiscoveryData(
-      base::span<const uint8_t, kCableQRSecretSize> qr_secret);
+  // 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;
 
@@ -82,6 +100,19 @@ struct COMPONENT_EXPORT(DEVICE_FIDO) CableDiscoveryData {
       base::span<const uint8_t, 32> 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, 32> 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, 32> 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.
@@ -102,6 +133,7 @@ struct COMPONENT_EXPORT(DEVICE_FIDO) CableDiscoveryData {
     CableEidGeneratorKey eid_gen_key;
     CablePskGeneratorKey psk_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
@@ -109,6 +141,10 @@ struct COMPONENT_EXPORT(DEVICE_FIDO) CableDiscoveryData {
     base::Optional<std::string> peer_name;
   };
   base::Optional<V2Data> v2;
+
+ private:
+  void InitFromQRSecret(
+      base::span<const uint8_t, kCableQRSecretSize> qr_secret);
 };
 
 }  // namespace device

device/fido/cable/fido_cable_discovery.cc

@@ -30,8 +30,10 @@
 #include "device/fido/fido_parsing_utils.h"
 #include "third_party/boringssl/src/include/openssl/aes.h"
 #include "third_party/boringssl/src/include/openssl/digest.h"
+#include "third_party/boringssl/src/include/openssl/ec.h"
 #include "third_party/boringssl/src/include/openssl/hkdf.h"
 #include "third_party/boringssl/src/include/openssl/mem.h"
+#include "third_party/boringssl/src/include/openssl/obj.h"
 
 namespace device {
 
@@ -104,6 +106,25 @@ std::unique_ptr<BluetoothAdvertisement::Data> ConstructAdvertisementData(
   return advertisement_data;
 }
 
+enum class QRValue : uint8_t {
+  QR_SECRET = 0,
+  IDENTITY_KEY_SEED = 1,
+};
+
+void DeriveQRValue(base::span<const uint8_t, 32> 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 -------------------------------------
@@ -124,22 +145,36 @@ CableDiscoveryData::CableDiscoveryData(
 }
 
 CableDiscoveryData::CableDiscoveryData(
-    base::span<const uint8_t, kCableQRSecretSize> qr_secret) {
-  version = Version::V2;
-  v2.emplace();
+    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 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
+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));
 
-  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);
+  return discovery_data;
 }
 
 CableDiscoveryData::CableDiscoveryData(const CableDiscoveryData& data) =
@@ -240,24 +275,69 @@ int64_t CableDiscoveryData::CurrentTimeTick() {
 std::array<uint8_t, kCableQRSecretSize> CableDiscoveryData::DeriveQRSecret(
     base::span<const uint8_t, 32> qr_generator_key,
     const int64_t tick) {
-  union {
-    int64_t i;
-    uint8_t bytes[8];
-  } current_tick;
-  current_tick.i = tick;
-
   std::array<uint8_t, kCableQRSecretSize> ret;
-  bool ok = HKDF(ret.data(), ret.size(), EVP_sha256(), qr_generator_key.data(),
-                 qr_generator_key.size(),
-                 /*salt=*/nullptr, 0, current_tick.bytes, sizeof(current_tick));
-  DCHECK(ok);
+  DeriveQRValue(qr_generator_key, tick, QRValue::QR_SECRET, ret);
   return ret;
 }
 
+// static
+CableIdentityKeySeed CableDiscoveryData::DeriveIdentityKeySeed(
+    base::span<const uint8_t, 32> 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, 32> 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);
+}
+
 // FidoCableDiscovery::CableV1DiscoveryEvent  ---------------------------------
 
 // CableV1DiscoveryEvent enumerates several things that can occur during a caBLE
@@ -373,7 +453,8 @@ FidoCableDiscovery::CreateHandshakeHandler(
 
       handler.reset(new FidoCableV2HandshakeHandler(
           device, discovery_data.v2->psk_gen_key, nonce, eid,
-          discovery_data.v2->peer_identity, *pairing_callback_));
+          discovery_data.v2->peer_identity,
+          discovery_data.v2->local_identity_seed, *pairing_callback_));
       break;
     }
 
@@ -782,7 +863,9 @@ FidoCableDiscovery::GetCableDiscoveryDataFromAuthenticatorEid(
     for (int i = 0; i < kNumPreviousTicks; i++) {
       auto qr_secret = CableDiscoveryData::DeriveQRSecret(*qr_generator_key_,
                                                           current_tick - i);
-      CableDiscoveryData candidate(qr_secret);
+      auto identity_key_seed = CableDiscoveryData::DeriveIdentityKeySeed(
+          *qr_generator_key_, current_tick - i);
+      CableDiscoveryData candidate(qr_secret, identity_key_seed);
       auto maybe_nonce = candidate.Match(authenticator_eid);
       if (maybe_nonce) {
         return Result(candidate, *maybe_nonce, authenticator_eid, i);
@@ -790,10 +873,11 @@ FidoCableDiscovery::GetCableDiscoveryDataFromAuthenticatorEid(
     }
 
     if (base::Contains(noted_obsolete_eids_, authenticator_eid)) {
+      std::array<uint8_t, kCableIdentityKeySeedSize> dummy_seed;
       for (int i = kNumPreviousTicks; i < 2 * kNumPreviousTicks; i++) {
         auto qr_secret = CableDiscoveryData::DeriveQRSecret(*qr_generator_key_,
                                                             current_tick - i);
-        CableDiscoveryData candidate(qr_secret);
+        CableDiscoveryData candidate(qr_secret, dummy_seed);
         if (candidate.Match(authenticator_eid)) {
           noted_obsolete_eids_.insert(authenticator_eid);
           FIDO_LOG(DEBUG)

device/fido/cable/fido_cable_handshake_handler.cc

@@ -187,11 +187,13 @@ FidoCableV2HandshakeHandler::FidoCableV2HandshakeHandler(
     base::span<const uint8_t, 8> nonce,
     base::span<const uint8_t, kCableEphemeralIdSize> eid,
     base::Optional<base::span<const uint8_t, kP256PointSize>> 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) {}
+      handshake_(psk_gen_key, nonce, eid, peer_identity, local_seed) {}
 
 FidoCableV2HandshakeHandler::~FidoCableV2HandshakeHandler() = default;
 

device/fido/cable/fido_cable_handshake_handler.h

@@ -87,6 +87,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableV2HandshakeHandler
       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;

device/fido/cable/fido_cable_handshake_handler_v2_fuzzer.cc

@@ -36,6 +36,11 @@ constexpr std::array<uint8_t, 65> kTestPeerIdentity = {
     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
 
@@ -48,14 +53,17 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* raw_data, size_t size) {
       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, base::DoNothing());
+      peer_identity, local_seed, base::DoNothing());
   handshake_handler_v2.InitiateCableHandshake(base::DoNothing());
   handshake_handler_v2.ValidateAuthenticatorHandshakeMessage(input);
   return 0;

device/fido/cable/noise.cc

@@ -59,11 +59,11 @@ Noise::~Noise() = default;
 
 void Noise::Init(Noise::HandshakeType type) {
   // See https://www.noiseprotocol.org/noise.html#the-handshakestate-object
-  static const char kNNProtocolName[] = "Noise_NNpsk0_P256_AESGCM_SHA256";
+  static const char kKNProtocolName[] = "Noise_KNpsk0_P256_AESGCM_SHA256";
   static const char kNKProtocolName[] = "Noise_NKpsk0_P256_AESGCM_SHA256";
-  static_assert(sizeof(kNKProtocolName) == sizeof(kNNProtocolName),
+  static_assert(sizeof(kNKProtocolName) == sizeof(kKNProtocolName),
                 "protocol names are different lengths");
-  static_assert(sizeof(kNNProtocolName) == crypto::kSHA256Length,
+  static_assert(sizeof(kKNProtocolName) == crypto::kSHA256Length,
                 "name may need padding if not HASHLEN bytes long");
   static_assert(
       std::tuple_size<decltype(chaining_key_)>::value == crypto::kSHA256Length,
@@ -76,8 +76,8 @@ void Noise::Init(Noise::HandshakeType type) {
       memcpy(chaining_key_.data(), kNKProtocolName, sizeof(kNKProtocolName));
       break;
 
-    case HandshakeType::kNNpsk0:
-      memcpy(chaining_key_.data(), kNNProtocolName, sizeof(kNNProtocolName));
+    case HandshakeType::kKNpsk0:
+      memcpy(chaining_key_.data(), kKNProtocolName, sizeof(kKNProtocolName));
       break;
   }
 

device/fido/cable/noise.h

@@ -22,7 +22,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) Noise {
  public:
   // HandshakeType enumerates the supported handshake patterns.
   enum class HandshakeType {
-    kNNpsk0,  // https://noiseexplorer.com/patterns/NNpsk0/
+    kKNpsk0,  // https://noiseexplorer.com/patterns/KNpsk0/
     kNKpsk0,  // https://noiseexplorer.com/patterns/NKpsk0/
   };
 

device/fido/cable/v2_handshake.cc

@@ -171,14 +171,20 @@ HandshakeInitiator::HandshakeInitiator(
     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, kP256PointSize>> peer_identity)
+    base::Optional<base::span<const uint8_t, kP256PointSize>> peer_identity,
+    base::Optional<base::span<const uint8_t, kCableIdentityKeySeedSize>>
+        local_seed)
     : eid_(fido_parsing_utils::Materialize(eid)) {
+  DCHECK(peer_identity.has_value() ^ local_seed.has_value());
   HKDF(psk_.data(), psk_.size(), EVP_sha256(), psk_gen_key.data(),
        psk_gen_key.size(), /*salt=*/nonce.data(), nonce.size(),
        /*info=*/nullptr, 0);
   if (peer_identity) {
     peer_identity_ = fido_parsing_utils::Materialize(*peer_identity);
   }
+  if (local_seed) {
+    local_seed_ = fido_parsing_utils::Materialize(*local_seed);
+  }
 }
 
 HandshakeInitiator::~HandshakeInitiator() = default;
@@ -188,7 +194,7 @@ std::vector<uint8_t> HandshakeInitiator::BuildInitialMessage() {
     noise_.Init(Noise::HandshakeType::kNKpsk0);
     noise_.MixHash(kPairedPrologue);
   } else {
-    noise_.Init(Noise::HandshakeType::kNNpsk0);
+    noise_.Init(Noise::HandshakeType::kKNpsk0);
     noise_.MixHash(kQRPrologue);
   }
 
@@ -246,11 +252,11 @@ HandshakeInitiator::ProcessResponse(base::span<const uint8_t> response) {
 
   bssl::UniquePtr<EC_POINT> peer_point(
       EC_POINT_new(EC_KEY_get0_group(ephemeral_key_.get())));
-  uint8_t shared_key[32];
+  uint8_t shared_key_ee[32];
   const EC_GROUP* group = EC_KEY_get0_group(ephemeral_key_.get());
   if (!EC_POINT_oct2point(group, peer_point.get(), peer_point_bytes.data(),
                           peer_point_bytes.size(), /*ctx=*/nullptr) ||
-      !ECDH_compute_key(shared_key, sizeof(shared_key), peer_point.get(),
+      !ECDH_compute_key(shared_key_ee, sizeof(shared_key_ee), peer_point.get(),
                         ephemeral_key_.get(), /*kdf=*/nullptr)) {
     FIDO_LOG(DEBUG) << "Peer's P-256 point not on curve.";
     return base::nullopt;
@@ -258,7 +264,19 @@ HandshakeInitiator::ProcessResponse(base::span<const uint8_t> response) {
 
   noise_.MixHash(peer_point_bytes);
   noise_.MixKey(peer_point_bytes);
-  noise_.MixKey(shared_key);
+  noise_.MixKey(shared_key_ee);
+
+  if (local_seed_) {
+    uint8_t shared_key_se[32];
+    bssl::UniquePtr<EC_KEY> identity_key(EC_KEY_derive_from_secret(
+        group, local_seed_->data(), local_seed_->size()));
+    if (!ECDH_compute_key(shared_key_se, sizeof(shared_key_se),
+                          peer_point.get(), identity_key.get(),
+                          /*kdf=*/nullptr)) {
+      return base::nullopt;
+    }
+    noise_.MixKey(shared_key_se);
+  }
 
   auto plaintext = noise_.DecryptAndHash(ciphertext);
   if (!plaintext || plaintext->empty() != peer_identity_.has_value()) {
@@ -268,7 +286,7 @@ HandshakeInitiator::ProcessResponse(base::span<const uint8_t> response) {
 
   base::Optional<std::unique_ptr<CableDiscoveryData>> discovery_data;
   if (!peer_identity_) {
-    // Handshakes without a peer identity (i.e. NNpsk0 handshakes setup from a
+    // Handshakes without a peer identity (i.e. KNpsk0 handshakes setup from a
     // QR code) send a padded message in the reply. This message can,
     // optionally, contain CBOR-encoded, long-term pairing information.
     const size_t padding_length = (*plaintext)[plaintext->size() - 1];
@@ -322,10 +340,12 @@ base::Optional<std::unique_ptr<Crypter>> RespondToHandshake(
     base::span<const uint8_t, 32> psk_gen_key,
     const NonceAndEID& nonce_and_eid,
     const EC_KEY* identity,
+    const EC_POINT* peer_identity,
     const CableDiscoveryData* pairing_data,
     base::span<const uint8_t> in,
     std::vector<uint8_t>* out_response) {
   DCHECK(identity == nullptr || pairing_data == nullptr);
+  DCHECK(identity == nullptr ^ peer_identity == nullptr);
 
   CBS cbs = CBSFromSpan(in);
 
@@ -348,7 +368,7 @@ base::Optional<std::unique_ptr<Crypter>> RespondToHandshake(
     noise.Init(device::Noise::HandshakeType::kNKpsk0);
     noise.MixHash(kPairedPrologue);
   } else {
-    noise.Init(device::Noise::HandshakeType::kNNpsk0);
+    noise.Init(device::Noise::HandshakeType::kKNpsk0);
     noise.MixHash(kQRPrologue);
   }
 
@@ -398,12 +418,21 @@ base::Optional<std::unique_ptr<Crypter>> RespondToHandshake(
   noise.MixHash(ephemeral_key_public_bytes);
   noise.MixKey(ephemeral_key_public_bytes);
 
-  uint8_t shared_key[32];
-  if (!ECDH_compute_key(shared_key, sizeof(shared_key), peer_point.get(),
+  uint8_t shared_key_ee[32];
+  if (!ECDH_compute_key(shared_key_ee, sizeof(shared_key_ee), peer_point.get(),
                         ephemeral_key.get(), /*kdf=*/nullptr)) {
     return base::nullopt;
   }
-  noise.MixKey(shared_key);
+  noise.MixKey(shared_key_ee);
+
+  if (peer_identity) {
+    uint8_t shared_key_se[32];
+    if (!ECDH_compute_key(shared_key_se, sizeof(shared_key_se), peer_identity,
+                          ephemeral_key.get(), /*kdf=*/nullptr)) {
+      return base::nullopt;
+    }
+    noise.MixKey(shared_key_se);
+  }
 
   std::vector<uint8_t> my_ciphertext;
   if (!identity) {

device/fido/cable/v2_handshake.h

@@ -78,7 +78,11 @@ class COMPONENT_EXPORT(DEVICE_FIDO) HandshakeInitiator {
       // peer_identity, if given, specifies that this is a paired handshake
       // and then contains an X9.62, P-256 public key for the peer. Otherwise
       // this is a QR-code handshake.
-      base::Optional<base::span<const uint8_t, kP256PointSize>> peer_identity);
+      base::Optional<base::span<const uint8_t, kP256PointSize>> peer_identity,
+      // local_identity must be provided if |peer_identity| is not. It contains
+      // the seed for deriving the local identity key.
+      base::Optional<base::span<const uint8_t, kCableIdentityKeySeedSize>>
+          local_identity);
 
   ~HandshakeInitiator();
 
@@ -100,6 +104,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) HandshakeInitiator {
   std::array<uint8_t, 32> psk_;
 
   base::Optional<std::array<uint8_t, kP256PointSize>> peer_identity_;
+  base::Optional<std::array<uint8_t, kCableIdentityKeySeedSize>> local_seed_;
   bssl::UniquePtr<EC_KEY> ephemeral_key_;
 };
 
@@ -112,6 +117,9 @@ base::Optional<std::unique_ptr<Crypter>> RespondToHandshake(
     // identity, if not nullptr, specifies that this is a paired handshake and
     // contains the long-term identity key for this authenticator.
     const EC_KEY* identity,
+    // peer_identity, which must be non-nullptr iff |identity| is nullptr,
+    // contains the peer's public key as derived from the QR-code data.
+    const EC_POINT* peer_identity,
     // pairing_data, if not nullptr, contains long-term pairing data that will
     // be shared with the peer. This is mutually exclusive with |identity|.
     const CableDiscoveryData* pairing_data,

device/fido/cable/v2_handshake_unittest.cc

@@ -19,6 +19,7 @@ class CableV2HandshakeTest : public ::testing::Test {
     std::fill(psk_gen_key_.begin(), psk_gen_key_.end(), 0);
     std::fill(nonce_and_eid_.first.begin(), nonce_and_eid_.first.end(), 1);
     std::fill(nonce_and_eid_.second.begin(), nonce_and_eid_.second.end(), 2);
+    std::fill(local_seed_.begin(), local_seed_.end(), 3);
 
     p256_key_.reset(EC_KEY_new_by_curve_name(NID_X9_62_prime256v1));
     const EC_GROUP* group = EC_KEY_get0_group(p256_key_.get());
@@ -28,13 +29,19 @@ class CableV2HandshakeTest : public ::testing::Test {
                                 POINT_CONVERSION_UNCOMPRESSED,
                                 p256_public_key_.data(),
                                 p256_public_key_.size(), /*ctx=*/nullptr));
+    bssl::UniquePtr<EC_KEY> qr_identity(EC_KEY_derive_from_secret(
+        group, local_seed_.data(), local_seed_.size()));
+    qr_identity_.reset(
+        EC_POINT_dup(EC_KEY_get0_public_key(qr_identity.get()), group));
   }
 
  protected:
   std::array<uint8_t, 32> psk_gen_key_;
   NonceAndEID nonce_and_eid_;
   bssl::UniquePtr<EC_KEY> p256_key_;
+  bssl::UniquePtr<EC_POINT> qr_identity_;
   std::array<uint8_t, kP256PointSize> p256_public_key_;
+  std::array<uint8_t, kCableIdentityKeySeedSize> local_seed_;
 };
 
 TEST_F(CableV2HandshakeTest, MessageEncrytion) {
@@ -73,12 +80,13 @@ TEST_F(CableV2HandshakeTest, OneTimeQRHandshake) {
     HandshakeInitiator initiator(
         use_correct_key ? psk_gen_key_ : wrong_psk_gen_key,
         nonce_and_eid_.first, nonce_and_eid_.second,
-        /*peer_identity=*/base::nullopt);
+        /*peer_identity=*/base::nullopt, local_seed_);
     std::vector<uint8_t> message = initiator.BuildInitialMessage();
     std::vector<uint8_t> response;
     base::Optional<std::unique_ptr<Crypter>> response_crypter(
         RespondToHandshake(psk_gen_key_, nonce_and_eid_, /*identity=*/nullptr,
-                           /*pairing_data=*/nullptr, message, &response));
+                           qr_identity_.get(), /*pairing_data=*/nullptr,
+                           message, &response));
     ASSERT_EQ(response_crypter.has_value(), use_correct_key);
     if (!use_correct_key) {
       continue;
@@ -105,12 +113,12 @@ TEST_F(CableV2HandshakeTest, PairingQRHandshake) {
 
   HandshakeInitiator initiator(psk_gen_key_, nonce_and_eid_.first,
                                nonce_and_eid_.second,
-                               /*peer_identity=*/base::nullopt);
+                               /*peer_identity=*/base::nullopt, local_seed_);
   std::vector<uint8_t> message = initiator.BuildInitialMessage();
   std::vector<uint8_t> response;
   base::Optional<std::unique_ptr<Crypter>> response_crypter(
       RespondToHandshake(psk_gen_key_, nonce_and_eid_, /*identity=*/nullptr,
-                         &pairing, message, &response));
+                         qr_identity_.get(), &pairing, message, &response));
   ASSERT_TRUE(response_crypter.has_value());
   base::Optional<std::pair<std::unique_ptr<Crypter>,
                            base::Optional<std::unique_ptr<CableDiscoveryData>>>>
@@ -138,12 +146,14 @@ TEST_F(CableV2HandshakeTest, PairedHandshake) {
     SCOPED_TRACE(use_correct_key);
 
     HandshakeInitiator initiator(psk_gen_key_, nonce_and_eid_.first,
-                                 nonce_and_eid_.second, p256_public_key_);
+                                 nonce_and_eid_.second, p256_public_key_,
+                                 /*local_seed=*/base::nullopt);
     std::vector<uint8_t> message = initiator.BuildInitialMessage();
     std::vector<uint8_t> response;
     base::Optional<std::unique_ptr<Crypter>> response_crypter(
         RespondToHandshake(psk_gen_key_, nonce_and_eid_,
                            use_correct_key ? p256_key_.get() : wrong_key.get(),
+                           /*peer_identity=*/nullptr,
                            /*pairing=*/nullptr, message, &response));
     ASSERT_EQ(response_crypter.has_value(), use_correct_key);