Hi,
On Mon, 20 Jun 2016 07:30:47 +0200 nisse@lysator.liu.se (Niels Möller) wrote:
I'm considering the below patch, making use of the side-channel silent mpz_powm_sec function. The idea is to make the RSA and DSA code less vulnerable to side-channel attacks.
This change introduces a security risk.
The function mpz_powm_sec() is not a drop in replacement for mpz_powm(), although it may sound like it. If you look at the docs [1] a requirement for mpz_powm_sec() is that the modulus must be odd.
If the modulus is even / divisible by 2 then mpz_powm_sec() will crash with a floating point exception. Quite frankly, I think this is bad from GMPs side as well. At the very least it shouldn't crash for invalid inputs, but return an error. (Ideally it would just work for all inputs that mathematically make sense). But nettle has to handle things with gmp as they are.
Attached is a certificate + key where I manually changed the modulus to be even (P.S.: This tool [2] fis very useful for such cases). The certificate is therefore obviously bogus, but that doesn't matter in our case.
Just try to run nettle with the mpz_powm_sec() patch and try to run gnutls with this crafted cert: gnutls-serv --x509keyfile=crashnettle.pem --x509certfile=crashnettle.pem
This crashes as expected with a floating point exception.
Now it may not be obvious that this is a security risk, because I usually control my own certificates. But there may well be situations where that's not so clear. E.g. think of a webhoster that allows his customers to set custom certificates and private keys. That doesn't mean one wants his customers to allow crashing the servers.
It may be that there are other situations where this matters.
Anyway: Changing to mpz_pown_sec() probably should only be done if the code is carefully checked to make sure that an even modulus never enters this function.
(P.S.: The autoconf gmp detection part of this patch breaks on my system and nettle gets built without gmp, so there seems to be something wrong with that, too. I have Gentoo with gmp 6.1.1. I haven't further analyzed this, I simply removed that part of the patch for my testing.)
[1] https://gmplib.org/manual/Integer-Exponentiation.html [2] https://github.com/google/der-ascii