Update to upstream be21ef7 #131
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
Change-Id: I073ca90b6f675bf0461ab1a953a5de6808d66968 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71047 Commit-Queue: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]>
Apparently Bazel has deprecated the built-in licenses() command and now expects you to pull in an external package. I think I've gotten this right? Since our new BUILD.bazel file is actually referenced by the old -with-bazel branch, this new dependency also needs to work there. That is still using the legacy WORKSPACE system, but I got that working there too, I think. However, as the legacy WORKSPACE system does not consider recursive dependencies, anyone using it and updating BoringSSL will need to, in turn, update their WORKSPACE file on update. Fixed: 365824757 Change-Id: I7b49f33d628cec2ec07a47f0e31f16765d0f532a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71147 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Ben Lawson <[email protected]> Reviewed-by: Adam Langley <[email protected]>
This makes sha1 be internal to bcm other than the shared (public) SHA_CTX structure and lengths. Change-Id: Ib30e0e54a988e6c74a171ecf1fb400e70a9187b0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/70567 Commit-Queue: Bob Beck <[email protected]> Reviewed-by: David Benjamin <[email protected]>
Per https://github.com/google/oss-policies-info/blob/main/foundational-cxx-support-matrix.md Change-Id: I26f4a3dcad0f9b448459f42a810895bd6fe06445 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71027 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
The prototype isn't included by the preprocessor if OPENSSL_NO_ASM is defined thus defining the function is an error. Change-Id: I7176bb2c406f5597ae4033274141f69342a2b226 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71167 Reviewed-by: David Benjamin <[email protected]> Auto-Submit: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Change-Id: I4844f20f6989be2d7fbe9687d9f8fc2c70f0d0f9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/70907 Reviewed-by: David Benjamin <[email protected]>
This compiler sometimes emits code like this: adrp x10, .Llocal_data2+16 ldr q0, [x10, :lo12:.Llocal_data2+16] We transform it into: adr x10, .Llocal_data2+16 ldr q0, [x10] Note this makes some assumptions on the compiler, which I've documented in comments. We already have a similar assumption on ADRP + ADD pairs, but it is a little more likely for the compiler to do this in the ADRP + LDR case. Hopefully we can get the delocate replacement working soon and make all this moot. Change-Id: Icf4ed701142a52edf38d285c0bc5d52c17032d4f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71267 Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]>
Change-Id: I953441ccf99321184a5b664cc446551fa5e295b4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/70947 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]>
BoringSSL is a "live at head" project and does not make release branches and thus releases. However, some systems cannot consume git revisions and expect git tags. In Bazel Central Registry's case, the toolchain is suspicious of non-"release" archives of https://github.blog/open-source/git/update-on-the-future-stability-of-source-code-archives-and-hashes/ To accommodate such systems, let's start periodically tagging snapshots as "releases". These versions do not represent any kind of stability or development milestone. BoringSSL does not branch at these releases and will not cherry-pick bugfixes to them. Unless there is a technical constraint to use one of these revisions, projects should simply use the latest revision when updating. Also, so that cutting such revisions is less tedious, probably the simplest is to set MODULE.bazel's version field in HEAD to whatever the last revision was. The process will then be: 1. Bump MODULE.bazel's version 2. Create a git tag 3. Create a GitHub "release" 4. Kick off a BCR update All that mess will ideally be automated, but for now we'll drive that manually. Update INCORPORATING.md to explain these tags, and also to discuss the new pregenerated build files. Versioning scheme is chosen as 0.YYYYMMDD.N so that: - It is deterministic from the date - It begins with zero lest anyone misinterpret these as semver versions - We have a digit at the end to bump when we need to cut two revisions in one day Change-Id: Ie256a5f0f7eaac5928b537c75f82402c934f9fc3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71227 Reviewed-by: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Not sure if these are correct. I added a README so that our future selves have some hope of figuring out what this is for. The metadata and presubmit files I copied from our existing BCR entries. https://github.com/bazelbuild/bazel-central-registry/blob/main/modules/boringssl/0.0.0-20240530-2db0eb3/presubmit.yml https://github.com/bazelbuild/bazel-central-registry/blob/main/modules/boringssl/metadata.json Change-Id: Ice8b7dee7360e44f77411db6fec8067a3204aa0b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71287 Commit-Queue: David Benjamin <[email protected]> Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]>
See bazelbuild/bazel-central-registry#2785 Change-Id: I32921c00b2f7d956b7c5b73050657aea295908b8 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71308 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]>
I had hoped to use Publish to BCR, but there are a few issues with it. First, a security issue: Publish to BCR requires granting a third-party app write access to the GitHub repository, even though it only reads from the repository, which requires no special privileges to read a repository: bazel-contrib/publish-to-bcr#157 Second, merely cutting a release is not sufficient to satisfy https://blog.bazel.build/2023/02/15/github-archive-checksum.html One needs to manually upload a release tarball that GitHub then stores explicitly. (Perhaps someone should define a deterministic tarball creation process for git revisions and end this silliness.) Since that tarball is added by an individual developer, it seems poor that nothing checks it against the git repository. The BCR repository itself has some tooling for making a release. It works by interactively asking questions (not automatable), but then saves an undocumented JSON file with the answers. I've written a script that generates the JSON file we need from a git tag. These JSON files need to reference file paths, so they cannot be made standalone. (See bazelbuild/bazel-central-registry#2781) Instead, the script drops everything into a temporary directory. Since BCR's limitations force us to do a lot of custom processing anyway, I made the script check that: 1. The release tarball matches the archive tarball, which are stable enough in practice. This allows anyone to perform an easy (still GitHub-dependent) check that they match, unless GitHub changes the hash. 2. The tarball's contents match the git tag in the local repository, so we verify GitHub against the developer's workstation. The script then prints a command to run in a local fork of the bazel-central-registry repository to make a PR. Alas, even downloading the tarball from GitHub takes a few seconds, so I had a bit of fun with the script output. Change-Id: I2a748309f63848ff097ee3c3e93e11751ef65cd7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71307 Reviewed-by: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
See EmbarkStudios/cargo-deny#611 - Instead of using the template from deny.template.toml, just specify the fields that we need to be changed - Added back comments on bans.allow, but the bssl-sys and bssl-crypto crates are not added to the allowlist because they are now allowed with `licenses.allow = ["ISC"]`. Change-Id: I9e693780d902671444bf90b4d158d6e099e87ccb Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/70147 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
After renaming from `SLHDSA_128sSHA2` to `SLHDSA_SHA2_128S`, I forgot to run clang-format. This change is purely the result of doing so. Change-Id: I133c417170a4350c129bb5391fd5c8ebb408c1bf Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71327 Commit-Queue: Adam Langley <[email protected]> Auto-Submit: Adam Langley <[email protected]> Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Change-Id: I9dca8533a59e214707862d6ece233a4332ced4d3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71328 Commit-Queue: Adam Langley <[email protected]> Auto-Submit: Adam Langley <[email protected]> Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
We're nowhere near this doing anything, but may as well. Bug: 715 Change-Id: I790e690c67b8501fa571e804b2bd20197a2ef028 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/70928 Reviewed-by: Nick Harper <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]> Reviewed-by: Bob Beck <[email protected]>
Change-Id: Ia07aa88be217a73846b0dc12f360cd1aaafcfaba Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69168 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> Auto-Submit: David Benjamin <[email protected]>
The SSLAEADContext class contains most of the state needed for each epoch, but instead of trying to shoehorn more state into it, this change creates a new struct to contain the needed state, including the next write sequence number for the epoch. Change-Id: I5c259275fc90920a5c1a4dec87ab83a80f62b47a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71347 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]> Auto-Submit: Nick Harper <[email protected]>
Defining the constants for the individual random engines is somewhat odd, but it looks like OpenSSL will itself silently ignore the option when built without a particular engine, so silently ignoring it on our end seems defensible. Change-Id: I45f955038e5325702d1e32ba7932ada0b4fc1ab6 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71387 Commit-Queue: Bob Beck <[email protected]> Reviewed-by: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]>
Change-Id: I79cf0a885bfdd15cf604f354f7eb576216707826 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71427 Auto-Submit: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Bob Beck <[email protected]>
Change-Id: I940875e06f13830f53532a430dd5b7a0d49248a1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71428 Auto-Submit: Adam Langley <[email protected]> Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Adam Langley <[email protected]>
runner/hpke's test data can just be embedded directly without worrying about where the files are when the test is run. runner/kyber is trickier because its in a different directory, but give it a command-line flag so a build file can easily redirect it. Change-Id: I05dc76633ddd35ee09e9eb8d9a45364d0dea72b9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71447 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]>
runner is a fork from a very old version of Go's crypto/tls. Back then, crypto/tls implemented some pooled memory thing to reduce allocations. The abstraction is pretty confusing. Replace it with (I think) more idiomatic patterns using []byte and bytes.Buffer. As part of this, I've moved things common to TLS and DTLS into the encrypt method so encrypt is now responsible for generating the explicit IV and adding TLS 1.3 padding. Change-Id: I21527dd406d2691bc5d24378a832114c43b8b753 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71367 Reviewed-by: Nick Harper <[email protected]> Commit-Queue: David Benjamin <[email protected]>
No need to store outSeq on the halfConn, removed unused parameter. Change-Id: I3f352acee0526be93531ce67fbb4c4634733771c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71407 Reviewed-by: Nick Harper <[email protected]> Commit-Queue: David Benjamin <[email protected]>
This change has DTLS 1.3 clients reject connections from servers that negotiate 1.3 after sending HVR. It also enables HRR tests in DTLS 1.3. Bug: 42290594 Change-Id: I7bd5ab0e968e6b4c301a5697b1166703c28d9ebc Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71348 Commit-Queue: Nick Harper <[email protected]> Commit-Queue: David Benjamin <[email protected]> Auto-Submit: Nick Harper <[email protected]> Reviewed-by: David Benjamin <[email protected]>
I briefly thought we would need to look at this for DTLS, but DTLS doesn't have the TLS 1.0 implicit IV bug. Add some basic tests to ensure it doesn't crash as we rework the record layer. Also add a missing error code in the implementation. Bug: 42290594 Change-Id: I16e4dfb85aa1b84c38fd2433e01499347b8bf8d3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71487 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Nick Harper <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Change-Id: I62027a3a9c3aa338721f42b045b9e028d307ab23 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/70967 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]>
This fixes an issue where :ssl_test in the Bazel build ran no tests. (The CMake build was fine.) Bazel has no way to handle C and C++ sources need different flags (bazelbuild/bazel#22041), so our Bazel rules transparently split mixed C/C++ targets in two. Bazel silently turns all static libraries into shared libraries for tests. This means that, even if we set linkstatic = True on the split-out library, the two halves don't end up in the same shared object. This is because if A(test) -> B(lib) -> C(lib) and C is linkstatic, C is statically linked into A, not B. This is probably to avoid diamond dependency issues. From what I can tell, there is no clean way to say "B can be made into a shared library but please statically link C into it, because C will never be referenced except by way of B". (This means our use of linkshared is wrong. The next CL will try to redo that.) This Bazel behavior does not recognize that static and shared libraries have very, very different symbol handling. In particular, our assembly files needed to be in the same shared object as OPENSSL_ia32cap_P, so all this required another workaround in https://boringssl-review.googlesource.com/c/boringssl/+/70690 This, in turn, triggered this latest issue: GoogleTest relies on static initializers of individual object files to register tests. This does not work with linker dead code elimination and needs --whole-archive, or alwayslink in Bazel parlance. The most recent Bazel workaround required the C++ sources be the ones that were extracted, so they lost the --whole-archive behavior. As a result, :ssl_test silently ran no tests! Work around this latest Bazel-induced problem by setting alwayslink on cc_test helpers. All this would go away if Bazel just fixed bazelbuild/bazel#22041 (We can probably revert https://boringssl-review.googlesource.com/c/boringssl/+/70690 now, but either way we should probably set alwayslink=True on the helpers so that the build is not sensitive to which were extracted.) Change-Id: I10745e1bcfe91ac929f154e66093b29723efc576 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71507 Reviewed-by: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]>
Due to a longstanding Bazel flaw (bazelbuild/bazel#22041), we need to split all our mixed C/C++ targets in two. Ideally this split would behave as if the Bazel flaw were fixed, with the split-out library statically linked with the other source files. Accordingly, the helper macro sets linkstatic = True. It turns out linkstatic = True does not work this way. Bazel interprets linkstatic such that, if A(test, linkshared) -> B(library) -> C(library, linkstatic), C will be statically linked into A, not B. This is probably to avoid diamond dependency problems but means it is not possible for a cc_library split to be transparent, only cc_binary and cc_test. So what is happening is that every cc_test that links libcrypto is getting mlkem.cc statically linked into it, separate from the rest of libcrypto! That means we're getting the worst of both worlds: worse cache hit rate for tests that link libcrypto AND our C/C++ bits are still not contained in the same library. linkstatic = True on the helper is still valuable in cc_test and cc_binary, but otherwise inherit the outer value. Change-Id: I1089c58c6ddaa90c89efd8cdcebd88169b0236c8 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71508 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Previously it was a separate state because we stored the received_hello_verify_request bit implicitly in the state. But now that we've burned a bit for that, let's make that be one state. This doesn't make a huge difference but avoids do_enter_early_data implicitly assuming that DTLS 1.3 doesn't have 0-RTT. (0-RTT DTLS 1.3 is not a priority. Rather I was getting caught up in the weird SetVersionIfNullCipher silliness, which is linked with 0-RTT.) Bug: 42290594 Change-Id: I28d5ea32657731bfbaff9c2bb9a02bfc33d8e41b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71527 Reviewed-by: Nick Harper <[email protected]> Commit-Queue: David Benjamin <[email protected]> Auto-Submit: David Benjamin <[email protected]>
https://boringssl-review.googlesource.com/c/boringssl/+/74268 broke the SSSE3 GHASH fallback because it no longer maintained alignment of Htable. We had been relying on the memcpy to copy Htable into something aligned. Maintaining the alignment requirement without the memcpy is kind of a nuisance because it now leaks into EVP_AEAD_CTX. Since we don't have a good way to make caller-allocatable structs aligned, it would mean allocating 15 extra bytes and then finding the right position. Benchmarks shows that the alignment makes no difference on a Intel(R) Xeon(R) Gold 6154 CPU @ 3.00GHz. Of course, this is artificial because that CPU would never run this code anyway. I recall adding the alignment requirement because it gave a bit of a perf boost on the old Mac Mini 2010 I was testing against, which actually is a CPU that would run it. I was able to dig it up, but apparently I no longer have a keyboard that's compatible with it. (That machine is also long EOL and cannot even run Chrome's minimum macOS version. Although its CPU may be representative for older Windows.) Regardless, I don't think it makes sense to expend this complexity for this. (See internal Chrome UMA Net.QuicSession.PreferAesGcm on Windows for the percentage of Windows that would be running this code. Though they should also be using ChaCha20-Poly1305 anyway.) Bug: 42290477 Change-Id: I4ef8c636bfc18200869f011ea50cc5d4988244ba Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74327 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]>
One more step towards making GCM128 into a coherent AES-GCM abstraction a pulling EVP_CIPHER and EVP_AEAD out of BCM. Bug: 42290602 Change-Id: I2efc6a57be194715b2b72d5eb60e4873de14f88b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74269 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
The HKDFExtract and HKDFExpandLabel commands used for TLS 1.3 KDF ACVP testing are also parameterized by hash function, like the HKDF command. This commit updates their entries in the ACVP.md command table to reflect this. Change-Id: I3cfecfeacb94019a0731e77f3f0212d32145831f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74307 Reviewed-by: Bob Beck <[email protected]> Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Bob Beck <[email protected]>
This is needed to cover the new VAES+AVX2 AES-GCM code. Change-Id: I3587113e9c761fd42cd317181e549428b0555050 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74367 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]>
We did this for Dilithium, but the tests themselves got lost for ML-DSA. This is just a matter of adding some declassifies so that the tests can survive running through with secret data marked secret. As for what to mark secret, I opted to mark: - Any internal secret output from the PRNG since that aligns better with crbug.com/42290551. Though it probably introduces a bunch of false positives on the TLS side because we haven't actually run all that through this yet. - Any *uniformly* secret inputs in test vectors. That is, seeds, but *not* the serialized long-form private keys (which ought to become seeds anyway). This is because a portion of those serializations include public keys and it's tricky to declassify that before the public key parser gets confused. To simplify comparisons, I added a Declassified() helper in test_util.h. I considered just making == on Bytes automatically declassify, but then we won't notice when we forget to, e.g. declassify ciphertext, so making the test do it explicitly seemed worthwhile? Change-Id: I2c53e25ca843ef876f2a89c15131a5b2b425603f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74387 Reviewed-by: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
…_prf The comment that a barrier-less reduce_once was safe turned out to not *quite* be true. In Clang configurations without auto-vectorization (notably -O1), Clang would emit a branch instead of a CMOV. Unfortunately, adding a barrier to reduce_once has significant performance costs. The problem seems to be that auto-vectorization breaks. I suspect it is primarily because the value barrier forces the value into a general-purpose register, while vectorized code puts it straight into a SIMD register. Though knowing the comparison is a comparison seems to also help a bit. Based on what we've understood of Clang's select transforms thus far, it would make sense that ML-KEM might not need the barrier. The main culprit is turning multiple selects with the same condition into a branch, and that does not happen in ML-KEM. Yet we observe a problem. Based on valgrind instrumentation, the problem seems to be limited to scalar_centered_binomial_distribution_eta_2_with_prf, likely because the value has such a limited range of values. For some reason, this causes many recent versions of Clang to emit a branch. I think this may actually be a misoptimization. Indeed the very latest trunk build of Clang on godbolt does not have this problem. Somewhere between 8cb44859cc31929521c09fc6a8add66d53db44de and 8daf4f16fa08b5d876e98108721dd1743a360326, LLVM seems to have fixed this issue. We can avoid this by computing it differently. We currently write reduce_once(kPrime + a + b - (c + d)), where a through d are 0 or 1. Instead, we can write a + b - (c + d), let the underflow happen, and then conditionally add kPrime based on the sign bit of the result. This seems to avoid mishaps, for now. If this breaks down again, we may need to get better value barriers, or to stop relying on auto-vectorization and vectorize ourselves. Change-Id: I917456348d63628880467d21138a57297532bc9a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74447 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Change-Id: I934bf60900ffe1f318e5929072844b26f4c35e44 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73667 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Adam Langley <[email protected]>
It looks like seeds will be the official private key format for ML-DSA and ML-KEM. Thus parsing the weird private key format will only be needed for processing NIST's test vectors. Change-Id: Id6273214ba98b73aaf96640ec25ea289801b9bd7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73848 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Adam Langley <[email protected]>
The referenced bug notes some ODR warnings in an unspecified
configuration. (Which I can't get AppleClang or GCC 14 to reproduce, at
least.) This is likely due to us switching to C++. The ODR rules in C++
are a little different, but also perhaps the detection is only kicking
in for C++.
Either way, structs in .cc files are generally not intended to leave
that compilation unit. (Unless it's an opaque struct listed in base.h.)
There's no such thing as `static struct` so this change wraps many
structs in .cc files in `namespace {`. There are also lots of structs in
test file. Those are less concerning, but test files should mostly
entirely be in a namespace so this change does that where possible for
test files containing structs.
Bug: 384186552
Change-Id: I6ccf715fbcdc3ea6260b5d5d05f305182b1a9450
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74407
Auto-Submit: Adam Langley <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Update-Note: BoringSSL now requires a C++17 toolchain. This aligns with Google's Foundational C++ Support Policy as it has now been 10 years since C++14 was published. See https://opensource.google/documentation/policies/cplusplus-support Bug: 42290600 Change-Id: I54b914ccdde2b61a76a4ebfbe67c1fb28f7ba6ac Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74467 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]>
This test used a data buffer of size 16*32, but calls the assembly
functions with lengths of up to 16*32+7. This was okay for
aesni_gcm_{en,de}crypt which round the length down to a multiple of 16.
But aes_gcm_{enc,dec}_update_vaes_avx10_{256,512} process the full
length, which caused a buffer overrun. Fix this.
Change-Id: Ib3651a38baff35cfd9624d24747a4e53cefe6cff
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74507
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Bug: 42290600 Change-Id: I0c6fec6f33ff03ac2f60276920eb6b878751478d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74468 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]>
Quite a lot more things can probably be std::optional, but this had a TODO. Bug: 42290600 Change-Id: Ia5fc40d1dea4c11fa2bfef21d38643732a9bb98b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74469 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
It's standard as part of C++17 apparently. Bug: 42290600 Change-Id: Ibc2a990c5c7937fee5c096e7e0540a5cf853566a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74487 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
p256-nistz-table.h was manually editted for the C++ conversion without updating the generator. Also put the alignas marker in a slightly less surprising location. Change-Id: I5ac0d1852caa7b8608c856f4fe7438ea5499f872 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74488 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
…aders Except for our public headers, our files no longer need to be consumed by C. That means inline can just be inline, and with C++17, OPENSSL_UNUSED can be [[maybe_unused]]. Bug: 42290600 Change-Id: Ibdb309bc413660e10d075fbb71d4d1dd87101c6d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74489 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]>
It turns out to be useful for tests to be able to read this value back. Change-Id: Icf21144c230dc59f7548b7f75749509c8b646b4a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74508 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> Auto-Submit: Adam Langley <[email protected]>
Later we'll probably want to square our preferred styles against this stricter requirement. (Maybe we should just prefer to put everything file-local in anonymous namespaces rather than mark things static? Not sure. I don't like that it's hard to see whether something is static, but if we have to use this for types anyway...) Bug: 384186552 Change-Id: Ie86a56b1c7358a32262f0b9c4edb3e503aa3d08a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74547 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]>
This is the same as what we did for ML-KEM earlier. The Kyber code will be removed in not too long, but since we haven't quite removed it yet, let's run it through the same thing. Change-Id: Ie0833d88fd49c4b475e1512d3f3b8f44c9e1fc66 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74567 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]> Auto-Submit: David Benjamin <[email protected]>
The FIPS module will export a pair of general functions for the prehashed modes. We'll split the non-standard and standard modes at the public interface. Change-Id: Ic824342678cd0ccbe3c9bd25c1a676268de6e367 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73927 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Adam Langley <[email protected]>
Found by code inspection. If vsnprintf wanted to write INT_MAX characters, allocating a INT_MAX + 1 scratch buffer will overflow. Since we always have INT_MAX < SIZE_MAX, just casting to size_t earlier avoids this. (If the malloc implementation is unwilling to allocate INT_MAX + 1, e.g. it is forbidden to on 32-bit, that's malloc's responsibility to detect.) Change-Id: I3c2a740ebc7ecd58464a9f63858ffcefe67f648f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74247 Auto-Submit: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]>
go get -u ./...
go mod tidy
Fixed: 385044782
Change-Id: I0ade613aa778b53a55c658122e4351f924790ddf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74587
Commit-Queue: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Change-Id: Ibc52fbe362728134fce3c90ee47a23065a2e31b2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74087 Reviewed-by: Adam Langley <[email protected]> Reviewed-by: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]>
SWilson4
approved these changes
Dec 20, 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.
No description provided.