Upstream merge 2023 07 28 #1122
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
samuel40791765
force-pushed
the
upstream-merge-2023-07-28
branch
from
6de5351 to
012cf44
Compare
samuel40791765
force-pushed
the
upstream-merge-2023-07-28
branch
from
012cf44 to
1e799d8
Compare
samuel40791765
force-pushed
the
upstream-merge-2023-07-28
branch
from
1e799d8 to
4debe34
Compare
torben-hansen
previously approved these changes
Aug 1, 2023
samuel40791765
force-pushed
the
upstream-merge-2023-07-28
branch
2 times, most recently
from
40bd012 to
fb392ea
Compare
nebeid
force-pushed
the
upstream-merge-2023-07-28
branch
from
d4e499e to
75021f0
Compare
rsa_test.cc and a few of the tests in evp_tests.txt still do, but refresh the easy one. I switched it to a P-256 key to keep the input small. Bug: 607 Change-Id: Ie0614280d12012bbd928f095eb4531e4d7550ae1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59666 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> (cherry picked from commit 722f5d878d3d8ed387738417f406f9aa9b9fd936)
Envoy needs to have the possible cipher, etc., strings predeclared to reduce synchronization needs in the steady state. It currently does this by (1) iterating over SSL_CTX_get_ciphers at SSL_CTX creation time and (2) hard-coding a lists of known TLS 1.3 ciphers, TLS versions, NamedGroups, etc. (1) would work for some applications, but it breaks any applications that configure ciphers on the SSL on a certificate callback, etc. If the callback configures a cipher that wasn't configured on the SSL_CTX (e.g. if the SSL_CTX were left at defaults), Envoy's logging breaks and we hit an ENVOY_BUG assertion. (2) breaks whenever BoringSSL adds a new feature. In principle, we could update Envoy when updating BoringSSL, but this is an unresasonable development overhead for just one of many BoringSSL consumers to impose. Such costs are particularly high when considering needing to coordinate updates to Envoy and BoringSSL across different repositories. Add APIs to enumerate the possible strings these functions can return. These string lists are a superset of those that any one application may care about (e.g. we may have a deprecated cipher that Envoy no longer needs, or an experimental cipher that's not yet ready for Envoy's stability goals), but this is fine provided this is just used to initialize the table. In particular, they are *not* intended to enumerate supported features. Bump BORINGSSL_API_VERSION to aid in patching these into Envoy. Bug: b:280350955 Change-Id: I4d11db980eebed5620d3657778c09dbec004653c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59667 Commit-Queue: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> (cherry picked from commit a972b78d1b11009cd07852fb4be2cc938489e031)
Bug: 574 Change-Id: Ief8030a4c9257a05defdc2dc02d496c63f06626a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59545 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> (cherry picked from commit c6dd304d2c628277b710ab50ce9eed660696756d)
Change-Id: I1a4914cc4859ed1e0797a046a1abec8921c4a9cf Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59746 Commit-Queue: Bob Beck <[email protected]> Reviewed-by: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]> (cherry picked from commit 4137759c20c7d72b5c48bce4d00d687926d279e6)
Trying to fix all the places where these formats go quadratic isn't a good use of time. We've already documented that they're not safe for use with untrusted inputs. Even without such DoS issues, they cannot be safely used anyway. (E.g. RUSTSEC-2023-0023.) Just cap the fuzzer input. It'd be nice if we could avoid this more systematically in the function, but they're not structured to make this easy to do, and anyone concerned about DoS in this function has worse problems. Bug: chromium:1444420, oss-fuzz:56048, 611 Change-Id: I53eeb346f59278ec2db3aac4a92573b927ed8003 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59785 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> Auto-Submit: David Benjamin <[email protected]> (cherry picked from commit b92fcfdc17f3ad794c220a86f4ae6695d0a0fb61)
Saves some duplicated logic. Change-Id: I202fa92a88101f9ad735648bc414ab05752641da Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59685 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> (cherry picked from commit c215ce7e8230786e0d4ec463d95a9e44af513e6a)
This was a bit of a mess. There are three assembly functions to juggle
here. Their current type signatures are:
void gcm_init_v8(u128 Htable[16], const uint64_t H[2]);
void gcm_gmult_v8(uint64_t Xi[2], const u128 Htable[16]);
void gcm_ghash_v8(uint64_t Xi[2], const u128 Htable[16], const uint8_t *inp,
size_t len);
Except for gcm_nohw.c, this is all assembly, so they don't follow the C
abstract machine's theory of typed memory. That means types are mostly
arbitrary and we have room to rearrange them. They do carry an implicit
alignment requirement, but none of these assembly files care about
this[*].
Values passed to gcm_gmult and gcm_ghash get XORed byte-by-byte in
places, which is inconvenient to do as uint64_t. They also get passed to
AES functions, which want bytes. Thus I think uint8_t[16] is the most
natural and convenient type to use.
H in gcm_init is interesting. gcm_init already doesn't take a GHASH key
in the natural byte representation. The two 8-byte halves are
byte-swapped, but the halves are not swapped, so it's not quite a byte
reversal. I opted to leave that as uint64_t[2], mostly to capture that
something odd is happening here.
[*] We only have GHASH assembly for x86, x86_64, armv7, and aarch64. We
used to have armv4 GHASH assembly, but that's been removed from
gcm_nohw.c. Thus we can assume none of these files care about alignment
for plain scalar loads. Alignment does matter for vmovdqa vs vmovdqu,
but that requires 16-byte alignment and uint64_t only implies 4- or
8-byte alignment on these architectures.
AES-LC:
In modes/internal.h:
- static_assert was replaced with OPENSSL_STATIC_ASSERT which was removed
from modes/cbc.c
- Xi parameter in gcm_<gmult|ghash>_avx512 and aesv8_gcm_8x_*
was changed from uint64_t to uint8_t to match the rest of this change.
Bug: 574
Change-Id: If7dba9b41ff62204f4cf8fcd54eb4a4c54214c6e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59528
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
(cherry picked from commit 5b32e81407cd044e76c995eb99343dca954a17b8)
samuel40791765
force-pushed
the
upstream-merge-2023-07-28
branch
from
75021f0 to
b020fa0
Compare
torben-hansen
approved these changes
Aug 3, 2023
skmcgrail
approved these changes
Aug 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes:
Merging from Upstream from google/boringssl@5b32e81 to google/boringssl@c215ce7
Call-outs:
See internal document as well as "AWS-LC" notes inserted in some of the commit messages for additions/deviations from the commit changes.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and
the ISC license.