mirror of
https://git.tartarus.org/simon/putty.git
synced 2025-01-09 17:38:00 +00:00
Reject all low-order points in Montgomery key exchange.
This expands our previous check for the public value being zero, to take in all the values that will _become_ zero after not many steps. The actual check at run time is done using the new is_infinite query method for Montgomery curve points. Test cases in cryptsuite.py cover all the dangerous values I generated via all that fiddly quartic- solving code. (DJB's page http://cr.yp.to/ecdh.html#validate also lists these same constants. But working them out again for myself makes me confident I can do it again for other similar curves, such as Curve448.) In particular, this makes us fully compliant with RFC 7748's demand to check we didn't generate a trivial output key, which can happen if the other end sends any of those low-order values. I don't actually see why this is a vital check to perform for security purposes, for the same reason that we didn't classify the bug 'diffie-hellman-range-check' as a vulnerability: I can't really see what the other end's incentive might be to deliberately send one of these nonsense values (and you can't do it by accident - none of these values is a power of the canonical base point). It's not that a DH participant couldn't possible want to secretly expose the session traffic - but there are plenty of more subtle (and less subtle!) ways to do it, so you don't really gain anything by forcing them to use one of those instead. But the RFC says to check, so we check.
This commit is contained in:
parent
1cad3c8255
commit
7751657811
22
sshecc.c
22
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);
|
||||
|
||||
|
@ -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"
|
||||
|
@ -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
|
||||
|
Loading…
Reference in New Issue
Block a user