Upstream merge 2024 05 10 #1590
Merged
Conversation
This file contains 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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1590 +/- ##
==========================================
+ Coverage 77.90% 78.03% +0.13%
==========================================
Files 561 562 +1
Lines 94632 94519 -113
Branches 13604 13562 -42
==========================================
+ Hits 73725 73761 +36
+ Misses 20315 20166 -149
Partials 592 592 ☔ View full report in Codecov by Sentry. |
nebeid
force-pushed
the
upstream-merge-2024-05-10
branch
3 times, most recently
from
eb1fce6
to
fff4fa3
Compare
nebeid
force-pushed
the
upstream-merge-2024-05-10
branch
from
fff4fa3
to
bc86971
Compare
nebeid
force-pushed
the
upstream-merge-2024-05-10
branch
from
bc86971
to
3bfef26
Compare
justsmth
approved these changes
May 17, 2024
Since it's otherwise pretty tedious, let's try this with C99 designated initializers. From testing, I remember they worked pretty reliably in C. (In C++, it's a little trickier because MSVC won't accept them outside C++20. Although I think all our supported MSVCs have a C++20 mode now...) AWS-LC: Changes to CMakeLists were not taken as that flag exists there already. Fixed: 671 Change-Id: Ia29ade8721ecfe2140a2d183ad60c8a730c631f0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64447 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> (cherry picked from commit a9a4c6dc89aff96f64c6ed93c1c3fc4d0c8e6e74)
X509_VERIFY_PARAM inheritance is unbelievably thorny, and I'm not sure it was ever thought through very well. We can make it slightly less complicated by removing the internal inh_flags value. We don't support X509_VERIFY_PARAM_set_inh_flags (and really should keep it that way!), so there are actually only two possible values, zero and X509_VP_FLAG_DEFAULT, used to implement X509_VERIFY_PARAM_inherit, and X509_VERIFY_PARAM_set1, respectively. This still leaves some weird behaviors that are expected through the public API, which I've documented in the headers. They'll probably need another pass when they're grouped into sections and whatnot. Bug: 441, 426 Change-Id: Ib0a855afd35e597c65c249627addfef76ed7099d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64253 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> (cherry picked from commit 51ae958ff7c99103932e07905c40cbc71a22b389)
This matches an upstream change. Add macros for the old names. We may be able to unexport these, but for now just pick up the less confusing names. (I found only one user of one of them, goma, which will be replaced next year. Though wanting *some* API to query the dirhash machinery is not completely implausible.) Change-Id: Idd2352c07c294c4f63c4ef12e5d97804f42225b9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64254 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> (cherry picked from commit 33a5e94645787d74593efa0e63200a31372325a2)
(Or second, if there's a method table.) CRYPTO_EX_free is passed the parent object, so the caller could, in principle, inspect the object. We should pass in an object in a self-consistent state. Also fix up the documentation. I think some bits of a since removed CRYPTO_EX_new got jumbled up in there. Change-Id: I316d00aee61bf544f59d4dac2efcd825e4bfa9b1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64255 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> (cherry picked from commit 59906b3aa8d9f48ad7303edc540912bd588a8e46)
Change-Id: Ifaef253aa82b07d0930dddbd773724132a7724c4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64587 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]> (cherry picked from commit c41de812877a5ba256ba78e64c06dcbe3c8343d3)
Get the remaining config APIs, extensions accessors, and the get1_email family. I'm not sure yet whether the various remaining extension-specific functions should get their own sections (probably), in which case, maybe we should move the accessors these into their sections? Put them with the rest of the certificate getters for now. As part of this, deduplicate the X509v3_KU_* and KU_* constants. See openssl/openssl#22955 Bug: 426 Change-Id: I31a9b887eb1e6cfa272f04d2ee80dbb5a9ed98f7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64256 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Bob Beck <[email protected]> (cherry picked from commit 314c2520eab450615d8e78df21169c090d6f51e5)
It turns out NSS and OpenSSL defined the same constants! All this time, Chromium has inadvertently relied on KU_* being defined in <openssl/x509v3.h> and not <openssl/x509.h>. Once we merged the two headers, this broke. Since we just deprecated these in favor of X509v3_KU_*, just move these back into <openssl/x509v3.h>. Change-Id: I93453527f30eee6df7630dc68c052c814aaeda02 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64607 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> (cherry picked from commit 540fcce9a5a8d92e4686d5d266dc19c91724ea0b)
I forgot half of https://boringssl-review.googlesource.com/c/boringssl/+/64607 Change-Id: Idbb827c5298c08bb1344272353ba4740802c55de Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64627 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> (cherry picked from commit 85e2f2c655a13e29988df777ed680ddf0969434d)
This is not thread-safe. Even if made thread-safe, it involves registering globals, so it's just not a good API. Note this means that there is no longer a way to configure custom trust OIDs or purpose checks. Evidently no one was doing that. Should a use case arise, I don't think it should be met by this API. The things one might want to configure here are: - Which OID to match against X509_add1_trust_object and X509_add1_reject_object - Whether self-signed certificates, if no trust objects are configured, also count as trust anchors - Which EKU OID to look for up the chain - Which legacy Netscape certificate type to look for (can we remove this?) - Which key usage bits to look for in the leaf We can simply add APIs for specifying those if we need them. Interestingly, there's a call to check_ca inside the purpose checks (which gets skipped if you don't configure a purpose!), but I think it may be redundant with the X509_check_ca call in the path verifier. Change-Id: If71ee3d0768b5fc71422852b4fcf7eb23e937dd2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64507 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> (cherry picked from commit 396f2ef0855fee63cd7fd2f60fc77b0a447b1dc7)
This gets the built-in tables out of mutable data. Update-Note: No one uses these APIs except for rust-openssl. rust-openssl may need fixes because they seem to not quite handle C const correctly. Change-Id: Ia23c61e2fe6a5637e59044e10ea2048cfe46e502 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64508 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> (cherry picked from commit 6099ab92c2447d41ab9e95f5c1bc8cfbc331439f)
STACK_OF(GENERAL_NAMES) is STACK_OF(STACK_OF(GENERAL_NAMES)). Nothing uses this. It appears to be a remnant of CMS and indirect CRL support. May as well trim the header slightly. STACK_OF(X509_VERIFY_PARAM) is a remnant of (non-thread-safe) global registration of X509_VERIFY_PARAMs. STACK_OF(X509_LOOKUP) is only used internally. May as well prune them from the header so the file expands to be a bit less code. Update-Note: A few obscure STACK_OF(T) types are unexported. This is not expected to impact anyone. Change-Id: I03757c8522531132a31270b6dab055966b6e9070 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64527 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> (cherry picked from commit e6489902b7fb692875341b8ab5e57f0515f47bc1)
Update-Note: In the process, unexport the ASN1_ITEMs, and the d2i/i2d functions for OTHERNAME and EDIPARTYNAME. These do not appear to be used and removing them will cut down on the amount of compatibility glue needed when we rewrite the parsers with a safer calling convention. Bug: 426 Change-Id: Ifc45867c0a0c832e5ef72deaec5a2c88b8d8ac6a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64628 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> (cherry picked from commit e89d99af0e4d7fb1df4d961d7aafdfed30d08d41)
nebeid
force-pushed
the
upstream-merge-2024-05-10
branch
from
3bfef26
to
ec57e90
Compare
skmcgrail
approved these changes
May 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 considering commits
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.