Tolerate curve25519 backends where the high bit of the pk isn't ignored

Right now, all our curve25519 backends ignore the high bit of the
public key. But possibly, others could treat the high bit of the
public key as encoding out-of-bounds values, or as something to be
preserved. This could be used to distinguish clients with different
backends, at the cost of killing a circuit.

As a workaround, let's just clear the high bit of each public key
indiscriminately before we use it. Fix for bug 8121, reported by
rransom. Bugfix on 0.2.4.8-alpha.

diff --git a/changes/bug8121 b/changes/bug8121
new file mode 100644 (file)
index 0000000..60cba72
--- /dev/null
+++ b/changes/bug8121
@@ -0,0 +1,7 @@
+  o Minor features:
+    - Clear the high bit on curve25519 public keys before passing them to
+      our backend, in case we ever wind up using a backend that doesn't do
+      so itself. If we used such a backend, and *didn't* clear the high bit,
+      we could wind up in a situation where users with such backends would
+      be distinguishable from users without. Fix for bug 8121; bugfix on
+      0.2.4.8-alpha.

diff --git a/src/common/crypto_curve25519.c b/src/common/crypto_curve25519.c
index 425a1a078c1f1c43a84408a3b887dab27e8abab3..3e4004db2e7e8bfe01e773d6cb89a7acc55b03f0 100644 (file)
--- a/src/common/crypto_curve25519.c
+++ b/src/common/crypto_curve25519.c
@@ -33,13 +33,20 @@ int
 curve25519_impl(uint8_t *output, const uint8_t *secret,
                 const uint8_t *basepoint)
 {
+  uint8_t bp[CURVE25519_PUBKEY_LEN];
+  int r;
+  memcpy(bp, basepoint, CURVE25519_PUBKEY_LEN);
+  /* Clear the high bit, in case our backend foolishly looks at it. */
+  bp[31] &= 0x7f;
 #ifdef USE_CURVE25519_DONNA
-  return curve25519_donna(output, secret, basepoint);
+  r = curve25519_donna(output, secret, bp);
 #elif defined(USE_CURVE25519_NACL)
-  return crypto_scalarmult_curve25519(output, secret, basepoint);
+  r = crypto_scalarmult_curve25519(output, secret, bp);
 #else
 #error "No implementation of curve25519 is available."
 #endif
+  memwipe(bp, 0, sizeof(bp));
+  return r;
 }
 
 /* ==============================

diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c
index 37843d1110f86390dd2650a96a37df8db3e70c9a..273e03b9d970702b709efd1ed74163a4b88b683c 100644 (file)
--- a/src/test/test_crypto.c
+++ b/src/test/test_crypto.c
@@ -941,6 +941,8 @@ test_crypto_curve25519_impl(void *arg)
   /* adapted from curve25519_donna, which adapted it from test-curve25519
      version 20050915, by D. J. Bernstein, Public domain. */
 
+  const int randomize_high_bit = (arg != NULL);
+
   unsigned char e1k[32];
   unsigned char e2k[32];
   unsigned char e1e2k[32];
@@ -952,12 +954,19 @@ test_crypto_curve25519_impl(void *arg)
   const int loop_max=10000;
   char *mem_op_hex_tmp = NULL;
 
-  (void)arg;
-
   for (loop = 0; loop < loop_max; ++loop) {
     curve25519_impl(e1k,e1,k);
     curve25519_impl(e2e1k,e2,e1k);
     curve25519_impl(e2k,e2,k);
+    if (randomize_high_bit) {
+      /* We require that the high bit of the public key be ignored. So if
+       * we're doing this variant test, we randomize the high bit of e2k, and
+       * make sure that the handshake still works out the same as it would
+       * otherwise. */
+      uint8_t byte;
+      crypto_rand((char*)&byte, 1);
+      e2k[31] |= (byte & 0x80);
+    }
     curve25519_impl(e1e2k,e1,e2k);
     test_memeq(e1e2k, e2e1k, 32);
     if (loop == loop_max-1) {
@@ -1135,6 +1144,7 @@ struct testcase_t crypto_tests[] = {
   { "hkdf_sha256", test_crypto_hkdf_sha256, 0, NULL, NULL },
 #ifdef CURVE25519_ENABLED
   { "curve25519_impl", test_crypto_curve25519_impl, 0, NULL, NULL },
+  { "curve25519_impl_hibit", test_crypto_curve25519_impl, 0, NULL, (void*)"y" },
   { "curve25519_wrappers", test_crypto_curve25519_wrappers, 0, NULL, NULL },
   { "curve25519_encode", test_crypto_curve25519_encode, 0, NULL, NULL },
   { "curve25519_persist", test_crypto_curve25519_persist, 0, NULL, NULL },