Commit Graph

736 Commits

Author SHA1 Message Date
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
Steven Valdez
74666da5b3 Update key share extension number for draft23.
Change-Id: I7561fc7e04d726ea9e26f645da10e45b62a20627
Reviewed-on: https://boringssl-review.googlesource.com/24704
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-09 15:22:02 +00:00
David Benjamin
6df6540766 Add a draft TLS 1.3 anti-downgrade signal.
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>
2017-12-21 01:50:33 +00:00
Steven Valdez
64cc121f41 Remove deprecated TLS 1.3 variants.
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>
2017-12-18 21:20:32 +00:00
David Benjamin
f88242d1c1 SSL_export_keying_material should work in half-RTT.
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>
2017-12-18 16:53:13 +00:00
Steven Valdez
f98b582ded Fix tls13_variant check to check max_version.
Change-Id: I946e8903d745b0bf0c48701d16aa866464233ab7
Reviewed-on: https://boringssl-review.googlesource.com/24044
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>
2017-12-12 17:20:07 +00:00
David Benjamin
650d8c393e Implement TLS 1.3 early exporters.
Bug: 222
Change-Id: I33ee56358a62afcd9c3921026d55efcc543a5c11
Reviewed-on: https://boringssl-review.googlesource.com/23945
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>
2017-12-11 21:33:26 +00:00
David Benjamin
8f53fc0a94 Fix fuzzer mode suppressions.
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>
2017-12-11 20:12:11 +00:00
David Benjamin
21baf6421a Fix CustomExtensions-Server-EarlyDataAccepted test.
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>
2017-12-11 15:54:15 +00:00
David Benjamin
494e4d0e89 Add an option for False Start without ALPN.
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>
2017-12-04 21:20:46 +00:00
Steven Valdez
9986f6b045 Fix renegotiation with TLS 1.3 draft 22.
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>
2017-12-01 17:56:55 +00:00
David Benjamin
fb535892e5 runner: Rewrite some more parsers.
These were easy.

Change-Id: I5fc764b83d641b08b58ccbff36dbd28cb66efed0
Reviewed-on: https://boringssl-review.googlesource.com/23564
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>
2017-11-30 17:05:06 +00:00
Steven Valdez
c5c31abe2b Enforce compression_method in TLS 1.3 draft 22.
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>
2017-11-29 22:19:04 +00:00
Steven Valdez
e6cefe41bb Update PR 1091 CL to use draft22 version.
Change-Id: Ifa811262fbca22222656da530f97daac3dcd6a5b
Reviewed-on: https://boringssl-review.googlesource.com/22944
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>
2017-11-29 16:11:24 +00:00
David Benjamin
a7673facf8 runner: Parse CertificateRequest with byteReader.
Bug: 212
Change-Id: I0ad4df330360789b16fc9db70565abdb3ae42a8f
Reviewed-on: https://boringssl-review.googlesource.com/23448
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>
2017-11-28 18:37:39 +00:00
David Benjamin
28b267b357 runner: Parse Certificate with byteReader.
Bug: 212
Change-Id: Ife51516ef0642730e601e146028b16ded99ab7ba
Reviewed-on: https://boringssl-review.googlesource.com/23447
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>
2017-11-28 17:42:49 +00:00
David Benjamin
bd911af514 runner: Parse SH/HRR/EE with byteReader.
Bug: 212
Change-Id: I454db0bfd59bac3729338c6f8d9e51efde0735eb
Reviewed-on: https://boringssl-review.googlesource.com/23446
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>
2017-11-28 17:03:39 +00:00
David Benjamin
7ce2378750 runner: Send the right alert for handshake message parsing failures.
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>
2017-11-28 16:55:49 +00:00
David Benjamin
be8c8b4b1d runner: Add a byteReader type and convert ClientHello parsing.
Bug: 212
Change-Id: Iecbd8fddef1b55a438947ad60780e08cb4260c48
Reviewed-on: https://boringssl-review.googlesource.com/23444
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>
2017-11-27 21:18:40 +00:00
David Benjamin
855d5046c7 Unwind legacy SSL_PRIVATE_KEY_METHOD hooks.
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>
2017-11-21 17:48:09 +00:00
David Benjamin
8793942c5c Fix fuzzer mode suppressions.
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>
2017-11-20 18:44:18 +00:00
David Benjamin
e7c95d91f8 Run TLS 1.3 tests at all variants and fix bugs.
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>
2017-11-20 18:19:18 +00:00
David Benjamin
3bba5ccf35 Add EndOfEarlyData to per-message tests.
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>
2017-11-20 18:10:38 +00:00
David Benjamin
e6f30e4ce1 Add tests for post-handshake CCS in draft "22".
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>
2017-11-14 05:40:38 +00:00
David Benjamin
13761f2833 Fix TLSInnerPlaintext limit.
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>
2017-11-14 05:38:48 +00:00
Steven Valdez
ba8f1864c1 Disable 'draft 22' by default.
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>
2017-11-12 03:26:05 +00:00
Steven Valdez
964b2377d0 Implement PR 1091 (TLS 1.3 draft '22').
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>
2017-11-11 06:24:55 +00:00
Steven Valdez
cd8470f7fa Adding support for draft 21 as a TLS 1.3 variant.
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>
2017-11-01 21:32:36 +00:00
David Benjamin
cfc120eb22 Remove RC4 remnants in runner.
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>
2017-11-01 16:45:36 +00:00
David Benjamin
5be3a74c49 Remove supports_cipher hook.
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>
2017-11-01 16:44:46 +00:00
David Benjamin
ba94746eb2 Remove temporary logging.
Bug: 199
Change-Id: Ic8eb3e7901b89e5a57c959c650ea316e2eeeb45a
Reviewed-on: https://boringssl-review.googlesource.com/22424
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>
2017-10-30 20:47:16 +00:00
David Benjamin
ed84291188 Revert "Pack encrypted handshake messages together."
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>
2017-10-27 14:36:37 +00:00
David Benjamin
883b5461d5 runner: Check c.hand before changing ciphers.
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>
2017-10-25 03:45:26 +00:00
David Benjamin
75d43b5785 Pack encrypted handshake messages together.
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>
2017-10-25 03:38:56 +00:00
David Benjamin
dd6c2e880f Check early ALPN before offering 0-RTT.
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>
2017-10-25 03:31:56 +00:00
David Benjamin
f6632dae5f Make all read errors idempotent.
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>
2017-10-17 21:28:51 +00:00
David Benjamin
a031b61230 Replace open_close_notify with open_app_data.
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>
2017-10-17 21:19:31 +00:00
David Benjamin
40e94701dc Always process handshake records in full.
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>
2017-10-17 14:53:11 +00:00
David Benjamin
e1068b76bd Test RSA premaster unpad better.
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>
2017-10-13 18:22:58 +00:00
David Benjamin
11ac519d79 Test DTLS record/packet packing more aggressively.
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>
2017-10-13 17:32:55 +00:00
Steven Valdez
be165a2e70 Fix missing TicketMaxEarlyDataInfo in first session ticket.
Change-Id: Ib24208e0ebdb1787c629ee29bd0115332ac36e73
Reviewed-on: https://boringssl-review.googlesource.com/21484
Reviewed-by: David Benjamin <davidben@google.com>
2017-10-10 21:03:10 +00:00
Adam Langley
2e9bb4eb96 Fix comment about EarlyCCS.
Thanks to Dimitar Vlahovski for pointing this out.

Change-Id: I417f52ec6c3e950bdab6079962b29976fb75c029
Reviewed-on: https://boringssl-review.googlesource.com/21324
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>
2017-10-06 22:46:57 +00:00
David Benjamin
861f28a624 Clear one more timeout when using gdb.
https://boringssl-review.googlesource.com/18605 got the other ones.

Change-Id: If00487a4dd8508496a31a0a565c965559e12879c
Reviewed-on: https://boringssl-review.googlesource.com/21264
Commit-Queue: David Benjamin <davidben@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>
2017-10-06 17:23:07 +00:00
David Benjamin
6c1f2b77de Test that we tolerate server-sent supported groups.
I should have added this test in
https://boringssl-review.googlesource.com/10320. This is necessary in
TLS 1.3 and spec compliance and TLS 1.2 to tolerate some broken servers.

Change-Id: Ibb52eaa1e370062f83e84856ef7f1c2c79d6a5d3
Reviewed-on: https://boringssl-review.googlesource.com/21124
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>
2017-10-05 15:14:29 +00:00
Steven Valdez
4c7f5fa023 Remove old TLS 1.3 variants (NoSessionID and RecordType).
Change-Id: I2428321218d0b5dce242e3843d39ca269e1eb686
Reviewed-on: https://boringssl-review.googlesource.com/20984
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>
2017-10-03 18:12:23 +00:00
David Benjamin
15868b3bba Revert "Work around a Java client bug when rotating certificates."
This reverts commit aba057a4e0 and
5a79ff5efd.

Change-Id: Ia53a3908491ec99ab25ea1d1bdedf322c2fbe5c4
Reviewed-on: https://boringssl-review.googlesource.com/20744
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-09-26 22:27:47 +00:00
David Benjamin
683ffbbe57 Fix fuzzer mode suppressions.
Some tests got renamed.

Change-Id: I7ef788c10dc40de244778b9e80ae3a04afee3dd4
Reviewed-on: https://boringssl-review.googlesource.com/20226
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-09-12 19:32:14 +00:00
Steven Valdez
c7d4d21413 Add experiment without client CCS and fix session ID bug.
Change-Id: Id6cf63caf5a00d4d4ca66a5c7530c48c2d9ed91f
Reviewed-on: https://boringssl-review.googlesource.com/20164
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>
2017-09-12 18:05:50 +00:00
David Benjamin
aba057a4e0 Work around a Java client bug when rotating certificates.
The Java client implementation of the 3SHAKE mitigation incorrectly
rejects initial handshakes when all of the following are true:

1. The ClientHello offered a session.
2. The session was successfully resumed previously.
3. The server declines the session.
4. The server sends a certificate with a different SAN list than in the
   previous session.

(Note the 3SHAKE mitigation is to reject certificates changes on
renegotiation, while Java's logic applies to initial handshakes as
well.)

The end result is long-lived Java clients break on some certificate
rotations. Fingerprint Java clients and decline all offered sessions.
This avoids (2) while still introducing new sessions to clear any
existing problematic sessions.

See also b/65323005.

Change-Id: Ib2b84c69b5ecba285ffb8c4d03de5626838d794e
Reviewed-on: https://boringssl-review.googlesource.com/20184
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>
2017-09-12 15:56:59 +00:00
Steven Valdez
1682126fd8 Add Experiment 2
Change-Id: If240cbeb133a23331cb6ca59eaacde7733592278
Reviewed-on: https://boringssl-review.googlesource.com/20144
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>
2017-09-11 16:53:16 +00:00
David Benjamin
a9c96bae8a Remove a DHE remnant from runner.
Change-Id: I98a42572af71e18839a29eb0f7547d17d08f2c22
Reviewed-on: https://boringssl-review.googlesource.com/20024
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>
2017-09-08 14:48:10 +00:00
David Benjamin
2ff44b183a Add DTLS fuzzers.
Bug: 124
Change-Id: Iff02be9df2806572e6d3f860b448f598f85778c3
Reviewed-on: https://boringssl-review.googlesource.com/20107
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>
2017-09-07 22:26:50 +00:00
David Benjamin
e51fb0fa71 Fix empty fragment handling in DTLS message reassembly.
Found with libFuzzer.

Bug: chromium:763097
Change-Id: I806bcfc714c0629ff7f725e37f4c0045d4ec7ac6
Reviewed-on: https://boringssl-review.googlesource.com/20105
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>
2017-09-07 22:11:10 +00:00
David Benjamin
d0beda01f9 Properly report SSL_session_reused after a renegotiation.
We forgot to reset that value.

Change-Id: Ic869cb61da332983cc40223cbbdf23b455dd9766
Reviewed-on: https://boringssl-review.googlesource.com/20084
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>
2017-09-06 20:56:06 +00:00
Martin Kreichgauer
a23b68f564 ssl/test/runner: Change ecdsa.PublicKey initialization
Change-Id: I4dea223825da4e4ab0bc789e738f470f5fe5d659
Reviewed-on: https://boringssl-review.googlesource.com/20044
Commit-Queue: Martin Kreichgauer <martinkr@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>
2017-09-06 18:19:16 +00:00
David Benjamin
0a471910b4 Test empty extensions fields are omitted.
For historical reasons, TLS allows ServerHellos (and ClientHellos)
without extensions to omit the extensions fields entirely.
https://github.com/openssl/openssl/pull/4296 reports this is even
necessary for compatibility with extension-less clients. We continue to
do so, but add a test for it anyway.

Change-Id: I63c2e3a5f298674eb21952fca6914dad07d7c245
Reviewed-on: https://boringssl-review.googlesource.com/19864
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>
2017-08-31 14:17:36 +00:00
David Benjamin
3969fdf860 Test invalid certificates.
The fuzzer should discover this instantly, but it's a sufficiently
important failure case (don't accidentally drop the certificate on the
floor or anything weird like that) that it's probably worth testing.

Change-Id: I684932c2e8a88fcf9b2318bf46980d312c66f6ef
Reviewed-on: https://boringssl-review.googlesource.com/19744
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>
2017-08-29 20:27:58 +00:00
David Benjamin
f21650709a Cut down on some redundant flags.
We have fancy -on-initial and -on-resume prefixes now that can apply to
every flag.

Change-Id: I6195a97f663ebc94db320ca35889c213c700a976
Reviewed-on: https://boringssl-review.googlesource.com/19666
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>
2017-08-24 16:18:32 +00:00
David Benjamin
5c4271f7cb Don't reauthenticate on renegotiation.
We currently forbid the server certificate from changing on
renegotiation. This means re-verifying the certificate is pointless and
indeed the callback being called again seems to surprise consumers more
than anything else.

Carry over the initial handshake's SCT lists and OCSP responses (don't
enforce they don't change since the server may have, say, picked up new
OCSP responses in the meantime), ignore new ones received on
renegotiation, and don't bother redoing verification.

For our purposes, TLS 1.2 renegotiation is an overcomplicated TLS 1.3
KeyUpdate + post-handshake auth. The server is not allowed to change
identity.

Bug: 126
Change-Id: I0dae85bcf243943b1a5a97fa4f30f100c9e6e41e
Reviewed-on: https://boringssl-review.googlesource.com/19665
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>
2017-08-24 16:14:22 +00:00
David Benjamin
302b818d4b Only enable DTLS post-handshake rexmits if we sent the final Finished.
I messed up https://boringssl-review.googlesource.com/8883 and caused
both sides to believe they had sent the final Finished. Use next_message
to detect whether our last flight had a reply.

Change-Id: Ia4d8c8eefa818c9a69acc94d63c9c863293c3cf5
Reviewed-on: https://boringssl-review.googlesource.com/19604
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>
2017-08-23 17:13:42 +00:00
Steven Valdez
f4ecc84644 Prevent both early data and custom extensions from being accepted.
This loosens the earlier restriction to match Channel ID. Both may be
configured and offered, but the server is obligated to select only one
of them. This aligns with the current tokbind + 0-RTT draft where the
combination is signaled by a separate extension.

Bug: 183
Change-Id: I786102a679999705d399f0091f76da236be091c2
Reviewed-on: https://boringssl-review.googlesource.com/19124
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
2017-08-14 20:15:54 +00:00
Dimitar Vlahovski
bd70845a80 Add tests for CertificateVerify
CertificateVerify must be sent after a non-empty Certificate msg for:
1) TLS1.2 client
2) TLS1.3 client and server

This CL adds tests for those use cases.

Change-Id: I696e9dd74dcd523c6f8868a4fb9ada28fd67746d
Reviewed-on: https://boringssl-review.googlesource.com/19044
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>
2017-08-10 18:44:57 +00:00
David Benjamin
4a37de076b Test that Finished checks are enforced in 0-RTT.
This is analogous to needing to test that Finished is enforced in False
Start.

Change-Id: I168a72ac51b0f75156aaf6ccc9724ae66ce1e734
Reviewed-on: https://boringssl-review.googlesource.com/18986
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2017-08-09 00:42:34 +00:00
David Benjamin
f6ae9e6c2c Fix more hard-coded TLS 1.3 variant strings.
These should use the shim/runner combined setting.

Change-Id: Iad6abb4e76f6e5accef446696aa4132073eca06a
Reviewed-on: https://boringssl-review.googlesource.com/18984
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-08-09 00:40:45 +00:00
David Benjamin
78b8b99cf7 Fix a bug in and test the message callback.
reuse_message and V2ClientHellos each caused messages to be
double-reported.

Change-Id: I8722a3761ede272408ac9cf8e1b2ce383911cc6f
Reviewed-on: https://boringssl-review.googlesource.com/18764
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>
2017-08-03 18:52:47 +00:00
David Benjamin
b0c761eb76 Tolerate early ChangeCipherSpec in DTLS.
This would only come up if the peer didn't pack records together, but
it's free to handle. Notably OpenSSL has a bug where it does not pack
retransmits together.

Change-Id: I0927d768f6b50c62bacdd82bd1c95396ed503cf3
Reviewed-on: https://boringssl-review.googlesource.com/18724
Reviewed-by: David Benjamin <davidben@google.com>
2017-08-01 22:00:52 +00:00
Adam Langley
182b573329 Don't set timeout in runner when using GDB.
I'm not that fast when debugging.

Change-Id: I37a120a77e9a35ac5255ad760513b983f83d9bd7
Reviewed-on: https://boringssl-review.googlesource.com/18605
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>
2017-07-28 21:26:42 +00:00
Adam Langley
e6c58ffa70 go fmt runner.go
Change-Id: I3357b733c69ff1fdbf64fd12653261383a310732
Reviewed-on: https://boringssl-review.googlesource.com/18604
Reviewed-by: David Benjamin <davidben@google.com>
2017-07-28 18:19:53 +00:00
Adam Langley
10e1060261 Send correct fatal alert the renegotation extension fails to match.
https://tools.ietf.org/html/rfc5746#section-3.4 says that
handshake_failure is the correct alert to send, but we were sending
illegal_parameter.

Change-Id: Ife951c5951f6f8e4c31a3f2f57307bfed1c24561
Reviewed-on: https://boringssl-review.googlesource.com/18408
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>
2017-07-25 21:02:43 +00:00
Steven Valdez
0e4a448ab8 Add ClientHello no_session_id variant.
Change-Id: I3d249582dea871d7b1c078a6b5f57679037d1b8f
Reviewed-on: https://boringssl-review.googlesource.com/17984
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
2017-07-18 19:58:10 +00:00
David Benjamin
3a1dd46e4e Add async certificate verification callback.
This also serves as a certificate verification callback for
CRYPTO_BUFFER-based consumers. Remove the silly
SSL_CTX_i_promise_to_verify_certs_after_the_handshake placeholder.

Bug: 54, chromium:347402
Change-Id: I4c6b445cb9cd7204218acb2e5d1625e6f37aff6f
Reviewed-on: https://boringssl-review.googlesource.com/17964
Reviewed-by: David Benjamin <davidben@google.com>
2017-07-17 20:55:23 +00:00
David Benjamin
09ed11928e Test that record-splitting splits records.
We probably should not have been able to land
https://boringssl-review.googlesource.com/17944 without a test
suppression.

Change-Id: Ie47ca324f94d2f03b7d31218b0379656c070b21b
Reviewed-on: https://boringssl-review.googlesource.com/17905
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>
2017-07-17 14:19:13 +00:00
David Benjamin
b853f315dd Fix handling of ServerHellos with omitted extensions.
Due to SSL 3.0 legacy, TLS 1.0 through 1.2 allow ClientHello and
ServerHello messages to omit the extensions field altogether, rather
than write an empty field. We broke this in
https://boringssl-review.googlesource.com/c/17704/ when we needed to a
second ServerHello parsing path.

Fix this and add some regression tests to explicitly test both the
omitted and empty extensions ClientHello and ServerHello cases.

Bug: chromium:743218
Change-Id: I8297ba608570238e19f12ea44a9fe2fe9d881d28
Reviewed-on: https://boringssl-review.googlesource.com/17904
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2017-07-14 23:17:40 +00:00
David Benjamin
c386440683 Add some timestamps to connect/accept failures.
This is in an attempt to debug the Mac flakiness. The timestamps will
hopefully help narrow down the order of operations here.

Bug: 199
Change-Id: I8b8dd7222e3a57a8b055b8bc1b7731334e0fcdf0
Reviewed-on: https://boringssl-review.googlesource.com/17886
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>
2017-07-14 20:55:40 +00:00
Steven Valdez
dbe01585ba Implement ContentType TLS 1.3 variant.
This implements PR #1051
(https://github.com/tlswg/tls13-spec/pull/1051).

Local experiments were not able to replicate the claims in the PR, but
implement this anyway for comparison purposes.

Change-Id: Ic9baf5e671f9a44565020466a553dd08f5ec0f1b
Reviewed-on: https://boringssl-review.googlesource.com/17844
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>
2017-07-14 19:48:00 +00:00
David Benjamin
a502239475 Actually test the TLS 1.3 experimental variant.
Adding it to tlsVersions is sort of pointless when we don't test it.

Change-Id: Ie0c0167cef887aee54e5be90bf7fc98619c1a6fb
Reviewed-on: https://boringssl-review.googlesource.com/17708
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>
2017-07-11 14:34:07 +00:00
Steven Valdez
038da9b939 Move the version to an extension in the experimental TLS 1.3 encoding.
Change-Id: I0726e11006235db9309a8370a11e00ede0216279
Reviewed-on: https://boringssl-review.googlesource.com/17704
Reviewed-by: David Benjamin <davidben@google.com>
2017-07-11 14:33:33 +00:00
David Benjamin
08fea48a91 Fix fuzzer mode test suppressions.
Change-Id: If59e911549f639976752c018ffd7253f41c6beda
Reviewed-on: https://boringssl-review.googlesource.com/17705
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
2017-07-10 19:52:00 +00:00
Steven Valdez
520e1220bb Implement experimental alternate encoding of TLS 1.3.
TLS 1.3 deployment is currently blocked by buggy middleboxes
throughout the ecosystem. As an experiment to better understand these bugs
and the problems they are causing, implement TLS 1.3 variants with
alternate encodings. These are still the same protocol, only encoded
slightly differently. We will use what we learn from these experiments to
guide the TLS 1.3 deployment strategy and proposals to the IETF, if any.

These experiments only target the basic 1-RTT TLS 1.3 handshake. Based on
what we learn from this experiment, we may try future variations to
explore 0-RTT and HelloRetryRequest.

When enabled, the server supports all TLS 1.3 variants while the client
is configured to use a particular variant.

Change-Id: I532411d1abc41314dc76acce0246879b754b4c61
Reviewed-on: https://boringssl-review.googlesource.com/17327
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>
2017-07-10 18:15:08 +00:00
David Benjamin
a818134b67 Simplify ChangeCipherSpec code in runner.
Not sure why it was expanded out like that.

Change-Id: I6899dbd23130ed7196c45c2784330b2a4fe9bdba
Reviewed-on: https://boringssl-review.googlesource.com/17666
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-07-10 17:39:43 +00:00
David Benjamin
d9cbb53562 Fix SSL_version on 0-RTT.
Like other handshake properties, when in 0-RTT on the client,
SSL_version should report the predicted version. This used to work on
accident because of how ssl->version got set in handshake_client.c early
(and that TLS 1.4 does not exist), but we no longer do that.

Change-Id: Ifb63a22b795fe8964ac553844a46040acd5d7323
Reviewed-on: https://boringssl-review.googlesource.com/17664
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>
2017-07-07 17:43:07 +00:00
David Benjamin
a1ce85696d Test record splitting at all ciphers.
We were missing AES256 and 3DES. Though this test dates to the old
record-splitting code which was much scarier than the new one.

Change-Id: Ia84a8c1a2bbd79fa70941f80cf6393013e4f13d5
Reviewed-on: https://boringssl-review.googlesource.com/17543
Reviewed-by: David Benjamin <davidben@google.com>
2017-07-05 23:52:12 +00:00
David Benjamin
634f475255 Test the Channel IDs are not requested without ECDHE.
This was a workaround for triple handshake put in way back, before
extended master secret.

Change-Id: Ie0112fa6323522b17c90a833d558f7182586d2c3
Reviewed-on: https://boringssl-review.googlesource.com/17541
Reviewed-by: David Benjamin <davidben@google.com>
2017-07-05 23:51:59 +00:00
David Benjamin
c3648faaa7 Add tests for SSL_VERIFY_PEER_IF_NO_OBC and fix TLS 1.3.
Also mirror the structure of the TLS 1.2 and TLS 1.3 code a bit.

Change-Id: I7b34bf30de63fa0bd47a39a90570846fb2314ad5
Reviewed-on: https://boringssl-review.googlesource.com/17539
Reviewed-by: David Benjamin <davidben@google.com>
2017-07-05 23:50:41 +00:00
David Benjamin
364af78407 Add some cipher negotiation tests.
We've never actually written tests for equipreference groups at the
BoringSSL level.

Change-Id: I571c081534efbfa8e7b84846fafed0b772721da1
Reviewed-on: https://boringssl-review.googlesource.com/17538
Reviewed-by: David Benjamin <davidben@google.com>
2017-07-05 23:50:36 +00:00
David Benjamin
413e79e947 Test the client rejects invalid compression methods from the server.
Change-Id: I90286da596d5822d4cfedf40995d80cf76adaf97
Reviewed-on: https://boringssl-review.googlesource.com/17536
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2017-07-05 23:50:21 +00:00
David Benjamin
9343b0b8b3 Don't check renegotiation_info in fuzzer mode.
Otherwise the fuzzer gets stuck at renegotiations.

Bug: 104
Change-Id: If37f9ab165d06e37bfc5c423fba35edaabed293b
Reviewed-on: https://boringssl-review.googlesource.com/17532
Reviewed-by: David Benjamin <davidben@google.com>
2017-07-05 23:48:42 +00:00
David Benjamin
0fde2eb0e3 Update TLS fuzzer format with prepended settings.
This allows us to fill in holes in our fuzzer coverage, notably client
resumption (and thus early data) and server client certificates. The
corpora are not refreshed yet. This will be done in upcoming changes.

Also add an option for debugging fuzzers. It's very useful to test it on
transcripts and make sure that fuzzer mode successfully makes things
compatible.

Bug: 104
Change-Id: I02f0be4045d1baf68efc9a4157f573df1429575d
Reviewed-on: https://boringssl-review.googlesource.com/17531
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>
2017-07-05 23:48:26 +00:00