-
Notifications
You must be signed in to change notification settings - Fork 739
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.
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
Merge BoringSSL through a85ef9aa6c6538e5ae014a408e9d69870bc4f404 #2397
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
We've since added a few more files that don't have a counterpart in OpenSSL, or are named differently from their OpenSSL counterparts. Change-Id: I7057d8b258cb9656924054022654359d11a164f8 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74667 Reviewed-by: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]>
OpenSSL ran a "copyright consolidation" process, driven by this script. We will need to modify it slightly because it sometimes sets the year based on OpenSSL's git. Initially, this checks in an unmodified copy of OpenSSL's script. Bug: 364634028 Change-Id: I5acd518a900d9c5bb4ac637c5a3986ddf0b22bd0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74707 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]>
OpenSSL's "copyright consolidation" script standardizes their various old copyright headers on a new one. In doing so, it recomputed the copyright year as follows: - The end year was always 2016, when they ran the script. - If the file began with an EAY copyright line, that starting year was used. - Otherwise, the start year was ignored and recomputed from version control. This final step will not run in BoringSSL, because we started a new history. Instead, modify the script to simply take the result of the process from the corresponding file in OpenSSL. Bug: 364634028 Change-Id: I6083a398c7d742210d1b67110dda755ba0509f6c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74708 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]>
We'll simply be picking up the copyright lines from upstream OpenSSL. But since OpenSSL's script ended up changing the years around, this script will let us check which start years changed. (End years in OpenSSL got all rewritten to 2016 in the "copyright consolidation" process, so there's nothing terribly interesting to check.) Bug: 364634028 Change-Id: Id7263b05a98898fe6a6a121af9655ac0857c3ba2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74709 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
This applies the OpenSSL "copyright consolidation" process from the following upstream changes: * openssl/openssl@e0a6519 * openssl/openssl@3fb2cf1 * openssl/openssl@ac3d0e1 * openssl/openssl@c2f312f * openssl/openssl@596d6b7 * openssl/openssl@e18cf66 * openssl/openssl@846e33c * openssl/openssl@440e5d8 * openssl/openssl@21dcbeb * openssl/openssl@6286757 * openssl/openssl@4f22f40 * openssl/openssl@d2e9e32 * openssl/openssl@2039c42 * openssl/openssl@b132225 * openssl/openssl@aa6bb13 * openssl/openssl@b6cff31 * openssl/openssl@9e20068 * openssl/openssl@6aa36e8 * openssl/openssl@44c8a5e This was mostly automated, but partially manual. The automated portion can be reproduced by checking OpenSSL to commit 44c8a5e2b9af8909844cc002c53049311634b314, and running the following: git grep -l -E 'Copyright remains Eric Young|Copyright.*The OpenSSL Project\.|Written by.*for the OpenSSL Project' crypto/ decrepit/ include/ ssl/ | grep -v objects.go > files.txt cat files.txt | xargs -n1 perl -i ./util/copyright.pl From there, some years were fixed up manually according to go/openssl-copyright-consolidation-comparison (internal-only). Three files required additional manual fixing: - crypto/ecdh_extra/ecdh_extra.cc - crypto/fipsmodule/ecdh/ecdh.cc.inc - include/openssl/ecdh.h These files have an OpenSSL header, but *after* a different header, so the script does not correctly detect the now redundant OpenSSL header. They were manually modified to remove it. This matches what seems to have been done to crypto/ec/ecdh_ossl.c in OpenSSL's 4f22f40507fea3f272637eb8e00cadf1f34b10d9. Bug: 364634028 Change-Id: I79a559a409ebe2476f2cb8a48a488ac5dd77c90a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74710 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]>
This allows the script to recreate the new header. Bug: 364634028 Change-Id: Ie399e95f284b0170e8073e60f71806bf16cf48e2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74711 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
This imports the following changes from OpenSSL: * openssl/openssl@aa8f3d7 * openssl/openssl@5aba2b6 * openssl/openssl@c80149d See the following links for some related discussion in OpenSSL's repository: * openssl/openssl#3663 * openssl/openssl#3684 * openssl/openssl#3585 (comment) * openssl/openssl#3685 The copyright_summary script may be used to compare this CL. Note there is one change to ecdsa_test.cc to align with OpenSSL. See go/openssl-copyright-consolidation-comparison (internal-only). Bug: 364634028 Change-Id: I987c4e145d2ccd0c32bbf9e7bb2cc69e89019d35 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74712 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
This imports openssl/openssl@624265c from upstream OpenSSL. The only part that applies to BoringSSL is x_spki.c. Bug: 364634028 Change-Id: I709ed9765ee78ed35983384b0a472071a3cee4ea Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74713 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Now that the change has been applied, we no longer need these. Bug: 364634028 Change-Id: I2979b62489d640807c6b2568227c015a05af4d4b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74767 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
We have some logic to implicitly define OPENSSL_STATIC_ARMCAP on some platforms. This was being done a hair too late for the NEED_CPUID logic in crypto/internal.h to pick up. I'm not sure why this is only tripping the Android build now. It seems to have been broken for a long while. I put it in the public headers because <openssl/crypto.h> is also sensitive to OPENSSL_STATIC_ARMCAP, so it seems prudent for it to be set all in one place. Change-Id: I53691b018282a71f5d0cb0f6a6c457e1ee4d1df9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74787 Auto-Submit: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]>
These aren't used externally. While I'm here, const-correct PEM_do_header. Really we could have just made these file-local except that PEM_X509_INFO_read_bio does something weird. Bug: 42290574 Change-Id: I455b9c31da0efb854925bbe38797d3c0e221fcdf Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74807 Reviewed-by: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
This imports upstream's openssl/openssl@6714cb1 Bug: 364634028 Change-Id: I270390f9a0ab8acb5ec508e2c992ccf6b1091a07 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74768 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
There's a lot more we could improve in this function, but fix this particular egregious issue first. Change-Id: Idc4b7f9aa62972293ead4f8534b8461942318e21 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74808 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]>
Adds: * SHA2-512/224, SHA2-512/256 * SHA3-224, SHA3-256, SHA3-384, SHA3-512 See https://pages.nist.gov/ACVP/draft-vassilev-acvp-drbg.html#section-7.4 Change-Id: I3f7d16062096a2c425f230374d44f1b29c95834d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74687 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Bob Beck <[email protected]>
This commit adjusts the ACVP.md documentation for the KDF-counter command to match the implementation. Prior to this the kdf struct in subprocess that dispatches KDF-counter command invocations had a few divergences from the docs: * If the test case has the "Deferred" property set to true, then the key argument provided to the wrapper is empty. * The wrapper is expected to output three values: the input key (since for deferred tests it was generated module-side), the fixed counter data, and the derived key. For deferred tests the returned key is written to the response `KeyIn`. For non-deferred tests the returned key is verified to match the one that was sent to the submodule as a command arg. Change-Id: If266383e279d2222f55975aa3376e8fb134899d7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74727 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Bob Beck <[email protected]> Reviewed-by: Adam Langley <[email protected]>
Add a VAES-optimized AES-GCM implementation that is optimized for AMD Zen 3 processors, using AVX2 instead of AVX512 / AVX10. With AVX2 only 16 vector registers are available and some instructions are missing, which is inconvenient and makes the code not easily sharable with the AVX512 / AVX10 version. However, using VAES still gives a significant performance improvement, about 80-85% on long messages as shown by the following tables which show the change in AES-256-GCM throughput in MB/s on a Zen 3 "Milan" processor for various message lengths in bytes. Encryption: | 16384 | 4096 | 4095 | 1420 | 512 | 500 | --------+-------+-------+-------+-------+-------+-------+ Before | 3955 | 3749 | 3597 | 3054 | 2411 | 2038 | After | 7128 | 6631 | 5975 | 4788 | 3807 | 2676 | | 300 | 200 | 64 | 63 | 16 | --------+-------+-------+-------+-------+-------+ Before | 1757 | 1405 | 856 | 602 | 356 | After | 1885 | 1430 | 940 | 593 | 381 | Decryption: | 16384 | 4096 | 4095 | 1420 | 512 | 500 | --------+-------+-------+-------+-------+-------+-------+ Before | 3962 | 3774 | 3593 | 2978 | 2510 | 1998 | After | 7378 | 6836 | 6282 | 4826 | 3868 | 2753 | | 300 | 200 | 64 | 63 | 16 | --------+-------+-------+-------+-------+-------+ Before | 1742 | 1428 | 856 | 535 | 383 | After | 1940 | 1534 | 940 | 573 | 383 | Change-Id: I583dd6b48b81ab3c6df51bfe8729366cad500537 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74368 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Since there is now a VAES+AVX2 implementation of AES-GCM, and the future of AVX10/256 is uncertain, disable the AES-GCM functions that use AVX10/256 (equivalently AVX512 with a maximum vector length of 256 bits). This leaves VAES+AVX2 as the sole 256-bit support for now. For now this just affects Intel Ice Lake and Tiger Lake (which actually support AVX512, but where downclocking issues make 256-bit arguably preferable to 512-bit), where a slight performance loss is seen on long messages. The following tables compare AES-256-GCM throughput in MB/s on Ice Lake server for various message lengths: Encryption: | 16384 | 4096 | 4095 | 1420 | 512 | 500 | --------+-------+-------+-------+-------+-------+-------+ Before | 7533 | 6990 | 6220 | 5096 | 4200 | 2702 | After | 7403 | 6879 | 6236 | 4980 | 4040 | 2868 | | 300 | 200 | 64 | 63 | 16 | --------+-------+-------+-------+-------+-------+ Before | 2086 | 1555 | 1031 | 657 | 433 | After | 2069 | 1635 | 1045 | 667 | 430 | Decryption: | 16384 | 4096 | 4095 | 1420 | 512 | 500 | --------+-------+-------+-------+-------+-------+-------+ Before | 7703 | 7140 | 6524 | 5283 | 4244 | 2990 | After | 7572 | 7056 | 6494 | 5155 | 4224 | 3073 | | 300 | 200 | 64 | 63 | 16 | --------+-------+-------+-------+-------+-------+ Before | 2276 | 1733 | 1070 | 680 | 447 | After | 2249 | 1743 | 1100 | 692 | 447 | This change should be reconsidered if AVX10/256 sees widespread support, as we shouldn't carry forward a restriction to AVX2 unnecessarily. This change also replaces gcm_init_vpclmulqdq_avx10 with gcm_init_vpclmulqdq_avx10_512, now instantiated using 512-bit vectors. Otherwise it would be the only avx10 function left using 256-bit. Change-Id: I7fd21568482118a2ce7a382e9042b187cd2739f7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74369 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
This imports openssl/openssl@0904e79 from upstream OpenSSL. The tree intentionally does not compile at this point. Bug: 364634028 Change-Id: I39001741cf0db059e76ad4940004a1d57bf8af12 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74827 Reviewed-by: Adam Langley <[email protected]>
…upstream OpenSSL This imports openssl/openssl@320a812 from upstream OpenSSL. This causes the following free functions to no longer check for NULL: * BIO_CONNECT_free * BUF_MEM_free * BN_CTX_free * BN_RECP_CTX_free * BN_MONT_CTX_free * BN_BLINDING_free * X509_STORE_free * SSL_SESSION_free (It also causes tls_free to no longer check for NULL, but that check was unnecessary.) Bug: 364634028 Change-Id: Ia625039a0a22b0bf368c39d6b8090ca15955f8e4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74828 Reviewed-by: Adam Langley <[email protected]>
…some functions. This change reimplements some OpenSSL changes based only on the description of the work in base.h. Change-Id: I1a8b3d2774216c43ab446aa56b31cbb40d58b29d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74847 Reviewed-by: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
This was another part of OpenSSL's "copyright consolidation" changes. See: https://github.com/openssl/openssl/blob/44c8a5e2b9af8909844cc002c53049311634b314/crypto/bn/asm/x86_64-gcc.c https://github.com/openssl/openssl/blob/44c8a5e2b9af8909844cc002c53049311634b314/crypto/md5/asm/md5-586.pl https://github.com/openssl/openssl/blob/44c8a5e2b9af8909844cc002c53049311634b314/crypto/md5/asm/md5-x86_64.pl Bug: 364634028 Change-Id: I9bb24c94a468ca419936118f4d1e5b3a359e1674 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74887 Commit-Queue: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]>
Years computed from version control. Bug: 364634028 Change-Id: I949149b156ea24966813f304699869a5ad304c98 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74888 Reviewed-by: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]>
Bug: 364634028 Change-Id: Id26a236e3cc74944111f1ce74e32dbb481c4b309 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74829 Auto-Submit: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
The crate won't build without it anyway, so unconditionally try and enable it. Change-Id: Ief8a7dbf8d0af3040b1832424007150987ce654d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74947 Auto-Submit: Pete Bentley <[email protected]> Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]>
Change-Id: If398ea31546f7be98abce4362cae1f7c821ff7aa Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74867 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Bob Beck <[email protected]>
Change-Id: I94077581037372ea658e60b86b05fa977e1c3ac6 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74868 Commit-Queue: Bob Beck <[email protected]> Reviewed-by: Adam Langley <[email protected]>
I think this was not supposed to be checked in. The generate scripts tend to drop things extra files into an out directory, but for better or worse, we don't actually end up checking them in. (They're not in the corresponding files in Chromium.) Probably these were lying around in the worktree at the time. Change-Id: Ibc423ce316ecdf2b4b62a4513d228618ff88bc67 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74830 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Change-Id: Id8447847e3cf6a48123cb625762ecbc4ddad8f16 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74907 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Bob Beck <[email protected]>
This reverts commit d678fbf. The original change broke Chromium: ``` error[E0725]: the feature `new_uninit` is not in the list of allowed features --> ../../third_party/boringssl/src/rust/bssl-crypto/src/lib.rs:28:12 | 28 | #![feature(new_uninit)] | ^^^^^^^^^^ ``` Change-Id: I6732a720a5705cfe7b0219b5d9d02a911af0f89a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74987 Auto-Submit: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
In ASN1_TIME_to_generalizedtime we were already parsing the input time string to check for validity at the start of the function. Instead of throwing away the tm we construct to do this, keep the tm, and just convert the time normally to what we want after the check instead of messing with the original string. Change-Id: I32b351abd135b1f9c30b228c09718c5be994187a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74848 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Bob Beck <[email protected]>
The foo / foo_extra split will make increasingly less sense as we stop putting public APIs in crypto/fipsmodule. Just call it crypto/foo and crypto/fipsmodule/foo. Bug: 42290602 Change-Id: I5143d3edfb768ed7a1aa288ff606f6f13faa9278 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/75151 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]>
…f" from upstream OpenSSL
…finition to target.h
…pstream OpenSSL
…ributor that we cannot find" from upstream OpenSSL
…and `sprintf`, and handle NULL in some functions.
…enSSL's "copyright consolidation"
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2397 +/- ##
=======================================
Coverage 96.61% 96.61%
=======================================
Files 176 176
Lines 21640 21640
Branches 529 529
=======================================
Hits 20908 20908
Misses 618 618
Partials 114 114 ☔ View full report in Codecov by Sentry. |
This was referenced Feb 18, 2025
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.