cablev2: hash in identity keys as Noise specifies.

Noise says[1] that identity keys should be hashed after the prologue,
but I missed this during the switch to KN handshakes. Thanks to Dirk
Balfanz for picking up on that.

Mobile counterparts will need to be updated to match this.

BUG=1002262

[1] http://www.noiseprotocol.org/noise.html#:~:text=once%20for%20each%20public%20key%20listed%20in%20the%20pre-messages%20from

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

device/fido/cable/noise.cc

@@ -9,7 +9,9 @@
 #include "crypto/aead.h"
 #include "crypto/sha2.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/obj.h"
 #include "third_party/boringssl/src/include/openssl/sha.h"
 
 namespace {
@@ -121,6 +123,17 @@ base::Optional<std::vector<uint8_t>> Noise::DecryptAndHash(
   return plaintext;
 }
 
+void Noise::MixHashPoint(const EC_POINT* point) {
+  constexpr size_t kP256PointSize = 65;
+  uint8_t x962[kP256PointSize];
+  bssl::UniquePtr<EC_GROUP> p256(
+      EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1));
+  CHECK_EQ(sizeof(x962),
+           EC_POINT_point2oct(p256.get(), point, POINT_CONVERSION_UNCOMPRESSED,
+                              x962, sizeof(x962), /*ctx=*/nullptr));
+  MixHash(x962);
+}
+
 std::tuple<std::array<uint8_t, 32>, std::array<uint8_t, 32>>
 Noise::traffic_keys() const {
   return HKDF2(chaining_key_, base::span<const uint8_t>());

device/fido/cable/noise.h

@@ -12,6 +12,7 @@
 
 #include "base/component_export.h"
 #include "base/containers/span.h"
+#include "third_party/boringssl/src/include/openssl/base.h"
 
 namespace device {
 
@@ -41,6 +42,10 @@ class COMPONENT_EXPORT(DEVICE_FIDO) Noise {
   base::Optional<std::vector<uint8_t>> DecryptAndHash(
       base::span<const uint8_t> ciphertext);
 
+  // MaxHashPoint calls |MixHash| with the uncompressed, X9.62 serialization of
+  // |point|.
+  void MixHashPoint(const EC_POINT* point);
+
   // traffic_keys() calls Split from the protocol spec but, rather than
   // returning CipherState objects, returns the raw keys.
   std::tuple<std::array<uint8_t, 32>, std::array<uint8_t, 32>> traffic_keys()

device/fido/cable/v2_handshake.cc

@@ -182,7 +182,10 @@ HandshakeInitiator::HandshakeInitiator(
     peer_identity_ = fido_parsing_utils::Materialize(*peer_identity);
   }
   if (local_seed) {
-    local_seed_ = fido_parsing_utils::Materialize(*local_seed);
+    bssl::UniquePtr<EC_GROUP> p256(
+        EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1));
+    local_identity_.reset(EC_KEY_derive_from_secret(
+        p256.get(), local_seed->data(), local_seed->size()));
   }
 }
 
@@ -192,9 +195,11 @@ std::vector<uint8_t> HandshakeInitiator::BuildInitialMessage() {
   if (peer_identity_) {
     noise_.Init(Noise::HandshakeType::kNKpsk0);
     noise_.MixHash(kPairedPrologue);
+    noise_.MixHash(*peer_identity_);
   } else {
     noise_.Init(Noise::HandshakeType::kKNpsk0);
     noise_.MixHash(kQRPrologue);
+    noise_.MixHashPoint(EC_KEY_get0_public_key(local_identity_.get()));
   }
 
   noise_.MixKeyAndHash(psk_);
@@ -267,12 +272,10 @@ HandshakeInitiator::ProcessResponse(base::span<const uint8_t> response) {
   noise_.MixKey(peer_point_bytes);
   noise_.MixKey(shared_key_ee);
 
-  if (local_seed_) {
+  if (local_identity_) {
     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(),
+                         local_identity_.get(),
                          /*kdf=*/nullptr) != sizeof(shared_key_se)) {
       return base::nullopt;
     }
@@ -368,9 +371,11 @@ base::Optional<std::unique_ptr<Crypter>> RespondToHandshake(
   if (identity) {
     noise.Init(device::Noise::HandshakeType::kNKpsk0);
     noise.MixHash(kPairedPrologue);
+    noise.MixHashPoint(EC_KEY_get0_public_key(identity));
   } else {
     noise.Init(device::Noise::HandshakeType::kKNpsk0);
     noise.MixHash(kQRPrologue);
+    noise.MixHashPoint(peer_identity);
   }
 
   std::array<uint8_t, 32> psk;

device/fido/cable/v2_handshake.h

@@ -104,7 +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> local_identity_;
   bssl::UniquePtr<EC_KEY> ephemeral_key_;
 };