Skip to content
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 bd20800c22fc8402611b537287bd6948c3f2a5a8 #1666

Merged
merged 172 commits into from
Sep 30, 2023

Conversation

briansmith
Copy link
Owner

No description provided.

davidben and others added 30 commits June 23, 2023 17:45
OPENSSL_WINDOWS doesn't *quite* imply that crypto/rand_extra/windows.c
is used, thanks to fuzzer mode.

The sea of ifdefs here is becoming quite a mess, so I've added
OPENSSL_RAND_* resolve the dispatch in one place. Perhaps later we
should also we can also simplify this by just including
CRYPTO_init_sysrand and CRYPTO_sysrand_if_available in all the C files.
But that'll be easier to do when Trusty's RNG is moved in tree.

While I'm here, fold some of the ifdefs in windows.c together.

Change-Id: Ic9c21c5c943a409ebb1d77f27daea1eeb9422e9d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61085
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
This is breaking the oss-fuzz build. Changes made:

- No need make a subproject, I think. In particular, keep the minimum
  CMake versions matching.

- Let's not include it in install just yet, since it's still
  experimental.

- I removed the comment about public headers. The target doesn't
  actually provide any public headers for the moment.

- Apparently the "modern CMake" way to set properties is per-target
  rather than using the directory-wide options, so I've switched to
  that.

Bug: oss-fuzz:60049
Change-Id: I511667ca8e83cb1997f82b329ed38f980574305e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61126
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Android have not updated their bindgen (see b/279198502), so they cannot
yet pick up inline functions automatically.

Bug: 596
Change-Id: I49d5adaaa3537ada545c9c6fce98ea2dbf2f40ae
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61165
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Change-Id: Id38833b329b0d661fb18e8a75b671379effe82a6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61166
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Change-Id: Ie5cb298398c1944704c74b63f51323800f6dc5b0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61065
Reviewed-by: David Benjamin <davidben@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The standard macro-based pattern does not work in bindgen because of
rust-lang/rust-bindgen#2544

Change-Id: Ic2b92e779ade2ed55a627bba9c76f7df5c0f6136
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61185
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
BN_MONT_CTX already has the modulus, so just use it. This is one less
value to initialize statically.

Bug: 20
Change-Id: I78f73994ab595b795e99d67851bdff3b73fc3dd6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60926
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
While I'm here, update x/crypto and x/net to their latest versions.
byteReader is a straightforward port, except there doesn't seem to be a
convenient way to read length-prefixed bytes without manually casting
from cryptobyte.String to []byte, so I've done that.

byteBuilder is a bit more involved because it's based on closures, but
still a mechanical change.

As part of this, I switched runner's ticket format to use u24 length
prefixes instead of u32, because cryptobyte.String doesn't have u32
length prefixes. (Although, oddly, cryptobyte.Builder does.)

Fixed: 374
Change-Id: If9bea0b41fe2b8bc48f040a667753b160da469bb
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61186
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
We require MSVC 2019 now, which has a /std:c11 flag. Enable it to match
the CMake build and remove a blocker for requiring C11 unconditionally.

(This select branch is also used by clang-cl. I had meant to figure out
the @bazel_tools business as part of this, but it turns out clang-cl
works better with the MSVC flags than the GCC ones anyway. -Wall in
clang-cl is like MSVC's /Wall and actually means all warnings. Ideally
we'd still condition this on the compiler, in case anyone uses MinGW,
but we can figure that out later.)

Tested with bazelisk build :all and
bazelisk build --compiler=clang-cl :all on Windows.

Change-Id: I4559789a221071eef39f9d34929f0e9c5994119e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61127
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
While we are at it, add a fillins/log.h and modify import_spec.json
to remove the need for patching out LOG and DVLOG (which removes
one more patch in the patch stack

Change-Id: I3b7b512490ee0e8c465734b2236b31da7d132ec7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61225
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This will make it easier to support other build systems rather
than relying on names of files matching a special pattern to
mean a special thing.

Bug: 1322914

Change-Id: I3d1df70e79934717275c0c331e7e5cd3cc5c99e5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61245
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
Inspired by
https://boringssl-review.googlesource.com/c/boringssl/+/61245, though it
looks like we do still need CMAKE_CONFIGURE_DEPENDS to tell CMake to
rerun itself.

Change-Id: Iec2d23590dd45c647c5c53dc80aa8420795a7e73
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61265
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
One less value to initialize statically. Instead, just check if r +
order < p. It's one additional comparison, but those have negligible
cost here.

Bug: 20
Change-Id: Iabc9c1894b58aeba45282e3360e38fe843eb7139
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60927
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This achieves much of the same goals as
https://boringssl-review.googlesource.com/c/boringssl/+/61245, but
reuses the existing source.cmake parser in generate_build_files.py.

There are two practical differences here:

First, CMake knows that, if it sees include(sources.cmake), it should
add sources.cmake as a dependency of build.ninja, and re-run cmake if
source.cmake changes. It does not know this for file(STRINGS). This can
be fixed with CMAKE_CONFIGURE_DEPENDS, but since we already have this,
may as well use it.

Second, sources.cmake wants paths rooted at the top-level BoringSSL
source directory, which means we want to define the targets at the top
level, rather than in subdirectories. While it will make the top-level
CMakeLists.txt larger, I think this is a good move:

- ./build/crypto_test --gtest_filter=... is just less annoying to type
  than ./build/crypto/crypto_test --gtest_filter=...

- It will (eventually) mean libcrypto.a lives at build/libcrypto.a
  instead of build/crypto/libcrypto.a. This means we can pass a single
  -L parameter when building things against BoringSSL, without relying
  on the install target.

- It aligns with the generated CMake build. I would like, long-term, to
  stop maintaining multiple CMake builds, and the generated one seems to
  have a better convention. And one that will be more disruptive to
  others if we change it.

- Top-level-rooted paths are more convenient for generate_build_files.py
  to work with, because that's what we want to output.

As I get further along in 542, I expect I'll move this once again,
possibly to some JSON file, because I'll need my new pregenerate tool to
parse it (and we'll no longer be constrained by what's easy to consume
in CMake), but use this for now.

Bug: 542
Change-Id: I1e8b894057264da5d7624a1fbb92f9e1198ea38e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61266
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
This removes a place where we make hard-coded assumes about test names.
Also it shards pki_test, as that test suite is large enough to benefit
from it.

Change-Id: I392254b73a2df2f022ccf13508552372c103bff7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61285
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
This reverts commit 28e4a1b. Bazel
broke --cxxopt on Windows in
bazelbuild/bazel#15073, which means projects
enabling, say, C++20 with --cxxopt=/std:c++20 are silently passing
/std:c++20 to our C files.

This is already a problem, but MSVC is smart enough to silently ignore
the flag when building C. However, MSVC will report an error if you then
pass /std:c++20 /std:c11 into the same command. It seems that check is
not aware of this ignoring behavior.

Ultimately, this is a Bazel bug, and one that makes the broken versions
of Bazel unsuitable for use with C. This was fixed in Bazel in
bazelbuild/bazel#18119 and backported to the
upcoming Bazel 6.3.0 release in
bazelbuild/bazel#18552

Temporarily revert the change. When Bazel 6.3.0 is released, we'll put
this back and require Windows users use a functioning version of Bazel.

Bug: 624
Fixed: 623
Change-Id: I68d9b2ed8751b4cf5dc7f42f8c1fbd42a97d6ca2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61365
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
One less value to initialize statically. Also this simplifies EC_GROUP
initialization. While I'm here, reorder EC_GROUP to pad better.

This lets us simplify the init bits slightly. It does mean p224-64.c,
the one EC_GROUP that doesn't use Montgomery reduction, carries around a
wasted Montgomery context, but it'll make generating the tables
statically much easier. Also once the data is pre-generated, the cost is
minimal.

Bug: 20
Change-Id: Ib66e655ce5a0902ab3ed6695fcbb46aa87683885
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60928
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CMake and the generate builds now broadly share a source of truth for
the test files.

Update-Note: In the standalone CMake build, build/crypto/crypto_test is
now build/crypto_test, etc. For now, the build still copies the outputs
to the subdirectories (it's cheap and avoids some workflow turbulence),
but I'm thinking we keep that for six months or so and then remove it.

Bug: 542
Change-Id: I8f97e1fcedea1375d48567dfd2da01a6e66ec4e8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61286
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
As part of this, align the generated and standalone builds in how
crypto/test/gtest_main.cc is added. Since not all test targets
(urandom_test) include gtest_main.cc, and the generated one just
manually adds it to the file lists, just put it into the file lists
ahead of time. That way we don't need to synchronize the dependency
information with the generated build.

This also aligns the generated build with
https://boringssl-review.googlesource.com/c/boringssl/+/56567 to put
files like crypto/test/abi_test.cc and crypto/test/file_test_gtest.cc in
the test_support library.

Update-Note: If something odd happens with the test_support library in
downstream builds, this CL is probably to blame.

Bug: 542
Change-Id: I235e0ccd0432f4b380a92b265ede35eb8a6a6e36
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61288
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
libpki.so moved and I forgot to upload this script.

Change-Id: I920c1bd506f7e3848a47729cb70ecf72402e0b49
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61345
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Bug: 542
Change-Id: I94684f2398a28792cc0a31c8e776eecac8cc3cba
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61305
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
This changes the order of things so that by default your
system is expected to provide us with a getentropy() in
<unistd.h> for integrators that are not explicitly
supported.

We preserve the getrandom/urandom dance for Linux and Android
for now.

Linux has had getentropy() in libc's since 2017
macOS, and all the BSD's have had it for any versions we
care about.

iOS hides it from us - so we use CommonCrypto CCRandomGenerateBytes

Update-Note: Non-macOS Apple platforms now use CCRandomGenerateBytes
instead of /dev/urandom. Linux behavior remains unchanged. Platforms
which were not explicitly supported with a different codepath will also
switch from /dev/urandom to getentropy. If your platform specifically
requires /dev/urandom, let us know.know

Bug: 287
Change-Id: I0c2b8c594c473e4395379f50b0c4e6713c0a4c02
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61325
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
The delocate machinery makes it annoying to have pointers in structures.
Also this is a hair more compact.

Bug: 20
Change-Id: I2bc2dd97018277b5be55fd560f4171b7b85928ff
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60929
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This was removed in
https://boringssl-review.googlesource.com/c/boringssl/+/56925

Change-Id: Id55859b26b7a87d77586c7b27008d07fe12a499a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61306
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Bug: 287
Change-Id: Ia907c5dd7fd31e95098730673d2da1bede6d79ed
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61405
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Instead, rely on internal.h to either include the header or polyfill it.
On Windows, we don't (yet) require C11, so we can't rely on the header
being directly includable. Though given it took a couple months to
notice this, it's clear the non-C11 path is pretty much untested, so we
need to get rid of it.

Bug: 624
Change-Id: I86a6961c93161c3adfacd374affb8bfb2be0a542
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61445
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
See openssl/openssl#21371

Change-Id: I4c2cf9a0f5cea1a65063d4a83c194b5e9eeb877c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61385
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Another point indirection removed. As part of this, remove the extra
copy of one and just use generator.raw.Z.

Bug: 20
Change-Id: I066f624fe02e17082383afc15871ab2431e97b61
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60930
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This allows operating systems to insist on IBT
enforcement as an exploit mitigation mechanism without
needing to make an exception for anything using a
bundled boringssl, such as chrome, mono, and qtwebengine.

Change-Id: Iac28dd3d2af177b89ffde10ae97bce23739feb94
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60625
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Also fix the comments for ERR_STATE because they were actually wrong.

Bug: 516
Change-Id: I3b352fc75e63075a9f02f33c6e23da0f821a323e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61425
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
davidben and others added 27 commits September 18, 2023 21:15
Instead the spelling is message(FATAL_ERROR "blah"). Although
error("blah") also works because it just complains that error doesn't
exist.

Change-Id: I80384e0198a9013f93f9403d0a4c256749905045
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63106
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
CMake 3.12 was released July 2018, so it meets our five year rule. [0]
also requires a CMake version newer than 3.12.

[0] https://github.com/google/oss-policies-info/blob/main/foundational-cxx-support-matrix.md

Change-Id: I2b21d68e3a379108edde9c7d13450bba52f41235
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63105
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Bug: 649
Change-Id: Ib47e843496e58a5cdb3cd04b3929e0a08ba09744
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63145
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This is an old workaround from the AMD K8 days. GCC stopped doing it for
their generic output in 2017.
https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=4ca47ced33cc0d6f9e336930d628a6fdbf22f6e2

b/65150507#comment2 says LLVM has never done it.

We can retire this now and recover a small handful of bytes.

Change-Id: I37ef47038b6b3a1a7500bcea8cbd1beefc83121c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63205
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
This removes one more patch, and adapts import to deal with gmock from chrome
which is now included in boring.

Bug: chromium:1322914
Change-Id: I2a5957f741252941fea76205a21e98fd655f8cae
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63225
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
Reviewed-by: Adam Langley <agl@google.com>
It's probably worth explaining in a comment that this is about
implementation-defined behavior, and why we consider it okay to make
assumptions like uint8_t == unsigned char.

Change-Id: Ia35248aef7895b0998831b6bac06993e845e6297
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63285
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
… about

Try to support more than what BoringSSL does w.r.t. aliasing pointers.
@briansmith briansmith self-assigned this Sep 30, 2023
@briansmith briansmith merged commit feb3942 into main Sep 30, 2023
265 checks passed
@briansmith briansmith deleted the b/merge-boringssl-18 branch September 30, 2023 03:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants