Commit Graph

135 Commits

Author SHA1 Message Date
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
Steven Valdez
8f36c51f98 Revise version negotiation logic on the C side.
This is in preparation for upcoming experiments which will require
supporting multiple experimental versions of TLS 1.3 with, on the
server, the ability to enable multiple variants at once. This means the
version <-> wire bijection no longer exists, even when limiting to a
single SSL*.  Thus version_to_wire is removed and instead we treat the
wire version as the canonical version value.

There is a mapping from valid wire versions to protocol versions which
describe the high-level handshake protocol in use. This mapping is not
injective, so uses of version_from_wire are rewritten differently.

All the version-munging logic is moved to ssl_versions.c with a master
preference list of all TLS and DTLS versions. The legacy version
negotiation is converted to the new scheme. The version lists and
negotiation are driven by the preference lists and a
ssl_supports_version API.

To simplify the mess around SSL_SESSION and versions, version_from_wire
is now DTLS/TLS-agnostic, with any filtering being done by
ssl_supports_version. This is screwy but allows parsing SSL_SESSIONs to
sanity-check it and reject all bogus versions in SSL_SESSION. This
reduces a mess of error cases.

As part of this, the weird logic where ssl->version is set early when
sending the ClientHello is removed. The one place where we were relying
on this behavior is tweaked to query hs->max_version instead.

Change-Id: Ic91b348481ceba94d9ae06d6781187c11adc15b0
Reviewed-on: https://boringssl-review.googlesource.com/17524
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2017-07-05 19:13:17 +00:00
David Benjamin
68161cb8ba Stash the computed version range in SSL_HANDSHAKE.
Avoid dealing with that function call everywhere.

Change-Id: I7de64b59c8d17e8286c18a6b20c704e8ba8b9ebe
Reviewed-on: https://boringssl-review.googlesource.com/17267
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-06-20 20:13:09 +00:00
David Benjamin
4414874f1f Simplify ssl_private_key_* state machine points.
The original motivation behind the sign/complete split was to avoid
needlessly hashing the input on each pass through the state machine, but
we're payload-based now and, in all cases, the payload is either cheap
to compute or readily available. (Even the hashing worry was probably
unnecessary.)

Tweak ssl_private_key_{sign,decrypt} to automatically call
ssl_private_key_complete as needed and take advantage of this in the
handshake state machines:

- TLS 1.3 signing now computes the payload each pass. The payload is
  small and we're already allocating a comparable-sized buffer each
  iteration to hold the signature. This shouldn't be a big deal.

- TLS 1.2 decryption code still needs two states due to reading the
  message (fixed in new state machine style), but otherwise it just
  performs cheap idempotent tasks again. The PSK code is reshuffled to
  guarantee the callback is not called twice (though this was impossible
  anyway because we don't support RSA_PSK).

- TLS 1.2 CertificateVerify signing is easy as the transcript is readily
  available. The buffer is released very slightly later, but it
  shouldn't matter.

- TLS 1.2 ServerKeyExchange signing required some reshuffling.
  Assembling the ServerKeyExchange parameters is moved to the previous
  state. The signing payload has some randoms prepended. This is cheap
  enough, but a nuisance in C. Pre-prepend the randoms in
  hs->server_params.

With this change, we are *nearly* rid of the A/B => same function
pattern.

BUG=128

Change-Id: Iec4fe0be7cfc88a6de027ba2760fae70794ea810
Reviewed-on: https://boringssl-review.googlesource.com/17265
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-06-20 19:37:05 +00:00
David Benjamin
8d606e361c Clear out f_err pattern from handshake_client.c.
This leaves just handshake_server.c.

BUG=128

Change-Id: I92503404eca0765539c95f107a726fb84c28e8bd
Reviewed-on: https://boringssl-review.googlesource.com/17247
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-06-16 19:21:08 +00:00
Steven Valdez
e831a81518 Adding support for sending early data on the client.
BUG=76

Change-Id: If58a73da38e46549fd55f84a9104e2dfebfda43f
Reviewed-on: https://boringssl-review.googlesource.com/14164
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: 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-06-15 19:34:59 +00:00
David Benjamin
a365138ac6 Factor out the default signature algorithm logic.
This is done in three different places.

Change-Id: I1e55a14c464b1953b3d4de22b50688082ea65129
Reviewed-on: https://boringssl-review.googlesource.com/15306
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-04-24 20:28:33 +00:00
Matthew Braithwaite
7e06de5d2d Really remove DHE ciphersuites from TLS.
This follows up on cedc6f18 by removing support for the
-DBORINGSSL_ENABLE_DHE_TLS compile flag, and the code needed to
support it.

Change-Id: I53b6aa7a0eddd23ace8b770edb2a31b18ba2ce26
Reviewed-on: https://boringssl-review.googlesource.com/14886
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-04-11 23:41:31 +00:00
David Benjamin
d3bca049ee Remove a batch of f_errs.
This function is particularly messy as it had a mix of goto err and
return -1, so if we added a cleanup, we may not have noticed a leak.

Change-Id: I7f363f69857b602c40f8d0f35ce6a83b07051e29
Reviewed-on: https://boringssl-review.googlesource.com/14825
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-04-10 14:11:41 +00:00
David Benjamin
cc17c24852 Stop pretending RSA and ECDSA sigalgs are configurable.
We don't allow consumers to enable and disable RSA and ECDSA signature
algorithms but will filter client-sent cipher suites and server-sent
client certificate types based on this hard-coded list.

This is two less places to update for Ed25519.

BUG=187

Change-Id: I62836b6980acc6d03ee254f0a84e9826668e9e57
Reviewed-on: https://boringssl-review.googlesource.com/14567
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-04-05 22:54:02 +00:00
David Benjamin
a232a7159c Deprecate SSL_PRIVATE_KEY_METHOD type and max_signature_len.
Instead, extract it from the certificate, which is what everyone was
doing anyway. A follow-up change will take advantage of this cleanup to
deduplicate code between signing and verifying for which keys are good
for which signature algorithms.

BUG=188

Change-Id: Ic3f83a6477e8fa53e5e7233f4545f4d2c4b58d01
Reviewed-on: https://boringssl-review.googlesource.com/14565
Reviewed-by: Adam Langley <agl@google.com>
2017-04-05 22:43:20 +00:00
Steven Valdez
681eb6ac2d Adding support for receiving early data on the server.
BUG=76

Change-Id: Ie894ea5d327f88e66b234767de437dbe5c67c41d
Reviewed-on: https://boringssl-review.googlesource.com/12960
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-03-26 19:30:27 +00:00
Steven Valdez
2d85062c4f Add Data-less Zero-RTT support.
This adds support on the server and client to accept data-less early
data. The server will still fail to parse early data with any
contents, so this should remain disabled.

BUG=76

Change-Id: Id85d192d8e0360b8de4b6971511b5e8a0e8012f7
Reviewed-on: https://boringssl-review.googlesource.com/12921
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-03-25 21:00:18 +00:00
David Benjamin
ab1d28e305 Trim x509.h includes.
There are still a few x509.h includes outside ssl_x509.c and ssl_file.c
due to referencing X509_V_* values, but otherwise these includes are no
longer needed.

Change-Id: Ide458e01358dc2ddb6838277d074ad249e599040
Reviewed-on: https://boringssl-review.googlesource.com/14026
Reviewed-by: Adam Langley <agl@google.com>
2017-03-07 23:13:49 +00:00
David Benjamin
a58baaf9e6 Forbid the server certificate from changing on renego.
This allows us to move the code from Chrome into BoringSSL itself.

BUG=126

Change-Id: I04b4f63008a6de0a58dd6c685c78e9edd06deda6
Reviewed-on: https://boringssl-review.googlesource.com/14028
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-03-01 23:26:50 +00:00
Adam Langley
94a62e61aa Hang ssl_auto_chain_if_needed off |X509_METHOD|.
Change-Id: I6609b0e62ea13d14578ccbd945803a27e01ed488
Reviewed-on: https://boringssl-review.googlesource.com/13972
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-02-28 23:07:02 +00:00
Adam Langley
2a3b3439c8 Move X509-related verification code into ssl_x509.c.
Change-Id: I9f94033036550ae85c98cd4e09584972b4441b5d
Reviewed-on: https://boringssl-review.googlesource.com/13971
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-02-28 22:57:34 +00:00
Adam Langley
34b4c829fd Hold CA names as |CRYPTO_BUFFER|s.
This change converts the CA names that are parsed from a server's
CertificateRequest, as well as the CA names that are configured for
sending to clients in the same, to use |CRYPTO_BUFFER|.

The |X509_NAME|-based interfaces are turned into compatibility wrappers.

Change-Id: I95304ecc988ee39320499739a0866c7f8ff5ed98
Reviewed-on: https://boringssl-review.googlesource.com/13585
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-02-28 22:24:31 +00:00
David Benjamin
fc02b59b27 Move tmp.extended_master_secret to SSL_HANDSHAKE.
The two non-trivial changes are:

1. The public API now queries it out of the session. There is a long
   comment over the old field explaining why the state was separate, but
   this predates EMS being forbidden from changing across resumption. It
   is not possible for established_session and the socket to disagree on
   EMS.

2. Since SSL_HANDSHAKE gets reset on each handshake, the check that EMS
   does not change on renego looks different. I've reworked that function a
   bit, but it should have the same effect.

Change-Id: If72e5291f79681381cf4d8ceab267f76618b7c3d
Reviewed-on: https://boringssl-review.googlesource.com/13910
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-02-17 21:36:37 +00:00
David Benjamin
45738dd496 Move new_cipher and new_session to SSL_HANDSHAKE.
This lets us trim another two pointers of per-connection state.

Change-Id: I2145d529bc25b7e24a921d01e82ee99f2c98867c
Reviewed-on: https://boringssl-review.googlesource.com/13804
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-02-17 20:48:31 +00:00
David Benjamin
16b1b1d4d1 Simplify state and info_callback management.
All the business with rewinding hs->state back or skipping states based
on reuse_message or a skip parameter isn't really worth the trouble for
a debugging callback. With SSL_state no longer exposed, we don't have to
worry about breaking things.

BUG=177

Change-Id: I9a0421f01c8b2f24c80a6b3e44de9138ea023f58
Reviewed-on: https://boringssl-review.googlesource.com/13829
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>
2017-02-13 19:52:27 +00:00
David Benjamin
35ed5233c2 Remove an unnecessary state transition.
The split was there out of paranoia that some caller may notice the
change in initial state. Now that SSL_state is neutered, simplify.

BUG=177

Change-Id: I7e2138c2b56821b0c79eec98bb09a82fc28238e8
Reviewed-on: https://boringssl-review.googlesource.com/13828
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>
2017-02-13 19:52:09 +00:00
David Benjamin
77458a436e Avoid transitioning into SSL_ST_OK and back out.
I doubt this matters, but this seems a little odd. In particular, this
avoids info_callback seeing the SSL_ST_OK once we stop switching
hs->state back and forth.

BUG=177

Change-Id: Ied39c0e94c242af9d5d0f26795d6e0f2f0b12406
Reviewed-on: https://boringssl-review.googlesource.com/13827
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>
2017-02-13 19:52:02 +00:00
Adam Langley
46db7af2c9 Remove |X509| things from SSL_SESSION.
|SSL_SESSION_from_bytes| now takes an |SSL_CTX*|, from which it uses the
|X509_METHOD| and buffer pool. This is our API so we can do this.

This also requires adding an |SSL_CTX*| argument to |SSL_SESSION_new|
for the same reason. However, |SSL_SESSION_new| already has very few
callers (and none in third-party code that I can see) so I think we can
get away with this.

Change-Id: I1337cd2bd8cff03d4b9405ea3146b3b59584aa72
Reviewed-on: https://boringssl-review.googlesource.com/13584
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-02-10 19:12:04 +00:00
Steven Valdez
908ac19e8e Moving transcript and PRF functions to SSL_TRANSCRIPT.
Change-Id: I98903df561bbf8c5739f892d2ad5e89ac0eb8e6f
Reviewed-on: https://boringssl-review.googlesource.com/13369
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>
2017-02-10 16:33:42 +00:00
Adam Langley
c68e5b99a7 Establish that the default value of an out-arg for alerts is SSL_AD_DECODE_ERROR.
We already have some cases where the default is DECODE_ERROR and, rather
than have two defaults, just harmonise on that. (INTERNAL_ERROR might
make more sense in some cases, but we don't want to have to remember
what the default is in each case and nobody really cares what the actual
value is anyway.)

Change-Id: I28007898e8d6e7415219145eb9f43ea875028ab2
Reviewed-on: https://boringssl-review.googlesource.com/13720
Reviewed-by: Adam Langley <agl@google.com>
2017-02-09 18:07:30 +00:00
David Benjamin
17b3083373 Use a separate timeout scheme for TLS 1.3.
In TLS 1.2, resumption's benefits are more-or-less subsumed by False
Start. TLS 1.2 resumption lifetime is bounded by how much traffic we are
willing to encrypt without fresh key material, so the lifetime is short.
Renewal uses the same key, so we do not allow it to increase lifetimes.

In TLS 1.3, resumption unlocks 0-RTT. We do not implement psk_ke, so
resumption incorporates fresh key material into both encrypted traffic
(except for early data) and renewed tickets. Thus we are both more
willing to and more interested in longer lifetimes for tickets. Renewal
is also not useless. Thus in TLS 1.3, lifetime is bound separately by
the lifetime of a given secret as a psk_dhe_ke authenticator and the
lifetime of the online signature which authenticated the initial
handshake.

This change maintains two lifetimes on an SSL_SESSION: timeout which is
the renewable lifetime of this ticket, and auth_timeout which is the
non-renewable cliff. It also separates the TLS 1.2 and TLS 1.3 timeouts.
The old session timeout defaults and configuration apply to TLS 1.3, and
we define new ones for TLS 1.3.

Finally, this makes us honor the NewSessionTicket timeout in TLS 1.3.
It's no longer a "hint" in 1.3 and there's probably value in avoiding
known-useless 0-RTT offers.

BUG=120

Change-Id: Iac46d56e5a6a377d8b88b8fa31f492d534cb1b85
Reviewed-on: https://boringssl-review.googlesource.com/13503
Reviewed-by: Adam Langley <agl@google.com>
2017-02-02 19:51:49 +00:00
David Benjamin
f71036e4e3 Remove ssl_hash_message_t from ssl_get_message.
Move to explicit hashing everywhere, matching TLS 1.2 with TLS 1.3. The
ssl_get_message calls between all the handshake states are now all
uniform so, when we're ready, we can rewire the TLS 1.2 state machine to
look like the TLS 1.3 one. (ssl_get_message calls become an
ssl_hs_read_message transition, reuse_message becomes an ssl_hs_ok
transition.)

This avoids some nuisance in processing the ServerHello at the 1.2 / 1.3
transition.

The downside of explicit hashing is we may forget to hash something, but
this will fail to interop with our tests and anyone else, so we should
be able to catch it.

BUG=128

Change-Id: I01393943b14dfaa98eec2a78f62c3a41c29b3a0e
Reviewed-on: https://boringssl-review.googlesource.com/13266
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-01-27 23:23:57 +00:00
David Benjamin
276b7e8127 Move optional message type checks out of ssl_get_message.
This aligns the TLS 1.2 state machine closer with the TLS 1.3 state
machine. This is more work for the handshake, but ultimately the
plan is to take the ssl_get_message call out of the handshake (so it is
just the state machine rather than calling into BIO), so the parameters
need to be folded out as in TLS 1.3.

The WrongMessageType-* family of tests should make sure we don't miss
one of these.

BUG=128

Change-Id: I17a1e6177c52a7540b2bc6b0b3f926ab386c4950
Reviewed-on: https://boringssl-review.googlesource.com/13264
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-01-27 23:15:52 +00:00
David Benjamin
2fe6e227fb Remove mask_a and mask_k from CERT.
This resolves a TODO, trims per-connection memory, and makes more sense.
These masks have nothing to do with certificate configuration.

Change-Id: I783e6158e51f58cce88e3e68dfa0ed965bdc894c
Reviewed-on: https://boringssl-review.googlesource.com/13368
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>
2017-01-27 15:44:17 +00:00
David Benjamin
16315f7cc7 Remove the rest of write_message.
The TLS 1.2 state machine now looks actually much closer to the TLS 1.3
one on the write side. Although the write states still have a BIO-style
return, they don't actually send anything anymore. Only the BIO flush
state does. Reads are still integrated into the states themselves
though, so I haven't made it match TLS 1.3 yet.

BUG=72

Change-Id: I7708162efca13cd335723efa5080718a5f2808ab
Reviewed-on: https://boringssl-review.googlesource.com/13228
Reviewed-by: Adam Langley <agl@google.com>
2017-01-25 23:39:23 +00:00
David Benjamin
daf207a52a Don't use the buffer BIO in TLS.
On the TLS side, we introduce a running buffer of ciphertext. Queuing up
pending data consists of encrypting the record into the buffer. This
effectively reimplements what the buffer BIO was doing previously, but
this resizes to fit the whole flight.

As part of this, rename all the functions to add to the pending flight
to be more uniform. This CL proposes "add_foo" to add to the pending
flight and "flush_flight" to drain it.

We add an add_alert hook for alerts but, for now, only the SSL 3.0
warning alert (sent mid-handshake) uses this mechanism.  Later work will
push this down to the rest of the write path so closure alerts use it
too, as in DTLS. The intended end state is that all the ssl_buffer.c and
wpend_ret logic will only be used for application data and eventually
optionally replaced by the in-place API, while all "incidental" data
will be handled internally.

For now, the two buffers are mutually exclusive. Moving closure alerts
to "incidentals" will change this, but flushing application data early
is tricky due to wpend_ret. (If we call ssl_write_buffer_flush,
do_ssl3_write doesn't realize it still has a wpend_ret to replay.) That
too is all left alone in this change.

To keep the diff down, write_message is retained for now and will be
removed from the state machines in a follow-up change.

BUG=72

Change-Id: Ibce882f5f7196880648f25d5005322ca4055c71d
Reviewed-on: https://boringssl-review.googlesource.com/13224
Reviewed-by: Adam Langley <agl@google.com>
2017-01-25 23:35:47 +00:00
David Benjamin
6d50f475e3 Remove support for RSA premaster logging.
This was replaced by the more general CLIENT_RANDOM scheme that records
the master secret. Support was added in Wireshark 1.8.0, released in
June 2012. At this point, ECDHE is sufficiently widely deployed that
anyone that cares about this feature must have upgraded their Wireshark
by now.

Change-Id: I9b708f245ec8728c1999daf91aca663be7d25661
Reviewed-on: https://boringssl-review.googlesource.com/13263
Reviewed-by: David Benjamin <davidben@google.com>
2017-01-25 16:48:35 +00:00
David Benjamin
8d5f9da2e3 Abstract away BIO_flush calls in the handshake.
This is the first part to removing the buffer BIO. The eventual end
state is the SSL_PROTOCOL_METHOD is responsible for maintaining one
flight's worth of messages. In TLS, it will just be a buffer containing
the flight's ciphertext. In DTLS, it's the existing structure for
retransmit purposes. There will be hooks:

- add_message (synchronous)
- add_change_cipher_spec (synchronous)
- add_warning_alert (synchronous; needed until we lose SSLv3 client auth
  and TLS 1.3 draft 18; draft 19 will switch end_of_early_data to a
  handshake message)
- write_flight (BIO; flush_flight will be renamed to this)

This also preserves the exact return value of BIO_flush. Eventually all
the BIO_write calls will be hidden behind BIO_flush to, to be consistent
with other BIO-based calls, preserve the return value.

BUG=72

Change-Id: I74cd23759a17356aab3bb475a8ea42bd2cd115c9
Reviewed-on: https://boringssl-review.googlesource.com/13222
Reviewed-by: Adam Langley <agl@google.com>
2017-01-24 16:16:02 +00:00
David Benjamin
a660e7ab67 Don't clear cert_request in ssl3_send_client_certificate.
Instead, add ssl_has_certificate to the ssl3_send_cert_verify check. If
writing the empty Certificate does not complete synchronously (it almost
always does due to the buffer BIO), but if the buffer boundary is at
exactly the wrong place, write_message will need a retry but, having
cleared cert_request, we never re-enter ssl3_send_client_certificate.

This will later be moot when we've gotten rid of the buffer BIO, but
this is cleaner anyway and is closer to the TLS 1.3 code.

With this change, blindly taking away the BIO buffer in TLS (which is
not what we want since we want the entire flight in one write but is a
nice sanity check), only the SSL 3.0 no client certificate tests fail.
They too rely on some writes completing synchronously due to SSL 3.0
sending a warning alert. There is a similar bug when
tlsext_servername_callback returns SSL_TLSEXT_ERR_ALERT_WARNING.

Those will be resolved after reworking the write path since it's a bit
of a mess.

Change-Id: I56b4df6163cae1df263cf36f0d93046d0375a5ac
Reviewed-on: https://boringssl-review.googlesource.com/13052
Reviewed-by: David Benjamin <davidben@google.com>
2017-01-04 04:50:00 +00:00
David Benjamin
650aa1c80a Clean up certificate auto-chaining.
Rather than doing it right before outputing, treat this as a part of the
pipeline to finalize the certificate chain, and run it right after
cert_cb to modify the certificate configuration itself. This means
nothing else in the stack needs to worry about this case existing.

It also makes it easy to support in both TLS 1.2 and TLS 1.3.

Change-Id: I6a088297a54449f1f5f5bb8b5385caa4e8665eb6
Reviewed-on: https://boringssl-review.googlesource.com/12966
Reviewed-by: Adam Langley <agl@google.com>
2017-01-04 01:36:26 +00:00
David Benjamin
17cf2cb1d2 Work around language and compiler bug in memcpy, etc.
Most C standard library functions are undefined if passed NULL, even
when the corresponding length is zero. This gives them (and, in turn,
all functions which call them) surprising behavior on empty arrays.
Some compilers will miscompile code due to this rule. See also
https://www.imperialviolet.org/2016/06/26/nonnull.html

Add OPENSSL_memcpy, etc., wrappers which avoid this problem.

BUG=23

Change-Id: I95f42b23e92945af0e681264fffaf578e7f8465e
Reviewed-on: https://boringssl-review.googlesource.com/12928
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-12-21 20:34:47 +00:00
Adam Langley
0567220b8b Don't use X.509 functions to check ECDSA keyUsage.
This removes another dependency on the crypto/x509 code.

Change-Id: Ia72da4d47192954c2b9a32cf4bcfd7498213c0c7
Reviewed-on: https://boringssl-review.googlesource.com/12709
Reviewed-by: Adam Langley <agl@google.com>
2016-12-14 17:51:03 +00:00
David Benjamin
f1050fd79a Preserve the peer signature algorithm across resumes.
So we can report it cleanly out of DevTools, it should behave like
SSL_get_curve_id and be reported on resumption too.

BUG=chromium:658905

Change-Id: I0402e540a1e722e09eaebadf7fb4785d8880c389
Reviewed-on: https://boringssl-review.googlesource.com/12694
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-14 01:12:40 +00:00
David Benjamin
4882a6c50b Replace key_exchange_info with group_id.
The only accessor for this field is the group/curve ID. Switch to only
storing that so no cipher checks are needed to interpret it. Instead,
ignore older values at parse time.

Change-Id: Id0946d4ac9e7482c69e64cc368a9d0cddf328bd3
Reviewed-on: https://boringssl-review.googlesource.com/12693
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-14 01:07:20 +00:00
Adam Langley
d515722d22 Don't depend on the X509 code for getting public keys.
This change removes the use of |X509_get_pubkey| from the TLS <= 1.2
code. That function is replaced with a shallow parse of the certificate
to extract the public key instead.

Change-Id: I8938c6c5a01b32038c6b6fa58eb065e5b44ca6d2
Reviewed-on: https://boringssl-review.googlesource.com/12707
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-12-13 21:27:31 +00:00
Adam Langley
d519bf6be0 Add |SSL_CTX_set0_buffer_pool|.
This currently only works for certificates parsed from the network, but
if making several connections that share certificates, some KB of memory
might be saved.

BUG=chromium:671420

Change-Id: I1c7a71d84e1976138641f71830aafff87f795f9d
Reviewed-on: https://boringssl-review.googlesource.com/12706
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-13 18:54:22 +00:00
Adam Langley
68e7124ddf Hold certificates in an SSL_SESSION as CRYPTO_BUFFERSs as well.
This change adds a STACK_OF(CRYPTO_BUFFER) to an SSL_SESSION which
contains the raw form of the received certificates. The X509-based
members still exist, but their |enc| buffer will alias the
CRYPTO_BUFFERs.

(This is a second attempt at
https://boringssl-review.googlesource.com/#/c/12163/.)

BUG=chromium:671420

Change-Id: I508a8a46cab89a5a3fcc0c1224185d63e3d59cb8
Reviewed-on: https://boringssl-review.googlesource.com/12705
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-12-13 17:28:25 +00:00
Adam Langley
364f7a6d21 Push the difference in chain semantics to the edge.
OpenSSL includes a leaf certificate in a certificate chain when it's a
client, but doesn't when it's a server. This is also reflected in the
serialisation of sessions.

This change makes the internal semantics consistent: the leaf is always
included in the chain in memory, and never duplicated when serialised.
To maintain the same API, SSL_get_peer_cert_chain will construct a copy
of the chain without the leaf if needed.

Since the serialised format of a client session has changed, an
|is_server| boolean is added to the ASN.1 that defaults to true. Thus
any old client sessions will be parsed as server sessions and (silently)
discarded by a client.

Change-Id: Ibcf72bc8a130cedb423bc0fd3417868e0af3ca3e
Reviewed-on: https://boringssl-review.googlesource.com/12704
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-12-13 17:18:53 +00:00
David Benjamin
cb0c29ff75 Move state and next_state to SSL_HANDSHAKE.
state is now initialized to SSL_ST_INIT in SSL_HANDSHAKE. If there is no
handshake present, we report SSL_ST_OK. This saves 8 bytes of
per-connection post-handshake memory.

Change-Id: Idb3f7031045caed005bd7712bc8c4b42c81a1d04
Reviewed-on: https://boringssl-review.googlesource.com/12697
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-12 22:09:01 +00:00
David Benjamin
5edfc8cc17 Emulate the client_cert_cb with cert_cb.
This avoids needing a extra state around client certificates to avoid
calling the callbacks twice. This does, however, come with a behavior
change: configuring both callbacks won't work. No consumer does this.

(Except bssl_shim which needed slight tweaks.)

Change-Id: Ia5426ed2620e40eecdcf352216c4a46764e31a9a
Reviewed-on: https://boringssl-review.googlesource.com/12690
Reviewed-by: Adam Langley <agl@google.com>
2016-12-12 21:58:24 +00:00
Adam Langley
c0fc7a1385 Revert "Add |SSL_CTX_set0_buffer_pool|." and "Hold certificates in an SSL_SESSION as CRYPTO_BUFFERSs as well."
This reverts commits 5a6e616961 and
e8509090cf. I'm going to unify how the
chains are kept in memory between client and server first otherwise the
mess just keeps growing.

Change-Id: I76df0d94c9053b2454821d22a3c97951b6419831
Reviewed-on: https://boringssl-review.googlesource.com/12701
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-09 23:31:12 +00:00
Matthew Braithwaite
651aaefb44 Remove CECPQ1 (experimental post-quantum key agreement).
Change-Id: Ie947ab176d10feb709c6e135d5241c6cf605b8e8
Reviewed-on: https://boringssl-review.googlesource.com/12700
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-09 19:16:56 +00:00
Adam Langley
e8509090cf Hold certificates in an SSL_SESSION as CRYPTO_BUFFERSs as well.
This change adds a STACK_OF(CRYPTO_BUFFER) to an SSL_SESSION which
contains the raw form of the received certificates. The X509-based
members still exist, but their |enc| buffer will alias the
CRYPTO_BUFFERs.

The serialisation format of SSL_SESSIONs is also changed, in a backwards
compatible way. Previously, some sessions would duplicate the leaf
certificate in the certificate chain. These sessions can still be read,
but will be written in a way incompatible with older versions of the
code. This should be fine because the situation where multiple versions
exchange serialised sessions is at the server, and the server doesn't
duplicate the leaf certifiate in the chain anyway.

Change-Id: Id3b75d24f1745795315cb7f8089a4ee4263fa938
Reviewed-on: https://boringssl-review.googlesource.com/12163
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-12-09 18:12:40 +00:00
David Benjamin
f04c2e9878 Move client_version into SSL_HANDSHAKE.
There is no need to retain it beyond this point.

Change-Id: Ib5722ab30fc013380198b1582d1240f0fe0aa770
Reviewed-on: https://boringssl-review.googlesource.com/12620
Reviewed-by: Adam Langley <agl@google.com>
2016-12-08 16:39:52 +00:00