Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Upstream merge 2024 07 09 #1694

Merged
merged 14 commits into from
Jul 19, 2024
Merged

Upstream merge 2024 07 09 #1694

merged 14 commits into from
Jul 19, 2024

Conversation

nebeid
Copy link
Contributor

@nebeid nebeid commented Jul 9, 2024 •
edited
Loading

Description of changes:

Merging from Upstream considering commits between google/boringssl@45f5e5d (Jan 10, 2024) and google/boringssl@f42be90 (Jan 19, 2024).
(This PR is to be rebased on #1661)

Call-outs:

See internal document as well as "AWS-LC" notes inserted in some of the commit messages for additions/deviations from the upstream commit.

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.

@nebeid nebeid requested a review from a team as a code owner July 9, 2024 16:04
@justsmth justsmth requested review from justsmth and skmcgrail July 9, 2024 17:33
@nebeid nebeid force-pushed the upstream-merge-2024-07-09 branch 4 times, most recently from cbf3dd6 to 084be3f Compare July 9, 2024 18:24
@codecov-commenter
Copy link

codecov-commenter commented Jul 9, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 88.70056% with 20 lines in your changes missing coverage. Please review.

Project coverage is 78.27%. Comparing base (4ac1742) to head (e2f938e).
Report is 185 commits behind head on main.

Files with missing lines Patch % Lines
crypto/fipsmodule/cipher/e_aesccm.c 80.00% 11 Missing ⚠️
crypto/conf/conf.c 89.55% 7 Missing ⚠️
crypto/fipsmodule/bn/gcd.c 77.77% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1694      +/-   ##
==========================================
- Coverage   78.28%   78.27%   -0.01%     
==========================================
  Files         572      573       +1     
  Lines       95803    95828      +25     
  Branches    13733    13731       -2     
==========================================
+ Hits        75003    75014      +11     
- Misses      20198    20211      +13     
- Partials      602      603       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nebeid nebeid force-pushed the upstream-merge-2024-07-09 branch 5 times, most recently from c3e33a6 to f157348 Compare July 16, 2024 15:37
@nebeid nebeid force-pushed the upstream-merge-2024-07-09 branch from f157348 to d16fd5b Compare July 19, 2024 13:05
justsmth
justsmth previously approved these changes Jul 19, 2024
View reviewed changes
davidben and others added 14 commits July 19, 2024 13:41
@davidben @nebeid
Move CRL_REASON_* back to x509v3.h
7fb4bd7 
strongswan defines conflicting symbols and has been relying on them only
being defined in <openssl/x509v3.h>. Defining the constants in
<openssl/x509.h> would break strongswan, so move them back for now.

Long term, we would like for new code to only need <openssl/x509.h>, so
I've left a TODO to introduce properly namespaced versions of these
constants and, separately, see if we can fix strongswan to similarly
avoid the conflict. Between OpenSSL, strongswan, and wincrypt.h all
defining these constants, it seems best for everyone to just avoid them
going forward.

Change-Id: I23ce4c5013a80a831e0dc74fda8623027017190c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65387
Commit-Queue: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 45f5e5da1235b13220599fa04b7a648f29db80c3)
@skmcgrail @nebeid
Fix segfault if CRYPTO_set_thread_local fails and calls rand_thread_s…
6b51185 
…tate_free.

If BORINGSSL_FIPS is defined, then rand_thread_state_free will attempt to manage the next and prev
pointers. These will be set to undefined values in the case of CRYPTO_set_thread_local calling rand_thread_state_free,
as theese pointers are not initalized until later on.

AWS-LC:
- The change was already made in
aws@3b58566
So this commit contains the comment.

Change-Id: Ie7712e7eb4acec7cca6890cfabc2030f18ab5244
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63885
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
(cherry picked from commit 9206d7ca41065a0e3c7cf0ba64f3bf496665770a)
@davidben @nebeid
Clear some false positives in constant-time validation
0121ec6 
These were flagged when patching the PRNG to return data marked secret.
This CL doesn't include that part (I didn't run all tests, and other
tests likely require further annotations), but it does include some of
the declassifications necessary to do that later.

Broadly:

- Whenever we compute public keys from private keys, we must tell
  valgrind the public key is now public. (Valgrind does not know
  that cryptography works.)

- Whenever we compute signatures from private keys, we must tell
  valgrind the signature is now public. (Ditto.)

- Whenever we pass a secret value but check it is fully reduced, we must
  tell valgrind the comparison may be leaked. (Valgrind doesn't know
  these values are always within range... that we have to check at all
  is a consequence of OpenSSL's API and/or defensive coding.)

- Valgrind does not know about the randomizing properties of blinding.
  We actually aim to be constant-time without RSA blinding, so that
  doesn't need an annotation, but the blinded inversion step in the
  process of computing the RSA blinding factor does.

Bug: 676
Change-Id: Ic3a47adddb23a61fe452b9be27b214eec2ea5235
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65367
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
(cherry picked from commit b628f8721e7e67302b0e26c92e0108a31066194e)
@briansmith @nebeid
Simplify Montgomery RR precomputation.
96360bd 
Use equivalent but simpler math, and explain the simpler math. Move the
discussion of multipying-vs-doubling to be after the discussion of
squaring-vs-doubling so that the discussion order follows the code
order, and so that we can combine the multipying-vs-doubling discussion
with the explanation of why no multiply/doubling is needed at all.
Expand the existing discussion to be a little more explicit.

Retain |threshold|, but change the type of |threshold| was changed to
|int| to avoid a signed/unsigned comparison in the added assertion
(|bn_mod_lshift_consttime| takes the shift count as an |int| anyway).

Change-Id: I24e4687e76944a34a8621b5f2fdee15a5201ac88
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63906
Reviewed-by: Bob Beck <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
(cherry picked from commit 3c88240d2c5a19be36a9980936006a0f5ee4ab76)
@davidben @nebeid
Make the debug vs release build note in BUILDING.md more prominent
2de02f6 
Change-Id: I3549fb623db267df5956c9412d758749abd2a4dc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65468
Commit-Queue: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit b96e8166f35ec679cb5afe94ea4581a373f25f66)
@davidben @nebeid
Elaborate a bit on static vs dynamic EC_GROUPs in documentation
9d08b7c 
Also move EC_GROUP_free and EC_GROUP_dup to the deprecated section,
because it's now usually unnecessary.

Change-Id: Ica4aa8d2b993555f977792bf284a5dbf29a3d710
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63245
Commit-Queue: David Benjamin <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
(cherry picked from commit c394713d0d84558865a5ed434310e8fefe445be6)
@davidben @nebeid
Replace CONF's internal representation with something more typesafe
cc932cf 
Sections are stored in a CONF structure as having name == NULL and value
being a STACK_OF(CONF_VALUE) with the wrong pointer type. This loses
type safety and complicates all the cleanup functions. (E.g.
crypto/x509 has its own X509V3_conf_free which is distinct from the copy
in crypto/conf.c.)

These objects are, happily, never exported outside the file. Replace
them with a CONF_SECTION and store the two values in separate hash
tables. This also means a CONF_VALUE's name is no longer nullable, so
all the comparisons and hashes become simpler.

Also fix up add_string slightly. It left the CONF in a slightly
precarious state if a malloc failed in the middle. Also v->section
would leak if add_string failed.

Change-Id: Ib54e9dd5037766804c8ddcd80d357237d2d357ea
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60106
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
(cherry picked from commit 313f9b0c6034e3337f50a91466bd8977442bdc21)
@davidben @nebeid
Fix a bug detecting BER deeply nested inside DER
3dd2af8 
When BER was nested deep inside otherwise valid DER, cbs_find_ber's
internally book-keeping would clobber *out_ber_found and lose track that
there was some BER to convert.

This is unlikely to come up in real world inputs, as the only reason to
use any of these awful BER features is to stream the output. I.e., there
is no reason to have indefinite-length inside definite-length, and there
is no reason to have definite-length constructed string. That means
cbs_find_ber, realistically, can just check for indefinite-length
encoding at the front and do nothing else.

Nonetheless, these inputs are nominally allowed, so fix the book-keeping
bug.

Change-Id: Idaf3c178fa511ce24af83d6ae27fa9f197f1789e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65507
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
(cherry picked from commit 520d211b842079f6343803c9635422a29e9e12df)
@davidben @nebeid
Reduce the BER conversion recursion depth
41675ba 
2048 is too high, particularly in heavily instrumented fuzzer builds
with large stack frames. Use a much more conservative limit.

Change-Id: If0b49f2ca04520c41400dcbfd83463766fded3a9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65508
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 7c1433eb1959ef7579cb460aab464bc0441467e3)
@davidben @nebeid
Clarify that X509_NAME_hash(_old) are specific to hash-dir
b84067e 
I'll also group them with hash-dir functions later, but it's worth
clarifying this inline.

Change-Id: Ia9166dd75d2546f8f2e5cf627823ec7206cf9fdc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65427
Reviewed-by: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 083f72d726097b4abb67982315adc5f7ceb5a69a)
@nebeid
Reworking bssl_crypto: support AES-GCM-SIV open_gather.
e2717ef 
On x86-64, the Rust tests notice that open_gather wasn't implemented for
AES-GCM-SIV when the assembly code is used. Fix that.

AWS-LC:
This change does not include rust code changes, but fixes to the support
of AES-GCM-SIV for open_gather.

Change-Id: I7de9d009922bcc0ed7e3e2f8a7a3cfb671893d63
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65187
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit e5d6b2fbb45b5638cc9a4c98508748436cb07a44)
@nebeid
Fix AES-GCM-SIV with huge inputs on 32-bit.
4e4c0cd 
The asm code is 64-bit only, so multipling a `size_t` by eight to get a
number of bits is valid and the bounds on the inputs are checked
accordingly. But on 32-bit, that calculation will overflow for huge
inputs.

Change-Id: I6d2171becd6b6259593b2aa80105d8cae1ec7ed4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65188
Reviewed-by: David Benjamin <[email protected]>
(cherry picked from commit a32596b0545d2f6192a6a1be2f8e2c7c4f0c8f44)
@davidben @nebeid
Avoid unions in CCM
26ccfec 
I believe this one was well-defined in C, because we never take the
address of the uint64_t arm, but this is fragile.

AWS-LC:
UBSAN complained about the new alignment of CIPHER_AES_CCM_CTX;
- As with EVP_AES_GCM_CTX, padding was added to ctx_size and a function
to retrieve the aligned cipher_ctx from within EVP_CIPHER_CTX.
- 32-bit build on x86 needed a CUSTOM_COPY implementation.

Bug: 574
Change-Id: I439801a3e0564b731f119c520096311f731523a5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65607
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
(cherry picked from commit f42be90d665b6a376177648ccbb76fbbd6497c13)
@nebeid
Update build files in generated-src.
e2f938e
@nebeid nebeid force-pushed the upstream-merge-2024-07-09 branch from d16fd5b to e2f938e Compare July 19, 2024 17:45
@nebeid nebeid merged commit 96dcfd0 into aws:main Jul 19, 2024
101 of 103 checks passed
@skmcgrail skmcgrail mentioned this pull request Aug 1, 2024
Merged
skmcgrail added a commit that referenced this pull request Aug 1, 2024
@skmcgrail
Prepare Release for v1.33.0 (#1734)
a265ac9 
## What's Changed
* Added options to x509 tool by @ecdeye in
#1696
* Add support to detect Neoverse V2 cores by @andrewhop in
#1706
* Move OCSP functions for Ruby out of internal.h by @samuel40791765 in
#1704
* Add aes-256-xts to EVP_get_cipherbyname by @torben-hansen in
#1707
* Match using CMAKE_SYSTEM_PROCESSOR_LOWER by @justsmth in
#1709
* Update MySQL to 9.0.0 by @skmcgrail in
#1685
* [EC] Unify scalar multiplication for P-256/384/521 by @dkostic in
#1693
* Adds const qualifier to ciphertext parameter in EVP_PKEY_decapsulate
by @maddeleine in #1713
* Upstream merge 2024 06 24 by @nebeid in
#1661
* NIST SP 800-108r1-upd1: KDF Counter Implementation by @skmcgrail in
#1644
* Upstream merge 2024 07 09 by @nebeid in
#1694
* Design for support of HMAC precomputed keys by @fabrice102 in
#1574
* Fix for select point from table in ec_nistp scalar_mul by @dkostic in
#1719
* X509toolcomparison by @ecdeye in
#1714
* AWS-LC s2n-bignum update 2024-07-22 by @dkostic in
#1718
* Add OpenVPN to CI by @smittals2 in
#1705
* Lower required Go version, add CI test for specific version by
@andrewhop in #1717
* ec2-test-framework enhancements and graviton 4 testing by
@samuel40791765 in #1715
* sha + chacha: Move AArch64/X86-64 dispatching to C. by @justsmth in
#1625
* Show number of pruned ec2 instances in dashboard by @samuel40791765 in
#1728
* rsa and md5 tools by @ecdeye in
#1722
* FIPS 203 IPD update: ML-KEM-IPD-768 and ML-KEM-IPD-1024 by @jakemas in
#1724
* bump mysql CI to 9.0.1 by @samuel40791765 in
#1727
* Support utility OCSP request functions by @samuel40791765 in
#1708
* add support for OCSP_SINGLERESP functions by @samuel40791765 in
#1703
This was referenced Aug 1, 2024
Closed
Merged
@nebeid nebeid deleted the upstream-merge-2024-07-09 branch December 10, 2024 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lilpoozie2005 lilpoozie2005 lilpoozie2005 approved these changes

@justsmth justsmth justsmth approved these changes

@skmcgrail skmcgrail skmcgrail approved these changes

Assignees
No one assigned
Labels
reviewers-assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@nebeid @codecov-commenter @skmcgrail @justsmth @lilpoozie2005 @davidben @briansmith