On Fri, Sep 4, 2015 at 8:49 AM, Niels Möller nisse@lysator.liu.se wrote:
Said that, there is rsa_pkcs1_sign_tr() which is supposed to be the side-channel resistant version of rsa_pkcs1_sign() and can have this additional check with no changes.
What about rsa_decrypt_tr? Does it also need this check? Attacks are a bit harder since even with known plaintext, there's padding that is supposed to be random. But I imagine it's possible with improved attacks that either work with partial knowledge, or exploit weeknesses in the randomness used for padding.
The issue with signatures is that you send or store the output once calculated and that can be observed by an adversary. When decrypting I don't think the threat model is the same. At least for the purposes of TLS protecting decryption wouldn't add anything.
Maybe factor out a function rsa_compute_root_tr (in particular, that would be useful if we want the same check in rsa_decrypt_tr).
I don't think that would be necessary (see above for the decrypt function).
- /* Try bad signature */
- mpz_combit(signature, 17);
- ASSERT (!rsa_pkcs1_verify(pub, di_length, di, signature));
It would be good to also do a test with an invalid private key, e.g, using p+2 instead of p. That should trigger the new check, so that rsa_pkcs1_sign_tr returns failure and sets s == 0.
Good idea.
--- a/nettle.texinfo +++ b/nettle.texinfo @@ -3646,7 +3646,14 @@ There is currently no support for using SHA224 or SHA384 with @acronym{RSA} signatures, since there's no gain in either computation time nor message size compared to using SHA256 and SHA512, respectively.
-Creation and verification of signatures is done with the following functions: +Creation and verification of signatures is recommended to be done with the following functions. +They provide a side-channel (fault and timing) resistant implementation. +@deftypefun int rsa_pkcs1_sign_tr(const struct rsa_public_key *@var{pub}, const struct rsa_private_key *@var{key}, void *@var{random_ctx}, nettle_random_func *@var{random}, size_t @var{length}, const uint8_t *@var{digest_info}, mpz_t @var{s}); +@deftypefunx int rsa_pkcs1_verify(const struct rsa_public_key *@var{key}, size_t @var{length}, const uint8_t *digest_info, const mpz_t @var{signature});
I wasn't aware that these functions lacked documentation. Good catch. I think we need to say something about the digest_info argument.
I think the rsa_pkcs1_verify function should move to the other verify functions (and be at the top of that list, if it's the recommended one).
Would that make sense if we deprecate everything else? I thought that everything else is deprecated first should be listed the recommended functions, rather than trying to look for them within a very long list of deprecated functions.
Side-channel resistance isn't relevant for verify.
I made that clear.
(We may also need some public functions for constructing digest_infos, a bit like pkcs1_rsa_sha256_encode. But the current internal functions don't fit well with rsa_pkcs1_sign_tr. So the "CRT-hardening" fixes shouldn't block on that).
That would be indeed useful (though unrelated with this patch set).
+Other functions are also available but cannot be used by application that require +a side-channel silent implementation. These are listed below.
For improved side-channel silence, one could also let rsa_compute_root use mpz_powm_sec if available. But the current GMP plans are to not try to provide side-schannel silent functions on the mpz level (mpz_powm_sec is an exception), so to really take advantage of GMP improvements in side-channel silence, one would need to move to using the lower-level mpn interface.
That's pretty sad. It means that applications couldn't take advantage of that without being rewritten.
In any case the attached patch set should address all your comments.
regards, Nikos