Add constant-time validation for curve25519

Also add some tests for X25519_public_from_private, as we apparently
weren't directly testing it with test vectors.

Change-Id: I1b73a9655323d507a8e022c62530ddd4610db4b9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60109
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>

crypto/curve25519/curve25519.c

diff --git a/crypto/curve25519/curve25519.c b/crypto/curve25519/curve25519.c
index c0289ff..8cd4496 100644
--- a/crypto/curve25519/curve25519.c
+++ b/crypto/curve25519/curve25519.c
@@ -2116,7 +2116,8 @@
   static const uint8_t kZeros[32] = {0};
   x25519_scalar_mult(out_shared_key, private_key, peer_public_value);
   // The all-zero output results when the input is a point of small order.
-  return CRYPTO_memcmp(kZeros, out_shared_key, 32) != 0;
+  return constant_time_declassify_int(
+             CRYPTO_memcmp(kZeros, out_shared_key, 32)) != 0;
 }
 
 void X25519_public_from_private(uint8_t out_public_value[32],
@@ -2147,4 +2148,5 @@
   fe_loose_invert(&zminusy_inv, &zminusy);
   fe_mul_tlt(&zminusy_inv, &zplusy, &zminusy_inv);
   fe_tobytes(out_public_value, &zminusy_inv);
+  CONSTTIME_DECLASSIFY(out_public_value, 32);
 }

crypto/curve25519/ed25519_test.cc

diff --git a/crypto/curve25519/ed25519_test.cc b/crypto/curve25519/ed25519_test.cc
index d56abe6..0b7c585 100644
--- a/crypto/curve25519/ed25519_test.cc
+++ b/crypto/curve25519/ed25519_test.cc
@@ -35,9 +35,15 @@
     ASSERT_TRUE(t->GetBytes(&expected_signature, "SIG"));
     ASSERT_EQ(64u, expected_signature.size());
 
+    // Signing should not leak the private key or the message.
+    CONSTTIME_SECRET(private_key.data(), private_key.size());
+    CONSTTIME_SECRET(message.data(), message.size());
     uint8_t signature[64];
     ASSERT_TRUE(ED25519_sign(signature, message.data(), message.size(),
                              private_key.data()));
+    CONSTTIME_DECLASSIFY(signature, sizeof(signature));
+    CONSTTIME_DECLASSIFY(message.data(), message.size());
+
     EXPECT_EQ(Bytes(expected_signature), Bytes(signature));
     EXPECT_TRUE(ED25519_verify(message.data(), message.size(), signature,
                                public_key.data()));
@@ -114,9 +120,12 @@
 
   uint8_t seed[32];
   OPENSSL_memcpy(seed, private_key1, sizeof(seed));
+  CONSTTIME_SECRET(seed, sizeof(seed));
 
   uint8_t public_key2[32], private_key2[64];
   ED25519_keypair_from_seed(public_key2, private_key2, seed);
+  CONSTTIME_DECLASSIFY(public_key2, sizeof(public_key2));
+  CONSTTIME_DECLASSIFY(private_key2, sizeof(private_key2));
 
   EXPECT_EQ(Bytes(public_key1), Bytes(public_key2));
   EXPECT_EQ(Bytes(private_key1), Bytes(private_key2));

crypto/curve25519/x25519_test.cc

diff --git a/crypto/curve25519/x25519_test.cc b/crypto/curve25519/x25519_test.cc
index 1bf398f..f9e321d 100644
--- a/crypto/curve25519/x25519_test.cc
+++ b/crypto/curve25519/x25519_test.cc
@@ -27,7 +27,7 @@
 
 
 TEST(X25519Test, TestVector) {
-  // Taken from https://tools.ietf.org/html/rfc7748#section-5.2
+  // Taken from https://www.rfc-editor.org/rfc/rfc7748#section-5.2
   static const uint8_t kScalar1[32] = {
       0xa5, 0x46, 0xe3, 0x6b, 0xf0, 0x52, 0x7c, 0x9d, 0x3b, 0x16, 0x15,
       0x4b, 0x82, 0x46, 0x5e, 0xdd, 0x62, 0x14, 0x4c, 0x0a, 0xc1, 0xfc,
@@ -39,8 +39,13 @@
       0x35, 0x3b, 0x10, 0xa9, 0x03, 0xa6, 0xd0, 0xab, 0x1c, 0x4c,
   };
 
-  uint8_t out[32];
-  EXPECT_TRUE(X25519(out, kScalar1, kPoint1));
+  // Copy all the secrets into a temporary buffer, so we can run constant-time
+  // validation on them.
+  uint8_t out[32], secret[32];
+  OPENSSL_memcpy(secret, kScalar1, sizeof(secret));
+  CONSTTIME_SECRET(secret, sizeof(secret));
+  EXPECT_TRUE(X25519(out, secret, kPoint1));
+  CONSTTIME_DECLASSIFY(out, sizeof(out));
 
   static const uint8_t kExpected1[32] = {
       0xc3, 0xda, 0x55, 0x37, 0x9d, 0xe9, 0xc6, 0x90, 0x8e, 0x94, 0xea,
@@ -60,7 +65,10 @@
       0x3c, 0x3e, 0xfc, 0x4c, 0xd5, 0x49, 0xc7, 0x15, 0xa4, 0x93,
   };
 
-  EXPECT_TRUE(X25519(out, kScalar2, kPoint2));
+  OPENSSL_memcpy(secret, kScalar2, sizeof(secret));
+  CONSTTIME_SECRET(secret, sizeof(secret));
+  EXPECT_TRUE(X25519(out, secret, kPoint2));
+  CONSTTIME_DECLASSIFY(out, sizeof(out));
 
   static const uint8_t kExpected2[32] = {
       0x95, 0xcb, 0xde, 0x94, 0x76, 0xe8, 0x90, 0x7d, 0x7a, 0xad, 0xe4,
@@ -68,6 +76,57 @@
       0xa1, 0x52, 0xe6, 0xf8, 0xf7, 0x64, 0x7a, 0xac, 0x79, 0x57,
   };
   EXPECT_EQ(Bytes(kExpected2), Bytes(out));
+
+  // Taken from https://www.rfc-editor.org/rfc/rfc7748.html#section-6.1
+  static const uint8_t kPrivateA[32] = {
+      0x77, 0x07, 0x6d, 0x0a, 0x73, 0x18, 0xa5, 0x7d, 0x3c, 0x16, 0xc1,
+      0x72, 0x51, 0xb2, 0x66, 0x45, 0xdf, 0x4c, 0x2f, 0x87, 0xeb, 0xc0,
+      0x99, 0x2a, 0xb1, 0x77, 0xfb, 0xa5, 0x1d, 0xb9, 0x2c, 0x2a,
+  };
+  static const uint8_t kPublicA[32] = {
+      0x85, 0x20, 0xf0, 0x09, 0x89, 0x30, 0xa7, 0x54, 0x74, 0x8b, 0x7d,
+      0xdc, 0xb4, 0x3e, 0xf7, 0x5a, 0x0d, 0xbf, 0x3a, 0x0d, 0x26, 0x38,
+      0x1a, 0xf4, 0xeb, 0xa4, 0xa9, 0x8e, 0xaa, 0x9b, 0x4e, 0x6a,
+  };
+  static const uint8_t kPrivateB[32] = {
+      0x5d, 0xab, 0x08, 0x7e, 0x62, 0x4a, 0x8a, 0x4b, 0x79, 0xe1, 0x7f,
+      0x8b, 0x83, 0x80, 0x0e, 0xe6, 0x6f, 0x3b, 0xb1, 0x29, 0x26, 0x18,
+      0xb6, 0xfd, 0x1c, 0x2f, 0x8b, 0x27, 0xff, 0x88, 0xe0, 0xeb,
+  };
+  static const uint8_t kPublicB[32] = {
+      0xde, 0x9e, 0xdb, 0x7d, 0x7b, 0x7d, 0xc1, 0xb4, 0xd3, 0x5b, 0x61,
+      0xc2, 0xec, 0xe4, 0x35, 0x37, 0x3f, 0x83, 0x43, 0xc8, 0x5b, 0x78,
+      0x67, 0x4d, 0xad, 0xfc, 0x7e, 0x14, 0x6f, 0x88, 0x2b, 0x4f,
+  };
+  static const uint8_t kSecret[32] = {
+      0x4a, 0x5d, 0x9d, 0x5b, 0xa4, 0xce, 0x2d, 0xe1, 0x72, 0x8e, 0x3b,
+      0xf4, 0x80, 0x35, 0x0f, 0x25, 0xe0, 0x7e, 0x21, 0xc9, 0x47, 0xd1,
+      0x9e, 0x33, 0x76, 0xf0, 0x9b, 0x3c, 0x1e, 0x16, 0x17, 0x42,
+  };
+
+  OPENSSL_memcpy(secret, kPrivateA, sizeof(secret));
+  CONSTTIME_SECRET(secret, sizeof(secret));
+  X25519_public_from_private(out, secret);
+  CONSTTIME_DECLASSIFY(out, sizeof(out));
+  EXPECT_EQ(Bytes(out), Bytes(kPublicA));
+
+  OPENSSL_memcpy(secret, kPrivateB, sizeof(secret));
+  CONSTTIME_SECRET(secret, sizeof(secret));
+  X25519_public_from_private(out, secret);
+  CONSTTIME_DECLASSIFY(out, sizeof(out));
+  EXPECT_EQ(Bytes(out), Bytes(kPublicB));
+
+  OPENSSL_memcpy(secret, kPrivateA, sizeof(secret));
+  CONSTTIME_SECRET(secret, sizeof(secret));
+  X25519(out, secret, kPublicB);
+  CONSTTIME_DECLASSIFY(out, sizeof(out));
+  EXPECT_EQ(Bytes(out), Bytes(kSecret));
+
+  OPENSSL_memcpy(secret, kPrivateB, sizeof(secret));
+  CONSTTIME_SECRET(secret, sizeof(secret));
+  X25519(out, secret, kPublicA);
+  CONSTTIME_DECLASSIFY(out, sizeof(out));
+  EXPECT_EQ(Bytes(out), Bytes(kSecret));
 }
 
 TEST(X25519Test, SmallOrder) {
@@ -141,9 +200,15 @@
       ASSERT_TRUE(t->GetBytes(&shared, "shared"));
       ASSERT_EQ(32u, priv.size());
       ASSERT_EQ(32u, pub.size());
+
+      // X25519 should not leak the private key.
+      CONSTTIME_SECRET(priv.data(), priv.size());
+
       uint8_t secret[32];
       int ret = X25519(secret, priv.data(), pub.data());
       EXPECT_EQ(ret, result.IsValid({"NonCanonicalPublic", "Twist"}) ? 1 : 0);
+
+      CONSTTIME_DECLASSIFY(secret, sizeof(secret));
       EXPECT_EQ(Bytes(secret), Bytes(shared));
   });
 }