In the STL, <iterator> has a std::size for arrays. Some of these could
also just be ranged for loops. One static_assert could not use
std::size(out->whatever) because out was not a compile-time value, but
std::extent_v<decltype(out->whatever)> works instead.
Change-Id: I28007c79f5583e09167b81a34a447e205ee6dd9b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81658
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Checking for an error on the error queue is very tedious. You have to
check both library and reason code separately, which often means you
need to save the error code to a local variable.
Make an ERR_equals function. I've gone ahead and just made it public API
because even within BoringSSL, it seems to come up everywhere.
Change-Id: Ia963c9ae743ac5c6939846f8f64bbc98b495ce0b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81627
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
We were simulating non-blocking writes, but not non-blocking flush.
Model it as consuming one byte in AsyncBio.
In doing so, fix an obscure corner case in DTLS: If flushing after ACK
blocked, the next try would write a new ACK to the transport. There's no
real harm in this (we're running over UDP), but our tests intentionally
check for exactly the right number of writes and this was easy to fix.
This completely doesn't matter as a non-blocking writes on UDP-like
sockets are not really a thing, much less buffered non-blocking writes
on them. I don't even concretely know of anyone relying on BIO_flush in
TCP-like BIOs anymore in libssl. But since we try to support it, we
should test that we get it right.
Fixed: 381906252
Change-Id: I5296fcb01ca409d3026ca1150d6bdeaccc868014
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81348
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Lily Chen <chlily@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We should never mix them up with a context that expects
id-rsaEncryption. Right now it fails because we can't parse the
certificate. Later it will fail at a slightly different point.
Bug: 384818542
Change-Id: I64dc99a0099f6423ffa2686bede369b14b7544b9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/80268
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This is a test certificate library that is slightly more convenient to
build certificate chains. More importantly, it reimplements the
crypto/x509 serializer with x/crypto/cryptobyte.
This is a fair amount of work, but means we can add new key types that
Go does not support, notably RSA-PSS keys. This is to ensure that later
work to add RSA-PSS keys to libcrypto (but *not* libssl) will not
regress libssl. (All this work to test that we *don't* support something
in libssl.)
Bug: 384818542
Change-Id: I54cd264cf9e774fc38d8f8780becb61f886466a3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/80267
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This field is unused and already wasn't filled in with
garbageCertificate. Removing it also makes it more obvious that runner
does not actually care if it can parse its own certificate.
Change-Id: I788a6f6fe8784579d03c1c4023728b5ca77c3f88
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79911
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
When runner fails a test at the first connection, but the shim tries to
make a second connection before it is killed, the dispatcher doesn't
recognize the shim ID and we get a confusing message:
> Error dispatching connection: shim ID 55 not found
Fix this by remembering closed shim IDs and silently rejecting them.
Change-Id: Ic8afdd853da2ab3c9ef6d7102a5a0a7d52f905df
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79808
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
P-224 is too small to meet our security requirements. It seems that
nothing is using this anymore, so remove it to avoid folks accidentally
turning it on when they don't mean to.
Update-Note: Attempting to configure P-224 in TLS will now fail. This
does not impact P-224 as a general cryptographic primitive. Note that
this was off by default, so unless your project was explicitly enabling
this, this will not impact you.
Change-Id: I7b931ef0f37e3fac87848a37c5173892442e4f4f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79427
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CL originally by Bob Beck.
We did not filter this list to things that would be usable in the
handshake. This allows the client to not bother retrying with a
credential that wouldn't be usable anyway.
Fixed: 402692373
Change-Id: I78850ada5014bfd18235cfe5463fa2973da91a30
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79188
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Converting libcrypto's is actually kind of hairy because we need to
either use CRYPTO_once or make sure downstream callers have
__cxa_guard_acquire. (Also bio->num is not part of public API.) But
let's at least demonstrate better hygiene for these few.
Bug: 412269080
Change-Id: I6a7e1fa063e4857b9f584413443dd7f5f8ae7d00
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/78848
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
A few things got lost in translation here:
OpenSSL has two (well, three with BIO_set_callback_ex) BIO callback
signatures: BIO_callback_fn and BIO_info_cb. BIO_info_cb was formerly
known as bio_info_cb.
typedef long (*BIO_callback_fn)(BIO *b, int oper, const char *argp, int argi,
long argl, long ret);
typedef int BIO_info_cb(BIO *, int, int);
Note that BIO_info_cb is a typedef for the function type, not the
pointer type.
BIO_callback_fn is used with BIO_set_callback, which is some callback
that hooks into every BIO operation, using BIO_CB_* constants.
BIO_info_cb* is used in two places. First, it is the type-erased
function pointer type for BIO_callback_ctrl, a separate control type for
BIO_CTRL_* that needs to pass in function pointers. Second, it is the
actual function pointer type for BIO_set_info_callback, which is the
only thing that uses BIO_callback_ctrl.
In the initial fork, we somehow got bio_info_cb defined as
BIO_callback_fn and passed into BIO_set_callback.
https://boringssl-review.googlesource.com/c/boringssl/+/19184 removed
BIO_set_callback, but forgot to remove the now outdated documentation
for bio_info_cb and the unusable BIO_CB_* constants.
This CL does the following:
1. Finish removing the remants of BIO_set_callback.
2. Introduce BIO_info_cb, OpenSSL's new preferred name for bio_info_cb,
and make it match OpenSSL.
3. Redefine bio_info_cb as a deprecated alias for BIO_info_cb.
4. Update all the callback_ctrl implementations to reflect the new
types (function pointer vs function). Although since function
arguments decay to function pointer arguments, it's actually moot.
5. Make the connect BIO's callback type match upstream OpenSSL and
BIO_info_cb and non-const.
This whole machinery is kinda pointless and we should probably unwind it
all, but this CL just fixes it for now.
Change-Id: I1972c62c638b96f45189d733067aa59602d9b18f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/78830
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
These seem to no longer be needed? Over time we have gotten a bit more
measured about enabling all of MSVC's warnings (MSVC is just not
designed to be used with -Wall), so I'm guessing that's what changed.
Change-Id: If71850136fb83841a423b63bdf65c2d546ba0223
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/77887
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Instead of having a pair of bespoke build definitions use the standard
FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION toggle. We actually originated
the idea of a fuzzing-specific build toggle, and then libFuzzer
standardized a toggle when we talked to them about what we were doing.
The problem is our fuzzer mode toggle substantially changed the TLS
stack behavior, such that downstream code would likely go haywire. So we
couldn't easily fold into the standard one, and all of BoringSSL's
downstream fuzzer builds were messy.
Instead, make a few changes:
1. Switch BORINGSSL_UNSAFE_DETERMINISTIC_MODE to
FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION. That flag is not expected
to cause downstream issues as it just makes the PRNG deterministic.
2. Replace BORINGSSL_UNSAFE_FUZZER_MODE with a runtime toggle that is
only available when building with
FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION.
3. Instead of the no_fuzzer_mode fuzzers being special corpora for the
client and server fuzzers, they're now just separate fuzzerrs and
follow the usual naming conventions between fuzzers and their
corpora.
Update-Note: Downstream fuzzer builds can now be simplified. If the
fuzzing infrastructure already builds with
FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION, the separate boringssl_fuzz
(or whatever) target can be removed.
Bug: 42290128
Change-Id: Ia1e479777f366908951e15067c96c9767c229f0a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/77749
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Instead, do it in a callback in the fuzzer itself. This removes a part
of BORINGSSL_UNSAFE_DETERMINISTIC_MODE that is probably a little tricky
to use automatically as FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION.
Bug: 42290128
Change-Id: Ie3a62e25b099b4cb4ca1c16581beb04fc9d9930e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/77748
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Although we only need a subset of draft-ietf-tls-tlsflags, go ahead and
implement helper functions good enough for response flags to get some
experience with the extension.
Change-Id: Iba1581686c9d1883439cfd6445e98801f8fad098
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/77128
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Now runner.go contains only the test runner, while the various test
suites are moved into their own files, named foo_tests.go. (foo_test.go
would be treated as a Go test.)
I broadly just split by the addFooTests functions, but in a few cases I
grouped them together.
Now we no longer have a single 24,000 line file with all the tests. That
was getting unwieldy.
Change-Id: I76f372f60f5f0de5f1ba0913317918a4053372a3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/77107
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
First, simplify the API a bit:
- Just take a boolean param rather than having both set and clear
functions.
- Unless we need it, no need to bother with a getter. We generally
assume that the caller knows what they configured.
Next, expand on the docs and move it with other credential APIs, not
SSL_PRIVATE_KEY_METHOD.
Finally, fix a bug and test this in runner: the TLS 1.2 handshake forgot
to check the issuer, which meant that it assumed all credentials were
viable. Fix this and add tests to cover it all. In doing so, this pulls
in the MustMatchIssuer runner plumbing out of
https://boringssl-review.googlesource.com/c/boringssl/+/73087 to land a
little sooner.
Also test that issuer matching works with delegated credentials. May as
well.
Change-Id: I22aee148dd81fb9804d80b4243b68a5ecdead480
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76708
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We had some extra logic for detecting empty certificate_authorities in a
funny way, but since this is a syntax error, runner can just check this
unconditionally and stick with a more straightforward data model.
Also individual CAs cannot be empty, so fix runner to enforce this.
Change-Id: I5e697687d6dcc44e36b4f98a6284889129a077a4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76707
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Update-Note: SSL_VERIFY_PEER_IF_NO_OBC is removed. This was used as the
transition plan between the long-deprecated TLS Channel ID, and its
even-longer-deprecated precessor, Origin-Bound Certificates. Callers
should have no more reason to use this feature. (See also cl/728350196.)
Change-Id: I7a02e92592c4f71bed343935fdb094564701bd37
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76687
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
One of our environments is using a slightly older development snapshot
leading to Go 1.24, which seems to be slightly incompatible with the
final crypto/mlkem API. Until that gets updated, revert back to the
external module.
Change-Id: I5715a6800219dc0a42bca1022fdc992a8bcbdfa3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76327
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
This lets us fold X25519 into the ECDH bits, and drop
x/crypto/curve25519, but then means we need to carry a copy of the
crypto/elliptic version for P-224 because crypto/ecdh doesn't support
it.
Change-Id: Ie053679f26462c68c1d553de97e06afdb77c7eed
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76208
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Currently, trying to "go install" something from BoringSSL's Go module
fails because the proxy-reachable name is
boringssl.googlesource.com/boringssl.git, but the go.mod name is
boringssl.googlesource.com/boringssl.
$ go install -v boringssl.googlesource.com/boringssl.git/util/fipstools/acvp/acvptool@master
go: downloading boringssl.googlesource.com/boringssl.git v0.0.0-20250122182937-e056f59c7dfd
go: boringssl.googlesource.com/boringssl.git/util/fipstools/acvp/acvptool@master: version constraints conflict:
boringssl.googlesource.com/boringssl.git@v0.0.0-20250122182937-e056f59c7dfd: parsing go.mod:
module declares its path as: boringssl.googlesource.com/boringssl
but was required as: boringssl.googlesource.com/boringssl.git
Using boringssl.googlesource.com/boringssl fails because without the
.git hint, the go tool will fetch
https://boringssl.googlesource.com/boringssl/util/fipstools/acvp/acvptool?go-get=1
which is not implemented by gitiles.
Adding .git to the module name makes the first command work.
Change-Id: I6a6a4656a34fac424114a5d65d23df677ca7de47
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76107
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Notably Go and rules_cc, but also pick up the rest while I'm here.
Updating libc++ once again required reworking the config. They seem to
change it every couple of weeks.
Bug: 396087264
Change-Id: Ied0f6fa11cd8c34fe9f0f87e63fd6283a699a22c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76167
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Bob pointed out that the previous CL didn't quiiiite move the errors
around right. I looked at the ssl_parse_client_hello_with_trailing_data
calls but not the SSL_parse_client_hello calls. As a result, we doubled
up some errors.
I'd also missed that we already have SSL_R_CLIENTHELLO_PARSE_FAILED.
That said, which error to use is a little interesting. Some codepaths
used to use SSL_R_DECODE_ERROR and some used
SSL_R_CLIENTHELLO_PARSE_FAILED. Further complicating things is that some
ClientHello error paths are unreachable because only the first time a
ClientHello is parsed matters. But we have the second ClientHello in HRR
and inner ClientHellos to content with. (A TLS connection can have up to
four ClientHellos now!)
I've erred towards picking the more specific one, given this whole mess.
Update-Note: The error when the server cannot parse the ClientHello is
now a bit more specific. This might be visible to server-specific
logging, but will not change what is sent over the wire.
Change-Id: I64a4305968616a9f414d3c95fb4ffbd1cfdc4ecc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76147
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
These were temporary flags in case of compatibility issues with some
older changes, set to be removed after June 2024. It is now well past
June 2024 and no one ever had to use these APIs. Remove them.
Update-Note: Removed some unused APIs.
Change-Id: I5fe34e0ebcb30f81281e413017d5a6a968a96a97
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76127
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
We use the standard Apache 2.0 file header, described in "APPENDIX: How
to apply the Apache License to your work."
This was primarily automated by running:
git ls-tree -r --name-only HEAD | xargs go run ./util/relicense.go
See go/boringssl-relicensing-triage for the results of triaging the
output of the tool.
As part of this, switch from taking fiat-crypto under MIT license to
Apache 2.0. (It is licensed under MIT OR Apache-2.0 OR BSD-1-Clause.)
The copyright_summary tool can also be used to confirm we didn't
accidentally drop any copyright lines:
# Run before the CL
git grep -l Copyright | xargs go run ./util/copyright_summary.go -out /tmp/old.json
# Run after the CL
git grep -l Copyright | xargs go run ./util/copyright_summary.go -compare /tmp/old.json
Bug: 364634028
Change-Id: I17c50e761e9d077a1f92e25969e50ed35e320c59
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/75852
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
I was going to add a corresponding test for PAKEs and noticed we
neglected this for PSKs. This also fixes a bug in runner where client
auth + resumption handshakes as a server didn't work right. (I guess
none of our tests exercise this case.)
Change-Id: I9e82dcbca54aedba4059e45c3e40a39b390de34e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/75667
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
I am not sure why we ran through this increasingly large block of code,
with side effects, twice. All this really needed was to send a second
HRR and make sure the client rejected.
Change-Id: I1122ef2c5f8f85e2f356a6112ae2042653469417
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/75631
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Right now, if you don't pass -expect-selected-credential in a test, it
implicitly asserts that you selected the default credential. This means
that every credential-based test must pass this flag, which is a bit
tedious when the test is not about credential dispatch.
Instead, default it not expressing a opinion about this either way.
Change-Id: I42591cee71df1e4db8c8b902efba3b646b65d4e5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/75630
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
David Benjamin