diff --git a/changes/bug8121 b/changes/bug8121 new file mode 100644 index 0000000000..60cba72848 --- /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 425a1a078c..3e4004db2e 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 37843d1110..273e03b9d9 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 },