diff --git a/sshecc.c b/sshecc.c index 1481254c..8ba2629e 100644 --- a/sshecc.c +++ b/sshecc.c @@ -1411,26 +1411,18 @@ static mp_int *ssh_ecdhkex_m_getkey(ecdh_key *dh, ptrlen remoteKey) * will be reduced mod p. */ mp_reduce_mod_2to(remote_x, dh->curve->fieldBits); - if (mp_eq_integer(remote_x, 0)) { - /* - * The libssh spec for Curve25519 key exchange says that - * 'every possible public key maps to a valid ECC Point' and - * therefore no validation needs to be done on the server's - * provided x-coordinate. However, I don't believe it: an - * x-coordinate of zero doesn't work sensibly, because you end - * up dividing by zero in the doubling formula - * (x+1)^2(x-1)^2/(4(x^3+ax^2+x)). (Put another way, although - * that point P is not the _identity_ of the curve, it is a - * torsion point such that 2P is the identity.) - */ - mp_free(remote_x); - return NULL; - } MontgomeryPoint *remote_p = ecc_montgomery_point_new( dh->curve->m.mc, remote_x); mp_free(remote_x); MontgomeryPoint *p = ecc_montgomery_multiply(remote_p, dh->private); + + if (ecc_montgomery_is_identity(p)) { + ecc_montgomery_point_free(remote_p); + ecc_montgomery_point_free(p); + return NULL; + } + mp_int *x; ecc_montgomery_get_affine(p, &x); diff --git a/test/cryptsuite.py b/test/cryptsuite.py index 18a2a330..5006a76e 100755 --- a/test/cryptsuite.py +++ b/test/cryptsuite.py @@ -1360,6 +1360,46 @@ class crypt(MyTestBase): '7964541892e7511798e61dd78429358f4d6a887a50d2c5ebccf0e04f48fc665c' )) + def testMontgomeryKexLowOrderPoints(self): + # List of all the bad input values for Curve25519 which can + # end up generating a zero output key. You can find the first + # five (the ones in canonical representation, i.e. in + # [0,2^255-19)) by running + # find_montgomery_power2_order_x_values(curve25519.p, curve25519.a) + # and then encoding the results little-endian. + bad_keys_25519 = [ + "0000000000000000000000000000000000000000000000000000000000000000", + "0100000000000000000000000000000000000000000000000000000000000000", + "5f9c95bca3508c24b1d0b1559c83ef5b04445cc4581c8e86d8224eddd09f1157", + "e0eb7a7c3b41b8ae1656e3faf19fc46ada098deb9c32b1fd866205165f49b800", + "ecffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f", + + # Input values less than 2^255 are reduced mod p, so those + # of the above values which are still in that range when + # you add 2^255-19 to them should also be caught. + "edffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f", + "eeffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f", + + # Input values are reduced mod 2^255 before reducing mod + # p. So setting the high-order bit of any of the above 7 + # values should also lead to rejection, because it will be + # stripped off and then the value will be recognised as + # one of the above. + "0000000000000000000000000000000000000000000000000000000000000080", + "0100000000000000000000000000000000000000000000000000000000000080", + "5f9c95bca3508c24b1d0b1559c83ef5b04445cc4581c8e86d8224eddd09f11d7", + "e0eb7a7c3b41b8ae1656e3faf19fc46ada098deb9c32b1fd866205165f49b880", + "ecffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "edffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "eeffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + ] + + with random_prng("doesn't matter"): + ecdh25519 = ssh_ecdhkex_newkey('curve25519') + for pub in bad_keys_25519: + key = ssh_ecdhkex_getkey(ecdh25519, unhex(pub)) + self.assertEqual(key, None) + def testPRNG(self): hashalg = 'sha256' seed = b"hello, world" diff --git a/testcrypt.h b/testcrypt.h index bc9dcea9..152af6ce 100644 --- a/testcrypt.h +++ b/testcrypt.h @@ -202,7 +202,7 @@ FUNC2(val_mpint, dh_find_K, val_dh, val_mpint) */ FUNC1(val_ecdh, ssh_ecdhkex_newkey, ecdh_alg) FUNC2(void, ssh_ecdhkex_getpublic, val_ecdh, out_val_string_binarysink) -FUNC2(val_mpint, ssh_ecdhkex_getkey, val_ecdh, val_string_ptrlen) +FUNC2(opt_val_mpint, ssh_ecdhkex_getkey, val_ecdh, val_string_ptrlen) /* * RSA key exchange, and also the BinarySource get function