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.

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.

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;
 }
 
 /* ==============================

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 },