tree 8e3e7ba5653430b743c0e0cf0e365806b17a3c1e
parent 5bdf5e4ac251e7e9eca5693104d802d94a28f28b
author David Benjamin <davidben@google.com> 1675272116 -0500
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> 1675442822 +0000

Clear various false positives in RSA constant-time validation

This silences a few false positives in the valgrind-based constant-time
validation.

First, there are a few precondition checks that are publicly true, but
valgrind doesn't know that. I've added a constant_time_declassify_int
function and stuck those in there, since the existing macro is mostly
suited for macros. It also adds a value barrier in production code (see
comment for why). If we more thoroughly decoupled RSA from BIGNUM, we
could probably avoid this, since a lot of comes from going through
public BIGNUM APIs.

Next, our BIGNUM strategy is such that bounds on bignums are sometimes
computed pessimally, and then clamped down later. Modular arithmetic is
trivially bounded and avoids that, but RSA CRT involves some non-modular
computations. As a result, we actually compute a few more words than
necessary in the RSA result, and then bn_resize_words down.
bn_resize_words also has a precondition check, which checks that all
discarded words are zero. They are, but valgrind does not know that.

Similarly, the BN_bn2bin_padded call at the end checks for discarded
non-zero bytes, but valgrind does not know that, because the output is
bounded by n, the discarded bytes are zero.

I've added a bn_assert_fits_in_bytes to clear this. It's an assert in
debug mode and a declassification in constant-time validation.

I suspect a different secret integer design would avoid needing this. I
think this comes from a combination of non-modular arithmetic, not
having callers pass explicit width, and tracking public widths at the
word granularity, rather than byte or bit. (Bit would actually be most
ideal.) Maybe worth a ponder sometime.

Change-Id: I1bc9443d571d2881e2d857c70be913074deac156
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56825
Commit-Queue: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>