Commit Graph

60 Commits

Author SHA1 Message Date
David Benjamin
e7c95d91f8 Run TLS 1.3 tests at all variants and fix bugs.
We were only running a random subset of TLS 1.3 tests with variants and
let a lot of bugs through as a result.

- HelloRetryRequest-EmptyCookie wasn't actually testing what we were
  trying to test.

- The second HelloRetryRequest detection needs tweaks in draft-22.

- The empty HelloRetryRequest logic can't be based on non-empty
  extensions in draft-22.

- We weren't sending ChangeCipherSpec correctly in HRR or testing it
  right.

- Rework how runner reads ChangeCipherSpec by setting a flag which
  affects the next readRecord. This cuts down a lot of cases and works
  correctly if the client didn't send early data. (In that case, we
  don't flush CCS until EndOfEarlyData and runner deadlocks waiting for
  the ChangeCipherSpec to arrive.)

Change-Id: I559c96ea3a8b350067e391941231713c6edb2f78
Reviewed-on: https://boringssl-review.googlesource.com/23125
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-11-20 18:19:18 +00:00
Steven Valdez
f4ecc84644 Prevent both early data and custom extensions from being accepted.
This loosens the earlier restriction to match Channel ID. Both may be
configured and offered, but the server is obligated to select only one
of them. This aligns with the current tokbind + 0-RTT draft where the
combination is signaled by a separate extension.

Bug: 183
Change-Id: I786102a679999705d399f0091f76da236be091c2
Reviewed-on: https://boringssl-review.googlesource.com/19124
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
2017-08-14 20:15:54 +00:00
David Benjamin
5aaaa98f8c Detect WatchGuard's TLS 1.3 interference failure mode.
WatchGuard's bug is very distinctive. Report a dedicated error code out
of BoringSSL so we can better track this.

Bug: chromium:733223
Change-Id: Ia42abd8654e7987b1d43c63a4f454f35f6aa873b
Reviewed-on: https://boringssl-review.googlesource.com/17328
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-06-22 19:49:23 +00:00
Alessandro Ghedini
de254b4c4e Enforce max_early_data_size on the server.
BUG=76

Change-Id: I8b754ba17b3e0beee425929e4b53785b2e95f0ae
Reviewed-on: https://boringssl-review.googlesource.com/15164
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-19 17:21:01 +00:00
David Benjamin
3cfeb9522b Disable SSLv3 by default.
As a precursor to removing the code entirely later, disable the protocol
by default. Callers must use SSL_CTX_set_min_version to enable it.

This change also makes SSLv3_method *not* enable SSL 3.0. Normally
version-specific methods set the minimum and maximum version to their
version. SSLv3_method leaves the minimum at the default, so we will
treat it as all versions disabled. To help debugging, the error code is
switched from WRONG_SSL_VERSION to a new NO_SUPPORTED_VERSIONS_ENABLED.

This also defines OPENSSL_NO_SSL3 and OPENSSL_NO_SSL3_METHOD to kick in
any no-ssl3 build paths in consumers which should provide a convenient
hook for any upstreaming changes that may be needed. (OPENSSL_NO_SSL3
existed in older versions of OpenSSL, so in principle one may encounter
an OpenSSL with the same settings.)

Change-Id: I96a8f2f568eb77b2537b3a774b2f7108bd67dd0c
Reviewed-on: https://boringssl-review.googlesource.com/14031
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 16:38:16 +00:00
Steven Valdez
2a0707210a Prevent Channel ID and Custom Extensions on 0-RTT.
Channel ID is incompatible with 0-RTT, so we gracefully decline 0-RTT
as a server and forbid their combination as a client. We'll keep this
logic around until Channel ID is removed.

Channel ID will be replaced by tokbind which currently uses custom
extensions. Those will need additional logic to work with 0-RTT.
This is not implemented yet so, for now, fail if both are ever
configured together at all. A later change will allow the two to
combine.

BUG=183

Change-Id: I46c5ba883ccd47930349691fb08074a1fab13d5f
Reviewed-on: https://boringssl-review.googlesource.com/14370
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 18:13:57 +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
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
Matthew Braithwaite
6ad20dc912 Move error-on-empty-cipherlist into ssl_create_cipher_list().
It's more consistent to have the helper function do the check that
its every caller already performs.  This removes the error code
SSL_R_LIBRARY_HAS_NO_CIPHERS in favor of SSL_R_NO_CIPHER_MATCH.

Change-Id: I522239770dcb881d33d54616af386142ae41b29f
Reviewed-on: https://boringssl-review.googlesource.com/13964
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-09 17:31:45 +00:00
Adam Langley
d04ca95356 Add |SSL[_CTX]_set_chain_and_key|.
This allows a caller to configure a serving chain without dealing with
crypto/x509.

Change-Id: Ib42bb2ab9227d32071cf13ab07f92d029643a9a6
Reviewed-on: https://boringssl-review.googlesource.com/14126
Commit-Queue: Adam Langley <alangley@gmail.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-03-08 19:11:57 +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
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
David Benjamin
aedf303cc2 Parse the entire PSK extension.
Although we ignore all but the first identity, keep clients honest by
parsing the whole thing. Also explicitly check that the binder and
identity counts match.

Change-Id: Ib9c4caae18398360f3b80f8db1b22d4549bd5746
Reviewed-on: https://boringssl-review.googlesource.com/12469
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:53:13 +00:00
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
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
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
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
1db9e1bc7a Add the certificate_required alert.
This is part of TLS 1.3 draft 16 but isn't much of a wire format change,
so go ahead and add it now. When rolling into Chromium, we'll want to
add an entry to the error mapping.

Change-Id: I8fd7f461dca83b725a31ae19ef96c890d603ce53
Reviewed-on: https://boringssl-review.googlesource.com/11563
Reviewed-by: David Benjamin <davidben@google.com>
2016-10-10 15:48:06 +00:00
Steven Valdez
803c77a681 Update crypto negotation to draft 15.
BUG=77

Change-Id: If568412655aae240b072c29d763a5b17bb5ca3f7
Reviewed-on: https://boringssl-review.googlesource.com/10840
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
2016-10-06 14:37:09 +00:00
David Benjamin
c8b6b4fe4a Only predict X25519 in TLS 1.3.
We'd previously been assuming we'd want to predict P-256 and X25519 but,
on reflection, that's nonsense. Although, today, P-256 is widespread and
X25519 is less so, that's not the right question to ask. Those servers
are all 1.2.

The right question is whether we believe enough servers will get to TLS
1.3 before X25519 to justify wasting 64 bytes on all other connections.
Given that OpenSSL has already shipped X25519 and Microsoft was doing
interop testing on X25519 around when we were shipping it, I think the
answer is no.

Moreover, if we are wrong, it will be easier to go from predicting one
group to two rather than the inverse (provided we send a fake one with
GREASE). I anticipate prediction-miss HelloRetryRequest logic across the
TLS/TCP ecosystem will be largely untested (no one wants to pay an RTT),
so taking a group out of the predicted set will likely be a risky
operation.

Only predicting one group also makes things a bit simpler. I haven't
done this here, but we'll be able to fold the 1.2 and 1.3 ecdh_ctx's
together, even.

Change-Id: Ie7e42d3105aca48eb9d97e2e05a16c5379aa66a3
Reviewed-on: https://boringssl-review.googlesource.com/10960
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-09-21 21:18:34 +00:00
David Benjamin
7e1f984a7c Fix some bugs in TLS 1.3 server key_share code.
Found by libFuzzer and then one more mistake caught by valgrind. Add a
test for this case.

Change-Id: I92773bc1231bafe5fc069e8568d93ac0df4c8acb
Reviewed-on: https://boringssl-review.googlesource.com/11129
Reviewed-by: David Benjamin <davidben@google.com>
2016-09-21 20:40:10 +00:00
David Benjamin
163c95691a Forbid EMS from changing during renegotation.
Changing parameters on renegotiation makes all our APIs confusing. This
one has no reason to change, so lock it down. In particular, our
preference to forbid Token Binding + renego may be overridden at the
IETF, even though it's insane. Loosening it will be a bit less of a
headache if EMS can't change.

https://www.ietf.org/mail-archive/web/unbearable/current/msg00690.html
claims that this is already in the specification and enforced by NSS. I
can't find anything to this effect in the specification. It just says
the client MUST disable renegotiation when EMS is missing, which is
wishful thinking. At a glance, NSS doesn't seem to check, though I could
be misunderstanding the code.

Nonetheless, locking this down is a good idea anyway. Accurate or not,
take the email as an implicit endorsement of this from Mozilla.

Change-Id: I236b05991d28bed199763dcf2f47bbfb9d0322d7
Reviewed-on: https://boringssl-review.googlesource.com/10721
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-30 15:43:35 +00:00
David Benjamin
311c2579f7 Declare SSL_R_BLOCK_CIPHER_PAD_IS_WRONG and SSL_R_NO_CIPHERS_SPECIFIED.
nginx consumes these error codes without #ifdefs. Continue to define
them for compatibility, even though we never emit them.

BUG=95

Change-Id: I1e991987ce25fc4952cc85b98ffa050a8beab92e
Reviewed-on: https://boringssl-review.googlesource.com/10446
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-24 01:15:19 +00:00
Steven Valdez
32635b828f Add limit for consecutive KeyUpdate messages.
Change-Id: I2e1ee319bb9852b9c686f2f297c470db54f72279
Reviewed-on: https://boringssl-review.googlesource.com/10370
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-18 23:43:12 +00:00
David Benjamin
3e51757de2 Enforce the server ALPN protocol was advertised.
The server should not be allowed select a protocol that wasn't
advertised. Callers tend to not really notice and act as if some default
were chosen which is unlikely to work very well.

Change-Id: Ib6388db72f05386f854d275bab762ca79e8174e6
Reviewed-on: https://boringssl-review.googlesource.com/10284
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:46:34 +00:00
Steven Valdez
143e8b3fd9 Add TLS 1.3 1-RTT.
This adds the machinery for doing TLS 1.3 1RTT.

Change-Id: I736921ffe9dc6f6e64a08a836df6bb166d20f504
Reviewed-on: https://boringssl-review.googlesource.com/8720
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-07-18 09:54:46 +00:00
David Benjamin
61672818ef Check for buffered handshake messages on cipher change in DTLS.
This is the equivalent of FragmentAcrossChangeCipherSuite for DTLS. It
is possible for us to, while receiving pre-CCS handshake messages, to
buffer up a message with sequence number meant for a post-CCS Finished.
When we then get to the new epoch and attempt to read the Finished, we
will process the buffered Finished although it was sent with the wrong
encryption.

Move ssl_set_{read,write}_state to SSL_PROTOCOL_METHOD hooks as this is
a property of the transport. Notably, read_state may fail. In DTLS
check the handshake buffer size. We could place this check in
read_change_cipher_spec, but TLS 1.3 has no ChangeCipherSpec message, so
we will need to implement this at the cipher change point anyway. (For
now, there is only an assert on the TLS side. This will be replaced with
a proper check in TLS 1.3.)

Change-Id: Ia52b0b81e7db53e9ed2d4f6d334a1cce13e93297
Reviewed-on: https://boringssl-review.googlesource.com/8790
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-07-16 08:25:02 +00:00
David Benjamin
1f61f0d7c3 Implement TLS 1.3's downgrade signal.
For now, skip the 1.2 -> 1.1 signal since that will affect shipping
code. We may as well enable it too, but wait until things have settled
down. This implements the version in draft-14 since draft-13's isn't
backwards-compatible.

Change-Id: I46be43e6f4c5203eb4ae006d1c6a2fe7d7a949ec
Reviewed-on: https://boringssl-review.googlesource.com/8724
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-12 19:17:43 +00:00
David Benjamin
ea9a0d5313 Refine SHA-1 default in signature algorithm negotiation.
Rather than blindly select SHA-1 if we can't find a matching one, act as
if the peer advertised rsa_pkcs1_sha1 and ecdsa_sha1. This means that we
will fail the handshake if no common algorithm may be found.

This is done in preparation for removing the SHA-1 default in TLS 1.3.

Change-Id: I3584947909d3d6988b940f9404044cace265b20d
Reviewed-on: https://boringssl-review.googlesource.com/8695
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-12 16:32:31 +00:00
Steven Valdez
2b8415e8ff Move the Digest/Sign split for SignatureAlgorithms to a lower level.
In order to delay the digest of the handshake transcript and unify
around message-based signing callbacks, a copy of the transcript is kept
around until we are sure there is no certificate authentication.

This removes support for SSL_PRIVATE_KEY_METHOD as a client in SSL 3.0.

Change-Id: If8999a19ca021b4ff439319ab91e2cd2103caa64
Reviewed-on: https://boringssl-review.googlesource.com/8561
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-01 19:01:33 +00:00
David Benjamin
c9ae27ca72 Build up TLS 1.3 record-layer tests.
This also adds a missing check to the C half to ensure fake record types are
always correct, to keep implementations honest.

Change-Id: I1d65272e647ffa67018c721d52c639f8ba47d647
Reviewed-on: https://boringssl-review.googlesource.com/8510
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-27 17:02:01 +00:00
Steven Valdez
b9824e2417 Handle SSL_shutdown while in init more appropriately
Calling SSL_shutdown while in init previously gave a "1" response,
meaning everything was successfully closed down (even though it
wasn't). Better is to send our close_notify, but fail when trying to
receive one.

The problem with doing a shutdown while in the middle of a handshake
is that once our close_notify is sent we shouldn't really do anything
else (including process handshake/CCS messages) until we've received a
close_notify back from the peer. However the peer might send a CCS
before acting on our close_notify - so we won't be able to read it
because we're not acting on CCS messages!

(Imported from upstream's f73c737c7ac908c5d6407c419769123392a3b0a9)
Change-Id: Iaad5c5e38983456d3697c955522a89919628024b
Reviewed-on: https://boringssl-review.googlesource.com/7207
Reviewed-by: David Benjamin <davidben@google.com>
2016-02-24 15:57:09 +00:00
David Benjamin
ece5ba2797 Reset ssl error codes.
38 error codes have fallen off the list since the last time we did this.

Change-Id: Id7ee30889a5da2f6ab66957fd8e49e97640c8489
Reviewed-on: https://boringssl-review.googlesource.com/6643
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 18:38:20 +00:00
David Benjamin
cd24a39f1b Limit DHE groups to 4096-bit.
dh.c had a 10k-bit limit but it wasn't quite correctly enforced. However,
that's still 1.12s of jank on the IO thread, which is too long. Since the SSL
code consumes DHE groups from the network, it should be responsible for
enforcing what sanity it needs on them.

Costs of various bit lengths on 2013 Macbook Air:
1024 - 1.4ms
2048 - 14ms
3072 - 24ms
4096 - 55ms
5000 - 160ms
10000 - 1.12s

UMA says that DHE groups are 0.2% 4096-bit and otherwise are 5.5% 2048-bit and
94% 1024-bit and some noise. Set the limit to 4096-bit to be conservative,
although that's already quite a lot of jank.

BUG=554295

Change-Id: I8e167748a67e4e1adfb62d73dfff094abfa7d215
Reviewed-on: https://boringssl-review.googlesource.com/6464
Reviewed-by: Adam Langley <agl@google.com>
2015-11-11 22:18:39 +00:00
David Benjamin
76c2efc0e9 Forbid a server from negotiating both ALPN and NPN.
If the two extensions select different next protocols (quite possible since one
is server-selected and the other is client-selected), things will break. This
matches the behavior of NSS (Firefox) and Go.

Change-Id: Ie1da97bf062b91a370c85c12bc61423220a22f36
Reviewed-on: https://boringssl-review.googlesource.com/5780
Reviewed-by: Adam Langley <agl@google.com>
2015-09-01 20:46:42 +00:00
Adam Langley
0950563a9b Implement custom extensions.
This change mirrors upstream's custom extension API because we have some
internal users that depend on it.

Change-Id: I408e442de0a55df7b05c872c953ff048cd406513
Reviewed-on: https://boringssl-review.googlesource.com/5471
Reviewed-by: Adam Langley <agl@google.com>
2015-07-31 01:12:00 +00:00
Adam Langley
33ad2b59da Tidy up extensions stuff and drop fastradio support.
Fastradio was a trick where the ClientHello was padding to at least 1024
bytes in order to trick some mobile radios into entering high-power mode
immediately. After experimentation, the feature is being dropped.

This change also tidies up a bit of the extensions code now that
everything is using the new system.

Change-Id: Icf7892e0ac1fbe5d66a5d7b405ec455c6850a41c
Reviewed-on: https://boringssl-review.googlesource.com/5466
Reviewed-by: Adam Langley <agl@google.com>
2015-07-21 21:44:55 +00:00
David Benjamin
34248d4cb7 Get rid of err function codes.
Running make_errors.go every time a function is renamed is incredibly
tedious. Plus we keep getting them wrong.

Instead, sample __func__ (__FUNCTION__ in MSVC) in the OPENSSL_PUT_ERROR macro
and store it alongside file and line number. This doesn't change the format of
ERR_print_errors, however ERR_error_string_n now uses the placeholder
"OPENSSL_internal" rather than an actual function name since that only takes
the uint32_t packed error code as input.

This updates err scripts to not emit the function string table. The
OPENSSL_PUT_ERROR invocations, for now, still include the extra
parameter. That will be removed in a follow-up.

BUG=468039

Change-Id: Iaa2ef56991fb58892fa8a1283b3b8b995fbb308d
Reviewed-on: https://boringssl-review.googlesource.com/5275
Reviewed-by: Adam Langley <agl@google.com>
2015-07-16 02:02:08 +00:00
Adam Langley
2e857bdad3 Convert the signature algorithms extension to the new system
Change-Id: Ia53b434acd11e9d2b0151b967387d86745ae441f
Reviewed-on: https://boringssl-review.googlesource.com/5362
Reviewed-by: Adam Langley <agl@google.com>
2015-07-09 23:12:51 +00:00
Adam Langley
5021b223d8 Convert the renegotiation extension to the new system.
This change also switches the behaviour of the client. Previously the
client would send the SCSV rather than the extension, but now it'll only
do that for SSLv3 connections.

Change-Id: I67a04b8abbef2234747c0dac450458deb6b0cd0a
Reviewed-on: https://boringssl-review.googlesource.com/5143
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 19:30:53 +00:00
Adam Langley
614c66a2f8 Add infrastructure for better extension handling.
Rather than four massive functions that handle every extension,
organise the code by extension with four smaller functions for each.

Change-Id: I876b31dacb05aca9884ed3ae7c48462e6ffe3b49
Reviewed-on: https://boringssl-review.googlesource.com/5142
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 18:25:28 +00:00
David Benjamin
6cacac033b Promote SSL_CTX_[gs]et_tlsext_ticket_keys to functions.
BUG=404754

Change-Id: Iae75a7ab24d4aa3b30edf578cbfc1058aeadd863
Reviewed-on: https://boringssl-review.googlesource.com/5233
Reviewed-by: Adam Langley <agl@google.com>
2015-06-25 22:39:36 +00:00
David Benjamin
fd67aa8c95 Add SSL_SESSION_from_bytes.
Mirrors SSL_SESSION_to_bytes. It avoids having to deal with object-reuse, the
non-size_t length parameter, and trailing data. Both it and the object-reuse
variant back onto an unexposed SSL_SESSION_parse which reads a CBS.

Note that this changes the object reuse story slightly. It's now merely an
optional output pointer that frees its old contents. No d2i_SSL_SESSION
consumer in Google that's built does reuse, much less reuse with the assumption
that the top-level object won't be overridden.

Change-Id: I5cb8522f96909bb222cab0f342423f2dd7814282
Reviewed-on: https://boringssl-review.googlesource.com/5121
Reviewed-by: Adam Langley <agl@google.com>
2015-06-16 18:12:39 +00:00
David Benjamin
24f346d77b Limit the number of warning alerts silently consumed.
Per review comments on
https://boringssl-review.googlesource.com/#/c/4112/.

Change-Id: I82cacf67c6882e64f6637015ac41945522699797
Reviewed-on: https://boringssl-review.googlesource.com/5041
Reviewed-by: Adam Langley <agl@google.com>
2015-06-08 22:16:14 +00:00
Adam Langley
ba5934b77f Tighten up EMS resumption behaviour.
The client and server both have to decide on behaviour when resuming a
session where the EMS state of the session doesn't match the EMS state
as exchanged in the handshake.

                        Original handshake
      |  No                                         Yes
------+--------------------------------------------------------------
      |
R     |  Server: ok [1]                     Server: abort [3]
e  No |  Client: ok [2]                     Client: abort [4]
s     |
u     |
m     |
e     |
  Yes |  Server: don't resume                   No problem
      |  Client: abort; server
      |    shouldn't have resumed

[1] Servers want to accept legacy clients. The draft[5] says that
resumptions SHOULD be rejected so that Triple-Handshake can't be done,
but we'll rather enforce that EMS was used when using tls-unique etc.

[2] The draft[5] says that even the initial handshake should be aborted
if the server doesn't support EMS, but we need to be able to talk to the
world.

[3] This is a very weird case where a client has regressed without
flushing the session cache. Hopefully we can be strict and reject these.

[4] This can happen when a server-farm shares a session cache but
frontends are not all updated at once. If Chrome is strict here then
hopefully we can prevent any servers from existing that will try to
resume an EMS session that they don't understand. OpenSSL appears to be
ok here: https://www.ietf.org/mail-archive/web/tls/current/msg16570.html

[5] https://tools.ietf.org/html/draft-ietf-tls-session-hash-05#section-5.2

BUG=492200

Change-Id: Ie1225a3960d49117b05eefa5a36263d8e556e467
Reviewed-on: https://boringssl-review.googlesource.com/4981
Reviewed-by: Adam Langley <agl@google.com>
2015-06-03 22:05:50 +00:00
David Benjamin
c933a47e6f Switch the ssl_write_bytes hook to ssl_write_app_data.
The SSL_PROTOCOL_METHOD table needs work, but this makes it clearer
exactly what the shared interface between the upper later and TLS/DTLS
is.

BUG=468889

Change-Id: I38931c484aa4ab3f77964d708d38bfd349fac293
Reviewed-on: https://boringssl-review.googlesource.com/4955
Reviewed-by: Adam Langley <agl@google.com>
2015-06-01 22:18:06 +00:00
David Benjamin
31a07798a5 Factor SSL_AEAD_CTX into a dedicated type.
tls1_enc is now SSL_AEAD_CTX_{open,seal}. This starts tidying up a bit
of the record-layer logic. This removes rr->input, as encrypting and
decrypting records no longer refers to various globals. It also removes
wrec altogether. SSL3_RECORD is now only used to maintain state about
the current incoming record. Outgoing records go straight to the write
buffer.

This also removes the outgoing alignment memcpy and simply calls
SSL_AEAD_CTX_seal with the parameters as appropriate. From bssl speed
tests, this seems to be faster on non-ARM and a bit of a wash on ARM.

Later it may be worth recasting these open/seal functions to write into
a CBB (tweaked so it can be malloc-averse), but for now they take an
out/out_len/max_out trio like their EVP_AEAD counterparts.

BUG=468889

Change-Id: Ie9266a818cc053f695d35ef611fd74c5d4def6c3
Reviewed-on: https://boringssl-review.googlesource.com/4792
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 17:59:15 +00:00
David Benjamin
d6e95eefba Get rid of ssl_undefined_*
The only place using it is export keying material which can do the
version check inline.

Change-Id: I1893966c130aa43fa97a6116d91bb8b04f80c6fb
Reviewed-on: https://boringssl-review.googlesource.com/4615
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:56:02 +00:00
David Benjamin
605641ed95 Move the NULL case in ssl_add_cert_chain up.
It's only called for client certificates with NULL. The interaction with
extra_certs is more obvious if we handle that case externally. (We
shouldn't attach extra_certs if there is no leaf.)

Change-Id: I9dc26f32f582be8c48a4da9aae0ceee8741813dc
Reviewed-on: https://boringssl-review.googlesource.com/4613
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:53:53 +00:00
David Benjamin
7133d428dd Promote SNI macros to functions.
BUG=404754

Change-Id: I2b2e27f3db0c97f2db65ca5e226c6488d2bee2fc
Reviewed-on: https://boringssl-review.googlesource.com/4570
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:36:19 +00:00