Drop custom P-224 implementation.

This dates back to when we still used NSS(!) and thus didn't have access
to group operations for any elliptic curves. We can just use BoringSSL
now.

Change-Id: I8bc4bc09fdd26feb25a76c17bf716808eb1cb2a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2623123
Commit-Queue: Adam Langley <agl@chromium.org>
Auto-Submit: Adam Langley <agl@chromium.org>
Reviewed-by: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842843}

crypto/BUILD.gn

@@ -26,8 +26,6 @@ component("crypto") {
     "hmac.h",
     "openssl_util.cc",
     "openssl_util.h",
-    "p224.cc",
-    "p224.h",
     "p224_spake.cc",
     "p224_spake.h",
     "random.cc",
@@ -145,7 +143,6 @@ test("crypto_unittests") {
     "encryptor_unittest.cc",
     "hmac_unittest.cc",
     "p224_spake_unittest.cc",
-    "p224_unittest.cc",
     "random_unittest.cc",
     "rsa_private_key_unittest.cc",
     "secure_hash_unittest.cc",

crypto/p224.h

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CRYPTO_P224_H_
-#define CRYPTO_P224_H_
-
-#include <stddef.h>
-#include <stdint.h>
-
-#include <string>
-
-#include "base/strings/string_piece.h"
-#include "crypto/crypto_export.h"
-
-namespace crypto {
-
-// P224 implements an elliptic curve group, commonly known as P224 and defined
-// in FIPS 186-3, section D.2.2.
-namespace p224 {
-
-// An element of the field (ℤ/pℤ) is represented with 8, 28-bit limbs in
-// little endian order.
-typedef uint32_t FieldElement[8];
-
-struct CRYPTO_EXPORT Point {
-  // SetFromString the value of the point from the 56 byte, external
-  // representation. The external point representation is an (x, y) pair of a
-  // point on the curve. Each field element is represented as a big-endian
-  // number < p.
-  bool SetFromString(base::StringPiece in);
-
-  // ToString returns an external representation of the Point.
-  std::string ToString() const;
-
-  // An Point is represented in Jacobian form (x/z², y/z³).
-  FieldElement x, y, z;
-};
-
-// kScalarBytes is the number of bytes needed to represent an element of the
-// P224 field.
-static const size_t kScalarBytes = 28;
-
-// ScalarMult computes *out = in*scalar where scalar is a 28-byte, big-endian
-// number.
-void CRYPTO_EXPORT ScalarMult(const Point& in,
-                              const uint8_t* scalar,
-                              Point* out);
-
-// ScalarBaseMult computes *out = g*scalar where g is the base point of the
-// curve and scalar is a 28-byte, big-endian number.
-void CRYPTO_EXPORT ScalarBaseMult(const uint8_t* scalar, Point* out);
-
-// Add computes *out = a+b.
-void CRYPTO_EXPORT Add(const Point& a, const Point& b, Point* out);
-
-// Negate calculates out = -a;
-void CRYPTO_EXPORT Negate(const Point& a, Point* out);
-
-}  // namespace p224
-
-}  // namespace crypto
-
-#endif  // CRYPTO_P224_H_

crypto/p224_spake.cc

@@ -12,9 +12,11 @@
 #include <algorithm>
 
 #include "base/logging.h"
-#include "crypto/p224.h"
 #include "crypto/random.h"
 #include "crypto/secure_util.h"
+#include "third_party/boringssl/src/include/openssl/bn.h"
+#include "third_party/boringssl/src/include/openssl/ec.h"
+#include "third_party/boringssl/src/include/openssl/obj.h"
 
 namespace {
 
@@ -82,22 +84,87 @@ namespace {
 //   return 0;
 // }
 
-const crypto::p224::Point kM = {
-  {174237515, 77186811, 235213682, 33849492,
-   33188520, 48266885, 177021753, 81038478},
-  {104523827, 245682244, 266509668, 236196369,
-   28372046, 145351378, 198520366, 113345994},
-  {1, 0, 0, 0, 0, 0, 0, 0},
+const uint8_t kM_X962[1 + 28 + 28] = {
+    0x04, 0x4d, 0x48, 0xc8, 0xea, 0x8d, 0x23, 0x39, 0x2e, 0x07, 0xe8, 0x51,
+    0xfa, 0x6a, 0xa8, 0x20, 0x48, 0x09, 0x4e, 0x05, 0x13, 0x72, 0x49, 0x9c,
+    0x6f, 0xba, 0x62, 0xa7, 0x4b, 0x6c, 0x18, 0x5c, 0xab, 0xd5, 0x2e, 0x2e,
+    0x8a, 0x9e, 0x2d, 0x21, 0xb0, 0xec, 0x4e, 0xe1, 0x41, 0x21, 0x1f, 0xe2,
+    0x9d, 0x64, 0xea, 0x4d, 0x04, 0x46, 0x3a, 0xe8, 0x33,
 };
 
-const crypto::p224::Point kN = {
-  {136176322, 263523628, 251628795, 229292285,
-   5034302, 185981975, 171998428, 11653062},
-  {197567436, 51226044, 60372156, 175772188,
-   42075930, 8083165, 160827401, 65097570},
-  {1, 0, 0, 0, 0, 0, 0, 0},
+const uint8_t kN_X962[1 + 28 + 28] = {
+    0x04, 0x0b, 0x1c, 0xfc, 0x6a, 0x40, 0x7c, 0xdc, 0xb1, 0x5d, 0xc1, 0x70,
+    0x4c, 0xd1, 0x3e, 0xda, 0xab, 0x8f, 0xde, 0xff, 0x8c, 0xfb, 0xfb, 0x50,
+    0xd2, 0xc8, 0x1d, 0xe2, 0xc2, 0x3e, 0x14, 0xf6, 0x29, 0x96, 0x08, 0x09,
+    0x07, 0xb5, 0x6d, 0xd2, 0x82, 0x07, 0x1a, 0xa7, 0xa1, 0x21, 0xc3, 0x99,
+    0x34, 0xbc, 0x30, 0xda, 0x5b, 0xcb, 0xc6, 0xa3, 0xcc,
 };
 
+// ToBignum returns |big_endian_bytes| interpreted as a big-endian number.
+bssl::UniquePtr<BIGNUM> ToBignum(base::span<const uint8_t> big_endian_bytes) {
+  bssl::UniquePtr<BIGNUM> bn(BN_new());
+  CHECK(BN_bin2bn(big_endian_bytes.data(), big_endian_bytes.size(), bn.get()));
+  return bn;
+}
+
+// GetPoint decodes and returns the given X.962-encoded point. It will crash if
+// |x962| is not a valid P-224 point.
+bssl::UniquePtr<EC_POINT> GetPoint(
+    const EC_GROUP* p224,
+    base::span<const uint8_t, 1 + 28 + 28> x962) {
+  bssl::UniquePtr<EC_POINT> point(EC_POINT_new(p224));
+  CHECK(EC_POINT_oct2point(p224, point.get(), x962.data(), x962.size(),
+                           /*ctx=*/nullptr));
+  return point;
+}
+
+// GetMask returns (M|N)**pw, where the choice of M or N is controlled by
+// |use_m|.
+bssl::UniquePtr<EC_POINT> GetMask(const EC_GROUP* p224,
+                                  bool use_m,
+                                  base::span<const uint8_t> pw) {
+  bssl::UniquePtr<EC_POINT> MN(GetPoint(p224, use_m ? kM_X962 : kN_X962));
+  bssl::UniquePtr<EC_POINT> MNpw(EC_POINT_new(p224));
+  bssl::UniquePtr<BIGNUM> pw_bn(ToBignum(pw));
+  CHECK(EC_POINT_mul(p224, MNpw.get(), nullptr, MN.get(), pw_bn.get(),
+                     /*ctx=*/nullptr));
+  return MNpw;
+}
+
+// ToMessage serialises |in| as a 56-byte string that contains the big-endian
+// representations of x and y, or is all zeros if |in| is infinity.
+std::string ToMessage(const EC_GROUP* p224, const EC_POINT* in) {
+  if (EC_POINT_is_at_infinity(p224, in)) {
+    return std::string(28 + 28, 0);
+  }
+
+  uint8_t x962[1 + 28 + 28];
+  CHECK(EC_POINT_point2oct(p224, in, POINT_CONVERSION_UNCOMPRESSED, x962,
+                           sizeof(x962), /*ctx=*/nullptr) == sizeof(x962));
+  return std::string(reinterpret_cast<const char*>(&x962[1]), sizeof(x962) - 1);
+}
+
+// FromMessage converts a message, as generated by |ToMessage|, into a point. It
+// returns |nullptr| if the input is invalid or not on the curve.
+bssl::UniquePtr<EC_POINT> FromMessage(const EC_GROUP* p224,
+                                      base::StringPiece in) {
+  if (in.size() != 56) {
+    return nullptr;
+  }
+
+  uint8_t x962[1 + 56];
+  x962[0] = 4;
+  memcpy(&x962[1], in.data(), sizeof(x962) - 1);
+
+  bssl::UniquePtr<EC_POINT> ret(EC_POINT_new(p224));
+  if (!EC_POINT_oct2point(p224, ret.get(), x962, sizeof(x962),
+                          /*ctx=*/nullptr)) {
+    return nullptr;
+  }
+
+  return ret;
+}
+
 }  // anonymous namespace
 
 namespace crypto {
@@ -120,19 +187,25 @@ P224EncryptedKeyExchange::P224EncryptedKeyExchange(PeerType peer_type,
 
 void P224EncryptedKeyExchange::Init() {
   // X = g**x_
-  p224::Point X;
-  p224::ScalarBaseMult(x_, &X);
+  bssl::UniquePtr<EC_GROUP> p224(EC_GROUP_new_by_curve_name(NID_secp224r1));
+  bssl::UniquePtr<EC_POINT> X(EC_POINT_new(p224.get()));
+  bssl::UniquePtr<BIGNUM> x_bn(ToBignum(x_));
+  // x_bn may be >= the order, but |EC_POINT_mul| handles that. It doesn't do so
+  // in constant-time, but the these values are locally generated and so this
+  // occurs with negligible probability. (Same with |pw_|, just below.)
+  CHECK(EC_POINT_mul(p224.get(), X.get(), x_bn.get(), nullptr, nullptr,
+                     /*ctx=*/nullptr));
 
   // The client masks the Diffie-Hellman value, X, by adding M**pw and the
   // server uses N**pw.
-  p224::Point MNpw;
-  p224::ScalarMult(is_server_ ? kN : kM, pw_, &MNpw);
+  bssl::UniquePtr<EC_POINT> MNpw(GetMask(p224.get(), !is_server_, pw_));
 
   // X* = X + (N|M)**pw
-  p224::Point Xstar;
-  p224::Add(X, MNpw, &Xstar);
+  bssl::UniquePtr<EC_POINT> Xstar(EC_POINT_new(p224.get()));
+  CHECK(EC_POINT_add(p224.get(), Xstar.get(), X.get(), MNpw.get(),
+                     /*ctx=*/nullptr));
 
-  next_message_ = Xstar.ToString();
+  next_message_ = ToMessage(p224.get(), Xstar.get());
 }
 
 const std::string& P224EncryptedKeyExchange::GetNextMessage() {
@@ -175,26 +248,31 @@ P224EncryptedKeyExchange::Result P224EncryptedKeyExchange::ProcessMessage(
     return kResultFailed;
   }
 
+  bssl::UniquePtr<EC_GROUP> p224(EC_GROUP_new_by_curve_name(NID_secp224r1));
+
   // Y* is the other party's masked, Diffie-Hellman value.
-  p224::Point Ystar;
-  if (!Ystar.SetFromString(message)) {
+  bssl::UniquePtr<EC_POINT> Ystar(FromMessage(p224.get(), message));
+  if (!Ystar) {
     error_ = "failed to parse peer's masked Diffie-Hellman value";
     return kResultFailed;
   }
 
   // We calculate the mask value: (N|M)**pw
-  p224::Point MNpw, minus_MNpw, Y, k;
-  p224::ScalarMult(is_server_ ? kM : kN, pw_, &MNpw);
-  p224::Negate(MNpw, &minus_MNpw);
-
+  bssl::UniquePtr<EC_POINT> MNpw(GetMask(p224.get(), is_server_, pw_));
   // Y = Y* - (N|M)**pw
-  p224::Add(Ystar, minus_MNpw, &Y);
+  CHECK(EC_POINT_invert(p224.get(), MNpw.get(), /*ctx=*/nullptr));
+  bssl::UniquePtr<EC_POINT> Y(EC_POINT_new(p224.get()));
+  CHECK(EC_POINT_add(p224.get(), Y.get(), Ystar.get(), MNpw.get(),
+                     /*ctx=*/nullptr));
 
   // K = Y**x_
-  p224::ScalarMult(Y, x_, &k);
+  bssl::UniquePtr<EC_POINT> K(EC_POINT_new(p224.get()));
+  bssl::UniquePtr<BIGNUM> x_bn(ToBignum(x_));
+  CHECK(EC_POINT_mul(p224.get(), K.get(), nullptr, Y.get(), x_bn.get(),
+                     /*ctx=*/nullptr));
 
   // If everything worked out, then K is the same for both parties.
-  key_ = k.ToString();
+  key_ = ToMessage(p224.get(), K.get());
 
   std::string client_masked_dh, server_masked_dh;
   if (is_server_) {

crypto/p224_spake.h

@@ -11,7 +11,6 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/strings/string_piece.h"
-#include "crypto/p224.h"
 #include "crypto/sha2.h"
 
 namespace crypto {
@@ -110,12 +109,14 @@ class CRYPTO_EXPORT P224EncryptedKeyExchange {
                      const std::string& k,
                      uint8_t* out_digest);
 
+  // kScalarBytes is the number of bytes in a P-224 scalar.
+  static constexpr size_t kScalarBytes = 28;
   // x_ is the secret Diffie-Hellman exponent (see paper referenced in .cc
   // file).
-  uint8_t x_[p224::kScalarBytes];
+  uint8_t x_[kScalarBytes];
   // pw_ is SHA256(P(password), P(session))[:28] where P() prepends a uint32_t,
   // big-endian length prefix (see paper referenced in .cc file).
-  uint8_t pw_[p224::kScalarBytes];
+  uint8_t pw_[kScalarBytes];
   // expected_authenticator_ is used to store the hash value expected from the
   // other party.
   uint8_t expected_authenticator_[kSHA256Length];