Commit Graph

852 Commits

Author SHA1 Message Date
Matthew Braithwaite
ce77762686 shim: extract a |DoSplitHandshake| helper function.
This makes the shim code read more naturally, in that the split-
handshake special case now lives in its own file.

This helps with creating a separate binary to perform split
handshakes.

Change-Id: I7970a8f368417791d18d4d44eeb379ef4b46c960
Reviewed-on: https://boringssl-review.googlesource.com/29347
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
2018-07-13 19:57:17 +00:00
Adam Langley
e0afc85719 Send an alert if we fail to pick a signature algorithm.
Change-Id: Id7f5ef9932c4c491bd15085e3c604ebfcf259b7c
Reviewed-on: https://boringssl-review.googlesource.com/29665
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>
2018-07-10 15:38:12 +00:00
Matthew Braithwaite
c312fd02f6 Remove MoveTestConfig().
In f2bc5f4 davidben pointed out that this function seems unnecessary
in my desired end-state.  In fact, I think it may have been
unnecessary since 56986f90.  (This was easier to miss at the time,
since at the time the function was part of MoveExData(), having not
yet been factored out.)

Change-Id: Ia9b4a909c93cb595666bcf7356a9f9a085901455
Reviewed-on: https://boringssl-review.googlesource.com/29604
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>
2018-07-09 18:50:50 +00:00
David Benjamin
50596f8f54 Switch some easy SSL fields to UniquePtr.
Change-Id: I982ecda5a19187708b15e8572e6d0000c22ed87c
Reviewed-on: https://boringssl-review.googlesource.com/29590
Reviewed-by: Adam Langley <agl@google.com>
2018-07-06 19:30:51 +00:00
David Benjamin
4979803755 Unsplit SSL and SSL_CTX.
This doesn't give them a destructor yet, just shifts things around. In
doing so, it reveals that we inconsistently allowed internal code, but
not external code, to call functions like bssl::SSL_CTX_set_handoff_mode
without a namespace because of ADL. External code doesn't get to do
this because it doesn't see that ssl_ctx_st has a base class in
namespace bssl.

Change-Id: I2ab3b00fff2d6369e850606eed63017e4f8cf8c4
Reviewed-on: https://boringssl-review.googlesource.com/29588
Reviewed-by: Adam Langley <agl@google.com>
2018-07-06 18:56:03 +00:00
Adam Langley
859679518d Drop C++ from certificate compression API.
It's 2018, but passing STL objects across the API boundary turns out to
still be more bother than it's worth. Since we're dropping UniquePtr in
the API anyway, go the whole way and make it a plain-C API.

Change-Id: Ic0202012e5d81afe62d71b3fb57e6a27a8f63c65
Update-note: this will need corresponding changes to the internal use of SSL_CTX_add_cert_compression_alg.
Reviewed-on: https://boringssl-review.googlesource.com/29564
Reviewed-by: David Benjamin <davidben@google.com>
2018-07-04 16:39:14 +00:00
Matthew Braithwaite
d2f87a7779 shim: move handshake helper functions into their own file.
To wit, |RetryAsync| and |CheckIdempotentError|.

This helps with creating a separate binary to perform split
handshakes.

Separate handshake utilities

Change-Id: I81d0bc38f58e7e1a92b58bf09407452b345213b4
Reviewed-on: https://boringssl-review.googlesource.com/29346
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>
2018-07-03 23:30:20 +00:00
Matthew Braithwaite
f2bc5f490a shim: move |TestState| and |TestConfig| to their own files.
This makes |TestState| and |TestConfig| accessible outside
bssl_shim.cc, as well as the functions SetupCtx() and NewSSL(), which
become methods on |TestConfig|.  A whole mess of callbacks move in
order to support this change.

Along the way, some bits of global state are moved (e.g. the global
test clock) and made self-initializing.

This helps with creating a separate binary to perform split
handshakes.

Change-Id: I39b00a1819074882353f5f04ed01312916f3cccb
Reviewed-on: https://boringssl-review.googlesource.com/29345
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>
2018-07-03 23:14:56 +00:00
David Benjamin
63c79122e0 Remove the redundant version check in ssl_session_cmp.
This partitions the session ID space of the internal cache by version,
which is nominally something we want, but we must check the version
externally anyway for both tickets and external session cache. That
makes this measure redundant. (Servers generate session IDs and 2^256 is
huge, so there would never accidentally be a collision.)

This cuts down on the "key" in the internal session cache, which will
simplify adding something like an lh_SSL_SESSION_retrieve_key function.
(LHASH is currently lax about keys because it can freely stack-allocate
partially-initialized structs. C++ is a bit more finicky about this.)

Change-Id: I656fd9dbf023dccb163d2e8049eff8f1f9a0e21b
Reviewed-on: https://boringssl-review.googlesource.com/29585
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>
2018-07-03 22:54:16 +00:00
David Benjamin
53d2c7a84a Remove fail_second_ddos_callback.
We have generic -on-resume prefixes now. This avoids the global counter.

Change-Id: I7596ed3273e826b744d8545f7ed2bdd5e9190958
Reviewed-on: https://boringssl-review.googlesource.com/29594
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>
2018-07-03 22:52:26 +00:00
David Benjamin
2908dd141f Add bssl::UpRef.
bssl::UniquePtr and FOO_up_ref do not play well together. Add a helper
to simplify this. This allows us to write things like:

   foo->cert = UpRef(bar->cert);

instead of:

   if (bar->cert) {
     X509_up_ref(bar->cert.get());
   }
   foo->cert.reset(bar->cert.get());

This also plays well with PushToStack. To append something to a stack
while taking a reference, it's just:

   PushToStack(certs, UpRef(cert))

Change-Id: I99ae8de22b837588a2d8ffb58f86edc1d03ed46a
Reviewed-on: https://boringssl-review.googlesource.com/29584
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>
2018-07-03 22:47:36 +00:00
Matthew Braithwaite
997ff094af shim: move |SettingsWriter| into its own file.
This helps with creating a separate binary to perform split
handshakes.

Change-Id: Ie4bab40bebf39e79a90d45fabb566b7ce90945bb
Reviewed-on: https://boringssl-review.googlesource.com/29344
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>
2018-07-02 22:26:28 +00:00
Alessandro Ghedini
a0373182eb Update QUIC transport parameters extension codepoint
This was changed in draft-ietf-quic-tls-13 to use a codepoint from the
reserved range.

Change-Id: Ia3cda249a3f37bc244d5c8a7765ec34a5708c9ae
Reviewed-on: https://boringssl-review.googlesource.com/29464
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>
2018-06-28 17:41:58 +00:00
David Benjamin
9bb15f58f7 Remove SSL 3.0 implementation.
Update-Note: SSL_CTX_set_min_proto_version(SSL3_VERSION) now fails.
   SSL_OP_NO_SSLv3 is now zero. Internal SSL3-specific "AEAD"s are gone.

Change-Id: I34edb160be40a5eea3e2e0fdea562c6e2adda229
Reviewed-on: https://boringssl-review.googlesource.com/29444
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>
2018-06-28 16:54:58 +00:00
Adam Langley
0080d83b9f Implement the client side of certificate compression.
Change-Id: I0aced480af98276ebfe0970b4afb9aa957ee07cb
Reviewed-on: https://boringssl-review.googlesource.com/29024
Reviewed-by: Adam Langley <agl@google.com>
2018-06-18 22:16:11 +00:00
David Benjamin
5267ef7b4a Reject unexpected application data in bidirectional shutdown.
Update-Note: This tweaks the SSL_shutdown behavior. OpenSSL's original
SSL_shutdown behavior was an incoherent mix of discarding the record and
rejecting it (it would return SSL_ERROR_SYSCALL but retrying the
operation would discard it). SSLeay appears to have intended to discard
it, so we previously "fixed" it actually discard.

However, this behavior is somewhat bizarre and means we skip over
unbounded data, which we typically try to avoid. If you are trying to
cleanly shutdown the TLS portion of your protocol, surely it is at a
point where additional data is a syntax error. I suspect I originally
did not realize that, because the discarded record did not properly
continue the loop, SSL_shutdown would appear as if it rejected the data,
and so it's unlikely anyone was relying on that behavior.

Discussion in https://github.com/openssl/openssl/pull/6340 suggests
(some of) upstream also prefers rejecting.

Change-Id: Icde419049306ed17eb06ce1a7e1ff587901166f3
Reviewed-on: https://boringssl-review.googlesource.com/28864
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>
2018-06-04 21:39:58 +00:00
Adam Langley
a307cb7d58 Preliminary support for compressed certificates.
This change adds server-side support for compressed certificates.

(Although some definitions for client-side support are included in the
headers, there's no code behind them yet.)

Change-Id: I0f98abf0b782b7337ddd014c58e19e6b8cc5a3c2
Reviewed-on: https://boringssl-review.googlesource.com/27964
Reviewed-by: David Benjamin <davidben@google.com>
2018-06-04 21:24:20 +00:00
David Benjamin
700631bdf0 Pack encrypted handshake messages together.
We have a successful TLS 1.3 deployment, in spite of non-compliant
middleboxes everywhere, so now let's get this optimization in. It would
have been nice to test with this from the beginning, but sadly we forgot
about it. Ah well. This shaves 63 bytes off the server's first flight,
and then another 21 bytes off the pair of NewSessionTickets.

So we'll more easily notice in case of anything catastrophic, tie this
behavior to draft 28.

Update-Note: This slightly tweaks our draft-28 behavior.

Change-Id: I4f176a919bf7181239d6ebb31e7870f12364e0f9
Reviewed-on: https://boringssl-review.googlesource.com/28744
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>
2018-05-29 14:28:56 +00:00
David Benjamin
fa544f1c05 Reject if the ALPN callback returned an empty protocol.
If the callback returns an empty ALPN, we forget we negotiated ALPN at
all (bssl::Array does not distinguish null and empty). Empty ALPN
protocols are forbidden anyway, so reject these ahead of time.

Change-Id: I42f1fc4c843bc865e23fb2a2e5d57424b569ee99
Reviewed-on: https://boringssl-review.googlesource.com/28546
Reviewed-by: Adam Langley <agl@google.com>
2018-05-15 23:02:39 +00:00
David Benjamin
911cc0a0aa The legacy client OCSP callback should run without server OCSP.
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>
2018-05-15 22:57:41 +00:00
David Benjamin
103ed08549 Implement legacy OCSP APIs for libssl.
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>
2018-05-11 22:21:26 +00:00
David Benjamin
5f001d1423 Const-correct some functions.
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>
2018-05-11 15:10:35 +00:00
Steven Valdez
56c4ed9ad7 Allow enabling all TLS 1.3 variants by setting |tls13_default|.
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>
2018-05-10 20:27:34 +00:00
David Benjamin
02de7bd3a0 Add some more accessors to SSL_SESSION.
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>
2018-05-08 22:50:45 +00:00
David Benjamin
ed188fd8ef Enforce supported_versions in the second ServerHello.
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>
2018-05-07 19:05:20 +00:00
Matthew Braithwaite
e30fac6371 Fuzz SSL_serialize_handoff() and SSL_serialize_handback().
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>
2018-05-05 02:41:04 +00:00
David Benjamin
6e678eeb6e Remove legacy SHA-2 CBC ciphers.
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>
2018-05-02 19:21:56 +00:00
David Benjamin
71666cb87c Allow renego and config shedding to coexist more smoothly.
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>
2018-05-01 23:28:59 +00:00
Matthew Braithwaite
b7bc80a9a6 SSL_CONFIG: new struct for sheddable handshake configuration.
|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>
2018-05-01 20:40:16 +00:00
Matthew Braithwaite
a2dd781884 Defer writing the shim settings.
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>
2018-05-01 19:49:46 +00:00
David Benjamin
06c28d8e51 Simplify shim timeout logic.
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>
2018-04-25 16:07:28 +00:00
David Benjamin
56b1a8efa6 Test the high-order bit in X25519.
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>
2018-04-19 00:56:35 +00:00
David Benjamin
e28552dec8 Add an API to disable RSA-PSS for certificates.
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>
2018-04-16 20:02:43 +00:00
David Benjamin
6879e19362 Rename SSL_SIGN_RSA_PSS_SHA* constants.
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>
2018-04-16 19:00:03 +00:00
Steven Valdez
861f384d7b Implement TLS 1.3 draft28.
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>
2018-04-05 03:36:11 +00:00
Matthew Braithwaite
56986f905f Hand back ECDHE split handshakes after the first server message.
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>
2018-04-04 17:58:15 +00:00
David Benjamin
8a1a5daa49 Send the fake session ID in the test suite.
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>
2018-03-19 21:06:05 +00:00
Adam Langley
8df8e64205 Record whether dummy PQ padding was used.
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>
2018-02-28 23:38:53 +00:00
Adam Langley
4702db6306 Update dummy PQ extension for round two.
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>
2018-02-27 20:13:53 +00:00
David Benjamin
1bf2337fe1 Reject compressed ECDH coordinates in TLS.
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>
2018-02-15 01:42:54 +00:00
David Benjamin
48669209b7 Fix fuzzer mode suppressions.
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>
2018-01-31 22:57:51 +00:00
Adam Langley
ddb57cfb51 Add tests for split handshakes.
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>
2018-01-31 22:33:42 +00:00
Steven Valdez
7e5dd25d47 Remove draft22 and experiment2.
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>
2018-01-31 18:07:53 +00:00
Nick Harper
3c034b2cf3 Add support for QUIC transport params.
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>
2018-01-30 23:54:40 +00:00
David Benjamin
0ab3f0ca25 Notice earlier if a server echoes the TLS 1.3 compatibility session ID.
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>
2018-01-26 21:53:27 +00:00
Adam Langley
449a9e6a9e Make the gdb window larger.
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>
2018-01-26 19:59:23 +00:00
Steven Valdez
cb15cfda29 Add draft23 tests.
Change-Id: Ic77bc2f55e1e997ea20641cae66b0deddf1ef99c
Reviewed-on: https://boringssl-review.googlesource.com/25084
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>
2018-01-22 20:29:38 +00:00
Nick Harper
36fcc4ca5d Implement Token Binding
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>
2018-01-22 20:08:28 +00:00
David Benjamin
afd1cd959e Work around an NDK / Android bug.
The NDK r16 sometimes generates binaries with the DF_1_PIE, which the
runtime linker on Android N complains about. The next NDK revision
should work around this but, in the meantime, strip its error out.

https://github.com/android-ndk/ndk/issues/602
https://android-review.googlesource.com/c/platform/bionic/+/259790
https://android-review.googlesource.com/c/toolchain/binutils/+/571550

Change-Id: I99306d42f11179d5d19bd3f107a7386cc5c690db
Reviewed-on: https://boringssl-review.googlesource.com/24884
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>
2018-01-16 16:52:46 +00:00
Adam Langley
512a289a8a Add support for dummy PQ padding.
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>
2018-01-10 00:27:31 +00:00