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>
When generating a signature with some external signing process, the
caller needs to fill in the TBSCertificate (including the signature
algorithms), serialize the TBSCertificate, and then fill in the
signature.
We have i2d_re_X509_tbs (originally from CT I believe), but there are no
setters for the signature algorithms or the signature. Add
X509_set1_signature_algo, which mirrors upstream's
X509_REQ_set1_signature_algo, and X509_set1_signature_value, which is
new. Upstream has X509_REQ_set0_signature, but that requires the caller
manually assemble an ASN1_BIT_STRING. Taking the byte string seems less
error-prone.
Additionally, add i2d_X509_tbs and i2d_X509_CRL_tbs, for the non-"re"
variants of those APIs. Conscrypt needs to extract the TBS portion of a
certificate and a CRL, to implement X509Certificate.getTBSCertificate()
and X509CRL.getTBSCertList(). There, the aim is to get the data to
verify on an existing immutable certificate. OpenSSL has avoided
exporting the X509_CINF type, which I think is correct, so instead this
mirrors i2d_re_X509_tbs. (This does mean mirroring the confusing i2d
calling convention though.)
These new functions should unblock getting rid of a bunch of direct
struct accesses.
Later on, we should reorganize this header into immutable APIs for
verification and mutable APIs for generation. Even though we're stuck
the mistake of a common type for both use cases, I think splitting up
them up will let us rationalize the caches in the X509 objects a bit.
Change-Id: I96e6ab5cee3608e07b2ed7465c449a72ca10a393
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43784
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Chromium's VS toolchains now maintain JSON files with the expected
environment, so we don't need to pull in gyp to figure out the batch
file to run. This drops a long obsolete dependency and will make it
possible to handle other VS architectures. (gyp internally only handled
x86 and x64.)
Also trim away the logic in vs_toolchain.py to account for
non-depot_tools toolchains. Unlike Chromium, we don't use these scripts
outside of CI/CQ.
Change-Id: I2c9fddac52eef7b4895731d78c637fdcf9c85033
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43504
Reviewed-by: Adam Langley <agl@google.com>
For FIPS reasons, one might wish to ensure that a random AES-GCM nonce
was generated entirely within the FIPS module. If so, then these are the
AEADs for you.
Change-Id: Ic2b7864b089f446401f700d7d55bfa6336c61e23
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43686
Commit-Queue: Adam Langley <alangley@gmail.com>
Reviewed-by: David Benjamin <davidben@google.com>
We use this constant a lot in e_aes.c, but we write it out every time.
Change-Id: Iaa92efb391def6640349940c682d9f70ddaa23d5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43685
Reviewed-by: David Benjamin <davidben@google.com>
We already support this, but there wasn't a test for it.
Change-Id: I14304b99b312fcf729703cf175ec41e3e60db363
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43704
Commit-Queue: Adam Langley <agl@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>
These booleans are a little hard to understand in context and adding
any more makes things even more complicated. Thus make them flags so
that the meaning is articulated locally.
Change-Id: I8cdb7fd5657bb12f28a73d7c6818d400c987ad3b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43684
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This is a looser reland of
https://boringssl-review.googlesource.com/c/boringssl/+/41804, which was
reverted in
https://boringssl-review.googlesource.com/c/boringssl/+/42804.
Enforcing that the ECDSA parameters were omitted rather than NULL hit
some compatibility issues, so instead allow either forms for now. To
align with the Chromium verifier, we'll probably want to later be
stricter with a quirks flag to allow the invalid form, and then add a
similar flag to Chromium. For now, at least try to reject the completely
invalid parameter values.
Update-Note: Some invalid certificates will now be rejected at
verification time. Parsing of certificates is unchanged.
Bug: b/167375496,342
Change-Id: I1cba44fd164660e82a7a27e26368609e2bf59955
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43664
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The constructor parameter vs. method name one is a little unfortunate
given Google C++ style, but I think we've done this elsewhere in libssl,
so let's run with it for now.
Bug: 378
Change-Id: I31fb6b4b16e3248369dae6f47cc150de0e4f04fe
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43545
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
With -Wundef one could get warnings of undefined symbols.
This patch tries to fix this issue.
Furthermore, the case where there is BTI but no Pointer Authentication
now uses GNU_PROPERTY_AARCH64_BTI in the check which should correctly
reflect that feature's enabled state.
Change-Id: I14902a64e5f403c2b6a117bc9f5fb1a4f4611ebf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43524
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Use empty() over size() == 0, and don't export the IterateAES*
functions. (They return private types.)
Change-Id: I8a8f33a64e28cc2eab789563c6ba91afa6df87f9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43544
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
A recent change broke this but I didn't notice. (Which suggests that the
test isn't very useful, which is true, but I'm not ready to pull the
trigger on deleting it just yet.)
Change-Id: If120a553c095fa0be9f8e85fc05ee996a486621f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43484
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CopyDiaDllTo is no longer needed after
9f7781171e.
As a bonus, this makes the script much easier to use outside of the
bots.
Change-Id: Ib59b7e6ff9276b860032134ad7eaa006492e76b9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43464
Reviewed-by: Adam Langley <agl@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>
(There's going to be more and it was getting too big.)
Change-Id: I16a49f77975697bb5a04f2adfd465b09c2a09ef3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43404
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@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 change adds a config parameter PrivateKeyFile (to replace
PrivateKeyDERFile, although that still exists) because taking PKCS#1 DER
is a little odd for people. Also probe for PEM/DER and PKCS#1/8
automatically to try and work with whatever private key the user has.
Change-Id: I0f4efcd79528cfb26f791e9ee8c5141fc6a93723
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43344
Reviewed-by: 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>
Sometimes the linker will generate rodata subsections even if we don't
have -fdata-sections enabled. That's ok, so include them in the FIPS
module. The other subsections continue to be discarded to ensure that
unexpected sections don't appear and escape the module.
Bug: b/142971559
Change-Id: Icebcf40bd3d0e63f20456e44f6c2564f4316b561
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43324
Commit-Queue: Adam Langley <alangley@gmail.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
It's a lot easier to copy certificates to and from x509_test.cc when
there isn't an indent to worry about. Note this does stick a newline in
front of each string, but the PEM parsers don't care.
Change-Id: I06aff263a2470596e8c50564c198693cfdbf9c59
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43285
Reviewed-by: Adam Langley <agl@google.com>
See 309e73dfe067b3b774ef6f57bf665f41373a81ca from upstream, though note
that v3_alt.c's fix was rewritten. (We don't have sk_reserve, and I
don't think their fix was quite right anyway.)
Change-Id: Ieabd19d87d4628658324b212cce2ed3ce451ad22
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43284
Reviewed-by: Adam Langley <agl@google.com>
Since we have support for reading vectors from files, this change adds
support for saving them. There's no support for uploading the saved
vectors, rather it's just for quicker debugging since the NIST server is
taking over a minute to produce vectors at the moment and that's a
little frustrating to iterate with.
Change-Id: I5da8a084eb06b81aefa838b4e7ad8d529d1d31a6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43144
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>
DES being deprecated is hopefully well-established enough that no one is
going start complaining our implementation isn't constant-time. But I
think this is the only non-constant-time code left without a warning, so
add one for completeness.
(It is possible to implement DES with bitslicing, but 3DES TLS ciphers
are too slow as it is and hopefully not long for the world.)
Change-Id: I6b0de915e89ffe2d11372f7109642fcff44b11bf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43244
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Adam Langley
David Benjamin
Steven Valdez
Tamas Petz
Adam Langley
Daniel McArdle
Nick Harper