-
Notifications
You must be signed in to change notification settings - Fork 134
Update run_analytics.sh to skip ARM FIPS Static builds with GCC #678
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
Merged
Conversation
This file contains hidden or 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
…tracking reference to run_fips_tests.sh
torben-hansen
approved these changes
Nov 8, 2022
samuel40791765
approved these changes
Nov 9, 2022
Taffer
added a commit
that referenced
this pull request
Nov 10, 2022
* rename tlskdf.go to kdf-components.go to more accurately reflect NIST test vector naming scheme (#666) * Check for TLS 1.3 in SSL_generate_key_block. SSL_generate_key_block is specific to TLS 1.2. It will output garbage in TLS 1.3 (wrong KDF), so fail instead. Update-Note: SSL_generate_key_block gets a new error case, but callers that hit this were getting back useless output anyway. Change-Id: Ib35384f902e03cd4654d25b39ca1808c4d878c3d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54705 Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com> (cherry picked from commit 19d6ec9c439323c869e28f4e496329e55179f1ee) * Make CBB_init_fixed infallible and allocationless. Today, every use of CBB, even CBB_init_fixed, requires a small, fallible allocation to allocate the top-level CBB's cbb_buffer_st. We could embed cbb_buffer_st directly in CBB, but then every child CBB wastes that space, and needs an extra pointer to point back to the cbb_buffer_st. But top-level and child CBBs have disjoint representations anyway. We share a cbb_buffer_st pointer, but it's owning in one case and borrowed in another. Child CBBs have length prefix information, but it's never filed in for a top-level CBB. Make this a sum type, with is_child as the discriminator and a union for the two structures. (Elsewhere I've been trying to get rid of unions, but this isn't using unions for type-punning, so it should valid even in C++. We never access inactive arms.) The implementation gains a few more branches, but now CBB_init_fixed is infallible and allocation-less. I'm hoping this will let us more freely convert functions like UTF8_putc into CBB because we don't need to worry about cleanup or introducing allocations. Change-Id: If0b28cd9e079418f35d5a614058c0aa73658822e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54645 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> (cherry picked from commit 15ba28f839c828ced30178a86a9c56b104d3b5d7) * Some CBB_init_fixed simplifications. CBB_init_fixed callers no longer need to check the return value, or handle any cleanup. The hpke.c instance was even already (incorrectly at the time) assuming this. Change-Id: I2f4cb124454fc7ba7ff6d2075d99f537a58c6c6b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54647 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit 254b8e113977aeb84ced559772537cd15bbe6cfc) * Add support for arm/aarch64 on FreeBSD Change-Id: Ib3495ddedec533b78884100ff2ff76f7370e7dc6 Bug: 505 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54105 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit d66bba912806b83350afec11a3104a7865d783aa) * Test (and, for CSRs, fix) TBS cache invalidation on signing. We didn't actually have a test that would have caught openssl/openssl#19388. This fixes this by further generalizing the signing tests to run through all combinations of {new object, reused object} x {X509_sign, X509_set_signature_value}. In doing so, align X509_REQ_sign and X509_REQ_sign_ctx, which were missing the TBS invalidation. Change-Id: I5028aa2a00e71da0ebc7a03b23823b1337a56fca Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54726 Commit-Queue: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> (cherry picked from commit da96eeb958ee8442f408a5e8fa78cbfb86cc3281) * Fix comments around TBSCertificate cache. We don't actually refresh the cache most of the time, just drop it and live without it. The distinction isn't really visible by callers, but make the comments accurate. Change-Id: I7321695337125ca648ab57667564d9578a6fd549 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54727 Commit-Queue: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> (cherry picked from commit 818c4aafa86fa3922f969220473a27c163214b73) * Also detect Armv8.2 SHA-512 extensions on FreeBSD. A small follow-up to https://boringssl-review.googlesource.com/c/boringssl/+/54105, to bring it up to feature parity with the other aarch64 backends. ID_AA64ISAR0_SHA2_512 seems to be present in FreeBSD 12.0, so I don't believe this needs any compatibility ifdefs. Bug: 505 Change-Id: I44891cf635adfd2ae26d4113fdc910549cf89193 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54725 Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Robert Clausecker <fuzxxl@gmail.com> Reviewed-by: Bob Beck <bbe@google.com> (cherry picked from commit 9cd85d0b4c3ea8fc0e1a1883bab3a46ffaa18bd9) * Further fixups on the new tests. I messed up the indentation in one place, and Shane Lontis pointed out that the comment is slightly out of date now that there are two codepaths involved. Change-Id: I1be69f3f9a3835fffc4801b4464b9fb8ecb092cc Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54745 Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com> (cherry picked from commit ca6fa61cdb1cd5c6402fd41908479bbbf1b4566b) * Add EC scalar mul functions to bssl speed (#660) Adding lower level EC scalar multiplication operations to bssl speed tool: mul, mul_base, mul_public. * Add SSHKDF to ACVP Tool (#668) * prepare kdf-components.go for both tls and ssh * update kdf-components.go to include logic for handling sshkdf * add sshkdf to acvp tool * fix bug where no result is produced for TLS-v1.2 * add SSH KDF to acvp test suite * update spacing in code * reorganize subprocess.go * Revert "rename tlskdf.go to kdf-components.go to more accurately reflect NIST test vector naming scheme" Also updated variable names to resolve conflicts. This reverts commit ce0e884. * rename tlskdf.go to kdf-components.go to more accurately reflect NIST test vector naming scheme (#666) * move magic numbers into variables * add more elegant error handling * remove duplicated variables from kdf-components.go * Rust - rename lib to avoid link conflicts (#669) * Remove pthread_atfork fork detection mechanism (#673) #142 added pthread_atfork() as a secondary fork detection mechanism. However, we have observed several issues consuming AWS-LC artifacts due to the symbol pthread_atfork being handled differently in distributions (and in a non-portable way). This affects our ability to distribute AWS-LC to our intended places. See internal document for details. Therefore, completely remove pthread_atfork() from the code-base. * Update run_analytics.sh to skip ARM FIPS Static builds with GCC, add tracking reference to run_fips_tests.sh (#678) * Import the source code of Kyber768 (#679) Import the source code of Kyber768. The source code was taken from the primary source of Kyber at [link](https://github.com/pq-crystals/kyber), at [commit](https://github.com/pq-crystals/kyber/tree/faf5c3fe33e0b61c7c8a7888dd862bf5def17ad2), as of September 13th 2021. * Switch RSA_sign to size_t. While I'm here, use a fixed-size uint64_t in RSA_generate_key, rather than unsigned long. This code also assumes unsigned long fits in BN_ULONG, which is probably true on all platforms we care about, but unnecessarily fussy. The RSA_sign -> RSA_METHOD transition does require a cast. Go ahead and check length/hash_nid consistency so we know it fits in the cast. This does mean RSA_METHOD-backed keys are restricted to implementing digests that we support, but that's probably fine. If anything, I think we should try to shift away from RSA_METHOD as a story for custom keys. Bug: 516 Change-Id: I3969da67d1daeff882279a534eb48ca831eb16cd Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54465 Commit-Queue: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> (cherry picked from commit 58f728eaecf9d1152df4977e50f90375e05c486d) * Enable -Wstring-concatenation and silence warning. Newer versions of Clang have a warning to detect "suspicious" uses of string concatenation, where they think a comma or so was missing. It flags a false positive in x509_test.cc, which we can silence with parentheses. Fuchsia builds with this warning enabled, so enable it to catch future instances. I couldn't find official documentation on when this was added, but empirically it's in my clang-12 but not my clang-11. That's recent enough that adding a version check seems prudent. Unfortunately, version-detecting Clang is complex because AppleClang uses completely different versions. There's a handy table on Wikipedia that maps them. Change-Id: I503c21d39bb5c68dda9bda6da693c7208f3af561 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54785 Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit 7d1fc2b014a940e4e9ecd4660146c5cc2accad9b) * Remove CMake install command for modulewrapper. I think it never picked up the fix in https://boringssl-review.googlesource.com/c/boringssl/+/52345 for older CMakes, but it doesn't have much reason to be part of the install in the first place. Bug: 524 Change-Id: Ifbb898b1e4686194c85e9902ee3d59d83b55b78a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54786 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit 14aa0de18f638a92be13597bc1b8a95ca8fcf8a4) * Break FIPS tests by zeroing out the entire value. Previously the code just flipped one bit. But, empirically, modern Clang will sometimes produce code that doesn't depend on the first 16 bytes of the data; they are encoded in the instructions instead. Thus zero out the full value. (If Clang ever starts embedding complete values into the instruction stream then we're going to have to do something more complex. Self tests are a bit funny: the compiler could reasonably optimise them away completely given that it sees all the inputs. Perhaps the inputs would have to be moved into a different object file.) Change-Id: I7bfb18cb7868def67fc791dcc31c5915c7728ac4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54825 Commit-Queue: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com> Auto-Submit: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com> (cherry picked from commit 1eea82a92a141416b3f0ee4d52434252cb948b47) * Fix linking with non-MSVC toolchain in Windows platform This adds the link libraries in CMakeLists.txt file. If the libraries are not in CMake files linking failed with undefined reference error. Change-Id: I8f8352f6149a6332eedc0be51f36634890e3db60 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54805 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: David Benjamin <davidben@google.com> (cherry picked from commit b819f7e9392d25db6705a6bd3c92be3bb91775e2) * Miscellaneous -Wshorten-64-to-32 fixes. Bug: 516 Change-Id: Iba2014da414658c08e42e0993912fa73848832d3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54945 Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit 9d64d8d2373db494d3460102b970f4aaf92ee274) * Clean up short BIGNUM handling in bn_print. We shouldn't print different things depending on sizeof(long). Change-Id: I5f97e17b838f8c9b119421b9ce0e93e95bd33dc0 Reviewed-on: https://boringssl-review.googlesource.com/c/ 8252 boringssl/+/54946 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com> (cherry picked from commit 7deb8314625b40aea2e0c4fdcf9784d18045e397) * Add tests for EVP_PKEY_print_* These are mostly to ensure they don't crash, and that subsequent changes don't unintentionally change the output. The current output is a little weird but, for now, I've just captured the current output, bugs and all. Change-Id: I9f1a4910ccc717764ef44551de9b3e0f9f2a1b40 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54947 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit 1ec335da79eaa8341308acd9dc804078b8593c84) * Simplify crypto/evp/print.c. First, stop trying to pre-size the buffer and just have bn_print allocate the buffer internally. That removes the need for all the algorithms being two-pass. While I'm here, stop passing the unused ASN1_PCTX parameters in everywhere. As a side effect, this fixes a int vs size_t instance that flagged -Wshorten-64-32, but it ended up being a much more substantial change. Bug: 516 Change-Id: Ic210604de85539559b1ed88889ca6a08dfb20bde Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54948 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> (cherry picked from commit 3592aa3009eb883f7e5431ad7ce08dc3ae979902) * Update CMakeLists.txt to support CMake version 3.0. VERSION_GREATER_EQUAL was added in 3.7, negate the expression and use the widely supported VERSION_LESS * Add support for 8192 bit RSA key in speed tool (#680) Speed tool has support for measuring various RSA operations using 2048 and 4096 bit RSA keys. In terms of bit-security, these can't be compared to all corresponding elliptic curves we support; e.g. p256 is somewhat equivalent to 4096 bit RSA keys, hence no comparison for p384 and/or p512. To compare performance of RSA against, for example, ECDSA using p384 a 8192 bit RSA key is required. This also shows how RSA scales past 4096 bit keys (and shows why we don't need to measure performance for larger RSA keys...). Embed such an 8192 bit RSA key and wire it up in the speed tool. * Improve Rust build/publish (#681) * Improve Rust build/publish * Add comment on crate generation * Add comment on crate publishing Co-authored-by: William Bo Yang <coolbillyang@gmail.com> Co-authored-by: David Benjamin <davidben@google.com> Co-authored-by: Sergey A. Osokin <osa@FreeBSD.org.ru> Co-authored-by: Nevine Ebeid <66388554+nebeid@users.noreply.github.com> Co-authored-by: dkostic <25055813+dkostic@users.noreply.github.com> Co-authored-by: Justin W Smith <103147162+justsmth@users.noreply.github.com> Co-authored-by: torben-hansen <50673096+torben-hansen@users.noreply.github.com> Co-authored-by: Andrew Hopkins <andhop@amazon.com> Co-authored-by: Adam Langley <agl@imperialviolet.org> Co-authored-by: Biswapriyo Nath <nathbappai@gmail.com>
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:
Fix an issue where the analytics build fails on the last build for ARM. Supporting the Static FIPS GCC ARM build is tracked in CryptoAlg-1399.
This will also address the false positive in #677.
Testing:
Existing CI tests skip the Static FIPS GCC ARM build, the Clang build is still tested.
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.