Start making asserts constant-time too

We've historically settled on treating asserts as not in scope for our
constant-time goals. Production binaries are expected to be optimized
builds, with debug assertions turned off. (We have a handful of
assertions in perf-sensitive code that you definitely do not want to run
with.) Secret data has invariants too, so it is useful to be able to
write debug assertions on them.

However, combined with our default CMake build being a debug build, this
seems to cause some confusion with researchers sometimes. Also, if we
ever get language-level constant-time support, we would need to resolve
this mismatch anyway. (I assume any language support would put enough
into the type system to force us to declassify any intentional branches
on secret-by-data-flow bools, notably those we assert on.) So I'm
inclined to just make our asserts constant-time.

There are two issues around asserts, at least with our valgrind-based
validation:

The first is that a couple of asserts over secret data compute their
condition leakily. We can just fix these. The only such ones I found
were in bn_reduce_once and bn_gcd_consttime.

The second is that almost every assert over secret data will be flagged
as an invalid branch by valgrind. However, presuming the condition
itself was computed in constant time, this branch is actually safe. If
we were willing to abort the process when false, the assert is clearly
publicly true. We just need to declassify the boolean to assert on it.

assert(constant_time_declassify_int(expr)) is really long, so I made an
internal wrapper macro declassify_assert(expr). Not sure if that's the
best name. constant_time_declassify_assert(expr) is kinda long.
constant_time_assert(expr) fits with the rest of that namespace, but
reads as if we're somehow running an assert without branching, when the
whole point is that we *are* branching and need to explicitly say it's
okay to.

Fixed: 339
Change-Id: Ie33b99bf9a269b11d2c48d246cc4934be7e239ff
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65467
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
14 files changed
tree: f375612a9d5969c3f869afe446c25dd02d40115d
  1. .github/
  2. cmake/
  3. crypto/
  4. decrepit/
  5. fuzz/
  6. include/
  7. pki/
  8. rust/
  9. ssl/
  10. third_party/
  11. tool/
  12. util/
  13. .clang-format
  14. .gitignore
  15. API-CONVENTIONS.md
  16. BREAKING-CHANGES.md
  17. BUILDING.md
  18. CMakeLists.txt
  19. codereview.settings
  20. CONTRIBUTING.md
  21. FUZZING.md
  22. go.mod
  23. go.sum
  24. INCORPORATING.md
  25. LICENSE
  26. PORTING.md
  27. README.md
  28. SANDBOXING.md
  29. sources.cmake
  30. STYLE.md
README.md

BoringSSL

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: