This aligns with OpenSSL. In particular, we clear not_resumable as soon
as the SSL_SESSION is complete, but it may not have an ID or ticket.
(Due to APIs like SSL_get_session, SSL_SESSION needs to act both as a
resumption handle and a bundle of connection properties.)
Along the way, use the modified function in a few internal checks which,
with the ssl_update_cache change, removes the last dependency within the
library on the placeholder SHA256 IDs.
Change-Id: Ic225109ff31ec63ec08625e9f61a20cf0d9dd648
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47447
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Callers using private key callbacks may retain non-trivial state with a
private key. In many cases, the private key is no longer necessary
immediately after the first round-trip (e.g. non-HRR TLS 1.3
connections). Add a function that callers can query to drop the state a
hair earlier.
This is tested in two ways. First, the asserts in front of using the
key, combined with existing tests, ensure we don't start reporting it
too early. Second, I've added tests in ssl_test.cc to assert we report
it as early as we expect to.
In doing so, the number of parameters on ConnectClientAndServer()
started getting tedious, so I've split that into a
CreateClientAndServer() and CompleteHandshakes(). Callers that need to
configure weird things or drive the handshake manually can call
CreateClientAndServer() (which takes care of the BIO pair business) and
continue from there.
Bug: b/183734559
Change-Id: I05e1edb6d269c8468ba7cde7dc90e0856694a0ca
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47344
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
See go/handshake-hints (internal).
CL originally by Bin Wu <wub@google.com>. I just reworked the tests and
tidied it up a bit. This is the start of a replacement for the split
handshakes API. For now, only TLS 1.3 is supported. It starts with an
initial set of hints, but we can add more later. (In particular, we
should probably apply the remote handshaker's extension order to avoid
needing to capability protect such changes.)
Change-Id: I7b6a6dfaa84c6c6e3436d2a4026c3652b8a79f0f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46535
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
See also 86a90dc749af91f8a7b8da6628c9ffca2bae3009 from upstream. This
differs from upstream's which treats {NULL, 2} as a valid way to spell
the empty list. (I think this is a mistake and have asked them about
it.)
Upstream's CL also, for them, newly makes the empty list disable ALPN,
when previously they'd disable it but misread it as a malloc failure.
For us, we'd already fixed the misreading due to our switch to
bssl::Array and bssl::Span, but the documentation was odd. This CL
preserves that behavior, but updates the documentation and writes a
test.
Update-Note: SSL_CTX_set_alpn_protos and SSL_set_alpn_protos will now
reject invalud inputs. Previously, they would accept them, but silently
send an invalid ALPN extension which the server would almost certainly
error on.
Change-Id: Id5830b2d8c3a5cee4712878fe92ee350c4914367
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46804
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This CL adds an initial implementation of the ECH server, with pieces of
the client in BoGo as necessary for testing. In particular, the server
supports ClientHelloInner compression with ech_outer_extensions. When
ECH decryption fails, it can send retry_configs back to the client.
This server passes the "ech-accept" and "ech-reject" test cases in
tls-interop-runner[0] when tested against both the cloudflare-go and nss
clients. For reproducibility, I started with the main branch at commit
707604c262d8bcf3e944ed1d5a675077304732ce and updated the endpoint's
script to pass the server's ECHConfig and private key to the boringssl
tool.
Follow-up CLs will update HPKE to the latest draft and catch us up to
draft-10.
[0]: https://github.com/xvzcf/tls-interop-runner
Bug: 275
Change-Id: I49be35af46d1fd5dd9c62252f07d0bae179381ab
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45285
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This reverts commit a3437c09c7. There was
a miscommunication and it does not seem like we currently need this. If
that changes later, it's in Git and we can bring it back easily.
Change-Id: Ibbce29df2258a2d893d725ab3ee6fd78c5b6cb00
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46286
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
See draft-davidben-tls13-pkcs1-00. The code point is disabled by default
and must be configured in SSL_set_verify_algorithm_prefs and
SSL_set_signing_algorithm_prefs. It is also only defined for TLS 1.3
client certificates and otherwise ignored.
This required reworking the tests a bit since this is the first
signature algorithm that's disabled by default, and the first algorithm
that behaves differently between client and server.
Change-Id: Iac4aa96a4963cbc33688c252e958a572c5c3b511
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46187
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This aligns with OpenSSL's behavior. RFC7301 says servers should return
no_application_protocol if the client supported ALPN but no common
protocol was found. We currently interpret all values as
SSL_TLSEXT_ERR_NOACK. Instead, implement both modes and give guidance on
whne to use each. (NOACK is still useful because the callback may be
shared across multiple configurations, some of which don't support ALPN
at all. Those would want to return NOACK to ignore the list.)
To match upstream, I've also switched SSL_R_MISSING_ALPN, added for
QUIC, to SSL_R_NO_APPLICATION_PROTOCOL.
Update-Note: Callers that return SSL_TLSEXT_ERR_ALERT_FATAL from the
ALPN callback will change behavior. The old behavior may be restored by
returning SSL_TLSEXT_ERR_NOACK, though see the documentation for new
recommendations on return values.
Change-Id: Ib7917b5f8a098571bed764c79aa7a4ce0f728297
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45504
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
IETF QUIC draft 33 is replacing the TLS extension
codepoint for QUIC transport parameters from 0xffa5
to 57. To support multiple versions of Chrome, we
need to support both codepoints in BoringSSL. This
CL adds support for the new codepoint in a way that
can be enabled on individual connections.
Note that when BoringSSL is not in QUIC mode, it
will error if it sees the new codepoint as a server
but it will ignore the legacy codepoint as that could
be a different private usage of that codepoint.
Change-Id: I314f8f0b169cedd96eeccc42b44153e97044388c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44704
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
IETF QUIC draft 33 is replacing the TLS extension
codepoint for QUIC transport parameters from 0xffa5
to 57. To support multiple versions of Chrome, we
need to support both codepoints in BoringSSL. This
CL adds support for the new codepoint in a way that
can be enabled on individual connections.
Change-Id: I3bf06ea0710702c0dc45bb3ff2e3d772e9f87f9b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44585
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
It's not even accurate. The term "master key" dates to SSL 2, which we
do not implement. (Starting SSL 3, "key" was replaced with "secret".)
The field stores, at various points, the TLS 1.2 master secret, the TLS
1.3 resumption master secret, and the TLS 1.3 resumption PSK. Simply
rename the field to 'secret', which is as descriptive of a name as we
can get at this point.
I've left SSL_SESSION_get_master_key alone for now, as it's there for
OpenSSL compatibility, as well as references to the various TLS secrets
since those refer to concepts in the spec. (When the dust settles a bit
on rfc8446bis, we can fix those.)
Change-Id: I3c1007eb7982788789cc5db851de8724c7f35baf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44144
Reviewed-by: Adam Langley <agl@google.com>
These APIs were used by Chromium to control the carve-out for the TLS
1.3 downgrade signal. As of
https://chromium-review.googlesource.com/c/chromium/src/+/2324170,
Chromium no longer uses them.
Update-Note: SSL_CTX_set_ignore_tls13_downgrade,
SSL_set_ignore_tls13_downgrade, and SSL_is_tls13_downgrade now do
nothing. Calls sites should be removed. (There are some copies of older
Chromium lying around, so I haven't removed the functions yet.) The
enforcement was already on by default, so this CL does not affect
callers that don't use those functions.
Change-Id: I016af8291cd92051472d239c4650602fe2a68f5b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44124
Reviewed-by: Adam Langley <agl@google.com>
QUICHE has a switch-case converting ssl_early_data_reason_t to a string
for logging. This causes a lot of churn when we add a new value.
Instead, add a function for this. Bump BORINGSSL_API_VERSION so we can
easily land a CL in QUICHE to start using the function without
coordinating repositories.
Change-Id: I176ca07b4f75a3ea7153a387219459665062aad9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43724
Reviewed-by: Nick Harper <nharper@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(Original CL by svaldez, reworked by davidben.)
Change-Id: I8570808fa5e96a1c9e6e03c4877039a22e73254f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42404
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This was introduced in OpenSSL 1.1.1, and wpa_supplicant expects us to
have it. We had this same function as SSL_CIPHER_get_value (to match
SSL_get_cipher_by_value). Align with upstream's name.
It seems we also had a ssl_cipher_get_value lying around, so fold them
together. (I've retained the assert in ssl_cipher_get_value as it seems
reasonable enough; casting a hypothetical SSLv2 cipher ID to uint16_t
would not behave correctly.)
Change-Id: Ifbec460435bbc483f2c3de988522e321f2708172
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42966
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This version adds signature algorithms to the extension
Change-Id: I91dc78d33ee81cb7a6221c7bdeefc8ea460a2d6c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42424
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Update-Note: If an SSL_QUIC_METHOD is set, connections will now fail if
ALPN is not negotiated. This new behavior can be detected by checking
if the value of BORINGSSL_API_VERSION is greater than 10.
Bug: 294
Change-Id: I42fb80aa09268e77cec4a51e49cdad79bd72fa58
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42304
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Reason codes 1000+N correspond to receiving an alert N from the peer,
rather than observing the corresponding error condition locally. This
has generally been a source of confusion for folks.
They were originally named like SSL_R_TLSV1_ALERT_DECRYPTION_FAILED, but
OpenSSL introduced a few without the "ALERT" token in
739a543ea863682f157e9aa0ee382367eb3d187c.
We then inadvertently carried the mistake over in
SSL_R_TLSV1_UNKNOWN_PSK_IDENTITY and SSL_R_TLSV1_CERTIFICATE_REQUIRED.
Fix all these to include the "ALERT" for consistency and make it
slightly less confusing. (Although perhaps it should have been
RECEIVED_ALERT or so.) Add compatibility #defines for the original
OpenSSL ones and SSL_R_TLSV1_CERTIFICATE_REQUIRED. The latter can be
removed when downstream code is fixed. The OpenSSL ones we'll probably
just leave around.
Update-Note: The renamed alerts will log slightly different strings, but
the constants used by external code are still there.
Bug: 366
Change-Id: I30c299c4ad4b2bed695bd71d0831fbe6755975a7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42384
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
David Benjamin
Daniel McArdle
Yuchen Dai
David Schinazi
Adam Langley
Steven Valdez
Watson Ladd
Nick Harper