Merge BoringSSL through 20c93abd47726624ab3e479466078f7e63f081f7 #2264
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
This files aren't built and don't build because of a fillins dependency. Change-Id: I3466fb50298922cfb21c9f60950d572df0d64ca8 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65907 Commit-Queue: Bob Beck <[email protected]> Reviewed-by: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]>
Update-Note: Building for 32-bit x86 may require fixing your builds to pass -msse2 to the compiler. This will also speed up the rest of the code in your project. If your project needs to support the Pentium III, please contact BoringSSL maintainers. As far as I know, all our supported 32-bit x86 consumers require SSE2. I think, in the past, I've asserted that our assembly skips SSE2 capability detection. Looking at it again, I don't think that's true. OPENSSL_IA32_SSE2 means to enable runtime detection of SSE2, not compile-time. Additionally, I don't believe we have *ever* tested the non-SSE2 assembly codepaths. Also, now that we want to take the OPENSSL_ia32cap_P accesses out of assembly, those runtime checks are problematic, as we'd need to bifurcafe functions all the way down to bn_mul_words. Unfortunately, the situation with compilers is... complicated. Ideally, everyone would build with the equivalent of -msse2. 32-bit x86 is so register-poor that running without SSE2 statically available seems especially foolish. However, per https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9868, while Clang defaults to enabling SSE2, GCC does not. We once broke gRPC's build, in grpc/grpc#17540, by inadvertently assuming SSE2. In that discussion, gRPC maintainers were okay requiring Pentium 4 as the minimum CPU, but it's unclear if they actually changed their build. That discussion also said GCC 8 assumes SSE2, but I'm not able to reproduce this. LLVM does indeed interpret "i686" as implying SSE2: llvm/llvm-project#61347 rust-lang/rust#82435 However, Debian LLVM does *not*. Debian carries a patch to turn this off! https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/blob/snapshot/debian/patches/disable-sse2-old-x86.diff?ref_type=heads Meanwhile, Fedora fixed their baseline back in 2018. https://fedoraproject.org/wiki/Changes/Update_i686_architectural_baseline_to_include_SSE2 So let's start by detecting builds that forgot to pass -msse2 and see if we can get them fixed. If this sticks, I'll follow up by unwinding all the SSE2 branches. Bug: 673 Change-Id: I851184b358aaae2926c3e3fe618f3155e71c2f71 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65875 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
After all, we have to keep this robust, modern cipher conforming to C well-definedness expectations! These functions should have simply taken uint8_t* pointers. Make internal _ex variants that fix this. I've not bothered updating the public APIs because it will cause a ton of downstream churn and make those APIs even more OpenSSL-incompatible. (OpenSSL's APIs take a const-incorrect uint8_t (*in)[8]. Both our struct and their pointers expect callers to call with &foo.) This does not seem worth the trouble. Also since the underlying functions now access as uint8_t*, I suspect this broadly fixes strict aliasing issues with callers that cast from a byte array. (Though perhaps in->bytes should be (const uint8_t*)in?) Ideally c2l and l2c would be replaced with CRYPTO_load_u32_le and CRYPTO_store_u32_le. (It's a little rude for a header to squat those names, especially when those name often vary in endianness.) I did that in a couple places where we'd otherwise increment a pointer declared with the funny array parameter syntax. Otherwise I left it alone for now. Fixed: 683 Change-Id: I7b0d8b2a16697095ebf42a71482c4ba805a193e4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65690 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
…structed GeneralNames object for permitted names. Change-Id: Ic9520ddcde12e3df61479f2cf4a95c29d1f1f5f2 Bug: chromium:1477317 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65707 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Matt Mueller <[email protected]>
from libressl Change-Id: Iaec558fb6d3c698deb000c45b34aa94911e60a06 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65967 Auto-Submit: Bob Beck <[email protected]> Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]>
See discussion in https://boringssl-review.googlesource.com/c/boringssl/+/65967/comment/4b0fb2a6_78bfab3a/ struct tm is defined with tm_ values as all ints. Conversion inside this code is all bounds checked around valid times and can't overflow, but because struct tm uses 1 based months and 1900 based years, we need to modify the input values when converting a struct tm. Rather than do awkward bounds checks, just accept an int64_t on input and we don't have to care. OPENSSL_gmtime_adj gains checks to ensure the cumulative days and seconds values passed in may not overflow. While we are here we also ensure that OPENSSL_gmtime_adj does not modify the output struct tm in the failure case. Also while we are here, just make the offset_seconds value of OPENSSL_gmtime_adj an int64_t - because long was never the correct type. Add tests for all this. Change-Id: I40ac019c4274b5388c97736cf85cede951d8b7ae Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66047 Commit-Queue: Bob Beck <[email protected]> Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> Auto-Submit: Bob Beck <[email protected]>
RSAES-PKCS1-v1_5 is broken and should be described as such. Change-Id: I11f74fbfcef7b44579a333798240147f67cf896c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66107 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Change-Id: I010f4dcf4d284e09326611c346369f74f65519cd Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66087 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Ok so this one is purely for keeping this the same between forks. Boring today doesn't need this (although some of our outside uses might be able to use it). Effectively, this function is the same as converting to a posix time, followed by a safe check to see if you can put the result in a time_t. posix_time.h is about to get added as public in LibreSSL, and while not strictly necessary there because they could inline what it does, Libre is finding that may be needed in rpki-client, ocsp-client, as well as libtls (including for libtls portable). In the interests of keeping the same API in the same file, Libre would like this to be here so it's just consistent from both places. Change-Id: I2f234005e83cdea5e61fa41fbf03ef61516c45f8 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66127 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]>
Re-ran clang-format on those regions, now that clang-format knows about STACK_OF(T). Change-Id: Ied350343b8aaef0dc25dbe3f215c1fae25af90c4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66147 Auto-Submit: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]>
These can replace the current der::Input-specific string_view methods. I converted many of the uses within the library, but we should take a pass over the library and turn many of the std::strings into std::vector<uint8_t>. On MSVC, we have to pass /Zc:__cplusplus for __cplusplus to be set correctly. For now, I'm going to try just adding the flag, but if pki gets used in more places before we bump our minimum to C++17, we may want to start looking at _MSVC_LANG instead. There are actually a few places outside of pki that could use this, but we sadly can't use them until we require C++17 across the board. Bug: 661 Change-Id: I1002d9f2e1e4a2592760e8938560894d42a9b58c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65908 Reviewed-by: Bob Beck <[email protected]> Reviewed-by: Matt Mueller <[email protected]> Commit-Queue: David Benjamin <[email protected]>
This matches upstream OpenSSL. It's also counting a total number of bytes, not a single buffer. On a 32-bit platform, one may still transfor more than 4GiB of data through a single BIO. Change-Id: I1c668d84ee5ce13f7ab5c476cb168ae9c0e0109e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66167 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]>
when building chains. Change-Id: I500b4a01e62c020548a88b5cc61db3bfaba4e06b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65767 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]>
This is effectively a derivation of certificate.cc from chromium_certificate_verifier, without absl. Bug: 660 Change-Id: I628827a00ab80a733582eee5a25e048dd65bf6b9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64171 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]>
MinGW defines __GNUC__, but the needed assembly file isn't built on Windows. See https://stackoverflow.com/questions/77983104/grpc-for-c-dependency-build-failed-curve25519-64-adx-h16-undefined-reference Change-Id: Idd1f12a5ab030ddd11e7d6e5a44a112acd6096d0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66189 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: David Benjamin <[email protected]>
Currently the way this is used by code checking for errors is to always assume that what it is looking for has actually been added as an error. If one of them were added as a warning, it would in theory do the wrong thing. We can either delete the warnings, or just change ContainsError to only consider errors to behave like the code is used today. Change-Id: Id916281203122fffd1ffc323dc979ff4f8b6425b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66187 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]> Reviewed-by: Matt Mueller <[email protected]>
Update-note: This moves signature_verify_cache.h from the pki directory to <openssl/pki/signature_verify_cache.h>. Nothing should really have been depending on this but if so it can be pre-conditioned on OPENSSL_API_VERSION of 30 or more. Bug: 660 Change-Id: Ia0ab6e9d16da2a86994ce0dfc6a086e43b79c9ae Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64787 Reviewed-by: David Benjamin <[email protected]>
Change-Id: I907932b04547c4cc779ec4e1edbebf2b12162ffa Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66247 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
This is not sufficient to get the correct type on these constants, because bindgen is broken and does not correctly bind constants. But in preparation for that bug being fixed, we should at least mark the headers correctly. Bug: 636 Change-Id: Ib42a2d4f42a84b385dd7bd286564ccfe457923eb Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66227 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
This will be needed for python/cpython#114573. Along the way, document the various functions that expose "query from X509_STORE". Most of them unfortunately leak the weird caching thing that hash_dir does, as well as OpenSSL's generally poor handling of issuers with the same name and CRL lookup, but I don't think it's really worth trying to unexport these APIs. Change-Id: I18137bdc4cbaa4bd20ff55116a18f350df386e4a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65787 Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
This is only used internally, for X509_PURPOSE_ANY to mark that it has no corresponding trust value. Countrary to the name, this doesn't mean to use the default X509_TRUST behavior, but to make it impossible to configure via X509_STORE_CTX_set_purpose. Since it's only used in one place, as any value that fails lookup, I've just put a local define in v3_purp.c. Change-Id: Id3e44c08528a303132ef09d0a94521af67cc2230 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65212 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
X509_PURPOSE can't be fully trimmed because rust-openssl uses a few APIs to look up purposes by string. Change-Id: I39e3cec4d8b01ecf7dec1f368fabea4a82eff8e9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65788 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
OpenSSL's API uses this weird "index" intermediate integer representation, which is the same as the ID but offset bit. Just use the IDs throughout. Also document and deprecate the string-based APIs that rust-openssl uses. As a bonus, we remove some int/size_t casts. Change-Id: I3ffd2ab59bf3c9d96014a028b667b0bd3288b16b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65789 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
These were mostly already documented, but fit the current style. Add a couple tests for some interesting cases. With this, all we have left to document are: - Built-in and custom extensions - Filesystem-based X509_STORE bits - The APIs to query X509_STORE (mildly annoying because the sort-of-a-cache-sort-of-not thing is exposed) Bug: 426 Change-Id: I68c16071b8781f560e6601fd65a7fba9b6efe862 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65790 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
This removes the need to hand-write rust_wrapper.c, because bindgen can generate it for us. bindgen 0.65 or later is needed. Earlier versions of this were buggy. I've also removed the claim that bssl-sys is somehow a solution for version skew. That was the original thinking from Android, but it hasn't worked out. The version skew solution is simply "use bindgen, don't handwrite bindings". Android are quite behind their originaly July timeline for adding the build half of this mechanism, but as this is now in the way of other work, we're going to proceed with using this now. There is now a unsupported_inline_wrappers cfg that Android can set to use the old mechanism. Update-Note: Rust support now requires your build correctly handle --wrap-static-fns. On Android, you will need to enable the unsupported_inline_wrappers cfg option until b/290347127 is fixed. Chromium doesn't actually use any of the inline functions yet, so we can handle --wrap-static-fns asynchronously, but I have a CL ready to enable that. Fixed: 596 Change-Id: I51fd1108a8c17a06f1bdd9171ebf352cea871723 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58985 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Bob Beck <[email protected]>
Change-Id: I82cddddba8752e1e6c95cfc64dd0b2cbd2870944 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66267 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Bob Beck <[email protected]> Auto-Submit: Adam Langley <[email protected]>
Things like Swift, which attempt to precompile all our headers in C mode, cannot cope without this. Change-Id: If59229f5e7cb617da370b65bb39a4af1fbe3f70e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66268 Reviewed-by: Bob Beck <[email protected]> Auto-Submit: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]>
Due to rust-lang/rust-bindgen#923, bindgen does not evaluate constants correctly. This means arithemetic is done with the wrong type, and more importantly the output has the wrong type. Ultimately, this is a bug in bindgen, but as that's remains unfixed, we'll have to work around it. rust-openssl's bindgen mode works around this by using the build.rs bindgen driver and registering a callback to fix the type. This won't work for some of our consumers, which require a hermetic and reproducible builds. Instead, apply bssl-sys's workaround at the lib.rs level. This removes a divergence between bssl-sys and rust-openssl's bindgen mode. Fixing these types does not mean we recommending using all of these constants! Many of the options here are ill-defined or produce even more ambiguous output than most. XN_FLAG_COMPAT is especially fun because it change the calling convention! The only option anyone should use is XN_FLAG_RFC2253, as that's at least a well-defined output. Fixed: 636 Change-Id: Id34b4a46e0cfd6dcb275477d9bb915bda66c787d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66228 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
This is a very thin wrapper over CBS_ASN1_* constants, dating from when the library wanted to avoid as strongly depending on BoringSSL. That's long stopped being the case, so we may as well just use the underlying constants. A tad less C++-y, but I don't think the wrapper is buying us much. Update-Note: pki/tag.h no longer exists. Use CBS_ASN1_TAG instead of bssl::der::Tag and CBS_ASN1_* instead of bssl::der::k*. Change-Id: I09f3f65b406d1a8fe60bfbe1a56d3e485ae84d97 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66067 Commit-Queue: David Benjamin <[email protected]> Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
It's less bad than I originally wrote because trust properties only matter if configured on the X509_STORE. Add a test for this. This is good because lots of functions trigger d2i_X509_AUX, so I think we have to assume attackers can specify these values. Nonetheless, this is surprising, so document which functions trigger this. Change-Id: I73ce44acfa2a373ef3f3ef09c3e46cea98124f33 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65791 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
I believe the original blocker, gRPC, is now cleared. While I'm here, make check_imported_libraries print all errors, rather than the first one. (Though if this sticks, we can probably remove this script. It was only needed for the C++ runtime check.) Update-Note: libssl now requires a C++ runtime, in addition to the pre-existing C++ requirement. Contact the BoringSSL team if this causes an issue. Some projects may need to switch the final link to use a C++ linker rather than a C linker. Change-Id: I94808bf1dad6695ef334e262f3d2426caab0520e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66288 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Probably we could remove this altogether. The new verifier doesn't support nameRelativeToCRLIssuer. Change-Id: Ibb2210d513827577656d816fad90f658c2875601 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65792 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
There are still a pile of functions left to document, but we're far enough now that the doc generation is happy to run on this header. Go ahead and start generating output. Bug: 426 Change-Id: I4c807d625df3a4a881936e99b5a3fc6559cda6c9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65793 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
The latest Rust toolchain has started flagging these (correctly) as unused. Change-Id: I702e0ac7bfe47c7546e44debf7f53c4ade8e5dd6 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66328 Commit-Queue: Bob Beck <[email protected]> Reviewed-by: Bob Beck <[email protected]> Auto-Submit: Adam Langley <[email protected]>
Upstream OpenSSL will, by default, register an atexit callback to free globals, even though the process is going to be destroyed anyway. Not only is this pointless, but it introduces UAFs on race condition if some thread outlives the main thread. Some projects have started to notice this, and use OPENSSL_INIT_NO_ATEXIT from 1.1.1b. Add the constant for compatibility. We already do not call atexit, so the option is a no-op. Change-Id: Ia17c529c27646507100ebb69c884e2db9cb70431 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66347 Commit-Queue: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]>
It's possible that https://boringssl-review.googlesource.com/c/boringssl/+/66268 wouldn't be needed if we split these back up. Update-Note: Downstream Bazel and GN builds that build libpki may need to also list the pki_headers variable. Change-Id: I61bfc231a9a8019f81b1ad4f7d43a6c4478b7283 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66407 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]>
Although the comments say draft-03, we're currently on draft-06. dcd6e44 forgot to update all the comments. The final RFC is identical to draft-06, except expected_cert_verify_algorithm was renamed to dc_cert_verify_algorithm, so this is just changing comment and renaming something. While I'm here, write the codepoint in decimal instead of hex, to match the document and how the other IANA codepoints are written out. Change-Id: I6d1f362a21eecafeef5bba5879f4158e31c8def4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66367 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]>
We always pass this, so checks are redundant. Note this doesn't control the SSE2 runtime checks, just whether SSE2 code is emitted. Change-Id: I159806928643915afecf738dcac218007ba94600 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65869 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
… on Windows. We were already doing this; just the conditions are reordered.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2264 +/- ##
=======================================
Coverage 96.81% 96.81%
=======================================
Files 169 169
Lines 20817 20817
Branches 494 494
=======================================
Hits 20153 20153
Misses 558 558
Partials 106 106 ☔ View full report in Codecov by Sentry. |
|
The clippy failure is unrelated and is addressed in PR #2265. |
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.
No description provided.