Swap endianness of the Curve25519 ECDH private key.

DJB's spec at http://cr.yp.to/ecdh/curve25519-20060209.pdf is clear
that we should be clearing the low 3 bits of the _LSB_ of the private
key bit string, and setting bit 6 and clearing bit 7 of the _MSB_. We
were doing the opposite, due to feeding the resulting bit string to
bignum_from_bytes() rather than bignum_from_bytes_le().

This didn't cause an interoperability issue, because the two DH
exponentiations still commute, but it goes against the Curve25519
spec, in particular the care taken to fix the position of the leading
exponent bit.

The code is now consistent with the test vectors in RFC 7748 section
6.1: if you modify the EC_MONTGOMERY branch of ssh_ecdhkex_newkey() to
replace the loop on random_byte() with a memcpy that fills bytes[]
with 77076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c2a
and then print out the resulting publicKey->x, you find that it's
(byte-reversed) the expected output value given in that RFC section,
8520f0098930a754748b7ddcb43ef75a0dbf3a0d26381af4eba4a98eaa9b4e6a.

diff --git a/sshecc.c b/sshecc.c
index 46e6fadbec500c077e886ff01c64f074f8b56139..3bb2082b6e4423bc2342429e8e0912912f35b5d0 100644 (file)
--- a/sshecc.c
+++ b/sshecc.c
@@ -2760,7 +2760,7 @@ void *ssh_ecdhkex_newkey(const struct ssh_kex *kex)
         bytes[0] &= 248;
         bytes[31] &= 127;
         bytes[31] |= 64;
         bytes[0] &= 248;
         bytes[31] &= 127;
         bytes[31] |= 64;

-        key->privateKey = bignum_from_bytes(bytes, sizeof(bytes));
+        key->privateKey = bignum_from_bytes_le(bytes, sizeof(bytes));

         smemclr(bytes, sizeof(bytes));
         if (!key->privateKey) {
             sfree(key);
         smemclr(bytes, sizeof(bytes));
         if (!key->privateKey) {
             sfree(key);