Roll src/third_party/boringssl/src d7421ebf6..3ac32b1ed

https://boringssl.googlesource.com/boringssl/+log/d7421ebf6cae07051caf657016f160585b64f8a6..3ac32b1eda0da7a99d9c2b6c605fe50af80ccd90

In doing so, switch crypto/curve25519.h to use the new BoringSSL curve25510
code to avoid shipping two copies. This includes a small subgroup check, so
callers need to be tweaked slightly.

BUG=none

Review URL: https://codereview.chromium.org/1459783002

Cr-Commit-Position: refs/heads/master@{#360507}

DEPS

@@ -67,7 +67,7 @@ vars = {
   # Three lines of non-changing comments so that
   # the commit queue can handle CLs rolling BoringSSL
   # and whatever else without interference from each other.
-  'boringssl_revision': 'd7421ebf6cae07051caf657016f160585b64f8a6',
+  'boringssl_revision': '3ac32b1eda0da7a99d9c2b6c605fe50af80ccd90',
   # Three lines of non-changing comments so that
   # the commit queue can handle CLs rolling nss
   # and whatever else without interference from each other.

components/certificate_reporting/error_reporter.cc

@@ -34,11 +34,13 @@ static const uint32 kServerPublicKeyVersion = 1;
 
 static const char kHkdfLabel[] = "certificate report";
 
-std::string GetHkdfSubkeySecret(size_t subkey_length,
-                                const uint8* private_key,
-                                const uint8* public_key) {
+bool GetHkdfSubkeySecret(size_t subkey_length,
+                         const uint8* private_key,
+                         const uint8* public_key,
+                         std::string* secret) {
   uint8 shared_secret[crypto::curve25519::kBytes];
-  crypto::curve25519::ScalarMult(private_key, public_key, shared_secret);
+  if (!crypto::curve25519::ScalarMult(private_key, public_key, shared_secret))
+    return false;
 
   // By mistake, the HKDF label here ends up with an extra null byte on
   // the end, due to using sizeof(kHkdfLabel) in the StringPiece
@@ -55,7 +57,8 @@ std::string GetHkdfSubkeySecret(size_t subkey_length,
                     base::StringPiece(kHkdfLabel, sizeof(kHkdfLabel)),
                     0 /* key bytes */, 0 /* iv bytes */, subkey_length);
 
-  return hkdf.subkey_secret().as_string();
+  *secret = hkdf.subkey_secret().as_string();
+  return true;
 }
 
 bool EncryptSerializedReport(const uint8* server_public_key,
@@ -70,9 +73,13 @@ bool EncryptSerializedReport(const uint8* server_public_key,
   crypto::curve25519::ScalarBaseMult(private_key, public_key);
 
   crypto::Aead aead(crypto::Aead::AES_128_CTR_HMAC_SHA256);
-  const std::string key =
-      GetHkdfSubkeySecret(aead.KeyLength(), private_key,
-                          reinterpret_cast<const uint8*>(server_public_key));
+  std::string key;
+  if (!GetHkdfSubkeySecret(aead.KeyLength(), private_key,
+                           reinterpret_cast<const uint8*>(server_public_key),
+                           &key)) {
+    LOG(ERROR) << "Error getting subkey secret.";
+    return false;
+  }
   aead.Init(&key);
 
   // Use an all-zero nonce because the key is random per-message.
@@ -157,10 +164,14 @@ bool ErrorReporter::DecryptErrorReport(
     const EncryptedCertLoggerRequest& encrypted_report,
     std::string* decrypted_serialized_report) {
   crypto::Aead aead(crypto::Aead::AES_128_CTR_HMAC_SHA256);
-  const std::string key =
-      GetHkdfSubkeySecret(aead.KeyLength(), server_private_key,
-                          reinterpret_cast<const uint8*>(
-                              encrypted_report.client_public_key().data()));
+  std::string key;
+  if (!GetHkdfSubkeySecret(aead.KeyLength(), server_private_key,
+                           reinterpret_cast<const uint8*>(
+                               encrypted_report.client_public_key().data()),
+                           &key)) {
+    LOG(ERROR) << "Error getting subkey secret.";
+    return false;
+  }
   aead.Init(&key);
 
   // Use an all-zero nonce because the key is random per-message.

crypto/BUILD.gn

@@ -19,8 +19,9 @@ component("crypto") {
     "cssm_init.cc",
     "cssm_init.h",
     "curve25519-donna.c",
-    "curve25519.cc",
     "curve25519.h",
+    "curve25519_nss.cc",
+    "curve25519_openssl.cc",
     "ec_private_key.h",
     "ec_private_key_nss.cc",
     "ec_private_key_openssl.cc",
@@ -136,6 +137,8 @@ component("crypto") {
   if (use_openssl) {
     # Remove NSS files when using OpenSSL
     sources -= [
+      "curve25519-donna.c",
+      "curve25519_nss.cc",
       "ec_private_key_nss.cc",
       "ec_signature_creator_nss.cc",
       "encryptor_nss.cc",
@@ -157,6 +160,7 @@ component("crypto") {
     sources -= [
       "aead_openssl.cc",
       "aead_openssl.h",
+      "curve25519_openssl.cc",
       "ec_private_key_openssl.cc",
       "ec_signature_creator_openssl.cc",
       "encryptor_openssl.cc",

crypto/crypto.gyp

@@ -103,6 +103,8 @@
             # TODO(joth): Use a glob to match exclude patterns once the
             #             OpenSSL file set is complete.
             'sources!': [
+              'curve25519-donna.c',
+              'curve25519_nss.cc',
               'ec_private_key_nss.cc',
               'ec_signature_creator_nss.cc',
               'encryptor_nss.cc',
@@ -126,6 +128,7 @@
             'sources!': [
               'aead_openssl.cc',
               'aead_openssl.h',
+              'curve25519_openssl.cc',
               'ec_private_key_openssl.cc',
               'ec_signature_creator_openssl.cc',
               'encryptor_openssl.cc',

crypto/crypto.gypi

@@ -37,9 +37,10 @@
       'crypto_export.h',
       'cssm_init.cc',
       'cssm_init.h',
-      'curve25519.cc',
-      'curve25519.h',
       'curve25519-donna.c',
+      'curve25519.h',
+      'curve25519_nss.cc',
+      'curve25519_openssl.cc',
       'ghash.cc',
       'ghash.h',
       'ec_private_key.h',

crypto/curve25519.h

@@ -5,7 +5,9 @@
 #ifndef CRYPTO_CURVE25519_H
 #define CRYPTO_CURVE25519_H
 
-#include "base/basictypes.h"
+#include <stddef.h>
+#include <stdint.h>
+
 #include "crypto/crypto_export.h"
 
 namespace crypto {
@@ -14,6 +16,10 @@ namespace crypto {
 // described in "Curve 25519: new Diffie-Hellman Speed Records",
 // by D.J. Bernstein. Additional information is available at
 // http://cr.yp.to/ecdh.html.
+//
+// TODO(davidben): Once iOS is switched to BoringSSL (https://crbug.com/338886),
+// remove this file altogether and switch callers to using BoringSSL's
+// curve25519.h directly.
 namespace curve25519 {
 
 // kBytes is the number of bytes in the result of the Diffie-Hellman operation,
@@ -28,18 +34,20 @@ static const size_t kScalarBytes = 32;
 // |peer_public_key|. This method is a wrapper for |curve25519_donna()|. It
 // calls that function with |private_key| as |secret| and |peer_public_key| as
 // basepoint. |private_key| should be of length |kScalarBytes| and
-// |peer_public_key| should be of length |kBytes|.
-// See "Computing shared secrets" section of/ http://cr.yp.to/ecdh.html.
-CRYPTO_EXPORT void ScalarMult(const uint8* private_key,
-                              const uint8* peer_public_key,
-                              uint8* shared_key);
+// |peer_public_key| should be of length |kBytes|. It returns true on success
+// and false if |peer_public_key| was invalid.
+// See the "Computing shared secrets" section of http://cr.yp.to/ecdh.html.
+CRYPTO_EXPORT bool ScalarMult(const uint8_t* private_key,
+                              const uint8_t* peer_public_key,
+                              uint8_t* shared_key);
 
 // ScalarBaseMult computes the |public_key| from |private_key|. This method is a
 // wrapper for |curve25519_donna()|. It calls that function with |private_key|
 // as |secret| and |kBasePoint| as basepoint. |private_key| should be of length
 // |kScalarBytes|. See "Computing public keys" section of
 // http://cr.yp.to/ecdh.html.
-CRYPTO_EXPORT void ScalarBaseMult(const uint8* private_key, uint8* public_key);
+CRYPTO_EXPORT void ScalarBaseMult(const uint8_t* private_key,
+                                  uint8_t* public_key);
 
 }  // namespace curve25519
 

crypto/curve25519.cc → crypto/curve25519_nss.cc

@@ -4,30 +4,36 @@
 
 #include "crypto/curve25519.h"
 
+#include "crypto/secure_util.h"
+
 // Curve25519 is specified in terms of byte strings, not numbers, so all
 // implementations take and return the same sequence of bits. So the byte
 // order is implicitly specified as in, say, SHA1.
 //
 // Prototype for |curve25519_donna| function in
 // third_party/curve25519-donna/curve25519-donna.c
-extern "C" int curve25519_donna(uint8*, const uint8*, const uint8*);
+extern "C" int curve25519_donna(uint8_t*, const uint8_t*, const uint8_t*);
 
 namespace crypto {
 
 namespace curve25519 {
 
-void ScalarMult(const uint8* private_key,
-                const uint8* peer_public_key,
-                uint8* shared_key) {
+bool ScalarMult(const uint8_t* private_key,
+                const uint8_t* peer_public_key,
+                uint8_t* shared_key) {
   curve25519_donna(shared_key, private_key, peer_public_key);
+
+  // The all-zero output results when the input is a point of small order.
+  static const uint8_t kZeros[32] = {0};
+  return !SecureMemEqual(shared_key, kZeros, 32);
 }
 
 // kBasePoint is the base point (generator) of the elliptic curve group.
 // It is little-endian version of '9' followed by 31 zeros.
 // See "Computing public keys" section of http://cr.yp.to/ecdh.html.
-static const unsigned char kBasePoint[32] = {9};
+static const uint8_t kBasePoint[32] = {9};
 
-void ScalarBaseMult(const uint8* private_key, uint8* public_key) {
+void ScalarBaseMult(const uint8_t* private_key, uint8_t* public_key) {
   curve25519_donna(public_key, private_key, kBasePoint);
 }
 

crypto/curve25519_openssl.cc

@@ -0,0 +1,25 @@
+// Copyright 2015 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.
+
+#include "crypto/curve25519.h"
+
+#include <openssl/curve25519.h>
+
+namespace crypto {
+
+namespace curve25519 {
+
+bool ScalarMult(const uint8_t* private_key,
+                const uint8_t* peer_public_key,
+                uint8_t* shared_key) {
+  return !!X25519(shared_key, private_key, peer_public_key);
+}
+
+void ScalarBaseMult(const uint8_t* private_key, uint8_t* public_key) {
+  X25519_public_from_private(public_key, private_key);
+}
+
+}  // namespace curve25519
+
+}  // namespace crypto

crypto/curve25519_unittest.cc

@@ -17,28 +17,41 @@ namespace crypto {
 // public key and shared key for alice and bob. It asserts that alice and bob
 // have the same shared key.
 TEST(Curve25519, SharedKeyIdentity) {
-  uint8 alice_private_key[curve25519::kScalarBytes] = {3};
-  uint8 bob_private_key[curve25519::kScalarBytes] = {5};
+  uint8_t alice_private_key[curve25519::kScalarBytes] = {3};
+  uint8_t bob_private_key[curve25519::kScalarBytes] = {5};
 
   // Get public key for alice and bob.
-  uint8 alice_public_key[curve25519::kBytes];
+  uint8_t alice_public_key[curve25519::kBytes];
   curve25519::ScalarBaseMult(alice_private_key, alice_public_key);
 
-  uint8 bob_public_key[curve25519::kBytes];
+  uint8_t bob_public_key[curve25519::kBytes];
   curve25519::ScalarBaseMult(bob_private_key, bob_public_key);
 
   // Get the shared key for alice, by using alice's private key and bob's
   // public key.
-  uint8 alice_shared_key[curve25519::kBytes];
+  uint8_t alice_shared_key[curve25519::kBytes];
   curve25519::ScalarMult(alice_private_key, bob_public_key, alice_shared_key);
 
   // Get the shared key for bob, by using bob's private key and alice's public
   // key.
-  uint8 bob_shared_key[curve25519::kBytes];
+  uint8_t bob_shared_key[curve25519::kBytes];
   curve25519::ScalarMult(bob_private_key, alice_public_key, bob_shared_key);
 
   // Computed shared key of alice and bob should be the same.
   ASSERT_EQ(0, memcmp(alice_shared_key, bob_shared_key, curve25519::kBytes));
 }
 
+TEST(Curve25519, SmallOrder) {
+  static const uint8_t kSmallOrderPoint[32] = {
+      0xe0, 0xeb, 0x7a, 0x7c, 0x3b, 0x41, 0xb8, 0xae, 0x16, 0x56, 0xe3,
+      0xfa, 0xf1, 0x9f, 0xc4, 0x6a, 0xda, 0x09, 0x8d, 0xeb, 0x9c, 0x32,
+      0xb1, 0xfd, 0x86, 0x62, 0x05, 0x16, 0x5f, 0x49, 0xb8,
+  };
+
+  uint8_t out[32], private_key[32];
+  memset(private_key, 0x11, sizeof(private_key));
+
+  EXPECT_FALSE(curve25519::ScalarMult(private_key, kSmallOrderPoint, out));
+}
+
 }  // namespace crypto

net/quic/crypto/curve25519_key_exchange.cc

@@ -67,10 +67,11 @@ bool Curve25519KeyExchange::CalculateSharedKey(
   }
 
   uint8 result[crypto::curve25519::kBytes];
-  crypto::curve25519::ScalarMult(
-      private_key_,
-      reinterpret_cast<const uint8*>(peer_public_value.data()),
-      result);
+  if (!crypto::curve25519::ScalarMult(
+          private_key_,
+          reinterpret_cast<const uint8*>(peer_public_value.data()), result)) {
+    return false;
+  }
   out_result->assign(reinterpret_cast<char*>(result), sizeof(result));
 
   return true;

third_party/boringssl/BUILD.gn

@@ -6,10 +6,10 @@ import("//build/config/sanitizers/sanitizers.gni")
 
 # Config for us and everybody else depending on BoringSSL.
 config("openssl_config") {
-  include_dirs = []
-  include_dirs += [ "src/include" ]
+  defines = [ "OPENSSL_SMALL" ]
+  include_dirs = [ "src/include" ]
   if (is_component_build) {
-    defines = [ "BORINGSSL_SHARED_LIBRARY" ]
+    defines += [ "BORINGSSL_SHARED_LIBRARY" ]
   }
 }
 

third_party/boringssl/boringssl.gyp

@@ -17,7 +17,6 @@
       'defines': [
         'BORINGSSL_IMPLEMENTATION',
         'BORINGSSL_NO_STATIC_INITIALIZER',
-        'OPENSSL_SMALL',
       ],
       # TODO(davidben): Fix size_t truncations in BoringSSL.
       # https://crbug.com/429039
@@ -102,6 +101,9 @@
         'src/include',
       ],
       'direct_dependent_settings': {
+        'defines': [
+          'OPENSSL_SMALL',
+        ],
         'include_dirs': [
           'src/include',
         ],

third_party/boringssl/boringssl.gypi

@@ -133,6 +133,7 @@
       'src/crypto/cpu-arm.c',
       'src/crypto/cpu-intel.c',
       'src/crypto/crypto.c',
+      'src/crypto/curve25519/curve25519.c',
       'src/crypto/des/des.c',
       'src/crypto/dh/check.c',
       'src/crypto/dh/dh.c',