diff --git a/Recipe b/Recipe index 2e2fb5f0..d4ef8c94 100644 --- a/Recipe +++ b/Recipe @@ -282,7 +282,7 @@ UXSSH = SSH uxnoise uxagentc uxgss uxshare SFTP = psftpcommon sftp sftpcommon logging cmdline # Components of the prime-generation system. -SSHPRIME = sshprime smallprimes +SSHPRIME = sshprime smallprimes mpunsafe # Miscellaneous objects appearing in all the utilities, or all the # network ones, or the Unix or Windows subsets of those in turn. diff --git a/mpint.c b/mpint.c index 3e0ab530..94060ae7 100644 --- a/mpint.c +++ b/mpint.c @@ -33,7 +33,7 @@ static inline BignumInt mp_word(mp_int *x, size_t i) return i < x->nw ? x->w[i] : 0; } -static mp_int *mp_make_sized(size_t nw) +mp_int *mp_make_sized(size_t nw) { mp_int *x = snew_plus(mp_int, nw * sizeof(BignumInt)); assert(nw); /* we outlaw the zero-word mp_int */ diff --git a/mpint_i.h b/mpint_i.h index 23f6dc9d..d37e75f0 100644 --- a/mpint_i.h +++ b/mpint_i.h @@ -319,3 +319,6 @@ struct MontyContext { */ mp_int *scratch; }; + +/* Functions shared between mpint.c and mpunsafe.c */ +mp_int *mp_make_sized(size_t nw); diff --git a/mpunsafe.c b/mpunsafe.c new file mode 100644 index 00000000..beec13fa --- /dev/null +++ b/mpunsafe.c @@ -0,0 +1,57 @@ +#include +#include +#include + +#include "defs.h" +#include "misc.h" +#include "puttymem.h" + +#include "mpint.h" +#include "mpint_i.h" + +/* + * This global symbol is also defined in ssh2kex-client.c, to ensure + * that these unsafe non-constant-time mp_int functions can't end up + * accidentally linked in to any PuTTY tool that actually makes an SSH + * client connection. + * + * (Only _client_ connections, however. Uppity, being a test server + * only, is exempt.) + */ +const int deliberate_symbol_clash = 12345; + +static size_t mp_unsafe_words_needed(mp_int *x) +{ + size_t words = x->nw; + while (words > 1 && !x->w[words-1]) + words--; + return words; +} + +mp_int *mp_unsafe_shrink(mp_int *x) +{ + x->nw = mp_unsafe_words_needed(x); + /* This potentially leaves some allocated words between the new + * and old values of x->nw, which won't be wiped by mp_free now + * that x->nw doesn't mention that they exist. But we've just + * checked they're all zero, so we don't need to wipe them now + * either. */ + return x; +} + +mp_int *mp_unsafe_copy(mp_int *x) +{ + mp_int *copy = mp_make_sized(mp_unsafe_words_needed(x)); + mp_copy_into(copy, x); + return copy; +} + +uint32_t mp_unsafe_mod_integer(mp_int *x, uint32_t modulus) +{ + uint64_t accumulator = 0; + for (size_t i = mp_max_bytes(x); i-- > 0 ;) { + accumulator = 0x100 * accumulator + mp_get_byte(x, i); + accumulator %= modulus; + } + return accumulator; +} diff --git a/mpunsafe.h b/mpunsafe.h new file mode 100644 index 00000000..0b6ba3bd --- /dev/null +++ b/mpunsafe.h @@ -0,0 +1,46 @@ +/* + * mpunsafe.h: functions that deal with mp_ints in ways that are *not* + * expected to be constant-time. Used during key generation, in which + * constant run time is a lost cause anyway. + * + * These functions are in a separate header, so that you can easily + * check that you're not calling them in the wrong context. They're + * also defined in a separate source file, which is only linked in to + * the key generation tools. Furthermore, that source file also + * defines a global symbol that intentionally conflicts with one + * defined in the SSH client code, so that any attempt to put these + * functions into the same binary as the live SSH client + * implementation will cause a link-time failure. They should only be + * linked into PuTTYgen and auxiliary test programs. + * + * Also, just in case those precautions aren't enough, all the unsafe + * functions have 'unsafe' in the name. + */ + +#ifndef PUTTY_MPINT_UNSAFE_H +#define PUTTY_MPINT_UNSAFE_H + +/* + * The most obvious unsafe thing you want to do with an mp_int is to + * get rid of leading zero words in its representation, so that its + * nominal size is as close as possible to its true size, and you + * don't waste any time processing it. + * + * mp_unsafe_shrink performs this operation in place, mutating the + * size field of the mp_int it's given. It returns the same pointer it + * was given. + * + * mp_unsafe_copy leaves the original mp_int alone and makes a new one + * with the minimal size. + */ +mp_int *mp_unsafe_shrink(mp_int *m); +mp_int *mp_unsafe_copy(mp_int *m); + +/* + * Compute the residue of x mod m. This is implemented in the most + * obvious way using the C % operator, which won't be constant-time on + * many C implementations. + */ +uint32_t mp_unsafe_mod_integer(mp_int *x, uint32_t m); + +#endif /* PUTTY_MPINT_UNSAFE_H */ diff --git a/ssh2kex-client.c b/ssh2kex-client.c index 64ec0331..61a01646 100644 --- a/ssh2kex-client.c +++ b/ssh2kex-client.c @@ -13,6 +13,12 @@ #include "ssh2transport.h" #include "mpint.h" +/* + * Another copy of the symbol defined in mpunsafe.c. See the comment + * there. + */ +const int deliberate_symbol_clash = 12345; + void ssh2kex_coroutine(struct ssh2_transport_state *s, bool *aborted) { PacketProtocolLayer *ppl = &s->ppl; /* for ppl_logevent */ diff --git a/sshprime.c b/sshprime.c index 602d6c15..f6c02500 100644 --- a/sshprime.c +++ b/sshprime.c @@ -5,6 +5,7 @@ #include #include "ssh.h" #include "mpint.h" +#include "mpunsafe.h" #include "sshkeygen.h" /* @@ -108,23 +109,6 @@ * but 1s.) */ -static unsigned short mp_mod_short(mp_int *x, unsigned short modulus) -{ - /* - * This function lives here rather than in mpint.c partly because - * this is the only place it's needed, but mostly because it - * doesn't pay careful attention to constant running time, since - * as far as I can tell that's a lost cause for key generation - * anyway. - */ - unsigned accumulator = 0; - for (size_t i = mp_max_bytes(x); i-- > 0 ;) { - accumulator = 0x100 * accumulator + mp_get_byte(x, i); - accumulator %= modulus; - } - return accumulator; -} - /* * Generate a prime. We can deal with various extra properties of * the prime: @@ -204,9 +188,9 @@ mp_int *primegen( * we're incrementing by multiples of factor). */ unsigned long residues[NSMALLPRIMES + 1], multipliers[NSMALLPRIMES + 1]; for (size_t i = 0; i < lenof(moduli); i++) { - residues[i] = mp_mod_short(p, moduli[i]); + residues[i] = mp_unsafe_mod_integer(p, moduli[i]); if (factor) - multipliers[i] = mp_mod_short(factor, moduli[i]); + multipliers[i] = mp_unsafe_mod_integer(factor, moduli[i]); else multipliers[i] = 1; }