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>
Kyber was the prestandard version of ML-KEM. Callers should be using
ML-KEM now. For now, keep the implementation behind an internal header
for libssl, but we expect to remove that soon too.
Update-Note: <openssl/experimental/kyber.h> is gone. Use
<openssl/mlkem.h> instead.
Bug: 438787615
Change-Id: I02919e5d627dcbe863e70e433c79e360a556d462
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81287
Reviewed-by: Lily Chen <chlily@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Historically, sha.h included both SHA-1 and SHA-2 functions. But SHA-1
functions mostly shouldn't be used now, and it's useful to be able to
audit at the level of header names in some contexts.
Therefore move SHA-2 things into a new sha2.h. In order not to break
everything, sha.h now includes sha2.h so no changes are needed in
existing callers.
Change-Id: I68d5e991f58a1c74ca377ba017caaff356acc870
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/80327
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: Adam Langley <agl@google.com>
Reviewed-by: 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>
EVP_PKEY_get_ec_point_conv_form comes from OpenSSL 3.x.
EVP_PKEY_get_ec_curve_nid is new because OpenSSL 3.x instead has a
string-based API.
Still a ways to go, but let's start making EVP_PKEYs more useful, so
folks don't have to dip into a parallel universe of "low-level" APIs
as much.
Bug: 42290405
Change-Id: Ide3fad08194d833a3f011116fcfb0365540e4fad
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/80009
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@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>
This function is not meaningful in BoringSSL. OpenSSL has an arbitrary
mapping from algorithms to "security levels" and offers an API to filter
TLS configuration by those levels. In OpenSSL, this function does not
return how secure |ctx| is, just what security level the caller
previously configured. As BoringSSL does not implement this API, we
return zero to report that the security levels mechanism is not used.
It is bizarre to implement a function getter without the corresponding
setter, but Python apparently wants this.
Change-Id: I0b40c9bbb2504a7a8ded28db33ece4b8535c1e7c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79687
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We used to pass the parent object to callbacks, but this isn't safe. The
object is in the middle of being destroyed, so the object's
invariants won't hold. We can't remove the parameters, but pass in NULL
instead.
This also has the side effect of making it possible to manage
CRYPTO_EX_DATA lifetime in its destructor, because we won't need to find
some handle back to the parent object. It also means that defensively
clearing fields in BIO destroy callbacks, while harmless, is more
obviously unnecessary.
Update-Note: CRYPTO_EX_free is no longer passed the parent object. We do
not expect any callers to be impacted. Callers that were impacted
probably had some object lifetime bug already.
Fixed: 412707574
Change-Id: Idda889898a9eca3b59e3200ba1d5b4f2e62f2d37
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79607
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This does not switch our internal BIOs to the public APIs yet. The
tricky aspects are:
- We need a CRYPTO_once to fill in the BIO_METHOD, which is a little
tedious without __cxa_guard_acquire.
- bio->num is not exposed. External BIOs are expected to just make their
BIO_get_data structure more involved.
Update-Note: Callers should switch to public APIs.
Bug: 412269080
Change-Id: I09a2f61c653cf7a48412d9088e437b2bbbea3bb2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79567
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@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>
- Use scopers
- BIO_new + BIO_read_filename can be replaced with BIO_new_file (note
BIO_read_filename is "rb", not "r", on Windows)
- SSL_CTX_use_certificate_chain_file is more straightforward if we use
SSL_CTX_add1_chain_cert. add0 doesn't even save a refcount bump
because we internally transform it into a CRYPTO_BUFFER anyway.
Bug: 394340436
Change-Id: I586214b6547d5d85e589242eb218d296223778c9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79010
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
SSL_CTX_use_certificate_chain_file has some extra logic where, if
SSL_CTX_use_certificate succeeds but leaves an error in the error queue,
it fails anyway.
The comment claims it's because, on cert/key mismatch, it succeeds and
leaves an error. When the function was added in upstream's
b3ca645f479b1f0bcfa1d1a65c955e8b7ff33de6, this appears to have been
true.
However, years later upstream's 6049399bafac74bd809ffac2108e8d5d284cd720
added an ERR_clear_error and made SSL_CTX_use_certificate behave more
consistently. Now the behavior is that installing a certificate silently
clears any pre-existing mismatched keys, so this case no longer exists.
The recently-added tests also demonstrate this behavior.
Change-Id: I4c7de44046f10bc653f8f1fd86aff0d6956b2b1a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79009
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Certificate chains are transfered to the respective peer during the TLS
handshake in the ServerHello, and sometimes the ClientHello, messages.
These certificate chains can be of arbitrary size due to the number of
intermediary issuers, and due to the extensions within the certificates.
To avoid resource exhaustion, BoringSSL limits the size of handshake
messages, and rejects handshakes beyond a certain size.
By default, the max message size is 16 KiB, but it can be increased with
the SSL_CTX_set_cert_max_list setting.
This setting allows handshakes to complete with large certificate
chains. However, a new problem surfaces for TLSv1.3 sessions.
In TLSv1.3, session tickets (for supporting session resumption) are sent
after the handshake in NewSessionTicket messages. BoringSSL currently
encode the entire peer certificate chain (and some other things) in the
session ticket, which means the size of the certificate chains
influence the size of the corresponding NewSessionTicket.
To avoid breaking the TLS session on oversized NewSessionTicket
messages, a BoringSSL server will refuse to send any NewSessionTicket
that looks like it will be larger than 16 KiB. Unfortunately, this
accounting is inaccurate and does not take the entire NewSessionTicket
message into account. Thus, certificate chains that are a few hundred
bytes smaller than 16 KiB can be accepted by a handshake when the
max_cert_list setting has been increased, and by the NewSessionTicket
size accounting, but then later fail on the client with an
EXCESSIVE_MESSAGE_SIZE error, because ssl_max_handshake_message_len()
returns the default 16 KiB, in turn because the handshake has finished
and that causes it to ignore the configured max_cert_list.
This patch fixes this problem by making TLSv1.3 clients keep using the
max_cert_list setting post-handshake, when it is greater than the
default 16 KiB.
Change-Id: I17d689906a12079add4ad48b679508bc09a79f7c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/78647
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@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>
The one remaining external reference was in an unused parameter where it
was impossible to obtain a non-null value. Remove the last of that
machinery from the public API. This leaves us free to completely rework
LHASH_OF(T) internally, including making it a template that understands
whether it owns its values.
(If we end up needing to revert it, we can still make the real
LHASH_OF(T) into a template. It just won't be called LHASH_OF(T)
anymore.)
Update-Note: Calling code which references LHASH_OF(T) will no longer
compile. We have had no public APIs that allow a caller to usefully
construct an LHASH_OF(T) for some time, so this only ever came up in odd
cases around bindings APIs.
Change-Id: I5a44310b04d8f8f3599f0f356b64786e62db5fbf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/78787
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>
DTLS record layer state is more complex and, with DTLS 1.3, we could not
make the simplifying assumptions that we did in DTLS 1.2. Expose enough
information so that, at idle points, we can extract state. See
go/dtls13-handover (internal) for details.
This requires maintaining some extra state on each epoch. We could
recover most of it if we did a better job not allocating unused fields
in DTLS by reworking the transport-specific portions.
Update-Note: Callers that need DTLS record layer state should migrate to
the new APIs. For now, the old APIs continue to work for DTLS 1.2, but
once caller have migrated, we'll make them TLS-only.
Bug: 42290608
Change-Id: Id3f7185e9715b9ee0e5e68108fd44d9484abd569
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/77967
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@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>
David Benjamin