[EC] harden EC_KEY against leaks from memory accesses
authorNicola Tuveri <nic.tuv@gmail.com>
Tue, 21 Jan 2020 15:00:41 +0000 (17:00 +0200)
committerNicola Tuveri <nic.tuv@gmail.com>
Fri, 24 Apr 2020 14:44:10 +0000 (17:44 +0300)
commit6a01f6f4b41d045e2a3abcb10163633d769db76a
treebc65467ad53fadb3343527f4be70b0ab3f3650fc
parentcd45a57aafddb908eb3a56e118b4c01899765d18
[EC] harden EC_KEY against leaks from memory accesses

We should never leak the bit length of the secret scalar in the key,
so we always set the `BN_FLG_CONSTTIME` flag on the internal `BIGNUM`
holding the secret scalar.

This is important also because `BN_dup()` (and `BN_copy()`) do not
propagate the `BN_FLG_CONSTTIME` flag from the source `BIGNUM`, and
this brings an extra risk of inadvertently losing the flag, even when
the called specifically set it.

The propagation has been turned on and off a few times in the past
years because in some conditions has shown unintended consequences in
some code paths, so at the moment we can't fix this in the BN layer.

In `EC_KEY_set_private_key()` we can work around the propagation by
manually setting the flag after `BN_dup()` as we know for sure that
inside the EC module the `BN_FLG_CONSTTIME` is always treated
correctly and should not generate unintended consequences.

Setting the `BN_FLG_CONSTTIME` flag alone is never enough, we also have
to preallocate the `BIGNUM` internal buffer to a fixed public size big
enough that operations performed during the processing never trigger
a realloc which would leak the size of the scalar through memory
accesses.

Fixed Length
------------

The order of the large prime subgroup of the curve is our choice for
a fixed public size, as that is generally the upper bound for
generating a private key in EC cryptosystems and should fit all valid
secret scalars.

For preallocating the `BIGNUM` storage we look at the number of "words"
required for the internal representation of the order, and we
preallocate 2 extra "words" in case any of the subsequent processing
might temporarily overflow the order length.

Future work
-----------

A separate commit addresses further hardening of `BN_copy()` (and
indirectly `BN_dup()`).

(cherry picked from commit 0401d766afcd022748763f5614188301c9856c6e)

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11127)
crypto/ec/ec_key.c