[tor-commits] [tor/master] Tolerate curve25519 backends where the high bit of the pk isn't ignored

[nickm at torproject.org]

commit 266419d244c1a4795407479693b016c5a8b5da96
Author: Nick Mathewson <nickm at torproject.org>
Date:   Mon Feb 4 12:50:01 2013 -0500

    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                |    7 +++++++
 src/common/crypto_curve25519.c |   11 +++++++++--
 src/test/test_crypto.c         |   14 ++++++++++++--
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/changes/bug8121 b/changes/bug8121
new file mode 100644
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 425a1a0..3e4004d 100644
--- 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 37843d1..273e03b 100644
--- 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 },