(The BN_num_bits_word implementation was originally written by Andy
Polyakov for OpenSSL. See also
https://github.com/openssl/openssl/pull/5154.)
BN_num_bits, by way of BN_num_bits_word, currently leaks the
most-significant word of its argument via branching and memory access
pattern.
BN_num_bits is called on RSA prime factors in various places. These have
public bit lengths, but all bits beyond the high bit are secret. This
fully resolves those cases.
There are a few places where BN_num_bits is called on an input where
the bit length is also secret. The two left in BoringSSL are:
- BN_mod_exp_mont_consttime calls it on the RSA private exponent.
- The timing "fix" to add the order to k in DSA.
This does *not* fully resolve those cases as we still only look at the
top word. Today, that is guaranteed to be non-zero, but only because of
the long-standing bn_correct_top timing leak. Once that is fixed (I hope
to have patches soon), a constant-time BN_num_bits on such inputs must
count bits on each word.
Instead, those cases should not call BN_num_bits at all. The former uses
the bit width to pick windows, but it should be using the maximum bit
width. The next patch will fix this. The latter is the same "fix" we
excised from ECDSA in a838f9dc7e. That
should be excised from DSA after the bn_correct_top bug is fixed.
Thanks to Dinghao Wu, Danfeng Zhang, Shuai Wang, Pei Wang, and Xiao Liu
for reporting this issue.
Change-Id: Idc3da518cc5ec18bd8688b95f959b15300a57c14
Reviewed-on: https://boringssl-review.googlesource.com/25184
Reviewed-by: Adam Langley <agl@google.com>
crypto/{asn1,x509,x509v3,pem} were skipped as they are still OpenSSL
style.
Change-Id: I3cd9a60e1cb483a981aca325041f3fbce294247c
Reviewed-on: https://boringssl-review.googlesource.com/19504
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
