Prepare to merge the rest of the not-yet-merged BoringSSL changes.
The recent refactoring of `PrivateExponent` lets us add a new test-only
constructor that can support a wider range of exponents. Also, there's
no reason to avoid test vectors with a base of zero. We do need to still
reject 0 as an *exponent* and too-small moduli.
Accordingly, merge in all the relevant test vectors from BoringSSL's
`bn_test.txt` into `bigint_elem_exp_consttime_tests.txt` as of
BoringSSL a8b1633d1c6be133b9f684cc5cdd778bfd8d564e, which is the last
commit of BoringSSL that has been merged into *ring* so far.
```
git diff \
a8b1633d1c:crypto/fipsmodule/bn/bn_tests.txt \
src/arithmetic/bigint_elem_exp_consttime_tests.txt
```
When constructing a `PrivateExponent` we enforce that the exponent is
appropriately-sized for its associated modulus; this check is relied on
in RSA private key construction for key component consistency checks.
However, once the `PrivateExponent` is constructed there is no reason
to relate its value to the modulus. Doing so has inhibited us from
using some test vectors that are in the BoringSSL test suite. Further
this usage blocks encapsulating `BoxedLimbs` into its own submodule.
Ensure all the functions responsible for maintaining invariants for
`Modulus` values are within `Modulus`.
Clarify the constraints on the relationship between the moduli in
`Modulus::from_elem`.
Clarify that `Nonnegative` values are immutable by enforcing this
through the module system.
Some read-only to-be-refactored-away methods of `Nonnegative` stay in
`bigint` to avoid moving them back-and-forth later.
`git diff HEAD^1:src/arithmetic/bigint.rs src/arithmetic/nonnegative.rs`
BoringSSL can currently be built in C11 or pre-C11 mode in MSVC. They're
broadly the same, but do use completely different implementations of
alignas and alignof. Now that every build configuration I'm aware of has
been moved to the C11 mode, we don't even test the pre-C11 mode anymore.
Start requiring it.
Update-Note: If building with MSVC, BoringSSL now requires building with
/std:c11 or later. (On non-MSVC compilers, we have required C11 for a
while now.)
Fixed: 624
Change-Id: Ie9f66eee0bebac8143c23a7229c6854afaefea6e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63065
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Old version Chrome with the existing ALPS codepoint can potentially cause network error due to an arithmetic overflow bug in Chrome ALPS decoder (We already fixed the issues starting from M100 in Chrome).
This CL add a new codepoint for ALPS extension in a way that can be enabled on individual connections., To support multiple versions of Chrome, we need to support both codepoints in BoringSSL.
For details: https://docs.google.com/document/d/16pysbV_ym_qAau_DBYnrw2A4h5ve2212wfcoYASt52U
Change-Id: Iea7822e757d23009648febc8eaff1c91b0f06e18
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61125
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Bug: chromium:1322914
Change-Id: Ic5a1349013bcfb279e5fee9f9838c63558d663b7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63025
Auto-Submit: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
https://boringssl-review.googlesource.com/c/boringssl/+/62585 made the
compiler emit multiple CRYPTO_library_init calls in functions which
dispatch between a tower of alternatives. Ideally, the compiler would
know that at most one call suffices.
There doesn't seem to be such an attribute, but we can get the same
effect with pure or const attributes. We tie init with returning the
capability vector. On Intel, because the vector is so large, we have to
go with a weaker version. Somewhat annoyingly, the getter must be
out-of-line, because otherwise the compiler inlines first and loses the
attribute.
I went with pure because we allow our unit tests to mutate
OPENSSL_armcap_P, which means the Arm one is, strictly speaking, pure,
not const. This slightly reduces optimization potential, but should
still allow deduping in most places. Confirmed that aes_init_key
now only calls a helper function once.
See discussion in
https://boringssl-review.googlesource.com/c/boringssl/+/62585/comment/26083b88_b3db2b75/
Bug: 35
Change-Id: I9bc464f0e5a0ed9601017a5037028f906693a137
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62985
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
All the C accesses have been sufficiently abstracted that this is pretty
easy to handle automatically.
We still have accesses from assembly, so we're not quite
initializationless yet. But this does get us most of the way there. I'm
thinking what's next is:
- Make a list of asm symbols that touch armcap or ia32cap
- For each, figure out the place(s) in the calling code where we need to
init manually and/or pull the dispatch up into C
One interesting subtlety with how this CL does it: although this CL
means you can freely call, say, CRYPTO_is_SSSE3_capable without
CRYPTO_library_init, you cannot *quite* assume that CRYPTO_library_init
has been called after you call CRYPTO_is_SSSE3_capable. It is possible
that the build defined __SSSE3__, in which case CRYPTO_is_SSSE3_capable
does nothing. This does complicate resolving the asm cases above.
Bug: 35
Change-Id: Ie52c74e4a59a7019c3af0526dbb35950604ada66
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62585
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Referencing a variable in a closure captures it by _address_. So
referencing a loop variable can go horribly wrong:
https://go.dev/play/p/f2ivPAIN_bG
This is accepted as essentially a bug by Go and will be fixed in a
future release (https://github.com/golang/go/wiki/LoopvarExperiment).
But, for now at least, work around it.
Our tests trim the ACVP inputs to only have a single test case per group
in many cases, which hides most of this issue from tests. When we run
run full ACVP sets, our modulewrapper is seemingly fast enough not to
notice there either. But I've updated one of the tests here by
duplicating a test case enough that it catches this a meaningful amount
of the time.
Change-Id: I8216c00f67636ab7dad927eae4b49ae45ae3cf31
Bug: 646
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62965
Reviewed-by: David Benjamin <davidben@google.com>
Auto-Submit: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Some tests in the Chromium verifier use gmock. Since upstream googletest
considers them a single project now, just import both of them.
Update-Note: As part of this, I've made gtest_main.cc now initializes
gmock alongside gtest. This means BoringSSL's test targets now depend on
both. Downstream builds where googletest and googlemock are separate
build targets may need to add a dependency. (Though every downstream
test I looked at treats them as one target, so this is most likely a
no-op.)
Bug: chromium:1322914
Change-Id: Ief38c675bc13a4639dee061d058580967ab99d41
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62945
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Previously, EVP_CIPHER_CTX_iv_length always returned the cipher's fixed IV length. Now, after modification with EVP_CTRL_AEAD_SET_IVLEN, it returns the correct value.
Fixed: 626
Change-Id: Id98c929439850b3e83a80111f35aabebc6e5d47a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62907
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
The CI jobs for this target are failing with this error:
```
error: component 'rust-std' for target 'mipsel-unknown-linux-gnu' is
unavailable for download for channel 'nightly'
```
Remove the target while we investigate.
Handshake hints work fine with TLS 1.2 resumption now. Also split
handshakes is really really dangerous, and I think hints has survived
long enough that we can just declare it the successor.
Change-Id: Ib5fe5e1b030034b853a96c3404608c56d7b7a7c7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62925
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
We have a corresponding check on the ServerHello, but not
HelloRetryRequest. See also https://github.com/rustls/rustls/pull/1374,
where rustls forgot to apply the compatibility logic to
HelloRetryRequest.
(From the perspective of a TLS-1.2-expecting observer, HelloRetryRequest
is the ServerHello, so encoding hacks need to apply to both.)
Change-Id: I9b711ea45c54770a76ecfbca8bc992a4eaef6fcd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62906
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
This reverts 1e2f1696636088626cb223aa5a10f64e07b62ffd. Bazel 6.3 has
since been released, which includes a fix for
https://github.com/bazelbuild/bazel/issues/15073. Envoy and gRPC have
both since updated to this Bazel version. The policies in
https://opensource.google/documentation/policies/cplusplus-support#build_systems
also imply a minimum Bazel version of 6.3.2.
I'm thinking we let this bake for a little while, to catch any
unexpected issues, and then, if it sticks, we try to go ahead and
require C11 across the board.
Update-Note: If using Bazel with MSVC, and the build fails with
something like "Command line error D8016 : '/std:c++20' and '/std:c11'
command-line options are incompatible", you are likely running into the
above Bazel bug. Update to Bazel 6.3 or later.
Bug: 623, 624
Change-Id: I8baa99392ca47bc7580bc2930e7f4b16beced91e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62905
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
For example, openssl/asm_base.h expands to include the line
.long ((1 << 0) | (1 << 1));
when BTI and PAC are enabled.
Change-Id: I07208e0430757721e97b88c706672375f8f58f1f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62525
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
unw_context_t is, at least on x86_64, the same type as ucontext_t.
Passing that into unw_init_local doesn't work, but there's a
unw_init_local2 in libunwind 1.3.0 or later, which has a flag for this
case.
This avoids needing to unwind past the signal handler stack frames,
which is both simpler and faster. (Shaved around 10 seconds off running
all the unwind tests on my machine.)
Change-Id: I09c130e76682d63e51b7b9de9ff5b91415e26f32
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62867
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Rather than sample it from the UnwindCursor, we can just save it
immediately before starting the test.
Change-Id: Ica1eaa215755b0b772eaa08e03c5885aacec4f70
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62866
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Although it works without these (we just refer the unwinder to the red
zone), older versions of libunwind seem to have a bug that cause it to
flakily fail to restore rbx without this. I've attempted to bisect the
problem, but the issue is very flaky and I've failed to find the culprit
four times now, so just give up and work around it. Explicit restores
match what we do in other files.
Hopefully this will clear some issues tha fiat-crypto's CI are running
into.
Change-Id: I6a19679a37cad8e93e6dee554b6a9b3b9b4bbe4a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62865
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Bug: chromium:1322914
Change-Id: I2efbb110747273188245530f9ab1964faba5201c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62825
Reviewed-by: David Benjamin <davidben@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
RBP pointed 8 bytes off of where it should be. I've left the RSP offsets
alone, though it does mean they're shifted by 8 from what they
previously were. Per Andres, the new version of CryptOpt will generate
an RBP-compatible prolog, but for now I've just fixed it up by hand.
(This part was already hand-written.)
Change-Id: I23720e76affff6fae46b8f85b0a509380ccc8bc0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62805
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
bug: chromium:1322914
Change-Id: I23b49ed6a9a739cddf17b0b4d9e26c74b7cb3de5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62785
Auto-Submit: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Frustratingly, simply writing the standard (void)write(...) does not
work because GCC is broken and intentionally leaves the warning enabled
there. This does not comply with the now standard semantics for
nodiscard.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425
Instead, what seems to work is to assign it to a variable and then
(void) the variable.
Fixed: 644
Change-Id: Ic418b4185aeae1a9ca424c45a05af063e8d50255
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62666
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Windows in chrome also does not like this
bug: chromium:1322914
Change-Id: I79c788e0b521964fdc07b530ec47d7fc3635e5a4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62765
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
Windows gets mad otherwise
bug: chromium:1322914
Change-Id: I3f0409ff9b397cb6a888f8c81642737721912cb0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62706
Auto-Submit: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I215f9090e12314bcc3b0e15f5e83b751fea42003
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62726
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This way the Chromium certificate verifier can more easily use them.
Bug: chromium:1322914
Change-Id: I51dafc4e70d74da8543688b6457563d78e298150
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62745
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Change-Id: I18ba860d28dd3fb55cc14904758d6a8dc95e3f89
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62725
Auto-Submit: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I368e98f7484bdafac8d8600a6b4d5d7013e08817
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62705
Auto-Submit: Bob Beck <bbe@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Where the Trusty and Android baremetal cases are unambiguously mistakes
in their respective builds, nanolibc is a bit more interesting.
nanolibc sometimes build for a non-Linux target (which should not define
__linux__), but also sometimes build for Linux. Although technically
running in Linux userspace, this lacks all the libc APIs we'd normally
expect on Linux, so we treat it as a non-Linux target.
Change-Id: Id36f6bbc6e790d96e31193532717630a86f124b8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62685
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
These are OpenSSL names for BN_bn2le_padded and BN_le2bn. We can just
replace BN_le2bn with BN_lebin2bn. BN_bn2lebinpad is not size_t-clean,
so handle it as a separate function like we did BN_bn2binpad.
Change-Id: I6999ca06140a0c8c25942362dc79d1821971d679
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62665
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Newer glibc have an attribute((nonnull(1))) on fclose. Attributes aren't
part of the language, so decltype(fclose) lose the attribute. It seems
this causes std::unique_ptr<FILE, decltype(fclose)> to trip
-Wignored-attributes in GCC.
This is a bit aggressive of a warning, but work around this with a
custom deleter, which makes the unique_ptr object smaller anyway.
(Though the compiler can, I hope, dissolve all of this anyway.)
Fixed: 642
Change-Id: I9a0206a8c5675f856e80c5266c90be42d66a5606
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62465
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
