Upstream merge 2024 08 23 #1799
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 #1799 +/- ##
==========================================
+ Coverage 78.34% 78.36% +0.01%
==========================================
Files 581 581
Lines 97343 97370 +27
Branches 13961 13965 +4
==========================================
+ Hits 76265 76300 +35
+ Misses 20458 20448 -10
- Partials 620 622 +2 ☔ View full report in Codecov by Sentry. |
justsmth
force-pushed
the
upstream-merge-2024-08-23
branch
3 times, most recently
from
603d773 to
c0fde4b
Compare
This is a reland of https://boringssl-review.googlesource.com/c/boringssl/+/64749, which was reverted in https://boringssl-review.googlesource.com/c/boringssl/+/65328 due to issues in Arm mode (i.e. not Thumb mode) builds that target Armv6+ instead of Armv7+. The issue was that sha256_block_data_order_nohw has slightly different sizes depending on __ARM_ARCH. Prior to moving the dispatch, the sizes worked out such that they were always encodable in ADR. After moving the dispatch, the instructions got shorter, such that the Armv7+ build still worked, but the Armv6+ build needed to encode an offset of 0x1060 (previously 0x1080), which does not fit. See https://alisdair.mcdiarmid.org/arm-immediate-value-encoding/ for details on Arm's very fussy immediate value encoding. It's not the only form used for ADR (Thumb2 works very differently), but it's the applicable one here. While we could shuffle things around, this is all far too fragile. Just use the LDR; ADD pattern we used for the other function. ADRL would avoid a load (it splits the offset into two constants without a constant bank), but that's a pseudo-instruction that's only supported by gas. clang-assembler didn't want to implement it. Android have a macro at https://android.googlesource.com/platform/ndk/+/refs/heads/master/docs/ClangMigration.md#arm, but it didn't work for me when I tried it. Also, searching around, it sounds like ADRL in gas only works in Arm mode and not Thumb mode? We could probably work through all that, but the compiler emits constant banks on 32-bit Arm all the time. (I got this pattern from Clang's output.) This is probably not worth the trouble. Bug: 673 Change-Id: I165544764a931b293aa66fb3fc9bb8f01eeb8092 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65808 Commit-Queue: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> (cherry picked from commit 12316ab445eef5317391a94bef733fa6ff175173)
Some 'return' lines snuck in which cause us to skip some tests. Change-Id: I2806d6fb4fe3a6bd1fa58932c213d6af8991352e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65827 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> (cherry picked from commit 97dc33638b40a428ce666d61d71ee3db08083bf3)
Replace |bn_sqr8x_mont|'s unused |bp| parameter with a flag that indicates whether MULX and ADX are enabled. Bug: 673 Change-Id: I56632ad51bdc2f7f5ddd4278419d67e467b84d8f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65587 Reviewed-by: Bob Beck <[email protected]> Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
This clears the last reference to OPENSSL_armcap_P from assembly! Bug: 673 Change-Id: Id5d6115535742b2e980ed262d920ae28941841e8 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65868 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> (cherry picked from commit 01ea563b92e2b50cfaff23ed9c99d7603c976f3e)
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]> (cherry picked from commit 56d3ad9d23bc130aa9404bfdd1957fe81b3ba498)
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]> (cherry picked from commit 608becc67282174594fdaf0ec9c96daca9710d2f)
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]>
justsmth
force-pushed
the
upstream-merge-2024-08-23
branch
2 times, most recently
from
9954901 to
66d5171
Compare
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]> (cherry picked from commit 34b51faf3a58fe36e3ab1db99a2a441d0f69c754)
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]> (cherry picked from commit 8ff5add548e89f3680da398a41ecfca95a863fcd)
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]> (cherry picked from commit 5dd15f3a3b26995aeced71da4b404438df2a6363)
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]> (cherry picked from commit 10605c0d1e4b408dbd8fcfcae3897581afefb730)
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]> (cherry picked from commit c39e6cd9ec5acebb6de2adffc03cfe03b07f08ab)
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]> (cherry picked from commit 38d17d345c73a3ac0cb451fa0696fd8efabadc0d)
justsmth
force-pushed
the
upstream-merge-2024-08-23
branch
from
66d5171 to
46cdb75
Compare
torben-hansen
approved these changes
Aug 28, 2024
skmcgrail
approved these changes
Aug 29, 2024
smittals2
added a commit
that referenced
this pull request
Sep 17, 2024
## What's Changed * Use OPENSSL_STATIC_ASSERT which handles all the platform/compiler/C s… by @andrewhop in #1791 * ML-KEM refactor by @dkostic in #1763 * ML-KEM-IPD to ML-KEM as defined in FIPS 203 by @dkostic in #1796 * Add KDA OneStep testing to ACVP by @skmcgrail in #1792 * Updating erroneous documentation for BIO_get_mem_data and subsequent usage by @smittals2 in #1752 * No-op impls for several EVP_PKEY_CTX functions by @justsmth in #1759 * Drop "ipd" suffix from ML-KEM related code by @dkostic in #1797 * Upstream merge 2024 08 19 by @skmcgrail in #1781 * ML-KEM move to the FIPS module by @dkostic in #1802 * Reduce collision probability for variable names by @torben-hansen in #1804 * Refactor ENGINE API and memory around METHOD structs by @smittals2 in #1776 * bn: Move x86-64 argument-based dispatching of bn_mul_mont to C. by @justsmth in #1795 * Check at runtime that the tool is loading the same libcrypto it was built with by @andrewhop in #1716 * Avoid matching prefixes of a symbol as arm registers by @torben-hansen in #1807 * Add CI for FreeBSD by @justsmth in #1787 * Move curve25519 implementations to fips module except spake25519 by @torben-hansen in #1809 * Add CAST for SP 800-56Cr2 One-Step function by @skmcgrail in #1803 * Remove custom PKCS7 ASN1 functions, add new structs by @WillChilds-Klein in #1726 * NASM use default debug format by @justsmth in #1747 * Add KDF in counter mode ACVP Testing by @skmcgrail in #1810 * add support for OCSP_request_verify by @samuel40791765 in #1778 * Fix GitHub/CodeBuild Purge Lambda by @justsmth in #1808 * KBKDF_ctr_hmac FIPS Service Indicator by @skmcgrail in #1798 * Update x509 tool to write all output to common BIO which is a file or stdout by @andrewhop in #1800 * Add ML-KEM to speed.cc, bump AWSLC_API_VERSION to 30 by @andrewhop in #1817 * Add EVP_PKEY_asn1_* functions by @justsmth in #1751 * Improve portability of CI integration script by @torben-hansen in #1815 * Upstream merge 2024 08 23 by @justsmth in #1799 * Replace ECDSA_METHOD with EC_KEY_METHOD and add the associated API by @smittals2 in #1785 * Cherrypick "Add some barebones support for DH in EVP" by @samuel40791765 in #1813 * Add KDA OneStep (SSKDF_digest and SSKDF_hmac) to FIPS indicator by @skmcgrail in #1793 * Add EVP_Digest one-shot test XOFs by @WillChilds-Klein in #1820 * Wire-up ACVP Testing for SHA3 Signatures with RSA by @skmcgrail in #1805 * Make SHA3 (not SHAKE) Approved for EVP_DigestSign/Verify, RSA and ECDSA. by @nebeid in #1821 * Begin tracking RelWithDebInfo library statistics by @andrewhop in #1822 * Move EVP ed25519 function table under FIPS module by @torben-hansen in #1826 * Avoid C11 Atomics on Windows by @justsmth in #1824 * Improve pre-sandbox setup by @torben-hansen in #1825 * Add OCSP round trip integration test with minor fixes by @samuel40791765 in #1811 * Add various PKCS7 getters and setters by @WillChilds-Klein in #1780 * Run clang-format on pkcs7 code by @WillChilds-Klein in #1830 * Move KEM API and ML-KEM definitions to FIPS module by @torben-hansen in #1828 * fix socat integration CI by @samuel40791765 in #1833 * Retire out-of-module KEM folder by @torben-hansen in #1832 * Refactor RSA_METHOD and expand API by @smittals2 in #1790 * Update benchmark documentation in tool/readme.md by @andrewhop in #1812 * Pre jail unit test by @torben-hansen in #1835 * Move EVP KEM implementation to in-module and correct OID by @torben-hansen in #1838 * More minor symbols Ruby depends on by @samuel40791765 in #1837 * ED25519 Power-on Self Test / CAST / KAT by @skmcgrail in #1834 * ACVP ML-KEM testing by @skmcgrail in #1840 * ACVP ECDSA SHA3 Digest Testing by @skmcgrail in #1819 * ML-KEM Service Indicator for EVP_PKEY_keygen, EVP_PKEY_encapsulate, EVP_PKEY_decapsulate by @skmcgrail in #1844 * Add ML-KEM CAST for KeyGen, Encaps, and Decaps by @skmcgrail in #1846 * ED25519 Service Indicator by @skmcgrail in #1829 * Update Allowed RSA KeySize Generation to FIPS 186-5 specification by @skmcgrail in #1823 * Add ED25519 ACVP Testing by @skmcgrail in #1818 * Make EDDSA/Ed25519 POST lazy initalized by @skmcgrail in #1848 * add support for PEM Parameters without ASN1 hooks by @samuel40791765 in #1831 * Add OpenVPN tip of main to CI by @smittals2 in #1843 * Ensure SSE2 is enabled when using optimized assembly for 32-bit x86 by @graebm in #1841 * Add support for `EVP_PKEY_CTX_ctrl_str` - Step #1 by @justsmth in #1842 * Added SHA3/SHAKE XOF functionality by @jakemas in #1839 * Migrated ML-KEM SHA3/SHAKE usage to fipsmodule by @jakemas in #1851 * AVX-512 support for RSA Signing by @pittma in #1273
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:
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.