Commit Graph

1649 Commits

Author SHA1 Message Date
Steven Valdez
a4ee74dadf Skipping early data on 0RTT rejection.
BUG=101

Change-Id: Ia1edbccee535b0bc3a0e18465286d5bcca240035
Reviewed-on: https://boringssl-review.googlesource.com/12470
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>
2016-12-01 20:16:08 +00:00
David Benjamin
8f820b4e43 Clean up resumption secret "derivation" step.
There is no more derivation step. We just use the resumption secret
directly. This saves us an unnecessary memcpy.

Change-Id: I203bdcc0463780c47cce655046aa1be560bb5b18
Reviewed-on: https://boringssl-review.googlesource.com/12472
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-12-01 19:26:14 +00:00
David Benjamin
3d622e554e Add missing bounds check in tls13_derive_resumption_secret.
This is fine because TLS PRFs only go up to SHA-384, but since
SSL_SESSION::master_key is sized to 48, not EVP_MAX_MD_SIZE, this should
explicitly check the bounds.

Change-Id: I2b1bcaab5cdfc3ce4d7a8b8ed5cc4c6d15d10270
Reviewed-on: https://boringssl-review.googlesource.com/12460
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-11-28 20:36:32 +00:00
David Benjamin
68f37b7a3f Run TestOneSidedShutdown at all versions.
Change-Id: I3a5d949eec9241ea43da40ce23e0e7f2a25e30e5
Reviewed-on: https://boringssl-review.googlesource.com/12381
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-11-21 18:56:48 +00:00
David Benjamin
0fef3056eb Add a ForEachVersion function to ssl_test.
This aligns with ec_test which has a ForEachCurve helper and avoids
writing these loops all the time. As a bonus, these tests start working
in DTLS now.

Change-Id: I613fc08b641ddc12a819d8a1268a1e6a29043663
Reviewed-on: https://boringssl-review.googlesource.com/12380
Reviewed-by: Steven Valdez <svaldez@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>
2016-11-21 18:56:34 +00:00
Adam Langley
9b885c5d0f Don't allow invalid SCT lists to be set.
This change causes SSL_CTX_set_signed_cert_timestamp_list to check the
SCT list for shallow validity before allowing it to be set.

Change-Id: Ib8a1fe185224ff02ed4ce53a0109e60d934e96b3
Reviewed-on: https://boringssl-review.googlesource.com/12401
Commit-Queue: 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>
Reviewed-by: David Benjamin <davidben@google.com>
2016-11-19 00:24:18 +00:00
Adam Langley
6f5f49f33d Flush TLS 1.3 certificate extensions.
(Otherwise we end up touching potentially unwound stack.)

I looked into why our builders didn't catch this and it appears that, at
least with Clang 3.7, ASAN doesn't notice this. Perhaps Clang at that
version is being lazy about destructing the scoped CBB and so doesn't
actually go wrong.

Change-Id: Ia0f73e7eb662676439f024805fc8287a4e991ce0
Reviewed-on: https://boringssl-review.googlesource.com/12400
Reviewed-by: Adam Langley <agl@google.com>
2016-11-18 22:01:38 +00:00
Adam Langley
cfa08c3b77 Enforce basic sanity of SCT lists.
According to the RFC[1], SCT lists may not be empty and nor may any SCT
itself be empty.

[1] https://tools.ietf.org/html/rfc6962#section-3.3

Change-Id: Ia1f855907588b36a4fea60872f87e25dc20782b4
Reviewed-on: https://boringssl-review.googlesource.com/12362
Reviewed-by: Adam Langley <agl@google.com>
2016-11-18 19:19:48 +00:00
David Benjamin
b5172a722c Make tls1_setup_key_block static.
It is not called outside of t1_enc.c.

Change-Id: Ifd9d109eeb432e931361ebdf456243c490b93ecf
Reviewed-on: https://boringssl-review.googlesource.com/12340
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>
2016-11-18 03:58:26 +00:00
Adam Langley
fbbef12918 Don't put a colon in the extra error message.
Since the printed format for errors uses colons to separate different
parts of the error message, this was confusing.

Change-Id: I4742becec2bcb56ad8dc2fdb9a3bb23e4452d1b2
Reviewed-on: https://boringssl-review.googlesource.com/12361
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: Adam Langley <alangley@gmail.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-11-17 21:46:34 +00:00
David Benjamin
35598ae8dd Remove ext_alpn_init.
We do not change ALPN on renego, so the value should carry over and not
be cleared.

Change-Id: Id54a083945542b4457d9c2787f0fe7c30239b76f
Reviewed-on: https://boringssl-review.googlesource.com/12306
Reviewed-by: David Benjamin <davidben@google.com>
2016-11-17 06:46:48 +00:00
David Benjamin
e7f60a2852 Fix alert on tls1_process_alert failure.
If the function fails, it's an internal_error.

Change-Id: I4b7cf7a6ca2527f04b708303ab1bc71df762b55b
Reviewed-on: https://boringssl-review.googlesource.com/12312
Reviewed-by: David Benjamin <davidben@google.com>
2016-11-17 06:45:38 +00:00
David Benjamin
12d6bafed8 Make ssl_ext_pre_shared_key_add_clienthello static.
It doesn't need to be exported out of t1_lib.c.

Change-Id: I000493e1e330457051da1719ca9f8152a4ff845a
Reviewed-on: https://boringssl-review.googlesource.com/12316
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>
2016-11-17 06:33:30 +00:00
David Benjamin
bbaf367969 Add |SSL_set_retain_only_sha256_of_client_certs|.
Previously the option to retain only the SHA-256 hash of client
certificates could only be set at the |SSL_CTX| level. This change makes
|SSL| objects inherit the setting from the |SSL_CTX|, but allows it to
be overridden on a per-|SSL| basis.

Change-Id: Id435934af3d425d5f008d2f3b9751d1d0884ee55
Reviewed-on: https://boringssl-review.googlesource.com/12182
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>
2016-11-17 02:49:19 +00:00
David Benjamin
a933c38f1a Test setting session ID context in early or SNI callback.
The former has always worked. The latter is new to the revised
processing order.

Change-Id: I993d29ccaca091725524847695df4d1944b609cf
Reviewed-on: https://boringssl-review.googlesource.com/11848
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-11-17 01:22:05 +00:00
David Benjamin
f01f42a2ce Negotiate ciphers before resumption.
This changes our resumption strategy. Before, we would negotiate ciphers
only on fresh handshakes. On resumption, we would blindly use whatever
was in the session.

Instead, evaluate cipher suite preferences on every handshake.
Resumption requires that the saved cipher suite match the one that would
have been negotiated anyway. If client or server preferences changed
sufficiently, we decline the session.

This is much easier to reason about (we always pick the best cipher
suite), simpler, and avoids getting stuck under old preferences if
tickets are continuously renewed. Notably, although TLS 1.2 ticket
renewal does not work in practice, TLS 1.3 will renew tickets like
there's no tomorrow.

It also means we don't need dedicated code to avoid resuming a cipher
which has since been disabled. (That dedicated code was a little odd
anyway since the mask_k, etc., checks didn't occur. When cert_cb was
skipped on resumption, one could resume without ever configuring a
certificate! So we couldn't know whether to mask off RSA or ECDSA cipher
suites.)

Add tests which assert on this new arrangement.

BUG=116

Change-Id: Id40d851ccd87e06c46c6ec272527fd8ece8abfc6
Reviewed-on: https://boringssl-review.googlesource.com/11847
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>
2016-11-17 01:02:42 +00:00
David Benjamin
34202b93b6 Call cert_cb before resolving resumption.
This is in preparation for determining the cipher suite (which, in TLS
1.2, requires the certificate be known) before resumption.

Note this has caller-visible effects:

- cert_cb is now called whether resumption occurs or not. Our only
  consumer which uses this as a server is Node which will require a
  patch to fix up their mucking about with SSL_get_session. (But the
  patch should be quite upstreamable. More 1.1.0-compatible and
  generally saner.)

- cert_cb is now called before new_session_cb and dos_protection_cb.

BUG=116

Change-Id: I6cc745757f63281fad714d4548f23880570204b0
Reviewed-on: https://boringssl-review.googlesource.com/11846
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>
2016-11-17 00:29:46 +00:00
David Benjamin
4eb95ccfd6 Parse ClientHello extensions before deciding on resumption.
This simplifies a little code around EMS and PSK KE modes, but requires
tweaking the SNI code.

The extensions that are more tightly integrated with the handshake are
still processed inline for now. It does, however, require an extra state
in 1.2 so the asynchronous session callback does not cause extensions to
be processed twice. Tweak a test enforce this.

This and a follow-up to move cert_cb before resumption are done in
preparation for resolving the cipher suite before resumption and only
resuming on match.

Note this has caller-visible effects:

- The legacy SNI callback happens before resumption.

- The ALPN callback happens before resumption.

- Custom extension ClientHello parsing callbacks also cannot depend on
  resumption state.

- The DoS protection callback now runs after all the extension callbacks
  as it is documented to be called after the resumption decision.

BUG=116

Change-Id: I1281a3b61789b95c370314aaed4f04c1babbc65f
Reviewed-on: https://boringssl-review.googlesource.com/11845
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>
2016-11-16 23:58:02 +00:00
David Benjamin
e1cc35e581 Tolerate cipher changes on TLS 1.3 resumption as a client.
As a client, we must tolerate this to avoid interoperability failures
with allowed server behaviors.

BUG=117

Change-Id: I9c40a2a048282e2e63ab5ee1d40773fc2eda110a
Reviewed-on: https://boringssl-review.googlesource.com/12311
Reviewed-by: David Benjamin <davidben@google.com>
2016-11-16 13:27:07 +00:00
David Benjamin
2b02f4b67d Loosen TLS 1.3 session/cipher matching in BoGo.
Draft 18 sadly loosens the requirements to only requiring the PRF hash
stay fixed.

BUG=117

Change-Id: Ic94d53fd9cabaee611fcf36b0071558075e10728
Reviewed-on: https://boringssl-review.googlesource.com/12310
Reviewed-by: Nick Harper <nharper@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>
2016-11-16 13:19:25 +00:00
David Benjamin
d0d532f169 Select TLS 1.3 cipher before resumption in BoGo.
This is generally much cleaner and makes it possible to implement the
more lax cipher matching in draft 18.

BUG=117

Change-Id: I595d7619d60bc92e598d75b43945286323c0b72b
Reviewed-on: https://boringssl-review.googlesource.com/12309
Reviewed-by: Nick Harper <nharper@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>
2016-11-16 13:14:28 +00:00
David Benjamin
6929f27ed5 Fix return values for TLS 1.3 state machine code.
This is a no-op because all affected codepaths are either unreachable or
are fine because ssl_hs_error (intentionally, since C doesn't help us
any) aligns with zero. Still, fix these.

Change-Id: Ieba4e3eec3881a56b5ddcd32abdd2c9dda875eda
Reviewed-on: https://boringssl-review.googlesource.com/12313
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-11-16 13:13:50 +00:00
David Benjamin
71186e85d1 Move ExpectTicketAge out of AcceptAnySession.
It doesn't particular matter, but AcceptAnySession should only skip the
things that would cause us to note accept a ticket. ExpectTicketAge is
an assertion, not part of protocol logic. Accordingly, fix the text.

Change-Id: I3bea9c58f4d5f912308252ec8834f183287d632f
Reviewed-on: https://boringssl-review.googlesource.com/12308
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Nick Harper <nharper@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-11-16 07:57:15 +00:00
David Benjamin
0b8f85ebe5 Fix AcceptAnyVersion bug.
The version check should run if AcceptAnyVersion is *not* set.

Change-Id: I4c137564f91a86cb5e6a26e09fd4670cce8f1dcb
Reviewed-on: https://boringssl-review.googlesource.com/12307
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Nick Harper <nharper@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-11-16 07:55:27 +00:00
David Benjamin
ba28dfc381 Add a -repeat-until-failure flag to runner.
When debugging a flaky test, it's useful to be able to run a given test
over and over.

Change-Id: I1a7b38792215550b242eb8238214d873d41becb6
Reviewed-on: https://boringssl-review.googlesource.com/12301
Reviewed-by: David Benjamin <davidben@google.com>
2016-11-16 04:19:27 +00:00
David Benjamin
53210cb48e Do not send unsolicited SCTs in TLS 1.3.
The draft 18 implementation did not compute scts_requested correctly. As
a result, it always believed SCTs were requested. Fix this and add tests
for unsolicited OCSP responses and SCTs at all versions.

Thanks to Daniel Hirche for the report.

Change-Id: Ifc59c5c4d7edba5703fa485c6c7a4055b15954b4
Reviewed-on: https://boringssl-review.googlesource.com/12305
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>
2016-11-16 00:20:09 +00:00
David Benjamin
ea80f9d5df obfuscated_ticket_age must also be reset when comparing.
Thanks to Eric Rescorla for catching this.

Change-Id: Id0a024d7f705519cfe76d350e0ef2688dbd11a22
Reviewed-on: https://boringssl-review.googlesource.com/12303
Reviewed-by: Nick Harper <nharper@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>
2016-11-15 21:56:03 +00:00
David Benjamin
75f9914e17 Align TLS 1.2 and 1.3 server session validity checks.
Having that logic in two different places is a nuisance when we go to
add new checks like resumption stuff. Along the way, this adds missing
tests for the ClientHello cipher/session consistency check. (We'll
eventually get it for free once the cipher/resumption change is
unblocked, but get this working in the meantime.)

This also fixes a bug where the session validity checks happened in the
wrong order relative to whether tickets_supported or renew_ticket was
looked at. Fix that by lifting that logic closer to the handshake.

Change-Id: I3f4b59cfe01064f9125277dc5834e62a36e64aae
Reviewed-on: https://boringssl-review.googlesource.com/12230
Reviewed-by: Adam Langley <agl@google.com>
2016-11-15 18:18:39 +00:00
David Benjamin
c5665c9ac9 Remove out-of-date BoGo earlyDataContext parsing bits.
This was removed a while ago. As of -18, the early data indication
extension is just a boolean.

Change-Id: I328b9abfafad326d4c2a3b5fe981af111f8401ad
Reviewed-on: https://boringssl-review.googlesource.com/12302
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Nick Harper <nharper@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-11-15 13:55:26 +00:00
David Benjamin
b8d74f5b6a Add tests for failing cert_cb.
We missed that the TLS 1.3 code was inconsistent with the TLS 1.2 code.
Only on the server did we push an error code. But consistency between
client and server is probably worthwhile so, fix the 1.2 code to match
for now.

Change-Id: I17952c72048697dc66eacf0f144a66ced9cb3be8
Reviewed-on: https://boringssl-review.googlesource.com/12260
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>
2016-11-15 07:15:54 +00:00
David Benjamin
9b63f2964d Fix run_tests on fuzzer-mode builds.
Change-Id: I0767cd4801924170ce13b8143a9586485b8f78af
Reviewed-on: https://boringssl-review.googlesource.com/12280
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>
2016-11-15 07:01:24 +00:00
David Benjamin
dfb4138197 Update suppressions for fuzzer mode.
Change-Id: I07c4b67206440d169b314f24e1b3c1c697dda24f
Reviewed-on: https://boringssl-review.googlesource.com/12204
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>
2016-11-15 07:00:35 +00:00
David Benjamin
ffb1107c91 Add a helper function for parsing extensions blocks.
TLS 1.3 adds a number of places with extensions blocks that don't easily
fit into our ClientHello/EncryptedExtensions callbacks. Between
HelloRetryRequest, ServerHello, draft 18 going nuts with Certificate,
and NewSessionTicket when we do 0-RTT, this passes the "abstract things
that are repeated three times" sniff test.

For now, it rejects unknown extensions, but it will probably grow an
allow_unknown parameter for NewSessionTicket.

This involves disabling some MSVC warnings, but they're invalid as of
C99 which we otherwise require. See
https://connect.microsoft.com/VisualStudio/feedback/details/1230248/remove-c99-related-warnings-or-make-them-off-by-default

Change-Id: Iea8bf8ab216270c081dd63e79aaad9ec73b3b550
Reviewed-on: https://boringssl-review.googlesource.com/12233
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>
2016-11-15 06:58:52 +00:00
David Benjamin
32b47a5e35 Allow PSK binder mismatches in fuzzer mode.
BUG=112

Change-Id: I88ef17e32e33b091ff1e27b7950f88e1d48f9278
Reviewed-on: https://boringssl-review.googlesource.com/12239
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>
2016-11-15 06:57:54 +00:00
Steven Valdez
a833c357ed Update to TLS 1.3 draft 18.
This is the squash of the following CLs:
https://boringssl-review.googlesource.com/c/12021/9
https://boringssl-review.googlesource.com/c/12022/9
https://boringssl-review.googlesource.com/c/12107/19
https://boringssl-review.googlesource.com/c/12141/22
https://boringssl-review.googlesource.com/c/12181/33

The Go portions were written by Nick Harper

BUG=112

Change-Id: I375a1fcead493ec3e0282e231ccc8d7c4dde5063
Reviewed-on: https://boringssl-review.googlesource.com/12300
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-11-15 06:57:21 +00:00
David Benjamin
ced9479fd1 Replace hash_current_message with get_current_message.
For TLS 1.3 draft 18, it will be useful to get at the full current
message and not just the body. Add a hook to expose it and replace
hash_current_message with a wrapper over it.

BUG=112

Change-Id: Ib9e00dd1b78e8b72e12409d85c80e96c5b411a8b
Reviewed-on: https://boringssl-review.googlesource.com/12238
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>
2016-11-15 06:52:10 +00:00
David Benjamin
2c51645c59 Add runner tests which send intermediate certificates.
Certificate chain with intermediate taken from Chromium's tests. Though
it doesn't really matter because the runner tests don't verify
certificates.

BUG=70

Change-Id: I46fd1d4be0f371b5bfd43370b97d2c8053cfad60
Reviewed-on: https://boringssl-review.googlesource.com/12261
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-11-15 01:36:37 +00:00
David Benjamin
e6f2221423 Enforce record-layer version numbers.
We used to enforce after the version was set, but stopped enforcing with
TLS 1.3. NSS enforces the value for encrypted records, which makes sense
and avoids the problems gating it on have_version. Add tests for this.

Change-Id: I7fb5f94ab4a22e8e3b1c14205aa934952d671727
Reviewed-on: https://boringssl-review.googlesource.com/12143
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>
2016-11-13 05:28:35 +00:00
David Benjamin
eab773a8aa Add missing PSK identity comment.
Change-Id: I1ca9f252afeea6cdcaa6d75e842eab019c82a7e4
Reviewed-on: https://boringssl-review.googlesource.com/12184
Reviewed-by: David Benjamin <davidben@google.com>
2016-11-13 05:22:38 +00:00
David Benjamin
78476f6065 Move tlsext_ticket_expected to SSL_HANDSHAKE.
It's all of one bit, but having it on the SSL object means we need
manually to reset it on renego.

Change-Id: I989dacd430fe0fa63d76451b95f036a942aefcfe
Reviewed-on: https://boringssl-review.googlesource.com/12229
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>
2016-11-12 07:32:42 +00:00
David Benjamin
ba1660b282 Tidy up finish_message logic.
dtls1_finish_message should NULL *out_msg before calling OPENSSL_free,
rather than asking ssl3_complete_message to do it. ssl3_finish_message
has no need to call it at all.

Change-Id: I22054217073690ab391cd19bf9993b1ceada41fd
Reviewed-on: https://boringssl-review.googlesource.com/12231
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-11-12 05:57:08 +00:00
Steven Valdez
5eead165fc Splitting finish_message to finish_message/queue_message.
This is to allow for PSK binders to be munged into the ClientHello as part of
draft 18.

BUG=112

Change-Id: Ic4fd3b70fa45669389b6aaf55e61d5839f296748
Reviewed-on: https://boringssl-review.googlesource.com/12228
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>
2016-11-12 05:01:20 +00:00
David Benjamin
231a475355 Test bad records at all cipher suites.
We have AEAD-level coverage for these, but we should also test this in
the TLS stack, and at maximum size per upstream's CVE-2016-7054.

Change-Id: I1f4ad0356e793d6a3eefdc2d55a9c7e05ea08261
Reviewed-on: https://boringssl-review.googlesource.com/12187
Commit-Queue: David Benjamin <davidben@google.com>
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>
2016-11-10 16:19:51 +00:00
Adam Langley
c5ac2b6c78 Rename X.509 members in |SSL_SESSION| and |CERT|.
This change renames |peer| to |x509_peer| and |cert_chain| to
|x509_chain| in |SSL_SESSION|. It also renames |x509| to |x509_leaf| and
|chain| to |x509_chain| in |CERT|. (All with an eye to maybe making
them lazily initialised in the future).

This a) catches anyone who might be accessing these members directly and
b) makes space for |CRYPTO_BUFFER|-based values to take the unprefixed
names.

Change-Id: I10573304fb7d6f1ea03f9e645f7fc0acdaf71ac2
Reviewed-on: https://boringssl-review.googlesource.com/12162
Reviewed-by: David Benjamin <davidben@google.com>
2016-11-09 20:07:57 +00:00
David Benjamin
a983b4c248 Set SSL_MODE_NO_AUTO_CHAIN by default.
In transition to removing it altogether, set SSL_MODE_NO_AUTO_CHAIN by
default. If we find some consumer was relying on it, this will allow
them to revert locally with SSL_(CTX_)clear_mode, but hopefully this was
just unused.

BUG=42

Change-Id: Iaf70a436a3324ce02e02dfb18213b6715c034ff2
Reviewed-on: https://boringssl-review.googlesource.com/12180
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-11-09 19:31:38 +00:00
David Benjamin
da4789e412 Fix BoGo HelloVerifyRequest version handling.
3c6a1ea674 switched what layer handled
the DTLS version mapping but forgot to correct the HelloVerifyRequest
logic to account for this.

Thanks to Jed Davis for noticing this.

Change-Id: I94ea18fc43a7ba15dd7250bfbcf44dbb3361b3ce
Reviewed-on: https://boringssl-review.googlesource.com/11984
Reviewed-by: David Benjamin <davidben@google.com>
2016-11-09 19:01:59 +00:00
David Benjamin
4e41926774 Move key_block into SSL_HANDSHAKE.
This is already manually released at the end of the handshake. With this
change, it can happen implicitly, and SSL3_STATE shrinks further by
another pointer.

Change-Id: I94b9f2e4df55e8f2aa0b3a8799baa3b9a34d7ac1
Reviewed-on: https://boringssl-review.googlesource.com/12121
Reviewed-by: Adam Langley <agl@google.com>
2016-11-09 17:02:33 +00:00
David Benjamin
ec978dd812 Add corpora for fuzzers with fuzzer mode disabled.
Fuzzer mode explores the handshake, but at the cost of losing coverage
on the record layer. Add a separate build flag and client/server
corpora for this mode.

Note this requires tweaks in consumers' fuzzer build definitions.

BUG=111

Change-Id: I1026dc7301645e165a761068a1daad6eedc9271e
Reviewed-on: https://boringssl-review.googlesource.com/12108
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-11-09 16:53:37 +00:00
David Benjamin
da86360852 Expose SSL_max_seal_overhead.
Change-Id: I0626f926cad033a19eeb977e454f3c9293f01fd6
Reviewed-on: https://boringssl-review.googlesource.com/12106
Reviewed-by: Adam Langley <agl@google.com>
2016-11-09 16:51:46 +00:00
David Benjamin
123db57009 Measure session->timeout from ticket issuance.
The distinction for full handshakes is not meaningful (the timestamp is
currently the start of the handshake), but for renewed sessions, we
currently retain the timestamp of the original issuance.

Instead, when minting or receiving tickets, adjust session->time and
session->timeout so that session->time is the ticket issuance time.

This is still not our final TLS 1.3 behavior (which will need a both
renewable and non-renewable times to honor the server ticket lifetime),
but it gets us closer and unblocks handling ticket_age_add from TLS 1.3
draft 18 and sends the correct NewSessionTicket lifetime.

This fixes the ticket lifetime hint which we emit on the server to
mirror the true ticket lifetime. It also fixes the TLS 1.3 server code
to not set the ticket lifetime hint. There is no need to waste ticket
size with it, it is no longer a "hint" in TLS 1.3, and even in the TLS
1.3 code we didn't fill it in on the server.

Change-Id: I140541f1005a24e53e1b1eaa90996d6dada1c3a1
Reviewed-on: https://boringssl-review.googlesource.com/12105
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>
2016-11-08 23:51:10 +00:00