Commit Graph

162 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
David Benjamin
b25a8999be Add the ability to save and restore the error state.
This will be useful for the SSL stack to properly resurface handshake
failures. Leave this in a private header and, along the way, hide the
various types.

(ERR_NUM_ERRORS didn't change in meaning. The old documentation was
wrong.)

Bug: 206
Change-Id: I4c6ca98d162d11ad5e17e4baf439a18fbe371018
Reviewed-on: https://boringssl-review.googlesource.com/21284
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-10-09 21:43:13 +00:00
David Benjamin
a65c252f78 Further simplify error queue flags.
ERR_FLAGS_STRING is meaningless and we can use a bitfield for the mark
bit.

Change-Id: I6f677b55b11316147512171629196c651cb33ca9
Reviewed-on: https://boringssl-review.googlesource.com/21084
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-10-04 16:02:16 +00:00
David Benjamin
e1c3dad959 Error data is always a NUL-terminated malloced string.
Cut down on the number of cases we need to worry about here. In
particular, it would be useful for the handshake to be able to replay an
error.

Change-Id: I2345faaff5503ede1324a5599e680de83f4b106e
Reviewed-on: https://boringssl-review.googlesource.com/21004
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-10-02 21:24:08 +00:00
David Benjamin
808f832917 Run the comment converter on libcrypto.
crypto/{asn1,x509,x509v3,pem} were skipped as they are still OpenSSL
style.

Change-Id: I3cd9a60e1cb483a981aca325041f3fbce294247c
Reviewed-on: https://boringssl-review.googlesource.com/19504
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-08-18 21:49:04 +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
David Benjamin
b529253bea Implement scrypt from RFC 7914.
This imports upstream's scrypt implementation, though it's been heavily
revised. I lost track of words vs. blocks vs. bigger blocks too many
times in the original code and introduced a typedef for the fixed-width
Salsa20 blocks. The downside is going from bytes to blocks is a bit
trickier, so I took advantage of our little-endian assumption.

This also adds an missing check for N < 2^32. Upstream's code is making
this assumption in Integerify. I'll send that change back upstream. I've
also removed the weird edge case where a NULL out_key parameter means to
validate N/r/p against max_mem and nothing else. That's just in there to
get a different error code out of their PKCS#12 code.

Performance-wise, the cleanup appears to be the same (up to what little
precision I was able to get here), but an optimization to use bitwise
AND rather than modulus makes us measurably faster. Though scrypt isn't
a fast operation to begin with, so hopefully it isn't anyone's
bottleneck.

This CL does not route scrypt up to the PKCS#12 code, though we could
write our own version of that if we need to later.

BUG=chromium:731993

Change-Id: Ib2f43344017ed37b6bafd85a2c2b103d695020b8
Reviewed-on: https://boringssl-review.googlesource.com/17084
Reviewed-by: Adam Langley <agl@google.com>
2017-06-12 20:32:21 +00:00
Steven Valdez
2f3404bb81 Enforce incrementing counter for TLS 1.2 AES-GCM.
Change-Id: I7e790bc176369f2a57cc486c3dc960971faf019d
Reviewed-on: https://boringssl-review.googlesource.com/16625
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-26 20:06:36 +00:00
Steven Valdez
8ebc9eafec Update BN_enhanced_miller_rabin_primality_test to enforce preconditions and accept BN_prime_checks.
Change-Id: Ie4ac57d39bca46db33280c500a2092350ccdae67
Reviewed-on: https://boringssl-review.googlesource.com/15371
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-04-21 22:24:01 +00:00
David Benjamin
6fdea2aba9 Move PKCS#7 functions into their own directory.
A follow-up change will add a CRYPTO_BUFFER variant. This makes the
naming match the header and doesn't require including x509.h. (Though
like ssl.h and pkcs8.h, some of the functions are implemented with code
that depends on crypto/x509.)

Change-Id: I5a7de209f4f775fe0027893f711326d89699ca1f
Reviewed-on: https://boringssl-review.googlesource.com/15128
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-19 17:24:51 +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
Steven Valdez
b15143fece Fix check_fips for public keys and synchronize the EC and RSA versions.
Change-Id: Ibebf787445578608845df8861d67cd1e65ed0b35
Reviewed-on: https://boringssl-review.googlesource.com/15004
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-13 17:33:40 +00:00
Steven Valdez
d0b988219f Add RSA_check_fips to support public key validation checks.
Change-Id: I0e00f099a17d88f56b49970e612b0911afd9661e
Reviewed-on: https://boringssl-review.googlesource.com/14866
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-12 20:00:30 +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
David Benjamin
d69d94e7e3 Teach crypto/x509 how to verify an Ed25519 signature.
BUG=187

Change-Id: I5775ce0886041b0c12174a7d665f3af1e8bce511
Reviewed-on: https://boringssl-review.googlesource.com/14505
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-04-05 23:35:30 +00:00
David Benjamin
417830d981 Support EVP_PKEY_{sign,verify}_message with Ed25519.
It's amazing how short p_ed25519.c is.

BUG=187

Change-Id: Ib2a5fa7a4acf2087ece954506f81e91a1ed483e1
Reviewed-on: https://boringssl-review.googlesource.com/14449
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-04-05 23:05:14 +00:00
David Benjamin
05bb1c5033 Implement draft-ietf-curdle-pkix-04's serialization.
The resulting EVP_PKEYs do not do anything useful yet, but we are able
to parse them. Teaching them to sign will be done in a follow-up.

Creating these from in-memory keys is also slightly different from other
types. We don't have or need a public ED25519_KEY struct in
curve25519.h, so I've added tighter constructor functions which should
hopefully be easier to use anyway.

BUG=187

Change-Id: I0bbeea37350d4fdca05b6c6c0f152c15e6ade5bb
Reviewed-on: https://boringssl-review.googlesource.com/14446
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-04-05 23:02:22 +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
David Benjamin
3cb047e56c Decouple PKCS#12 hash lookup from the OID table.
This isn't strictly necessary for Chromium yet, but we already have a
decoupled version of hash algorithm parsing available. For now, don't
export it but eventually we may wish to use it for OCSP.

BUG=54

Change-Id: If460d38d48bd47a2b4a853779f210c0cf7ee236b
Reviewed-on: https://boringssl-review.googlesource.com/14211
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-03-25 21:22:50 +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
cfb9d147bb Update pkcs8 error data.
We forgot to run the script at some point.

Change-Id: I0bd142fdd13d64c1ed81d9b1515449220d1c936b
Reviewed-on: https://boringssl-review.googlesource.com/14329
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-03-23 15:07:28 +00:00
David Benjamin
2d05568a7b Fix out-of-memory condition in conf.
conf has the ability to expand variables in config files. Repeatedly doing
this can lead to an exponential increase in the amount of memory required.
This places a limit on the length of a value that can result from an
expansion.

Credit to OSS-Fuzz for finding this problem.

(Imported from upstream's 6a6213556a80ab0a9eb926a1d6023b8bf44f2afd. This
also import's upstream's ee1ccd0a41ad068957fe65ba7521e593b51bbad4 which
we had previously missed.)

Change-Id: I9be06a7e8a062b5adcd00c974a7b245226123563
Reviewed-on: https://boringssl-review.googlesource.com/14316
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-03-21 16:19:22 +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
David Benjamin
bc6ef7a83f Convert err_test to GTest.
BUG=129

Change-Id: I227ffa2da4e220075de296fb5b94d043f4e032e0
Reviewed-on: https://boringssl-review.googlesource.com/13627
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 17:38:22 +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
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
David Benjamin
8cf79af7d1 Always use Fermat's Little Theorem in ecdsa_sign_setup.
The case where ec_group_get_mont_data is NULL is only for arbitrary groups
which we now require to be prime order. BN_mod_exp_mont is fine with a NULL
BN_MONT_CTX. It will just compute it. Saves a bit of special-casing.

Also don't mark p-2 as BN_FLG_CONSTTIME as the exponent is public anyway.

Change-Id: Ie868576d52fc9ae5f5c9f2a4039a729151bf84c7
Reviewed-on: https://boringssl-review.googlesource.com/8307
Reviewed-by: Adam Langley <agl@google.com>
2016-06-20 17:11:42 +00:00