| commit | aa83c12069f3d62704fce3d499b068b5bf1b6e31 | [log] [tgz] |
|---|---|---|
| author | David Benjamin <davidben@google.com> | Wed Feb 01 12:21:56 2023 -0500 |
| committer | Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> | Fri Feb 03 16:47:02 2023 +0000 |
| tree | 8e3e7ba5653430b743c0e0cf0e365806b17a3c1e | |
| parent | 5bdf5e4ac251e7e9eca5693104d802d94a28f28b [diff] |
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>
BoringSSL is a fork of OpenSSL that is designed to meet Google's needs.
Although BoringSSL is an open source project, it is not intended for general use, as OpenSSL is. We don't recommend that third parties depend upon it. Doing so is likely to be frustrating because there are no guarantees of API or ABI stability.
Programs ship their own copies of BoringSSL when they use it and we update everything as needed when deciding to make API changes. This allows us to mostly avoid compromises in the name of compatibility. It works for us, but it may not work for you.
BoringSSL arose because Google used OpenSSL for many years in various ways and, over time, built up a large number of patches that were maintained while tracking upstream OpenSSL. As Google's product portfolio became more complex, more copies of OpenSSL sprung up and the effort involved in maintaining all these patches in multiple places was growing steadily.
Currently BoringSSL is the SSL library in Chrome/Chromium, Android (but it's not part of the NDK) and a number of other apps/programs.
Project links:
There are other files in this directory which might be helpful: