Advertise and pair with long-term keys.

With this change, the caBLE authenticator will advertise with it's
long-term keys and (always) exchange pairing information when doing QR
handshakes. This removes the need to do a QR pairing for every
operation.

BUG=1002262

Change-Id: I1999e3de347f278a460f126c491d185d1cb18e95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2101288
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@{#750613}

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

@@ -12,6 +12,7 @@
 #include "components/cbor/writer.h"
 #include "components/device_event_log/device_event_log.h"
 #include "crypto/aead.h"
+#include "crypto/random.h"
 #include "device/fido/authenticator_get_info_response.h"
 #include "device/fido/authenticator_supported_options.h"
 #include "device/fido/cable/cable_discovery_data.h"
@@ -25,7 +26,6 @@
 #include "third_party/boringssl/src/include/openssl/ecdh.h"
 #include "third_party/boringssl/src/include/openssl/hkdf.h"
 #include "third_party/boringssl/src/include/openssl/obj.h"
-#include "third_party/boringssl/src/include/openssl/rand.h"
 #include "third_party/boringssl/src/include/openssl/sha.h"
 
 // This "header" is actually contains several function definitions and thus can
@@ -36,6 +36,11 @@ using device::fido_parsing_utils::CopyCBORBytestring;
 
 namespace {
 
+// TODO: this string is currently in the protocol, and saved in the
+// desktop's prefs, but not otherwise surfaced. See if we can get a better
+// value for it.
+constexpr char kDeviceName[] = "Android phone";
+
 // Defragmenter accepts CTAP2 message fragments and reassembles them.
 // See
 // https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html#ble-framing
@@ -123,17 +128,24 @@ class Defragmenter {
 
 // AuthenticatorState contains the keys for a caBLE v2 authenticator.
 struct AuthenticatorState {
-  device::CableEidGeneratorKey eid_gen_key;
-  device::CablePskGeneratorKey psk_gen_key;
+  // pairing_data contains long-term keys, and information that is potentially
+  // sent to peers during QR pairing. The |v2| member of this structure will be
+  // populated.
+  device::CableDiscoveryData pairing_data;
+  // identity_key is the long-term signing key.
   bssl::UniquePtr<EC_KEY> identity_key;
 
-  std::pair<std::array<uint8_t, device::kCableNonceSize>,
-            std::array<uint8_t, device::kCableEphemeralIdSize>>
-      pairing_nonce_and_eid;
-  base::Optional<std::pair<std::array<uint8_t, device::kCableNonceSize>,
-                           std::array<uint8_t, device::kCableEphemeralIdSize>>>
-      qr_nonce_and_eid;
-  base::Optional<device::CableDiscoveryData> qr_discovery_data;
+  // pairing_advert contains information about the BLE advert that is sent based
+  // on the long-term keys.
+  device::cablev2::NonceAndEID pairing_advert;
+
+  // If doing a QR pairing, the following two members will be present.
+
+  // qr_advert contains information about the BLE advert that is sent based on
+  // QR pairing keys.
+  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;
 };
 
 // Client represents the state of a single BLE peer.
@@ -188,12 +200,37 @@ class Client {
           return false;
         }
 
+        // The handshake is prefixed with the EID that the peer is responding
+        // to. This allows us to handle the case where we have started
+        // advertising for a QR code, but the desktop is already paired and is
+        // connecting based on long-term keys.
+        device::CableEidArray requested_eid;
+        if (!device::fido_parsing_utils::ExtractArray(message->second, 0,
+                                                      &requested_eid)) {
+          return false;
+        }
+
         base::Optional<std::unique_ptr<device::cablev2::Crypter>>
-            handshake_result = device::cablev2::RespondToHandshake(
-                auth_state_->qr_discovery_data->v2->psk_gen_key,
-                auth_state_->qr_nonce_and_eid->first,
-                auth_state_->qr_nonce_and_eid->second, /*identity=*/nullptr,
-                /*pairing_data=*/nullptr, message->second, &response);
+            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(),
+              /*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);
+        } 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;
@@ -358,14 +395,24 @@ class CableInterface {
     if (!ParseState(state_bytes)) {
       GenerateFreshStateAndStore();
     }
+
+    // At this point, the version two pairing data has been established, either
+    // because it was parsed from the state, or because it was freshly generated
+    // and saved.
+    DCHECK(auth_state_.pairing_data.v2.has_value());
+    DCHECK(auth_state_.identity_key);
+
+    StartAdvertising(auth_state_.pairing_data.v2->eid_gen_key,
+                     &auth_state_.pairing_advert);
   }
 
   void Stop() {
     ble_handler_.Reset();
     clients_.clear();
     known_mtus_.clear();
-    auth_state_.qr_nonce_and_eid.reset();
-    auth_state_.qr_discovery_data.reset();
+    auth_state_.identity_key.reset();
+    auth_state_.qr_advert.reset();
+    auth_state_.qr_psk_gen_key.reset();
     env_ = nullptr;
   }
 
@@ -386,33 +433,11 @@ class CableInterface {
 
     uint8_t qr_secret[device::kCableQRSecretSize];
     memcpy(qr_secret, qr_secret_str.data(), sizeof(qr_secret));
-    auth_state_.qr_discovery_data.emplace(qr_secret);
+    const device::CableDiscoveryData discovery_data(qr_secret);
+    auth_state_.qr_psk_gen_key.emplace(discovery_data.v2->psk_gen_key);
 
-    std::array<uint8_t, device::kCableNonceSize> nonce;
-    RAND_bytes(nonce.data(), nonce.size());
-
-    uint8_t eid_plaintext[AES_BLOCK_SIZE];
-    static_assert(sizeof(eid_plaintext) == AES_BLOCK_SIZE,
-                  "EIDs are not AES blocks");
-    AES_KEY key;
-    CHECK(
-        AES_set_encrypt_key(
-            auth_state_.qr_discovery_data->v2->eid_gen_key.data(),
-            /*bits=*/8 * auth_state_.qr_discovery_data->v2->eid_gen_key.size(),
-            &key) == 0);
-    memcpy(eid_plaintext, nonce.data(), nonce.size());
-    memset(eid_plaintext + nonce.size(), 0,
-           sizeof(eid_plaintext) - nonce.size());
-
-    std::array<uint8_t, AES_BLOCK_SIZE> eid;
-    AES_encrypt(/*in=*/eid_plaintext, /*out=*/eid.data(), &key);
-
-    auth_state_.qr_nonce_and_eid.emplace(nonce, eid);
-
-    base::android::ScopedJavaLocalRef<jbyteArray> jbytes(
-        env_, env_->NewByteArray(sizeof(eid)));
-    env_->SetByteArrayRegion(jbytes.obj(), 0, eid.size(), (jbyte*)eid.data());
-    Java_BLEHandler_sendBLEAdvert(env_, ble_handler_, jbytes);
+    StartAdvertising(discovery_data.v2->eid_gen_key,
+                     &auth_state_.qr_advert.emplace());
   }
 
   void RecordClientMTU(uint64_t client_adr, uint16_t mtu_bytes) {
@@ -471,11 +496,32 @@ class CableInterface {
   friend struct base::DefaultSingletonTraits<CableInterface>;
   CableInterface() = default;
 
-  bssl::UniquePtr<EC_KEY> P256KeyFromSeed(base::span<const uint8_t, 32> seed) {
-    bssl::UniquePtr<EC_GROUP> p256(
-        EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1));
-    return bssl::UniquePtr<EC_KEY>(
-        EC_KEY_derive_from_secret(p256.get(), seed.data(), seed.size()));
+  void StartAdvertising(const device::CableEidGeneratorKey& eid_gen_key,
+                        device::cablev2::NonceAndEID* out_nonce_and_eid) {
+    std::array<uint8_t, device::kCableNonceSize> nonce;
+    crypto::RandBytes(nonce);
+
+    uint8_t eid_plaintext[device::kCableEphemeralIdSize];
+    static_assert(sizeof(eid_plaintext) == AES_BLOCK_SIZE,
+                  "EIDs are not AES blocks");
+    AES_KEY key;
+    CHECK(AES_set_encrypt_key(eid_gen_key.data(),
+                              /*bits=*/8 * eid_gen_key.size(), &key) == 0);
+    memcpy(eid_plaintext, nonce.data(), nonce.size());
+    static_assert(sizeof(nonce) < sizeof(eid_plaintext), "Nonces too large");
+    memset(eid_plaintext + nonce.size(), 0,
+           sizeof(eid_plaintext) - nonce.size());
+
+    std::array<uint8_t, AES_BLOCK_SIZE> eid;
+    AES_encrypt(/*in=*/eid_plaintext, /*out=*/eid.data(), &key);
+
+    out_nonce_and_eid->first = nonce;
+    out_nonce_and_eid->second = eid;
+
+    base::android::ScopedJavaLocalRef<jbyteArray> jbytes(
+        env_, env_->NewByteArray(sizeof(eid)));
+    env_->SetByteArrayRegion(jbytes.obj(), 0, eid.size(), (jbyte*)eid.data());
+    Java_BLEHandler_sendBLEAdvert(env_, ble_handler_, jbytes);
   }
 
   bool ParseState(const base::android::JavaParamRef<jbyteArray>& state_bytes) {
@@ -493,28 +539,38 @@ class CableInterface {
     }
 
     const cbor::Value::MapValue& state_map(state->GetMap());
+    device::CableDiscoveryData::V2Data& pairing_data =
+        auth_state_.pairing_data.v2.emplace();
     std::array<uint8_t, 32> identity_key_seed;
-    if (!CopyCBORBytestring(&auth_state_.eid_gen_key, state_map, 1) ||
-        !CopyCBORBytestring(&auth_state_.psk_gen_key, state_map, 2) ||
+    if (!CopyCBORBytestring(&pairing_data.eid_gen_key, state_map, 1) ||
+        !CopyCBORBytestring(&pairing_data.psk_gen_key, state_map, 2) ||
         !CopyCBORBytestring(&identity_key_seed, state_map, 3)) {
       return false;
     }
 
     auth_state_.identity_key = P256KeyFromSeed(identity_key_seed);
+    pairing_data.peer_identity.emplace(
+        X962PublicKeyOf(auth_state_.identity_key.get()));
+    pairing_data.peer_name.emplace(kDeviceName);
     return true;
   }
 
   void GenerateFreshStateAndStore() {
-    RAND_bytes(auth_state_.eid_gen_key.data(), auth_state_.eid_gen_key.size());
-    RAND_bytes(auth_state_.psk_gen_key.data(), auth_state_.psk_gen_key.size());
+    device::CableDiscoveryData::V2Data& pairing_data =
+        auth_state_.pairing_data.v2.emplace();
+    crypto::RandBytes(pairing_data.eid_gen_key);
+    crypto::RandBytes(pairing_data.psk_gen_key);
 
     std::array<uint8_t, 32> identity_key_seed;
-    RAND_bytes(identity_key_seed.data(), identity_key_seed.size());
+    crypto::RandBytes(identity_key_seed);
     auth_state_.identity_key = P256KeyFromSeed(identity_key_seed);
+    pairing_data.peer_identity.emplace(
+        X962PublicKeyOf(auth_state_.identity_key.get()));
+    pairing_data.peer_name.emplace(kDeviceName);
 
     cbor::Value::MapValue map;
-    map.emplace(1, cbor::Value(auth_state_.eid_gen_key));
-    map.emplace(2, cbor::Value(auth_state_.psk_gen_key));
+    map.emplace(1, cbor::Value(pairing_data.eid_gen_key));
+    map.emplace(2, cbor::Value(pairing_data.psk_gen_key));
     map.emplace(3, cbor::Value(identity_key_seed));
 
     base::Optional<std::vector<uint8_t>> bytes =
@@ -528,6 +584,25 @@ class CableInterface {
     Java_BLEHandler_setState(env_, ble_handler_, jbytes);
   }
 
+  static bssl::UniquePtr<EC_KEY> P256KeyFromSeed(
+      base::span<const uint8_t, 32> seed) {
+    bssl::UniquePtr<EC_GROUP> p256(
+        EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1));
+    return bssl::UniquePtr<EC_KEY>(
+        EC_KEY_derive_from_secret(p256.get(), seed.data(), seed.size()));
+  }
+
+  static device::CableAuthenticatorIdentityKey X962PublicKeyOf(
+      const EC_KEY* ec_key) {
+    device::CableAuthenticatorIdentityKey ret;
+    CHECK_EQ(ret.size(),
+             EC_POINT_point2oct(EC_KEY_get0_group(ec_key),
+                                EC_KEY_get0_public_key(ec_key),
+                                POINT_CONVERSION_UNCOMPRESSED, ret.data(),
+                                ret.size(), /*ctx=*/nullptr));
+    return ret;
+  }
+
   JNIEnv* env_ = nullptr;
   base::android::ScopedJavaGlobalRef<jobject> ble_handler_;
   AuthenticatorState auth_state_;

crypto/random.h

@@ -19,7 +19,6 @@ CRYPTO_EXPORT void RandBytes(void *bytes, size_t length);
 
 // Fills |bytes| with cryptographically-secure random bits.
 CRYPTO_EXPORT void RandBytes(base::span<uint8_t> bytes);
-
 }
 
 #endif  // CRYPTO_RANDOM_H_

device/fido/cable/v2_handshake.cc

@@ -320,8 +320,7 @@ HandshakeInitiator::ProcessResponse(base::span<const uint8_t> response) {
 
 base::Optional<std::unique_ptr<Crypter>> RespondToHandshake(
     base::span<const uint8_t, 32> psk_gen_key,
-    base::span<const uint8_t, 8> nonce,
-    base::span<const uint8_t, kCableEphemeralIdSize> expected_eid,
+    const NonceAndEID& nonce_and_eid,
     const EC_KEY* identity,
     const CableDiscoveryData* pairing_data,
     base::span<const uint8_t> in,
@@ -339,8 +338,8 @@ base::Optional<std::unique_ptr<Crypter>> RespondToHandshake(
     return base::nullopt;
   }
 
-  if (eid.size() != expected_eid.size() ||
-      memcmp(eid.data(), expected_eid.data(), eid.size()) != 0) {
+  if (eid.size() != nonce_and_eid.second.size() ||
+      memcmp(eid.data(), nonce_and_eid.second.data(), eid.size()) != 0) {
     return base::nullopt;
   }
 
@@ -356,7 +355,7 @@ base::Optional<std::unique_ptr<Crypter>> RespondToHandshake(
   std::array<uint8_t, 32> psk;
   HKDF(psk.data(), psk.size(), EVP_sha256(), psk_gen_key.data(),
        psk_gen_key.size(),
-       /*salt=*/nonce.data(), nonce.size(),
+       /*salt=*/nonce_and_eid.first.data(), nonce_and_eid.first.size(),
        /*info=*/nullptr, 0);
 
   noise.MixKeyAndHash(psk);

device/fido/cable/v2_handshake.h

@@ -21,6 +21,12 @@
 namespace device {
 namespace cablev2 {
 
+// NonceAndEID contains both the random nonce chosen for an advert, as well as
+// the EID that was generated from it.
+typedef std::pair<std::array<uint8_t, device::kCableNonceSize>,
+                  std::array<uint8_t, device::kCableEphemeralIdSize>>
+    NonceAndEID;
+
 // kP256PointSize is the number of bytes in an X9.62 encoding of a P-256 point.
 constexpr size_t kP256PointSize = 65;
 
@@ -102,8 +108,7 @@ COMPONENT_EXPORT(DEVICE_FIDO)
 base::Optional<std::unique_ptr<Crypter>> RespondToHandshake(
     // See |HandshakeInitiator| comments about these first three arguments.
     base::span<const uint8_t, 32> psk_gen_key,
-    base::span<const uint8_t, 8> nonce,
-    base::span<const uint8_t, kCableEphemeralIdSize> expected_eid,
+    const NonceAndEID& nonce_and_eid,
     // 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,

device/fido/cable/v2_handshake_unittest.cc

@@ -17,8 +17,8 @@ class CableV2HandshakeTest : public ::testing::Test {
  public:
   CableV2HandshakeTest() {
     std::fill(psk_gen_key_.begin(), psk_gen_key_.end(), 0);
-    std::fill(nonce_.begin(), nonce_.end(), 1);
-    std::fill(eid_.begin(), eid_.end(), 2);
+    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);
 
     p256_key_.reset(EC_KEY_new_by_curve_name(NID_X9_62_prime256v1));
     const EC_GROUP* group = EC_KEY_get0_group(p256_key_.get());
@@ -32,8 +32,7 @@ class CableV2HandshakeTest : public ::testing::Test {
 
  protected:
   std::array<uint8_t, 32> psk_gen_key_;
-  std::array<uint8_t, 8> nonce_;
-  std::array<uint8_t, kCableEphemeralIdSize> eid_;
+  NonceAndEID nonce_and_eid_;
   bssl::UniquePtr<EC_KEY> p256_key_;
   std::array<uint8_t, kP256PointSize> p256_public_key_;
 };
@@ -72,12 +71,13 @@ TEST_F(CableV2HandshakeTest, OneTimeQRHandshake) {
 
   for (const bool use_correct_key : {false, true}) {
     HandshakeInitiator initiator(
-        use_correct_key ? psk_gen_key_ : wrong_psk_gen_key, nonce_, eid_,
+        use_correct_key ? psk_gen_key_ : wrong_psk_gen_key,
+        nonce_and_eid_.first, nonce_and_eid_.second,
         /*peer_identity=*/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_, eid_, /*identity=*/nullptr,
+        RespondToHandshake(psk_gen_key_, nonce_and_eid_, /*identity=*/nullptr,
                            /*pairing_data=*/nullptr, message, &response));
     ASSERT_EQ(response_crypter.has_value(), use_correct_key);
     if (!use_correct_key) {
@@ -103,12 +103,13 @@ TEST_F(CableV2HandshakeTest, PairingQRHandshake) {
   pairing.v2->peer_identity = p256_public_key_;
   pairing.v2->peer_name = "Unittest";
 
-  HandshakeInitiator initiator(psk_gen_key_, nonce_, eid_,
+  HandshakeInitiator initiator(psk_gen_key_, nonce_and_eid_.first,
+                               nonce_and_eid_.second,
                                /*peer_identity=*/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_, eid_, /*identity=*/nullptr,
+      RespondToHandshake(psk_gen_key_, nonce_and_eid_, /*identity=*/nullptr,
                          &pairing, message, &response));
   ASSERT_TRUE(response_crypter.has_value());
   base::Optional<std::pair<std::unique_ptr<Crypter>,
@@ -136,11 +137,12 @@ TEST_F(CableV2HandshakeTest, PairedHandshake) {
   for (const bool use_correct_key : {false, true}) {
     SCOPED_TRACE(use_correct_key);
 
-    HandshakeInitiator initiator(psk_gen_key_, nonce_, eid_, p256_public_key_);
+    HandshakeInitiator initiator(psk_gen_key_, nonce_and_eid_.first,
+                                 nonce_and_eid_.second, p256_public_key_);
     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_, eid_,
+        RespondToHandshake(psk_gen_key_, nonce_and_eid_,
                            use_correct_key ? p256_key_.get() : wrong_key.get(),
                            /*pairing=*/nullptr, message, &response));
     ASSERT_EQ(response_crypter.has_value(), use_correct_key);