It's conditioned in OpenSSL on client offer, not server accept.
Change-Id: Iae5483a33d9365258446ce0ae34132aeb4a92c66
Reviewed-on: https://boringssl-review.googlesource.com/28545
Reviewed-by: Adam Langley <agl@google.com>
This is so Chromium can verify the session before offering it, rather
than doing it after the handshake (at which point it's too late to punt
the session) as we do today. This should, in turn, allow us to finally
verify certificates off a callback and order it correctly relative to
CertificateRequest in TLS 1.3.
(It will also order "correctly" in TLS 1.2, but this is useless. TLS 1.2
does not bind the CertificateRequest to the certificate at the point the
client needs to act on it.)
Bug: chromium:347402
Change-Id: I0daac2868c97b820aead6c3a7e4dc30d8ba44dc4
Reviewed-on: https://boringssl-review.googlesource.com/28405
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Previously, we'd omitted OpenSSL's OCSP APIs because they depend on a
complex OCSP mechanism and encourage the the unreliable server behavior
that hampers using OCSP stapling to fix revocation today. (OCSP
responses should not be fetched on-demand on a callback. They should be
managed like other server credentials and refreshed eagerly, so
temporary CA outage does not translate to loss of OCSP.)
But most of the APIs are byte-oriented anyway, so they're easy to
support. Intentionally omit the one that takes a bunch of OCSP_RESPIDs.
The callback is benign on the client (an artifact of OpenSSL reading
OCSP and verifying certificates in the wrong order). On the server, it
encourages unreliability, but pyOpenSSL/cryptography.io depends on this.
Dcument that this is only for compatibility with legacy software.
Also tweak a few things for compatilibility. cryptography.io expects
SSL_CTX_set_read_ahead to return something, SSL_get_server_tmp_key's
signature was wrong, and cryptography.io tries to redefine
SSL_get_server_tmp_key if SSL_CTRL_GET_SERVER_TMP_KEY is missing.
Change-Id: I2f99711783456bfb7324e9ad972510be8a95e845
Reviewed-on: https://boringssl-review.googlesource.com/28404
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Callers should not mutate these.
Update-Note: I believe I've fixed up everything. If I missed one, the
fix should be straightforward.
Change-Id: Ifbce4961204822f57502a0de33aaa5a2a08b026d
Reviewed-on: https://boringssl-review.googlesource.com/28266
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Update-Note: Enabling TLS 1.3 now enables both draft-23 and draft-28
by default, in preparation for cycling all to draft-28.
Change-Id: I9405f39081f2e5f7049aaae8a9c85399f21df047
Reviewed-on: https://boringssl-review.googlesource.com/28304
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Hopefully this is the last of it before we can hide the struct. We're
missing peer_sha256 accessors, and some test wants to mutate the ticket
in a test client.
Change-Id: I1a30fcc0a1e866d42acbc07a776014c9257f7c86
Reviewed-on: https://boringssl-review.googlesource.com/28268
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
gRPC builds on Debian Jessie, which has GCC 4.9.2, and builds with
-Wtype-limits, which makes it warn about code intended for 64-bit
systems when building on 32-bit systems.
We have tried to avoid these issues with Clang previously by guarding
with “sizeof(size_t) > 4”, but this version of GCC isn't smart enough to
figure that out.
Change-Id: I800ceb3891436fa7c81474ede4b8656021568357
Reviewed-on: https://boringssl-review.googlesource.com/28247
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We forgot to do this in our original implementation on general ecosystem
grounds. It's also mandated starting draft-26.
Just to avoid unnecessary turbulence, since draft-23 is doomed to die
anyway, condition this on our draft-28 implementation. (We don't support
24 through 27.)
We'd actually checked this already on the Go side, but the spec wants a
different alert.
Change-Id: I0014cda03d7129df0b48de077e45f8ae9fd16976
Reviewed-on: https://boringssl-review.googlesource.com/28124
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This is done by adding two new tagged data types to the shim's
transcript: one for the serialized handoff, and another for the
serialized handback.
Then, the handshake driver in |TLSFuzzer| is modified to be able to
drive a handoff+handback sequence in the same way as was done for
testing: by swapping |BIO|s into additional |SSL| objects. (If a
particular transcript does not contain a serialized handoff, this is a
no-op.)
Change-Id: Iab23e4dc27959ffd3d444adc41d40a4274e83653
Reviewed-on: https://boringssl-review.googlesource.com/27204
Commit-Queue: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Found by fuzzing.
Change-Id: I831f7869b16486eef7ac887ee199450e38461086
Reviewed-on: https://boringssl-review.googlesource.com/28044
Commit-Queue: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Along the way, check the version against the cipher to make sure the
combination is possible.
(Found by fuzzing: a bad version trips an assert.)
Change-Id: Ib0a284fd5fd9b7ba5ceba63aa6224966282a2cb7
Reviewed-on: https://boringssl-review.googlesource.com/27265
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
(It complains that the comparison is always false with NDK r17 beta 2.)
Change-Id: I6b695fd0e86047f0c1e4267290e63db3184a958a
Reviewed-on: https://boringssl-review.googlesource.com/28025
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
https://boringssl-review.googlesource.com/27944 inadvertently caused
SHA256 and SHA384 aliases to be rejected in
SSL_CTX_set_strict_cipher_list. While this is the desired end state, in
case the removal needs to be reverted, we should probably defer this to
post-removal cleanup.
Otherwise we might update someone's "ALL:!SHA256" cipher string to
account for the removal, and then revert the removal underneath them.
Change-Id: Id516a27a2ecefb5871485d0ae18067b5bbb536bb
Reviewed-on: https://boringssl-review.googlesource.com/28004
Reviewed-by: Adam Langley <agl@google.com>
These are also not needed after the handshake.
Change-Id: I5de2d5cf18a3783a6c04c0a8fe311069fb51b939
Reviewed-on: https://boringssl-review.googlesource.com/27986
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The TLS 1.3 client logic used ctx instead. This is all moot as
SSL_set_SSL_CTX on a client really wouldn't work, but we should be
consistent. Unfortunately, this moves moving the pointer back to SSL
from SSL_CONFIG.
Change-Id: I45f8241e16f499ad416afd5eceb52dc82af9c4f4
Reviewed-on: https://boringssl-review.googlesource.com/27985
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
All CBC ciphers in TLS are broken and insecure. TLS 1.2 introduced
AEAD-based ciphers which avoid their many problems. It also introduced
new CBC ciphers based on HMAC-SHA256 and HMAC-SHA384 that share the same
flaws as the original HMAC-SHA1 ones. These serve no purpose. Old
clients don't support them, they have the highest overhead of all TLS
ciphers, and new clients can use AEADs anyway.
Remove them from libssl. This is the smaller, more easily reverted
portion of the removal. If it survives a week or so, we can unwind a lot
more code elsewhere in libcrypto. This removal will allow us to clear
some indirect calls from crypto/cipher_extra/tls_cbc.c, aligning with
the recommendations here:
https://github.com/HACS-workshop/spectre-mitigations/blob/master/crypto_guidelines.md#2-avoid-indirect-branches-in-constant-time-code
Update-Note: The following cipher suites are removed:
- TLS_RSA_WITH_AES_128_CBC_SHA256
- TLS_RSA_WITH_AES_256_CBC_SHA256
- TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256
- TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384
- TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256
- TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384
Change-Id: I7ade0fc1fa2464626560d156659893899aab6f77
Reviewed-on: https://boringssl-review.googlesource.com/27944
Reviewed-by: Adam Langley <agl@google.com>
Chrome needs to support renegotiation at TLS 1.2 + HTTP/1.1, but we're
free to shed the handshake configuration at TLS 1.3 or HTTP/2.
Rather than making config shedding implicitly disable renegotiation,
make the actual shedding dependent on a combination of the two settings.
If config shedding is enabled, but so is renegotiation (including
whether we are a client, etc.), leave the config around. If the
renegotiation setting gets disabled again after the handshake,
re-evaluate and shed the config then.
Bug: 123
Change-Id: Ie833f413b3f15b8f0ede617991e3fef239d4a323
Reviewed-on: https://boringssl-review.googlesource.com/27904
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Matt Braithwaite <mab@google.com>
|SSL_CONFIG| is a container for bits of configuration that are
unneeded after the handshake completes. By default it is retained for
the life of the |SSL|, but it may be shed at the caller's option by
calling SSL_set_shed_handshake_config(). This is incompatible with
renegotiation, and with SSL_clear().
|SSL_CONFIG| is reachable by |ssl->config| and by |hs->config|. The
latter is always non-NULL. To avoid null checks, I've changed the
signature of a number of functions from |SSL*| arguments to
|SSL_HANDSHAKE*| arguments.
When configuration has been shed, setters that touch |SSL_CONFIG|
return an error value if that is possible. Setters that return |void|
do nothing.
Getters that request |SSL_CONFIG| values will fail with an |assert| if
the configuration has been shed. When asserts are compiled out, they
will return an error value.
The aim of this commit is to simplify analysis of split-handshakes by
making it obvious that some bits of state have no effects beyond the
handshake. It also cuts down on memory usage.
Of note: |SSL_CTX| is still reachable after the configuration has been
shed, and a couple things need to be retained only for the sake of
post-handshake hooks. Perhaps these can be fixed in time.
Change-Id: Idf09642e0518945b81a1e9fcd7331cc9cf7cc2d6
Bug: 123
Reviewed-on: https://boringssl-review.googlesource.com/27644
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
This is prefactoring for a coming change to the shim that will write
handoff and handback messages (which are serialized SSLConnection
objects) to the transcript.
This breaks the slightly tenuous ordering between the runner and the
shim. Fix the runner to wait until the shim has exited before
appending the transcript.
Change-Id: Iae34d28ec1addfe3ec4f3c77008248fe5530687c
Reviewed-on: https://boringssl-review.googlesource.com/27184
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Chromium has some code which reaches into this field for memory
accounting.
This fixes a bug in doc.go where this line-wrapping confuses it. doc.go
needs a bit of a rewrite, but this is a bit better.
Change-Id: Ic9cc2c2fe9329d7bc366ccf91e0c9a92eae08ed2
Reviewed-on: https://boringssl-review.googlesource.com/27764
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
I don't think this lock is actually needed. If the process exited by the
time we call shim.Process.Kill(), then the test ultimately finished. If
not, wait() will return that the process died by a signal.
Change-Id: I668a86583aba16fd00e0cd05071acc13059a2c42
Reviewed-on: https://boringssl-review.googlesource.com/27325
Reviewed-by: Adam Langley <agl@google.com>
After e325c3f471, this typo bites and
causes SSL_CTX_get_extra_chain_certs to return an empty stack.
Change-Id: I6aa7093d1ca4f3ba0f520a644b14de5b3a3ccaa6
Reviewed-on: https://boringssl-review.googlesource.com/27604
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This schism came up in passing again, and I realized we never added a
TLS-level test for this. Fix that.
Change-Id: I10f910bb5a975d6b3b73d99e7412ade35654fddb
Reviewed-on: https://boringssl-review.googlesource.com/27224
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This too is connection-level state to be reset on SSL_clear.
Change-Id: I071c9431c28a7d0ff3eb20c679784d4aa4c236a5
Reviewed-on: https://boringssl-review.googlesource.com/27490
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Chrome uses the platform certificate verifier and thus cannot reliably
expect PSS signatures to work in all configurations. Add an API for the
consumer to inform BoringSSL of this ability. We will then adjust our
advertisements accordingly.
Note that, because TLS 1.2 does not have the signature_algorithms_cert
extension, turning off TLS 1.3 and using this API will stop advertising
RSA-PSS. I believe this is the correct behavior given the semantics of
that code point.
The tests check the various combinations here, as well as checking that
the peer never sends signature_algorithms_cert identical to
signature_algorithms.
Bug: 229
Change-Id: I8c33a93efdc9252097e3899425b49548fc42a93a
Reviewed-on: https://boringssl-review.googlesource.com/27488
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
It's hard to diagnose "20".
Change-Id: I57e8d0fb6e4937ddeca45b3645463ca0dc872ea6
Reviewed-on: https://boringssl-review.googlesource.com/27487
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This reflects the change to add the key type into the constant. The old
constants are left around for now as legacy aliases and will be removed
later.
Change-Id: I67f1b50c01fbe0ebf4a2e9e89d3e7d5ed5f5a9d7
Reviewed-on: https://boringssl-review.googlesource.com/27486
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Update-Note: I believe everything relying on this overload has since
been updated.
Change-Id: I7facf59cde56098e5e3c79470293b67abb715f4c
Reviewed-on: https://boringssl-review.googlesource.com/27485
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
These are connection state, so they should be reset on SSL_clear.
Change-Id: I861fe52578836615d2719c9e1ff0911c798f336e
Reviewed-on: https://boringssl-review.googlesource.com/27384
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Conscrypt need this function right now. They ought to be fixed up to not
need this but, in the meantime, this API is also provided by OpenSSL and
will clear one most consumer reaching into SSL_SESSION.
Bumping the API since Conscrypt often involves multi-sided stuff.
Change-Id: I665ca6b6a17ef479133c29c23fc639f278128c69
Reviewed-on: https://boringssl-review.googlesource.com/27405
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
In the handoff+handback case, bssl_shim.cc creates 3 |SSL| objects:
one to receive the ClientHello, one to receive the handoff, and a
third one to receive the handback.
Before 56986f9, only the first of these received any configuration.
Since that commit, all 3 of them receive the same configuration. That
means that the handback message no longer needs to serialize as many
things.
N.B. even before 56986f9, not all of the fields were necessary. For
example, there was no reason to serialize |conf_max_version| and
|conf_min_version| in the handback, so far as I can tell.
This commit is mechanical: it simply removes everything that doesn't
cause any tests to fail. In the long run, I'll need to carefully
check for two possibilities:
- Knobs that affect the handshake after the server's first message it
sent. These are troublesome because that portion of the handshake
may run on a different |SSL|, depending on whether the handback is
early or late.
- Getters that may be called post-handshake, and that callers may
reasonably expect to reflect the value that was used during
handshake.
(I'm not sure that either case exists!)
Change-Id: Ibf6e0be6609ad6e83ab50e69199e9b2d51e59a87
Reviewed-on: https://boringssl-review.googlesource.com/27364
Commit-Queue: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
(Found by fuzzing: a zero value causes an infinite loop.)
Change-Id: I984fd88d85fb87616b5e806795c10334f4379744
Reviewed-on: https://boringssl-review.googlesource.com/27345
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The last-minute TLS 1.3 change was done partly for consistency with DTLS
1.3, where authenticating the record header is less obviously pointless
than in TLS. There, reconstructing it would be messy. Instead, pass in
the record header and let SSLAEADContext decide whether or not to
assemble its own.
(While I'm here, reorder all the flags so the AD and nonce ones are
grouped together.)
Change-Id: I06e65d526b21a08019e5ca6f1b7c7e0e579e7760
Reviewed-on: https://boringssl-review.googlesource.com/27024
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I7298c878bd2c8187dbd25903e397e8f0c2575aa4
Reviewed-on: https://boringssl-review.googlesource.com/26846
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
This changes the contract for split handshakes such that on the
receiving side, the connection is to be driven until it returns
|SSL_ERROR_HANDBACK|, rather than until SSL_do_handshake() returns
success.
Change-Id: Idd1ebfbd943d88474d7c934f4c0ae757ff3c0f37
Reviewed-on: https://boringssl-review.googlesource.com/26864
Commit-Queue: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
NSS only enables compatibility mode on the server if the client
requested it by way of the session ID. This is slightly off as a client
has no way not to request it when offering a TLS 1.2 session, but it is
in the spec.
So our tests are usable for other stacks, send a fake session ID in the
runner by default. The existing EmptySessionID-TLS13* test asserts that
BoringSSL behaves as we expect it to on empty session IDs too. The
intent is that NSS will disable that test but can otherwise leave the
rest enabled.
Change-Id: I370bf90aba1805c2f6970ceee0d29ecf199f437d
Reviewed-on: https://boringssl-review.googlesource.com/26504
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
It has now been folded into ServerHello. Additionally, TLS 1.2 and TLS
1.3 ServerHellos are now more uniform, so we can avoid the extra
ServerHello parser.
Change-Id: I46641128c3f65fe37e7effca5bef4a76bf3ba84c
Reviewed-on: https://boringssl-review.googlesource.com/26524
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Checking |initial_handshake_complete| was a mistake—it's not true for
False Start connections at the time when Chrome wants to measure whether
PQ padding was used or not.
Change-Id: I51757e00f3e02129666ee1ce31c30d63f1bcbe74
Reviewed-on: https://boringssl-review.googlesource.com/26444
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
On reflection, I think we'll need to note whether dummy PQ padding was
echoed on a given connection. Otherwise measurements in Chrome will be
mixed with cases where people have MITM proxies that ignored the
extension, or possibly Google frontends that haven't been updated.
Therefore this change will be used to filter latency measurements in
Chrome to only include those where the extension was echoed and we'll
measure at levels of 1 byte (for control), 400 bytes, and 1100 bytes.
This also makes it an error if the server didn't echo an extension of
the same length as was sent.
Change-Id: Ib2a0b29cfb8719a75a28f3cf96710c57d88eaa68
Reviewed-on: https://boringssl-review.googlesource.com/26284
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
In this round, Google servers will echo the extension in order to test
the latency of both parties sending a PQ key-agreement message.
The extension is sent (and echoed) for both full and resumption
handshakes. This is intended to mirror the overhead of TLS 1.3 (even
when using TLS 1.2), as a resumption in TLS 1.3 still does a fresh key
agreement.
Change-Id: I9ad163afac4fd1d916f9c7359ec32994e283abeb
Reviewed-on: https://boringssl-review.googlesource.com/26185
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
SSLv3_method, SSLv3_client_method, and SSLv3_server_method produce
SSL_CTXs which fail every handshake. They appear no longer necessary for
compatibility, so remove them.
SSLv3 is still accessible to callers who explicitly re-enable SSLv3 on a
TLS_method, but that will be removed completely later this year.
Meanwhile, clear out a weird hack we had here.
Update-Note: I believe there are no more callers of these functions. Any
that were were already non-functional as these methods haven't been
unable to handshake for a while now.
Change-Id: I622f785b428ab0ceab77b5a9db05b2b0df28145a
Reviewed-on: https://boringssl-review.googlesource.com/26004
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We don't advertise compressed coordinates (and point format negotiation
was deprecated in TLS 1.3), so reject them. Both Internet Explorer and
Firefox appear to reject them already.
Later I hope to add an easier to use ECDH API that acts on bytes, not
EC_POINT. This clears the way for that API to only accept uncompressed
coordinates. Compressed coordinates never got deployed over NIST curves,
for better or worse. At this point, there is no sense in changing that
as new protocols should use curve25519.
Change-Id: Id2f1be791ddcf155d596f4eb0b79351766c5cdab
Reviewed-on: https://boringssl-review.googlesource.com/26024
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
The private key callback may not push one of its own (it's possible to
register a custom error library and whatnot, but this is tedious). If
the callback does not push any, we report SSL_ERROR_SYSCALL. This is not
completely wrong, as "syscall" really means "I don't know, something you
gave me, probably the BIO, failed so I assume you know what happened",
but most callers just check errno. And indeed cert_cb pushes its own
error, so this probably should as well.
Update-Note: Custom private key callbacks which push an error code on
failure will report both that error followed by
SSL_R_PRIVATE_KEY_OPERATION_FAILED. Callbacks which did not push any
error will switch from SSL_ERROR_SYSCALL to SSL_ERROR_SSL with
SSL_R_PRIVATE_KEY_OPERATION_FAILED.
Change-Id: I7e90cd327fe0cbcff395470381a3591364a82c74
Reviewed-on: https://boringssl-review.googlesource.com/25544
Reviewed-by: Adam Langley <agl@google.com>
All the patterns need to account for a possible "-Split" version now.
Change-Id: Ie1b38ce10777d61d70a4d5a8bb2d44cdc98e4bfb
Reviewed-on: https://boringssl-review.googlesource.com/25504
Reviewed-by: Adam Langley <agl@google.com>
This change adds a couple of focused tests to ssl_test.cc, but also
programmically duplicates many runner tests in a split-handshake mode.
Change-Id: I9dafc8a394581e5daf1318722e1015de82117fd9
Reviewed-on: https://boringssl-review.googlesource.com/25388
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Split handshakes allows the handshaking of a TLS connection to be
performed remotely. This encompasses not just the private-key and ticket
operations – support for that was already available – but also things
such as selecting the certificates and cipher suites.
The the comment block in ssl.h for details. This is highly experimental
and will change significantly before its settled.
Change-Id: I337bdfa4c3262169e9b79dd4e70b57f0d380fcad
Reviewed-on: https://boringssl-review.googlesource.com/25387
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Change-Id: I2486dc810ea842c534015fc04917712daa26cfde
Update-Note: Now that tls13_experiment2 is gone, the server should remove the set_tls13_variant call. To avoid further churn, we'll make the server default for future variants to be what we'd like to deploy.
Reviewed-on: https://boringssl-review.googlesource.com/25104
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
This adds support for sending the quic_transport_parameters
(draft-ietf-quic-tls) in ClientHello and EncryptedExtensions, as well as
reading the value sent by the peer.
Bug: boringssl:224
Change-Id: Ied633f557cb13ac87454d634f2bd81ab156f5399
Reviewed-on: https://boringssl-review.googlesource.com/24464
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Since SSL{,_CTX}_set_custom_verify take a |mode| parameter that may be
|SSL_VERIFY_NONE|, it should do what it says on the tin, which is to
perform verification and ignore the result.
Change-Id: I0d8490111fb199c6b325cc167cf205316ecd4b49
Reviewed-on: https://boringssl-review.googlesource.com/25224
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
This function can serialise a session to a |CBB|.
Change-Id: Icdb7aef900f03f947c3fa4625dd218401eb8eafc
Reviewed-on: https://boringssl-review.googlesource.com/25385
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Mono's legacy TLS 1.0 stack, as a server, does not implement any form of
resumption, but blindly echos the ClientHello session ID in the
ServerHello for no particularly good reason.
This is invalid, but due to quirks of how our client checked session ID
equality, we only noticed on the second connection, rather than the
first. Flaky failures do no one any good, so break deterministically on
the first connection, when we realize something strange is going on.
Bug: chromium:796910
Change-Id: I1f255e915fcdffeafb80be481f6c0acb3c628846
Reviewed-on: https://boringssl-review.googlesource.com/25424
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Running can spawn gdb in an xterm, but the default xterm is rather
small. We could have everyone set their .Xdefaults, I presume, to solve
this, but very few people are running the old xterm these days.
Change-Id: I46eb3ff22f292eb44ce8c5124e83f1ab8aef9547
Reviewed-on: https://boringssl-review.googlesource.com/24846
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This change reslices how the functions that generate the key block and
initialise the TLS AEADs are cut. This makes future changes easier.
Change-Id: I7e0f7327375301bed96f33c195b80156db83ce6d
Reviewed-on: https://boringssl-review.googlesource.com/24845
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I7932258890b0b2226ff6841af45926e1b11979ba
Reviewed-on: https://boringssl-review.googlesource.com/24844
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
No sense in tempting middleboxes unnecessarily.
Change-Id: Iec66f77195f6b8aa62be681917342e59eb7aba31
Reviewed-on: https://boringssl-review.googlesource.com/24964
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Update-Note: Token Binding can no longer be configured with the custom
extensions API. Instead, use the new built-in implementation. (The
internal repository should be all set.)
Bug: 183
Change-Id: I007523a638dc99582ebd1d177c38619fa7e1ac38
Reviewed-on: https://boringssl-review.googlesource.com/20645
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
NIAP requires that the TLS KDF be tested by CAVP so this change moves
the PRF into crypto/fipsmodule/tls and adds a test harness for it. Like
the KAS tests, this is only triggered when “-niap” is passed to
run_cavp.go.
Change-Id: Iaa4973d915853c8e367e6106d829e44fcf1b4ce5
Reviewed-on: https://boringssl-review.googlesource.com/24666
Reviewed-by: Adam Langley <agl@google.com>
This extension will be used to measure the latency impact of potentially
sending a post-quantum key share by default. At this time it's purely
measuring the impact of the client sending the key share, not the server
replying with a ciphertext.
We could use the existing padding extension for this but that extension
doesn't allow the server to echo it, so we would need a different
extension in the future anyway. Thus we just create one now.
We can assume that modern clients will be using TLS 1.3 by the time that
PQ key-exchange is established and thus the key share will be sent in
all ClientHello messages. However, since TLS 1.3 isn't quite here yet,
this extension is also sent for TLS 1.0–1.2 ClientHellos. The latency
impact should be the same either way.
Change-Id: Ie4a17551f6589b28505797e8c54cddbe3338dfe5
Reviewed-on: https://boringssl-review.googlesource.com/24585
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
The language of RFC 5246 is "A certificate has expired or is not
currently valid", which sounds to me like |certificate_expired| should
pertain to any case where the current time is outside the
certificate's validity period.
Along the way, group the |unknown_ca| errors together.
Change-Id: I92c1fe3fc898283d0c7207625de36662cd0f784e
Reviewed-on: https://boringssl-review.googlesource.com/24624
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
CMake targets are visible globally but gtest_main has boringssl-specific
behavior that isn't appropriate for general use.
This change makes it possible to use boringssl and abseil-cpp in the
same project (since abseil-cpp expects gtest_main to exist and be useful
for its own tests).
Change-Id: Icc81c11b8bb4b1e21cea7c9fa725b6c082bd5369
Reviewed-on: https://boringssl-review.googlesource.com/24604
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This function maps |X509_V_ERR_*| to SSL alarm codes. It's used
internally when certs are verified with X509_verify_cert(), and is
helpful to callers who want to call that function, but who also want
to report its errors in a less implementation-dependent way.
Change-Id: I2900cce2eb631489f0947c317beafafd3ea57a75
Reviewed-on: https://boringssl-review.googlesource.com/24564
Commit-Queue: Matt Braithwaite <mab@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
TLS 1.3 includes a server-random-based anti-downgrade signal, as a
workaround for TLS 1.2's ServerKeyExchange signature failing to cover
the entire handshake. However, because TLS 1.3 draft versions are each
doomed to die, we cannot deploy it until the final RFC. (Suppose a
draft-TLS-1.3 client checked the signal and spoke to a final-TLS-1.3
server. The server would correctly negotiate TLS 1.2 and send the
signal. But the client would then break. An anologous situation exists
with reversed roles.)
However, it appears that Cisco devices have non-compliant TLS 1.2
implementations[1] and copy over another server's server-random when
acting as a TLS terminator (client and server back-to-back).
Hopefully they are the only ones doing this. Implement a
measurement-only version with a different value. This sentinel must not
be enforced, but it will tell us whether enforcing it will cause
problems.
[1] https://www.ietf.org/mail-archive/web/tls/current/msg25168.html
Bug: 226
Change-Id: I976880bdb2ef26f51592b2f6b3b97664342679c8
Reviewed-on: https://boringssl-review.googlesource.com/24284
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This is connection state, not configuration, so it must live on
ssl->s3, otherwise SSL_clear will be confused.
Change-Id: Id7c87ced5248d3953e37946e2d0673d66bfedb08
Reviewed-on: https://boringssl-review.googlesource.com/24264
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Otherwise it leaves something on the error queue and confuses
SSL_get_error, should the handshake state machine fail immediately
afterwards because of a BIO-level error.
Change-Id: I2c7b5e31368b9c5b2efa324166f52972430d6074
Reviewed-on: https://boringssl-review.googlesource.com/24247
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Upgrade-Note: SSL_CTX_set_tls13_variant(tls13_experiment) on the server
should switch to SSL_CTX_set_tls13_variant(tls13_experiment2).
(Configuring any TLS 1.3 variants on the server enables all variants,
so this is a no-op. We're just retiring some old experiments.)
Change-Id: I60f0ca3f96ff84bdf59e1a282a46e51d99047462
Reviewed-on: https://boringssl-review.googlesource.com/23784
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
QUIC will need to derive keys at this point. This also smooths over a
part of the server 0-RTT abstraction. Like with False Start, the SSL
object is largely in a functional state at this point.
Bug: 221
Change-Id: I4207d8cb1273a1156e728a7bff3943cc2c69e288
Reviewed-on: https://boringssl-review.googlesource.com/24224
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: Ic79f189c0bb2abf5d87f59ee410cafb4fb116ab8
Reviewed-on: https://boringssl-review.googlesource.com/24004
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
It's misnamed but, more importantly, doesn't do anything because the
test client isn't sending early data to begin with. We really need to
make these tests less error-prone to write. With this fix, the test
actually notices if we remove the server-side 0-RTT check.
Also remove MaxEarlyDataSize from the other server tests which
erroneously set it. Any test with sets that was likely copy-and-pasted
incorrectly.
Change-Id: Idc24bc1590e0316946022341185285418ab8c77b
Reviewed-on: https://boringssl-review.googlesource.com/23944
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We can probably do this globally at this point since the cipher
requirements are much more restrict than they were in the beginning.
(Firefox, in particular, has done so far a while.) For now add a flag
since some consumer wanted this.
I'll see about connecting it to a Chrome field trial after our breakage
budget is no longer reserved for TLS 1.3.
Change-Id: Ib61dd5aae2dfd48b56e79873a7f3061a7631a5f8
Reviewed-on: https://boringssl-review.googlesource.com/23725
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I87edf7e1fee07da4bc93cc7ab524b79991a4206e
Reviewed-on: https://boringssl-review.googlesource.com/23724
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: Ic99a949258e62cad168c2c39507ca63100a8ffe5
Reviewed-on: https://boringssl-review.googlesource.com/23264
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This throws me off every time.
Change-Id: I19848927fe821f7656dea0343361d70dae4007c9
Reviewed-on: https://boringssl-review.googlesource.com/23445
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
After much procrastinating, we finally moved Chromium to the new stuff.
We can now delete this. This is a breaking change for
SSL_PRIVATE_KEY_METHOD consumers, but it should be trivial (remove some
unused fields in the struct). I've bumped BORINGSSL_API_VERSION to ease
any multi-sided changes that may be needed.
Change-Id: I9fe562590ad938bcb4fcf9af0fadeff1d48745fb
Reviewed-on: https://boringssl-review.googlesource.com/23224
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Change-Id: I82f92019dccfaf927f7180a5af53c9ffae111861
Reviewed-on: https://boringssl-review.googlesource.com/23145
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We were only running a random subset of TLS 1.3 tests with variants and
let a lot of bugs through as a result.
- HelloRetryRequest-EmptyCookie wasn't actually testing what we were
trying to test.
- The second HelloRetryRequest detection needs tweaks in draft-22.
- The empty HelloRetryRequest logic can't be based on non-empty
extensions in draft-22.
- We weren't sending ChangeCipherSpec correctly in HRR or testing it
right.
- Rework how runner reads ChangeCipherSpec by setting a flag which
affects the next readRecord. This cuts down a lot of cases and works
correctly if the client didn't send early data. (In that case, we
don't flush CCS until EndOfEarlyData and runner deadlocks waiting for
the ChangeCipherSpec to arrive.)
Change-Id: I559c96ea3a8b350067e391941231713c6edb2f78
Reviewed-on: https://boringssl-review.googlesource.com/23125
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I9da9734625d1d9d2c783830d8b4aecd34f51acc6
Reviewed-on: https://boringssl-review.googlesource.com/23124
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Error paths must always have OPENSSL_PUT_ERROR.
Change-Id: I0ed8c8288484a4ea69ec58317064ad3cd90ddd64
Reviewed-on: https://boringssl-review.googlesource.com/23104
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The current PR says the sender only skips it during the handshake. Add a
test that we got this right.
Change-Id: Ib27eb942f11d955b8a24e32321efe474037f5254
Reviewed-on: https://boringssl-review.googlesource.com/23024
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
See https://github.com/tlswg/tls13-spec/pull/1083. We misread the
original text spec, but it turns out the original spec text required
senders have version-specific maximum send fragments. The PR fixes this
off-by-one issue. Align with the new spec text uniformly.
This is a wire format change for our existing drafts *only if* records
have padding. We don't currently send padding, so this is fine. Unpadded
records continue to be capped at 2^14 bytes of plaintext (or 2^14+1
bytes of TLSInnerPlaintext structure).
Change-Id: I01017cfd13162504bb163dd59afd74aff0896cc4
Reviewed-on: https://boringssl-review.googlesource.com/23004
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I1a0f264cbfa0eb5d4adac96d0fc24fa342f2b6a3
Reviewed-on: https://boringssl-review.googlesource.com/22946
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This introduces a wire change to Experiment2/Experiment3 over 0RTT, however
as there is never going to be a 0RTT deployment with Experiment2/Experiment3,
this is valid.
Change-Id: Id541d195cbc4bbb3df7680ae2a02b53bb8ae3eab
Reviewed-on: https://boringssl-review.googlesource.com/22744
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We end up writing these switch cases everywhere. Let consumers decompose
these a bit. The original thought was folks should write switch-cases so
they handle everything they support, but that's a pain. As long as
algorithm preferences are always configured, we can still add new
dimensions because folks won't be asked to sign algorithms that depend
on dimensions they don't understand.
Change-Id: I3dd7f067f2c55212f0201876546bc70fee032bcf
Reviewed-on: https://boringssl-review.googlesource.com/22524
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Node's default settings spell P-256 as prime256v1. This comes from
OpenSSL additionally allowing the long and short names of each curve's
NID. This works out to one additional name per curve for the ones we
support. To avoid depending on the giant OID table, this replicates the
names in libssl.
Change-Id: I456a2db6939eb6745e5a9d2f12cf6886e6265b9f
Reviewed-on: https://boringssl-review.googlesource.com/22545
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I46686aea9b68105cfe70a11db0e88052781e179c
Reviewed-on: https://boringssl-review.googlesource.com/22164
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
RC4 is dead and gone. This trims away the suiteNoDTLS flag.
Change-Id: I1ddc5d0811ad8cfb073e6e3c73100240bc649615
Reviewed-on: https://boringssl-review.googlesource.com/22469
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This avoids needing to keep track of which rules do and don't need it.
Change-Id: Id086b0622305f7f4acd3892f5d24d8e0c970febb
Reviewed-on: https://boringssl-review.googlesource.com/22468
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
RC4 is gone. The only remaining exception was the dumb SSL_eNULL cipher,
which works fine in DTLS. It doesn't seem worth the trouble to retain
this special-case.
Change-Id: I31023b71192808e4d21e82109255dc4d6d381df8
Reviewed-on: https://boringssl-review.googlesource.com/22467
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This reverts commit 75d43b5785. Chatting
with EKR, there is some reason to believe that doing this might cause
more middlebox issues. Since we're still in the middle of working
towards viable deployment in the first place, revert this.
We can experiment with this later. I should have arranged for this to be
controlled more carefully anyway.
Change-Id: I0c8bf578f9d7364e913894e1bf3c2b8123dfd770
Reviewed-on: https://boringssl-review.googlesource.com/22204
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Both of these changes have stuck in Chrome for quite a while now. Let's
clear them.
Change-Id: I13094451be2584ecaaf6b60eedefb7212b7bcde2
Reviewed-on: https://boringssl-review.googlesource.com/22226
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We never implemented psk_ke, so there's no need to define the constant.
Change-Id: I6e52596e1a2cf0b3db5e7cd96db6836f4290bf0b
Reviewed-on: https://boringssl-review.googlesource.com/22144
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This doesn't matter in so far as runner is not a real TLS
implementation, but it should enforce what there is to enforce just to
keep BoringSSL honest.
Bug: 80
Change-Id: I68940c33712d34a2437dc4dee31342e7f0f57c23
Reviewed-on: https://boringssl-review.googlesource.com/22069
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This does not affect TLS 1.2 (beyond Channel ID or NPN) but, in TLS 1.3,
we send several encrypted handshake messages in a row. For the server,
this means 66 wasted bytes in TLS 1.3. Since OpenSSL has otherwise used
one record per message since the beginning and unencrypted overhead is
less interesting, leave that behavior as-is for the time being. (This
isn't the most pressing use of the breakage budget.) But TLS 1.3 is new,
so get this tight from the start.
Change-Id: I64dbd590a62469d296e1f10673c14bcd0c62919a
Reviewed-on: https://boringssl-review.googlesource.com/22068
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
We enforce that servers don't send bogus ALPN values, so consumers may
assume that SSL_get0_alpn_selected won't have anything terribly weird.
To maintain that invariant in the face of folks whose ALPN preferences
change (consider a persisted session cache), we should decline to offer
0-RTT if early_alpn would have been rejected by the check anyway.
Change-Id: Ic3a9ba4041d5d4618742eb05e27033525d96ade1
Reviewed-on: https://boringssl-review.googlesource.com/22067
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This is in preparation for giving DTLS_STATE one.
Change-Id: I3dfeeaad2d20c547d8e65d739bd0ad5bc1acf74a
Reviewed-on: https://boringssl-review.googlesource.com/22065
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
new_*_len can just be computed rather than maintained as state.
Change-Id: If097ee9e68d8791fcfeb69052151faf0134c7c52
Reviewed-on: https://boringssl-review.googlesource.com/21948
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This finally clears most of the SSL_clear special-cases.
Change-Id: I00fc240ccbf13f4290322845f585ca6f5786ad80
Reviewed-on: https://boringssl-review.googlesource.com/21947
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
As with SSLTranscript before, we temporarily need some nastiness in
SSL3_STATE, but this is in preparation of giving SSL3_STATE a
constructor and destructor.
Change-Id: Ifc0ce34fdcd8691d521d8ea03ff5e83dad43b4a3
Reviewed-on: https://boringssl-review.googlesource.com/21944
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Now that we've gotten everything, test this by just making bssl_shim run
all errors twice. The manual tests added to ssl_test.cc may now be
removed.
Bug: 206
Change-Id: Iefa0eae83ba59b476e6b6c6f0f921d5d1b72cbfb
Reviewed-on: https://boringssl-review.googlesource.com/21886
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
While a fairly small hook, open_close_notify is pretty weird. It
processes things at the record level and not above. Notably, this will
break if it skips past a TLS 1.3 KeyUpdate.
Instead, it can share the core part of SSL_read/SSL_peek, with slight
tweaks to post-handshake processing. Note this does require some tweaks
to that code. Notably, to retain the current semantics that SSL_shutdown
does not call funny callbacks, we suppress tickets.
Change-Id: Ia0cbd0b9f4527f1b091dd2083a5d8c7efb2bac65
Reviewed-on: https://boringssl-review.googlesource.com/21885
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Ideally we'd put this deep in the record layer, but sending alerts
currently awkwardly sets the field early, so we can't quite lock it out
this deep down.
This is mostly a sanity-check, but a later CL will fix SSL_shutdown's
post-handshake message processing, so this will help catch errors there.
Change-Id: I78e627c19547dbcdc85fb168795240d692baf031
Reviewed-on: https://boringssl-review.googlesource.com/21884
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This gets us closer to exposing BIO-free APIs. The next step is probably
to make the experimental bssl::OpenRecord function call a split out core
of ssl_read_impl.
Change-Id: I4acebb43f708df8c52eb4e328da8ae3551362fb9
Reviewed-on: https://boringssl-review.googlesource.com/21865
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
With this change, it should now always be the case that rr->length is
zero on entry to ssl3_read_message. This will let us detach everything
but application data from rr. This pushes some init_buf invariants down
into tls_open_record so we don't need to maintain them everywhere.
Change-Id: I206747434e0a9603eea7d19664734fd16fa2de8e
Reviewed-on: https://boringssl-review.googlesource.com/21524
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Enough were to make record processing idempotent (we either consume a
record or we don't), but some errors would cause us to keep processing
records when we should get stuck.
This leaves errors in the layer between the record bits and the
handshake. I'm hoping that will be easier to resolve once they do not
depend on BIO, at which point the checks added in this CL may move
around.
Bug: 206
Change-Id: I6b177079388820335e25947c5bd736451780ab8f
Reviewed-on: https://boringssl-review.googlesource.com/21366
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
We'll probably want to either move or add additional checks later, but
meanwhile this gets more code on the BIO-free side of the divide.
Change-Id: I3e2b570cdf1d70a262d952c20fd2d76ff4f70dd0
Reviewed-on: https://boringssl-review.googlesource.com/21365
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Ultimately the ssl_buffer_* code will be above SSL_PROTOCOL_METHOD, so
having the processing be analogous is simpler. This also means that DTLS
can surface errors out of dtls_open_record without the caller reading an
extra record.
Bug: 206
Change-Id: Ic1cb3a884763c8e875e1129b1cda226f72bc95b7
Reviewed-on: https://boringssl-review.googlesource.com/21364
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
We only support non-blocking BIOs for DTLS as of
https://boringssl-review.googlesource.com/13945. This logic is a remnant
of that. It should not be necessary. All users of DTLSv1_get_timeout
call DTLSv1_handle_timeout. This gets it out of the way for
dtls_open_record calls which don't use dtls1_get_record.
We can restore it elsewhere if necessary, but I don't think we need it.
Change-Id: Idb737868358e4b59ad3cb2c994c7084ffcdb3709
Reviewed-on: https://boringssl-review.googlesource.com/21349
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This removes the last place where non-app-data hooks leave anything
uncomsumed in rrec. (There is still a place where non-app-data hooks see
a non-empty rrec an entrance. read_app_data calls into read_handshake.
That'll be fixed in a later patch in this series.)
This should not change behavior, though some error codes may change due
to some processing happening in a slightly different order.
Since we do this in a few places, this adds a BUF_MEM_append with tests.
Change-Id: I9fe1fc0103e47f90e3c9f4acfe638927aecdeff6
Reviewed-on: https://boringssl-review.googlesource.com/21345
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
RSABadValueTooLong should have the true one as a suffix, not a prefix,
so that the version check still works. Also do the padding manually to
catch a few other bad padding cases. This is sufficient coverage so that
disabling any one comparison in the padding check flags some failure.
Change-Id: Ibcad284e5ecee3e995f43101c09e4cf7694391e9
Reviewed-on: https://boringssl-review.googlesource.com/21904
Reviewed-by: Steven Valdez <svaldez@google.com>
Application records may be packed with other application data records or
with handshake records. We also were never testing CCS and handshake
being packed together. Implement this by moving the packing logic to the
bottom of BoGo's DTLS record layer.
Change-Id: Iabc14ec4ce7b99ed1f923ce9164077efe948c7a0
Reviewed-on: https://boringssl-review.googlesource.com/21844
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
To make sure I don't break it later on.
Change-Id: I0a326800593cd3196efaf2ec9f4042935ecf8eb8
Reviewed-on: https://boringssl-review.googlesource.com/21864
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
I think that's the last of the ssl3_ prefix being used for common
functions.
Change-Id: Id83e6f2065c3765931250bd074f6ebf1fc251696
Reviewed-on: https://boringssl-review.googlesource.com/21347
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
These are common between TLS and DTLS so should not have the ssl3_
prefix. (TLS-only stuff should really have a tls_ prefix, but we still
have a lot of that one.)
This also fixes a stray reference to ssl3_send_client_key_exchange..
Change-Id: Ia05b360aa090ab3b5f075d5f80f133cbfe0520d4
Reviewed-on: https://boringssl-review.googlesource.com/21346
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
It's no longer needed in the public header at all, now that we've hidden
the SSL_CTX struct.
Change-Id: I2fc6ddbeb52f000487627b433b9cdd7a4cde37a8
Reviewed-on: https://boringssl-review.googlesource.com/21684
Reviewed-by: Steven Valdez <svaldez@google.com>
GCC 7.2.0 (in Release builds) can't figure out that |type| is always
set:
../ssl/tls_record.cc: In function ‘bssl::OpenRecordResult bssl::OpenRecord(SSL*, bssl::Span<unsigned char>*, size_t*, uint8_t*, bssl::Span<unsigned char>)’:
../ssl/tls_record.cc:595:44: error: ‘type’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (type != SSL3_RT_APPLICATION_DATA && type != SSL3_RT_ALERT) {
cc1plus: all warnings being treated as errors
Change-Id: I1ca9683a18d89097288018f48b50991bce185da8
Reviewed-on: https://boringssl-review.googlesource.com/21724
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The handshake should be generic between TLS and DTLS.
Change-Id: I6feb2f013dd5d771f206750653ab9d117d7ea716
Reviewed-on: https://boringssl-review.googlesource.com/21348
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
This frees us up to make SSL_CTX a C++ type and avoids a lot of
protrusions of otherwise private types into the global namespace.
Bug: 6
Change-Id: I8a0624a53a4d26ac4a483fa270c39ecdd07459ee
Reviewed-on: https://boringssl-review.googlesource.com/21584
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
