When the handshaker fails to parse a config it currently exits. This
causes the two pipes to signal EOF to the shim, but the control channel
is a datagram socket in order to be atomic, thus doesn't signal an
error.
In the shim, EOF on the wfd pipe causes a short loop and thus a hang
forever. Catching the EOF and returning an error doesn't work because
some tests will close the pipe but still return information over the
control channel. We can start a timeout once wfd is closed, but that
seems like it might be flakey.
Thus this change makes the handshaker send an explicit error over the
control channel. It doesn't catch crashes, but it will catch config
errors, which are much more common in cross-version tests.
Change-Id: I4b1afed17694c57e4713d1b0fa4e9ecb12f09ec5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43865
Reviewed-by: David Benjamin <davidben@google.com>
In order to skip groups of tests in the cross-version testing (like
ALPS-*), it's useful to be able to match them by pattern.
Change-Id: Ic7e40c04a33b4bcbb08494fa04deb5e862f09d8f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43864
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@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>
Add an earlyData and earlyDataRejected flag to configure the standard
0-RTT test options. It's too tedious otherwise. Along the way, I added
an -expect-cipher flag to a few of the tests which could do with them.
This does cause most 0-RTT tests to exchange a quick burst of data, so a
few more fuzzer mode suppressions are needed. I think that's probably
fine. Maybe we should mess with fuzzer mode so it's able to trial
decrypt as this is getting a little tedious.
Change-Id: Ib6490fe006d91294aab1a06d88f7793c6ae840c8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43086
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
OpenSSL synchronizes bio->next_bio and ssl->rbio with a variety of
callbacks, so BIO_copy_next_retry worked. We do not, so attempting to
flush the BIO crashed.
The SSL BIO is a compatibility hack and intentionally much more limited,
so start by just copying things from the right BIO directly. Add a basic
unit test for SSL BIOs. If we need to, we can implement a more complex
synchronization later.
Additionally reject reconfiguring an SSL BIO because that will leak the
object right now.
Change-Id: I724c95ab6f1a3a1aa1889b0483c81ce3bdc534ae
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43424
Reviewed-by: Adam Langley <agl@google.com>
The Go TLS implementation, at the time runner forked, had custom
testing-only equal methods on all the handshake messages. We've since
removed all of them except for ClientHello, where we repurposed the
function to check ClientHello consistency on HelloVerifyRequest and
HelloRetryRequest.
These are tedious to update. Upstream has since replaced them with
reflect.DeepEqual, but the comparison we want is even tighter. Even
unknown extensions aren't allowed to change. Replace the check with a
custom one that works on the byte serialization and remove
clientHelloMsg.equal.
Along the way, I've fixed the HRR PSK identity logic to match the spec a
bit more and check binders more consistently.
Change-Id: Ib39e8791201c42d37e304ae5110c7aeed62c8b3f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43364
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This CL replaces clientHelloMsg's npnAfterAlpn and pskBinderFirst fields
with a new field: prefixExtensions. The extensions in prefixExtensions
are tried first when marshalling clientHelloMsg.
The ability to control extensions' marshalling order will make it
simpler to implement the "outer_extensions" behavior defined in
draft-ietf-tls-esni-07.
Bug: 275
Change-Id: Ib6dcc1e6fa0281f312cb65a9e204415c3f3ef2c6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43064
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The quic_early_data_context should always be saved in the SSL_SESSION.
This change fixes a bug where it was only saved in the SSL_SESSION on
full handshakes (but not resumption handshakes).
Bug: 376
Change-Id: I3659d3398e85ac4263760b504d7ea8458fc7e1e2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43264
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
If a test fails due to an unexpected SSL_get_error result, notably 0-RTT
rejection, it's very difficult to debug.
Change-Id: I08585551f6d3d3c4ea414bf3ac7bc9ba0ed4063b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43085
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We have an ad-hoc expectResumeVersion, and I'll want to add one for ALPS
(to confirm changing ALPS on resumption works). We probably could have
done with such a test for ALPN too. Wrap all the ConnectionState
expectations into a struct with an optional resumeExpectations field.
Also move the OCSPResponse() method to ConnectionState for consistency.
Change-Id: Icaabf5571c51e78ed078f57de0e04928d3f3fa8d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43084
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
On the server, we echo the selected ALPN back out of the same getter as
the client.
Change-Id: I11978b9cd0a3e7c611f162e9cf5054e52005a195
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43045
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This will make it less tedious to add new fields later.
Change-Id: I82fe8f1859e4eaf0c72551d4ca5cf9534bee10b5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43005
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
This function is still a bit too C-like, but this is slightly better.
Change-Id: Id8931753c9b8a2445d12089af5391833a68c4901
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43004
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>
We don't currently use this type anywhere with a nontrivial destructor,
so this doesn't matter right now. But handle this correctly in case we
ever do. (One of these days, we should sort out using the STL and Abseil
in here...)
Change-Id: I6a198ccf87f953cedcdbe658fa508a3b79d47305
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42825
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>
The CI should be set up to retain the Go module and build caches, so
we'll avoid downloading it multiple times. This avoids having to
replicate some code.
Update-Note: The tests now have a golang.org/x/crypto dependency. This
should be fetched transparently with Go modules. Monorepos with
different import path conventions may need to rewrite these imports.
Change-Id: If5ba52e051f180536d72109c2e690bbd13d58e7c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42044
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We should probably ponder what to do about clang-format. That we
disagree on chains of && is a little annoying, but peppering the code
with clang-format off seems problematic.
Change-Id: I0547e4e41817e8c0b585d5fabe759ef25ed00cf7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42264
Reviewed-by: Adam Langley <agl@google.com>
Adam Langley
David Benjamin
Steven Valdez
Daniel McArdle
Nick Harper
Watson Ladd