Commit Graph

80 Commits

Author SHA1 Message Date
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
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
d94682dce5 Remove ex_data's dup hook.
The only place it is used is EC_KEY_{dup,copy} and no one calls that
function on an EC_KEY with ex_data. This aligns with functions like
RSAPublicKey_dup which do not copy ex_data. The logic is also somewhat
subtle in the face of malloc errors (upstream's PR 3323).

In fact, we'd even changed the function pointer signature from upstream,
so BoringSSL-only code is needed to pass this pointer in anyway. (I
haven't switched it to CRYPTO_EX_unused because there are some callers
which pass in an implementation anyway.)

Note, in upstream, the dup hook is also used for SSL_SESSIONs when those
are duplicated (for TLS 1.2 ticket renewal or TLS 1.3 resumption). Our
interpretation is that callers should treat those SSL_SESSIONs
equivalently to newly-established ones. This avoids every consumer
providing a dup hook and simplifies the interface.

(I've gone ahead and removed the TODO(fork). I don't think we'll be able
to change this API. Maybe introduce a new one, but it may not be worth
it? Then again, this API is atrocious... I've never seen anyone use argl
and argp even.)

BUG=21

Change-Id: I6c9e9d5a02347cb229d4c084c1e85125bd741d2b
Reviewed-on: https://boringssl-review.googlesource.com/16344
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-05-23 22:43:59 +00:00
Adam Langley
4c341d0299 Support asynchronous ticket decryption with TLS 1.0–1.2.
This change adds support for setting an |SSL_TICKET_AEAD_METHOD| which
allows a caller to control ticket encryption and decryption to a greater
extent than previously possible and also permits asynchronous ticket
decryption.

This change only includes partial support: TLS 1.3 work remains to be
done.

Change-Id: Ia2e10ebb3257e1a119630c463b6bf389cf20ef18
Reviewed-on: https://boringssl-review.googlesource.com/14144
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-03-11 00:04:18 +00:00
David Benjamin
be49706c42 Rename initial_ctx to session_ctx.
This makes its purpose clearer. That the session cache is based on the
initial SSL_CTX is confusing (it's a remnant of OpenSSL's backwards
session resumption ordering), but we're probably stuck with it.
Relatedly, document SSL_set_SSL_CTX better.

Change-Id: I2832efc63f6c959c5424271b365825afc7eec5e4
Reviewed-on: https://boringssl-review.googlesource.com/14204
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-03-10 22:50:46 +00:00
David Benjamin
ad8f5e1de9 Don't use long for timestamps.
This is the first part to fixing the SSL stack to be 2038-clean.
Internal structures and functions are switched to use OPENSSL_timeval
which, unlike timeval and long, are suitable for timestamps on all
platforms.

It is generally accepted that the year is now sometime after 1970, so
use uint64_t for the timestamps to avoid worrying about serializing
negative numbers in SSL_SESSION.

A follow-up change will fix SSL_CTX_set_current_time_cb to use
OPENSSL_timeval. This will require some coordinating with WebRTC.
DTLSv1_get_timeout is left alone for compatibility and because it stores
time remaining rather than an absolute time.

BUG=155

Change-Id: I1a5054813300874b6f29e348f9cd8ca80f6b9729
Reviewed-on: https://boringssl-review.googlesource.com/13944
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 21:58:38 +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
5960a90964 Move sid_ctx from SSL/SSL_CTX to CERT.
This reduces us from seven different configuration patterns to six (see
comment #2 of linked bug). I do not believe there is any behavior change
here as SSL_set_SSL_CTX already manually copied the field. It now gives
us a nice invariant: SSL_set_SSL_CTX overrides all and only the
dual-SSL/SSL_CTX options hanging off of CERT.

BUG=123

Change-Id: I1ae06b791fb869917a6503cee41afb2d9be53d89
Reviewed-on: https://boringssl-review.googlesource.com/13865
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-16 18:59:24 +00:00
Steven Valdez
27a9e6ae1b Adding ALPN to session.
0-RTT requires matching the selected ALPN parameters against those in
the session. Stash the ALPN value in the session in TLS 1.3, so we can
recover it.

BUG=76

Change-Id: I8668b287651ae4deb0bf540c0885a02d189adee0
Reviewed-on: https://boringssl-review.googlesource.com/13845
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-16 17:33:23 +00:00
David Benjamin
9e766d7532 Unexport the handshake's internal state.
Code which manages to constrain itself on this will limit our ability to
rework the handshake. I believe, at this point, we only need to expose
one bit of information (there's some code that compares SSL_state to
SSL_ST_OK), if even that.

BUG=177

Change-Id: Ie1c43006737db0b974811f1819755c629ae68e7b
Reviewed-on: https://boringssl-review.googlesource.com/13826
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>
2017-02-13 19:44:30 +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
Alessandro Ghedini
33fe4a0d14 Remove support for setting per-connection default session timeout
As previously discussed, it turns out we don't actually need this, so
there's no point in keeping it.

Change-Id: If549c917b6bd818cd36948e37cb7839c8d122b1a
Reviewed-on: https://boringssl-review.googlesource.com/13641
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-07 17:46:25 +00:00
Steven Valdez
2f82a0e51b Don't stash tlsext_hostname in ssl_get_new_session.
ssl_get_new_session would stash a copy of the configured hostname
into the SSL_SESSION on the server. Servers have no reason to
configuring that anyway, but, if one did, we'd leak when filling in
the client-supplied SNI later.

Remove this code and guard against this by remembering to OPENSSL_free
when overwriting that field (although it should always be NULL).

Reported-By: Robert Swiecki <swiecki@google.com>
Change-Id: Ib901b5f82e5cf818060ef47a9585363e05dd9932
Reviewed-on: https://boringssl-review.googlesource.com/13631
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-06 18:42:53 +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
0b1bb12ce8 Push the SSL_CTX session_timeout zero logic up.
This special-case is almost unexposed (the timeout is initialized to the
default) except if the caller calls SSL_CTX_set_timeout(0). Preserve
that behavior by mapping 0 to SSL_DEFAULT_SESSION_TIMEOUT in
SSL_CTX_set_timeout but simplify the internal state.

Change-Id: Ice03a519c25284b925f1e0cf485f2d8c54dc5038
Reviewed-on: https://boringssl-review.googlesource.com/13502
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-02-02 17:52:07 +00:00
David Benjamin
b5c58db9ff TLS 1.3 sessions should not be added to the server session cache.
Fix this and add a test. Otherwise enabling TLS 1.3 will cause a server
to blow through its session cache.

Change-Id: I67edbc468faedfd94a6c30cf842af085a6543b50
Reviewed-on: https://boringssl-review.googlesource.com/13501
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-01 23:07:55 +00:00
Steven Valdez
08b65f4e31 Enabling 0-RTT on new Session Tickets.
This adds support for setting 0-RTT mode on tickets minted by
BoringSSL, allowing for testing of the initial handshake knowledge.

BUG=76

Change-Id: Ic199842c03b5401ef122a537fdb7ed9e9a5c635a
Reviewed-on: https://boringssl-review.googlesource.com/12740
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-06 16:24:43 +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
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
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
David Benjamin
5888946777 Remove SSL_CTX_get_client_cert_cb.
This is never used. Removing it allows us to implement the old callback
using the new one.

Change-Id: I4be70cc16e609ce79b51836c19fec565c67ff3d6
Reviewed-on: https://boringssl-review.googlesource.com/12689
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-12-12 21:43:32 +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
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
30c4c30d4a Revise some integer sizes.
size_t at the public API, uint8_t on the SSL structs since everything
fits in there comfortably.

Change-Id: I837c3b21e04e03dfb957c1a3e6770300d0b49c0b
Reviewed-on: https://boringssl-review.googlesource.com/12638
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-08 16:48:44 +00:00
David Benjamin
731058ec8e Typedef ssl_early_callback_ctx to SSL_CLIENT_HELLO.
It's our ClientHello representation. May as well name it accordingly.
Also switch away from calling the variable name ctx as that conflicts
with SSL_CTX.

Change-Id: Iec0e597af37137270339e9754c6e08116198899e
Reviewed-on: https://boringssl-review.googlesource.com/12581
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-07 19:52:11 +00:00
David Benjamin
f3c8f8d19d Pass explicit parameters elsewhere.
The remaining direct accesses are in functions which expect to be called
in and out of the handshake. Accordingly, they are NULL-checked.

Change-Id: I07a7de6bdca7b6f8d09e22da11b8863ebf41389a
Reviewed-on: https://boringssl-review.googlesource.com/12343
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-06 19:54:58 +00:00
Alessandro Ghedini
bf48364a8c Support setting per-connection default session lifetime value
Due to recent changes, changing the SSL session timeout from cert_cb is
not possible anymore since the new |SSL_SESSION| is initialized *after*
cert_cb is run. The alternative would be using |SSL_CTX_set_timeout| but
the specific |SSL_CTX| could be shared by multiple |SSL|s.

Setting a value on a per-connection basis is useful in case timeouts
need to be calculated dynamically based on specific certificate/domain
information that would be retrieved from inside cert_cb (or other
callbacks).

It would also be possible to set the value to 0 to prevent session
resumption, which is not otherwise doable in the handshake callbacks.

Change-Id: I730a528c647f83f7f77f59b5b21d7e060e4c9843
Reviewed-on: https://boringssl-review.googlesource.com/12440
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 21:01: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
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
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
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
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
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
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
David Benjamin
8e816eb7b6 Treat sessions as expired on the boundary second.
TLS 1.3 clarifies that a ticket lifetime of zero means the session is
unusable. We don't currently pay attention to that field (to be fixed in
later changes) but, in preparation for this, switch the >= to a >.

Change-Id: I0e67a0d97bc8def04914f121e84d3e7a2d640d2c
Reviewed-on: https://boringssl-review.googlesource.com/12102
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-11-08 23:00:04 +00:00
David Benjamin
1b22f85a56 Reject tickets from the future.
This shouldn't happen, but it is good to check to avoid the potential
underflow in ssl_session_is_time_valid.

This required tweaking the mock clock in bssl_shim to stop going back in
time.

Change-Id: Id3ab8755139e989190d0b53d4bf90fe1ac203022
Reviewed-on: https://boringssl-review.googlesource.com/11841
Reviewed-by: David Benjamin <davidben@google.com>
2016-10-27 22:32:19 +00:00
Steven Valdez
5b9860827f Updating NewSessionTicket message and updating PSK to Draft 15.
BUG=77

Change-Id: Id8c45e98c4c22cdd437cbba1e9375239e123b261
Reviewed-on: https://boringssl-review.googlesource.com/10763
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-10-06 14:36:12 +00:00
David Benjamin
fbc45d7228 No-op ticket encryption in fuzzer mode.
This allows the fuzzer to discover server-side resumption paths by
simply supplying what we'd like the ticket to decrypt to in the clear.
We also have a natural way to get transcripts out of runner. We record
the runner-side transcripts, so all resumption handshakes will replay
the shim-created unencrypted tickets.

BUG=104

Change-Id: Icf9cbf4af520077d38e2c8c2766b6f8bfa3c9ab5
Reviewed-on: https://boringssl-review.googlesource.com/11224
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>
2016-09-22 21:26:23 +00:00
David Benjamin
01a905717c Fix remaining non-determinism in fuzzer transcripts.
Both the C and Go code were sampling the real clock. With this, two
successive iterations of runner transcripts give the same output.

Change-Id: I4d9e219e863881bf518c5ac199dce938a49cdfaa
Reviewed-on: https://boringssl-review.googlesource.com/11222
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>
2016-09-22 21:14:00 +00:00
David Benjamin
4d0be24319 Only allow SSL_set_session before the handshake.
Otherwise things break horribly. Explicitly abort to help catch bugs.

Change-Id: I66e2bf8808199b3331b3adde68d73758a601eb8c
Reviewed-on: https://boringssl-review.googlesource.com/10761
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-09-12 19:16:46 +00:00
Adam Langley
81f433540e Don't crash when a session callback returns NULL.
4aa154e08f changed the code to assume that
a session callback will zero the |copy| out-arg before returning NULL.
In practice this doesn't always happen and we should be robust against
it.

Change-Id: I0fd14969df836e0fa4f68ded8648fea8094ff9d7
Reviewed-on: https://boringssl-review.googlesource.com/10640
Reviewed-by: 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>
2016-08-26 17:42:41 +00:00
David Benjamin
29e5cf7209 Remove SSL_SESSION_get_key_exchange_info.
Chromium has switched to better APIs.

Change-Id: I26209b3a03c6a0db1ddce2f1fc99c8750cf6e56a
Reviewed-on: https://boringssl-review.googlesource.com/10501
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-08-19 20:05:06 +00:00
David Benjamin
7aa31d68fc Remove ssl->verify_result.
Having two copies of this is confusing. This field is inherently tied to
the certificate chain, which lives on SSL_SESSION, so this should live
there too. This also wasn't getting reset correctly on SSL_clear, but
this is now resolved.

Change-Id: I22b1734a93320bb0bf0dc31faa74d77a8e1de906
Reviewed-on: https://boringssl-review.googlesource.com/10283
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-08-19 18:29:43 +00:00
Steven Valdez
4aa154e08f Adding code to send session as PSK Identity.
BUG=75

Change-Id: Ied864cfccbc0e68d71c55c5ab563da27b7253463
Reviewed-on: https://boringssl-review.googlesource.com/9043
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-08-16 20:35:32 +00:00
David Benjamin
96a16cd10e Finish aligning up_ref functions with OpenSSL 1.1.0.
All external callers should be resolved now.

BUG=89

Change-Id: I6055450e8202c59cca49e4a824be3ec11c32a15a
Reviewed-on: https://boringssl-review.googlesource.com/10285
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-08-11 16:51:52 +00:00