Commit Graph

1604 Commits

Author SHA1 Message Date
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
David Benjamin
5d9ba81b6c Enable more TLS 1.3 resumption tests.
We missed these two.

Change-Id: I2bc45f6c88e882c36abaa12a02931d1af0b1f29f
Reviewed-on: https://boringssl-review.googlesource.com/11562
Reviewed-by: David Benjamin <davidben@google.com>
2016-10-10 15:47:31 +00:00
David Benjamin
52bf690ba4 Saved Finished messages are twelve bytes.
We only save them at TLS 1.0 through 1.2. This saves 104 bytes of
per-connection memory.

Change-Id: If397bdc10e40f0194cba01024e0e9857d6b812f0
Reviewed-on: https://boringssl-review.googlesource.com/11571
Reviewed-by: Adam Langley <agl@google.com>
2016-10-09 17:52:18 +00:00
David Benjamin
34941c0cab Forbid renego in SSL 3.0.
We need to retain a pair of Finished messages for renegotiation_info.
SSL 3.0's is actually larger than TLS 1.2's (always 12 bytes). Take
renegotiation out in preparation for trimming them to size.

Change-Id: I2e238c48aaf9be07dd696bc2a6af75e9b0ead299
Reviewed-on: https://boringssl-review.googlesource.com/11570
Reviewed-by: Adam Langley <agl@google.com>
2016-10-09 17:44:54 +00:00
David Benjamin
49ddf41557 Remove redundant copies of the Finished messages.
We only need one copy, not two. This trims 130 bytes of per-connection
memory.

Change-Id: I334aa7b1f8608e72426986bfa68534d416f3bda9
Reviewed-on: https://boringssl-review.googlesource.com/11569
Reviewed-by: Adam Langley <agl@google.com>
2016-10-09 17:43:03 +00:00
David Benjamin
ced00b4258 Turn off Finished-based APIs at TLS 1.3 and SSL 3.0.
tls-unique isn't defined at TLS 1.3 yet. (Given that it was too small in
1.2, they may just define a new one entirely?) SSL_get_(peer_)finished
doesn't work at 1.3 and is only used in lieu of computing tls-unique,
also undefined at SSL 3.0.

This is in preparation for trimming the copies of the Finished messages
we retain.

Change-Id: Iace99f2baea92c511c4041c592300dfbbe7226e2
Reviewed-on: https://boringssl-review.googlesource.com/11568
Reviewed-by: Adam Langley <agl@google.com>
2016-10-09 17:39:25 +00:00
David Benjamin
a4c8ff0190 Move TLS 1.2 key exchange fields to SSL_HANDSHAKE.
SSL_HANDSHAKE is dropped after the handshake, so I've removed the logic
around smaller sizes. It's much simpler when we can use CBS_stow and
CBB_finish without extra bounds-checking.

Change-Id: Idafaa5d69e171aed9a8759f3d44e52cb01c40f39
Reviewed-on: https://boringssl-review.googlesource.com/11567
Reviewed-by: Adam Langley <agl@google.com>
2016-10-09 17:30:32 +00:00
David Benjamin
43612b6bc7 Move peer_supported_group_list to SSL_HANDSHAKE.
Now not only the pointers but also the list itself is released after the
handshake completes.

Change-Id: I8b568147d2d4949b3b0efe58a93905f77a5a4481
Reviewed-on: https://boringssl-review.googlesource.com/11528
Reviewed-by: Adam Langley <agl@google.com>
2016-10-09 17:20:33 +00:00
David Benjamin
f04976ba25 Remove the get_peer_groups parameter to tls1_get_grouplist.
It's weird and makes things more confusing. Only use it for local
preferences as there is a default. Peer preferences can be read
directly. Also simplify the logic for requiring a non-empty peer group
list for ECDHE. The normal logic will give us this for free.

Change-Id: I1916155fe246be988f20cbf0b1728380ec90ff3d
Reviewed-on: https://boringssl-review.googlesource.com/11527
Reviewed-by: Adam Langley <agl@google.com>
2016-10-09 17:19:24 +00:00
David Benjamin
9d0b4bcb92 Trim tls1_check_group_id.
This function is now only ever called as a client, so there are no peer
preferences to check against. It is also now only called on peer curves,
so it only needs to be compared against local preferences.

Change-Id: I87f5b10cf4fe5fef9a9d60aff36010634192e90c
Reviewed-on: https://boringssl-review.googlesource.com/11526
Reviewed-by: Adam Langley <agl@google.com>
2016-10-09 17:15:49 +00:00
David Benjamin
938fa7cc84 Inline tls1_check_ec_cert.
These functions are only called once. It ends up being not much code if
just done inline.

Change-Id: Ic432b313a6f7994ff9f51436cffbe0c3686a6c7c
Reviewed-on: https://boringssl-review.googlesource.com/11525
Reviewed-by: Adam Langley <agl@google.com>
2016-10-09 17:14:50 +00:00
David Benjamin
34de91e377 Revise server-side ECDSA certificate checks.
This is in preparation for simplifying tls1_check_group_id, called by
tls1_check_ec_cert, which, in turn, is in preparation for moving the
peer group list to SSL_HANDSHAKE.

It also helps with bug #55. Move the key usage check to the certificate
configuration sanity check. There's no sense in doing it late. Also
remove the ECDSA peer curve check as we configure certificates
externally. With only one certificate, there's no sense in trying to
remove it.

BUG=55

Change-Id: I8c116337770d96cc9cfd4b4f0ca7939a4f05a1a9
Reviewed-on: https://boringssl-review.googlesource.com/11524
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-10-09 17:04:41 +00:00
David Benjamin
b74b08144e Move next_proto_neg_seen into SSL_HANDSHAKE.
Change-Id: I7f1d546f735ca854ac58c65b529218afda164ec0
Reviewed-on: https://boringssl-review.googlesource.com/11523
Reviewed-by: Adam Langley <agl@google.com>
2016-10-09 16:50:13 +00:00
David Benjamin
f5d2cd0808 Move extensions bitmasks into SSL_HANDSHAKE.
Change-Id: I3ab30a44b7f90ef1159e022cd17b7f50ffe27a93
Reviewed-on: https://boringssl-review.googlesource.com/11522
Reviewed-by: Adam Langley <agl@google.com>
2016-10-09 16:48:52 +00:00
David Benjamin
a048678cd6 Move some fields from tmp to hs.
This releases memory associated with them after the handshake. Note this
changes the behavior of |SSL_get0_certificate_types| and
|SSL_get_client_CA_list| slightly. Both functions now return NULL
outside of the handshake. But they were already documented to return
something undefined when not called at the CertificateRequest.

A survey of callers finds none that would care. (Note
SSL_get_client_CA_list is used both as a getter for the corresponding
server config setter and to report client handshake properties. Only the
latter is affected.) It's also pretty difficult to imagine why a caller
would wish to query this stuff at any other time, and there are clear
benefits to dropping the CA list after the handshake (some servers send
ABSURDLY large lists).

Change-Id: I3ac3b601ff0cfa601881ce77ae33d99bb5327004
Reviewed-on: https://boringssl-review.googlesource.com/11521
Reviewed-by: Adam Langley <agl@google.com>
2016-10-09 16:47:31 +00:00
David Benjamin
1286beef94 Test that unknown TLS 1.3 ticket extensions are tolerated.
Change-Id: Ifcdbeab9291d1141605a09a1960702c792cffa86
Reviewed-on: https://boringssl-review.googlesource.com/11561
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-10-07 21:00:59 +00:00
David Benjamin
1a5e8ecd64 Apply GREASE to TLS 1.3 tickets.
Change-Id: I5d4fc0d3204744e93d71a36923469035c19a5b10
Reviewed-on: https://boringssl-review.googlesource.com/11560
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-10-07 20:58:26 +00:00
Steven Valdez
3cbdc34619 Add GENERIC selector for TLS 1.3 AEAD-only cipher suites.
Change-Id: Ib499b3393962a4d41cf9694e055ed3eb869d91a2
Reviewed-on: https://boringssl-review.googlesource.com/11504
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 19:37:40 +00:00
David Benjamin
7f78df470b Add a few more tests around processing the server PSK extension.
The server acknowledging a non-existent session is a particularly
interesting case since getting it wrong means a NULL crash.

Change-Id: Iabde4955de883595239cfd8e9d84a7711e60a886
Reviewed-on: https://boringssl-review.googlesource.com/11500
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>
2016-10-06 14:38:01 +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
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
5ecb88b95b Make EnableAllCiphers client-only and rename.
EnableAllCiphers is problematic since some (version, cipher)
combinations aren't even defined and crash. Instead, use the
SendCipherSuite bug to mask the true cipher (which is becomes arbitrary)
for failure tests. The shim should fail long before we get further.

This lets us remove a number of weird checks in the TLS 1.3 code.

This also fixes the UnknownCipher tests which weren't actually testing
anything. EnableAllCiphers is now AdvertiseAllConfiguredCiphers and
does not filter out garbage values.

Change-Id: I7102fa893146bb0d096739e768c5a7aa339e51a8
Reviewed-on: https://boringssl-review.googlesource.com/11481
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-10-05 14:34:58 +00:00
David Benjamin
daa8850c83 Add tests for OCSP's interaction with resumption.
This is another case where the specification failed to hammer things
down and OpenSSL messed it up as a result. Also fix the SCT test in TLS
1.3.

Change-Id: I47541670447d1929869e1a39b2d9671a127bfba0
Reviewed-on: https://boringssl-review.googlesource.com/11480
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-10-04 20:53:21 +00:00
David Benjamin
6dbde984a2 Fix TLS 1.3 minimum version tests.
The client/server split didn't actually make sense. We're interested in
whether the client will notice the bad version before anything else, so
ignore peer cipher preferences so all combinations work.

Change-Id: I52f84b932509136a9b39d93e46c46729c3864bfd
Reviewed-on: https://boringssl-review.googlesource.com/11413
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-10-04 14:57:24 +00:00
Adam Langley
3e9e043229 Add dummy |SSL_COMP_free_compression_methods|.
cURL calls this function if |OPENSSL_VERSION_NUMBER| is in [0x10002003,
0x10002fff], which it now is for BoringSSL after 0aecbcf6.

Change-Id: I3f224f73f46791bd2232a1a96ed926c32740a6f6
Reviewed-on: https://boringssl-review.googlesource.com/11461
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-10-04 00:31:21 +00:00
David Benjamin
ad75a661bf Improve version extension tests.
ConflictingVersionNegotiation really should be about, say 1.1 and 1.2
since those may be negotiated via either mechanism. (Those two cases are
actually kinda weird and we may wish to change the spec. But, in the
meantime, test that we have the expected semantics.)

Also test that we ignore true TLS 1.3's number for now, until we use it,
and that TLS 1.3 suitably ignores ClientHello.version.

Change-Id: I76c660ddd179313fa68b15a6fda7a698bef4d9c9
Reviewed-on: https://boringssl-review.googlesource.com/11407
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-10-03 18:30:10 +00:00
David Benjamin
592b532dda Fix TLS 1.3 downgrade detection tests.
They weren't updated for the new version negotiation. (Though right now
they're just testing that we *don't* implement the downgrade detection
because it's a draft version.)

Change-Id: I4c983ebcdf3180d682833caf1e0063467ea41544
Reviewed-on: https://boringssl-review.googlesource.com/11406
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-10-03 18:26:55 +00:00
David Benjamin
7f0965a66d Check versions before trying to send KeyUpdate.
Otherwise we panic. Thanks to EKR for reporting.

Change-Id: Ie4b6c2e18e1c77c7b660ca5d4c3bafb38a82cb6a
Reviewed-on: https://boringssl-review.googlesource.com/11405
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-10-03 18:26:19 +00:00
Adam Langley
f139c9957c Return immediately when cipher-string processing fails.
Rather than clear variables and break out of a loop that just ends up
returning anyway, just return. This makes all the abort points
consistent in this function.

Change-Id: I51d862e7c60a9e967773f15a17480b783af8c456
Reviewed-on: https://boringssl-review.googlesource.com/11422
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-03 18:10:26 +00:00
Adam Langley
f99f2448bd Return immediately if a cipher command is invalid.
Breaking from inside the inner loop doesn't do what the code wants.
Instead the outer loop will continue running and it's possible for it to
read off the end of the buffer. (Found with libFuzzer.)

Next change will update the other abort points in this code to match.

Change-Id: I006dca0cd4c31db1c4b5e84b996fe24b2f1e6c13
Reviewed-on: https://boringssl-review.googlesource.com/11421
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-03 18:03:08 +00:00
David Benjamin
31f5b3c605 Document that malloc tests require a longer timeout.
I always forget this.

Change-Id: I9fa15cebb6586985ddc48cdbf9d184a49a8bfb02
Reviewed-on: https://boringssl-review.googlesource.com/11402
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-09-30 19:13:05 +00:00
Alessandro Ghedini
5fd1807d95 Implement SSL_CTX_set1_curves_list()
This function is used by NGINX to enable specific curves for ECDH from a
configuration file. However when building with BoringSSL, since it's not
implmeneted, it falls back to using EC_KEY_new_by_curve_name() wich doesn't
support X25519.

Change-Id: I533df4ef302592c1a9f9fc8880bd85f796ce0ef3
Reviewed-on: https://boringssl-review.googlesource.com/11382
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-30 00:45:19 +00:00
David Benjamin
a252b34d66 Add tests for very large handshake messages.
OpenSSL recently had a regression here (CVE-2016-6309). We're fine,
but so that we stay that way, add some tests.

Change-Id: I244d7ff327b7aad550f86408c5e5e65e6d1babe5
Reviewed-on: https://boringssl-review.googlesource.com/11321
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-29 16:31:54 +00:00
David Benjamin
d9791bf10a Apply GREASE to the version extension.
BUG=106

Change-Id: Iaa12aeb67627f3c22fe4a917c89c646cb3dc1843
Reviewed-on: https://boringssl-review.googlesource.com/11325
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-27 21:07:52 +00:00
Steven Valdez
fdd10998e1 Moving TLS 1.3 version negotiation into extension.
Change-Id: I73f9fd64b46f26978b897409d817b34ec9d93afd
Reviewed-on: https://boringssl-review.googlesource.com/11080
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-09-27 20:12:22 +00:00
David Benjamin
cb18ac2bc3 Add a test for SSL_version.
We were never really testing this.

Change-Id: Ia953870053d16d3994ae48172017d384c7bc3601
Reviewed-on: https://boringssl-review.googlesource.com/11341
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-09-27 18:16:26 +00:00
David Benjamin
b1dd8cdab5 Prepare runner's wire/version conversions for the version extension.
This mirror's 2dc0204603 on the C side.

BUG=90

Change-Id: Iebb72df5a5ae98cb2fd8db519d973cd734ff05ea
Reviewed-on: https://boringssl-review.googlesource.com/11320
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-09-27 15:09:38 +00:00
David Benjamin
3c6a1ea674 Apply version/wire mapping at a higher layer in runner.
This is in preparation for implementing the version extension and is
probably what we should have done from the beginning as it makes
intolerance bugs simpler.

This means knobs like SendClientVersion and SendServerVersion deal with
the wire values while knobs like NegotiateVersion and MaxVersion deal
with logical versions. (This matches how the bugs have always worked.
SendFoo is just a weird post-processing bit on the handshake messages
while NegotiateVersion actually changes how BoGo behaves.)

BUG=90

Change-Id: I7f359d798d0899fa2742107fb3d854be19e731a4
Reviewed-on: https://boringssl-review.googlesource.com/11300
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-09-27 15:09:16 +00:00
David Benjamin
5ab4596070 Fix ssl_ctx_make_profiles error handling.
It didn't clean up |profiles| on error or check for
sk_SRTP_PROTECTION_PROFILE_push failures.

Change-Id: I44d7f64896ad73347fbb0fc79752be4de70d3ab7
Reviewed-on: https://boringssl-review.googlesource.com/11323
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-09-27 13:27:06 +00:00
David Benjamin
1eeb0b00ba Check for sk_X509_NAME_push failures.
Also tidy up the logic slightly.

Change-Id: I708254406b2df52435ec434ac9806e8eb2cbe928
Reviewed-on: https://boringssl-review.googlesource.com/11322
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-09-27 13:18:37 +00:00
David Benjamin
04fe9013c4 Require custom private keys to specify curve in 1.3.
If someone is still using EVP_PKEY_EC (I really should get on converting
Chromium...), don't silently skip the curve match check in TLS 1.3,
otherwise it may work on accident. Refuse to sign anything so this gets
caught.

Change-Id: I4ea46efb0b8f31a656771b9d2e5f882bba64eb99
Reviewed-on: https://boringssl-review.googlesource.com/11244
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-09-26 17:22:48 +00:00
David Benjamin
65ac997f20 Implement draft-davidben-tls-grease-01.
This GREASEs cipher suites, groups, and extensions. For now, we'll
always place them in a hard-coded position. We can experiment with more
interesting strategies later.

If we add new ciphers and curves, presumably we prefer them over current
ones, so place GREASE values at the front. This prevents implementations
from parsing only the first value and ignoring the rest.

Add two new extensions, one empty and one non-empty. Place the empty one
in front (IBM WebSphere can't handle trailing empty extensions) and the
non-empty one at the end.

Change-Id: If2e009936bc298cedf2a7a593ce7d5d5ddbb841a
Reviewed-on: https://boringssl-review.googlesource.com/11241
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-23 21:11:15 +00:00
David Benjamin
1032df56e7 Disable Channel ID signature checking in fuzzer mode.
Get us a little bit more room here.

BUG=79

Change-Id: Ifadad94ead7794755a33f02d340111694b3572af
Reviewed-on: https://boringssl-review.googlesource.com/11228
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-09-22 21:35:12 +00:00
David Benjamin
7364719655 Rename NPN-Server test.
That is an extremely confusing name. It should be NPN-Declined-TLS13.

Change-Id: I0e5fa50a3ddb0b80e88a8bc10d0ef87d0fff0a54
Reviewed-on: https://boringssl-review.googlesource.com/11227
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-09-22 21:35:07 +00:00
David Benjamin
c07afb79f6 Record resumption and renewal transcripts separately.
We recently added a three-connection option, but the transcripts were
still assuming just -Normal and -Resume.

Change-Id: I8816bce95dd7fac779af658e3eb86bc78bb95c91
Reviewed-on: https://boringssl-review.googlesource.com/11226
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-09-22 21:32:14 +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
ac5e47f300 Add a fuzzer mode suppressions file.
We want to ensure -fuzzer passes tests, except for the tests it
intentionally fails on. This ensures that we don't lose our ability to
refresh the fuzzer transcripts.

Change-Id: I761856c30379a3934fd46a24627ef8415b136f93
Reviewed-on: https://boringssl-review.googlesource.com/11221
Reviewed-by: Adam Langley <agl@google.com>
2016-09-22 21:11:23 +00:00
David Benjamin
196df5bfa2 Add a InvalidChannelIDSignature test.
Apparently we never wrote one of those. Also send a decrypt_error alert
to be consistent with all the other signature checks.

Change-Id: Ib5624d098d1e3086245192cdce92f5df26005064
Reviewed-on: https://boringssl-review.googlesource.com/11180
Reviewed-by: David Benjamin <davidben@google.com>
2016-09-22 20:41:41 +00:00
David Benjamin
f3fbadeae0 Add tests for SSL_peek.
SSL_peek works fine for us, but OpenSSL 1.1.0 regressed this
(https://github.com/openssl/openssl/issues/1563), and we don't have
tests either. Fix this.

SSL_peek can handle all weird events that SSL_read can, so use runner
and tell bssl_shim to do a SSL_peek + SSL_peek + SSL_read instead of
SSL_read. Then add tests for all the events we may discover.

Change-Id: I9e8635e3ca19653a02a883f220ab1332d4412f98
Reviewed-on: https://boringssl-review.googlesource.com/11090
Reviewed-by: Adam Langley <agl@google.com>
2016-09-22 18:45:20 +00:00
David Benjamin
e34bcc91c0 Support default versions with set_{min,max}_proto_version.
Upstream makes 0 mean "min/max supported version". Match that behavior,
although call it "default" instead. It shouldn't get you TLS 1.3 until
we're ready to turn it on everywhere.

BUG=90

Change-Id: I9f122fceb701b7d4de2ff70afbc1ffdf370cb97e
Reviewed-on: https://boringssl-review.googlesource.com/11181
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-21 21:41:49 +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
af56fbd62a Renumber TLS 1.3 signature algorithms.
The old numbers violate a MUST-level requirement in TLS 1.2 to not
advertise anonymous (0x0700 ends in 0x00). The spec has been updated
with new allocations which avoid these.

BUG=webrtc:6342

Change-Id: Ia5663ada98fa1ebf0f8a7f50fe74a0e9206c4194
Reviewed-on: https://boringssl-review.googlesource.com/11131
Reviewed-by: Adam Langley <agl@google.com>
2016-09-21 20:54:15 +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
e470690633 Align SSL_set_{min,max}_version with upstream.
Upstream added these functions after we did but decided to change the
names slightly. I'm not sure why they wanted to add the "proto" in
there, but align with them nonetheless so the ecosystem only has one set
of these functions.

BUG=90

Change-Id: Ia9863c58c9734374092051f02952b112806040cc
Reviewed-on: https://boringssl-review.googlesource.com/11123
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 20:06:18 +00:00
David Benjamin
2dc0204603 Don't return invalid versions in version_from_wire.
This is in preparation for using the supported_versions extension to
experiment with draft TLS 1.3 versions, since we don't wish to restore
the fallback. With versions begin opaque values, we will want
version_from_wire to reject unknown values, not attempt to preserve
order in some way.

This means ClientHello.version processing needs to be separate code.
That's just written out fully in negotiate_version now. It also means
SSL_set_{min,max}_version will notice invalid inputs which aligns us
better with upstream's versions of those APIs.

This CL doesn't replace ssl->version with an internal-representation
version, though follow work should do it once a couple of changes land
in consumers.

BUG=90

Change-Id: Id2f5e1fa72847c823ee7f082e9e69f55e51ce9da
Reviewed-on: https://boringssl-review.googlesource.com/11122
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 19:51:45 +00:00
David Benjamin
c027999c28 Take the version parameter out of ssl_do_msg_callback.
This will make it a little easier to store the normalized version rather
than the wire version. Also document the V2ClientHello behavior.

Change-Id: I5ce9ccce44ca48be2e60ddf293c0fab6bba1356e
Reviewed-on: https://boringssl-review.googlesource.com/11121
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 18:55:27 +00:00
David Benjamin
d2ba8891e0 Improve -valgrind error-handling.
Passing --quiet makes valgrind only print out errors, so we don't need
to suppress things. Combine that with checking valgrind's dedicated exit
code so we notice errors that happen before the "---DONE---" marker.

This makes that marker unnecessary for valgrind. all_tests.go was not
sensitive to this, but still would do well to have valgrind be silent.

Change-Id: I841edf7de87081137e38990e647e989fd7567295
Reviewed-on: https://boringssl-review.googlesource.com/11128
Reviewed-by: Adam Langley <agl@google.com>
2016-09-21 17:25:32 +00:00
David Benjamin
9aafb64849 Don't swallow tool output on failure.
If the test failed due to non-ASan reasons but ASan also had errors,
output those too.

Change-Id: Id908fe2a823c59255c6a9585dfaa894a4fcd9f59
Reviewed-on: https://boringssl-review.googlesource.com/11127
Reviewed-by: Adam Langley <agl@google.com>
2016-09-21 17:24:42 +00:00
David Benjamin
7a4aaa4ce7 Fix TLS 1.3 fuzzer mode in Go.
Runner needs to implement fuzzer mode as well so we can record
transcripts from it. A bunch of tests were failing:

- C and Go disagreed on what fuzzer mode did to TLS 1.3 padding. So we
  fuzz more code, align Go with C. Fuzzer mode TLS 1.3 still pads but
  just skips the final AEAD.

- The deterministic RNG should be applied per test, not per exchange. It
  turns out, if your RNG is deterministic, one tends to pick the same
  session ID over and over which confuses clients. (Resumption is
  signaled by echoing the session ID.)

Now the only failing tests are the ones one would expect to fail.

BUG=79

Change-Id: Ica23881a6e726adae71e6767730519214ebcd62a
Reviewed-on: https://boringssl-review.googlesource.com/11126
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-21 17:18:40 +00:00
David Benjamin
e0ff767025 Remove SSL_set_fallback_version.
Ding-dong the fallback's dead.
https://mailarchive.ietf.org/arch/msg/tls/xfCh7D7hISFs5x-eA0xHwksoLrc

Also we'll need to tweak the versioning code slightly to implement
supported_versions and it's nice to have this out of the way.

Change-Id: I0961e19ea56b4afd828f6f48858ac6310129503d
Reviewed-on: https://boringssl-review.googlesource.com/11120
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 17:03:42 +00:00
David Benjamin
e63d9d7625 Test interaction of RSA key exchange and ClientHello.version.
If we see garbage in ClientHello.version and then select static RSA,
that garbage is what goes in the premaster.

Change-Id: I65190a44439745e6b5ffaf7669f063da725c8097
Reviewed-on: https://boringssl-review.googlesource.com/11092
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-20 23:13:52 +00:00
David Benjamin
786793411a Do not distinguish NULL and empty PSK identity hints.
Plain PSK omits the ServerKeyExchange when there is no hint and includes
it otherwise (it should have always sent it), while other PSK ciphers
like ECDHE_PSK cannot omit the hint. Having different capabilities here
is odd and RFC 4279 5.2 suggests that all PSK ciphers are capable of
"[not] provid[ing] an identity hint".

Interpret this to mean no identity hint and empty identity hint are the
same state. Annoyingly, this gives a plain PSK implementation two
options for spelling an empty hint. The spec isn't clear and this is not
really a battle worth fighting, so I've left both acceptable and added a
test for this case.

See also https://android-review.googlesource.com/c/275217/. This is also
consistent with Android's PskKeyManager API, our only consumer anyway.

https://developer.android.com/reference/android/net/PskKeyManager.html

Change-Id: I8a8e6cc1f7dd1b8b202cdaf3d4f151bebfb4a25b
Reviewed-on: https://boringssl-review.googlesource.com/11087
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-20 23:00:47 +00:00
David Benjamin
bac75b80cc Move peer_psk_identity_hint to SSL_HANDSHAKE.
One less field to reset on renego and save a pointer of post-handshake
memory.

Change-Id: Ifc0c3c73072af244ee3848d9a798988d2c8a7c38
Reviewed-on: https://boringssl-review.googlesource.com/11086
Reviewed-by: Adam Langley <agl@google.com>
2016-09-20 22:37:24 +00:00
David Benjamin
4fe3c90b7d Release TLS 1.3 key shares earlier in TLS 1.2.
This isn't hugely important since the hs object will actually be
released at the end of the handshake, but no sense in holding on to them
longer than needed.

Also release |public_key| when we no longer need it and document what
the fields mean.

Change-Id: If677cb4a915c75405dabe7135205630527afd8bc
Reviewed-on: https://boringssl-review.googlesource.com/10360
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-09-19 20:35:35 +00:00
David Benjamin
2c66e079ab Don't send the access_denied alert innappropriately.
access_denied is only used to indicate client cert errors and Chrome
maps it to ERR_SSL_BAD_CLIENT_AUTH_CERT accordingly:

   access_denied
      A valid certificate was received, but when access control was
      applied, the sender decided not to proceed with negotiation.  This
      message is always fatal.

We don't appear to be the cause of Chrome's recent
ERR_SSL_BAD_CLIENT_AUTH_CERT spike, but we should send these correctly
nonetheless.

If the early callback fails, handshake_failure seems the most
appropriate ("I was unable to find suitable parameters"). There isn't
really an alert that matches DoS, but internal_error seems okay?

   internal_error
      An internal error unrelated to the peer or the correctness of the
      protocol (such as a memory allocation failure) makes it impossible
      to continue.  This message is always fatal.

There's nothing wrong, per se, with your ClientHello, but I just can't
deal with it right now. Please go away.

Change-Id: Icd1c998c09dc42daa4b309c1a4a0f136b85eb69d
Reviewed-on: https://boringssl-review.googlesource.com/11084
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-16 20:12:09 +00:00
David Benjamin
9a5f49eec0 Remove a few more remnants of RC4/TLS.
Change-Id: I5d7fd9ba0688a3ebd6f6d36768cc3c0e33e2da52
Reviewed-on: https://boringssl-review.googlesource.com/11081
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Matt Braithwaite <mab@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-09-16 16:34:50 +00:00
Matthew Braithwaite
8aaa9e12c2 Remove RC4 from TLS for real.
This withdraws support for -DBORINGSSL_ENABLE_RC4_TLS, and removes the
RC4 AEADs.

Change-Id: I1321b76bfe047d180743fa46d1b81c5d70c64e81
Reviewed-on: https://boringssl-review.googlesource.com/10940
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-16 03:06:36 +00:00
David Benjamin
6e3f5cc7e1 Add SSL_CTX_get_keylog_callback.
Conscrypt would like to write a CTS test that the callback isn't set
unexpectedly.

Change-Id: I11f987422daf0544e90f5cff4d7aaf557ac1f5a2
Reviewed-on: https://boringssl-review.googlesource.com/11060
Reviewed-by: Kenny Root <kroot@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-09-14 19:20:33 +00:00
David Benjamin
45bdb2e1e3 Remove identical tests.
I'm not sure what happened here. These are both the same as
MissingKeyShare-Client.

Change-Id: I6601ed378d8639c1b59034f1e96c09a683bb62ca
Reviewed-on: https://boringssl-review.googlesource.com/11007
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-09-13 15:50:35 +00:00
David Benjamin
3d458dc048 Revert of Determining certificate_auth and key_exchange based on SSL.
Reason for revert:  Right now in TLS 1.3, certificate_auth is exactly
the same as whether we're doing resumption. With the weird reauth
stuff punted to later in the spec, having extra state is just more
room for bugs to creep in.

Original issue's description:
> Determining certificate_auth and key_exchange based on SSL.
> 
> This allows us to switch TLS 1.3 to use non-cipher based negotiation
> without needing to use separate functions between 1.3 and below.
> 
> BUG=77
> 
> Change-Id: I9207e7a6793cb69e8300e2c15afe3548cbf82af2
> Reviewed-on: https://boringssl-review.googlesource.com/10803
> 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>
> 

Change-Id: I240e3ee959ffd1f2481a06eabece3af554d20ffa
Reviewed-on: https://boringssl-review.googlesource.com/11008
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-12 22:54:08 +00:00
David Benjamin
639846e5e4 Add tests for trailing data in handshake messages.
It's easy to forget to check those. Unfortunately, it's also easy to
forget to check inner structures, which is going to be harder to stress,
but do these to start with. In doing, so fix up and unify some
error-handling, and add a missing check when parsing TLS 1.2
CertificateRequest.

This was also inspired by the recent IETF posting.

Change-Id: I27fe3cd3506258389a75d486036388400f0a33ba
Reviewed-on: https://boringssl-review.googlesource.com/10963
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-12 21:00:50 +00:00
David Benjamin
cd2c806530 Factor per-message test machinery out.
This will let us use the same test scenarios for testing messages with
trailing garbage or skipped messages.

Change-Id: I9f177983e8dabb6c94d3d8443d224b79a58f40b1
Reviewed-on: https://boringssl-review.googlesource.com/10962
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-09-12 19:56:40 +00:00
David Benjamin
54091230cd Use C99 for size_t loops.
This was done just by grepping for 'size_t i;' and 'size_t j;'. I left
everything in crypto/x509 and friends alone.

There's some instances in gcm.c that are non-trivial and pulled into a
separate CL for ease of review.

Change-Id: I6515804e3097f7e90855f1e7610868ee87117223
Reviewed-on: https://boringssl-review.googlesource.com/10801
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:44:24 +00:00
Steven Valdez
84b5c003cc Serialize unresumable sessions to a placeholder value.
Change-Id: I676d7fb00d63d74946b96c22ae2705072033c5f7
Reviewed-on: https://boringssl-review.googlesource.com/10620
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-12 19:25:02 +00:00
Steven Valdez
bd09eccd6d Determining certificate_auth and key_exchange based on SSL.
This allows us to switch TLS 1.3 to use non-cipher based negotiation
without needing to use separate functions between 1.3 and below.

BUG=77

Change-Id: I9207e7a6793cb69e8300e2c15afe3548cbf82af2
Reviewed-on: https://boringssl-review.googlesource.com/10803
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-12 19:23:14 +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
David Benjamin
fc0c9d9665 Use a common state to begin the handshake.
This simplifies the logic around SSL_clear to reset the state for a new
handshake. The state around here is still a little iffy, but this is a
slight improvement.

The SSL_ST_CONNECT and SSL_ST_ACCEPT states are still kept separate to
avoid problems with the info callback reporting SSL_ST_INIT. Glancing
through info callback consumers, although they're all debugging, they
tend to assume that all intermediate states either have only
SSL_ST_CONNECT set or only SSL_ST_ACCEPT set.

(They also all look identical which makes me think it's copy-and-pasted
from OpenSSL command-line tool or something.)

Change-Id: I55503781e52b51b4ca829256c14de6f5942dae51
Reviewed-on: https://boringssl-review.googlesource.com/10760
Reviewed-by: Adam Langley <agl@google.com>
2016-09-12 19:00:50 +00:00
David Benjamin
5510863fbd Temporary remove the TLS 1.3 anti-downgrade mechanism.
This mechanism is incompatible with deploying draft versions of TLS 1.3.

Suppose a draft M client talks to a draft N server, M != N. (Either M or
N could also be the final standard revision should there be lingering
draft clients or servers.) The server will notice the mismatch and
pretend ClientHello.version is TLS 1.2, not TLS 1.3. But this will
trigger anti-downgrade signal and cause an interop failure! And if it
doesn't trigger, all the clever tricks around ServerHello.random being
signed in TLS 1.2 are moot.

We'll put this back when the dust has settled.

Change-Id: Ic3cf72b7c31ba91e5cca0cfd7a3fca830c493a43
Reviewed-on: https://boringssl-review.googlesource.com/11005
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-09-12 18:10:23 +00:00
David Benjamin
c241d79261 Add tests around compression methods.
Not that this matters in the slightest, but the recent IETF mailing
reminded me we don't test this.

Change-Id: I300c96d6a63733d538a7019a7cb74d4e65d0498f
Reviewed-on: https://boringssl-review.googlesource.com/10961
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-09 17:29:21 +00:00
David Benjamin
abe94e3b0d Test that SNI warning alerts are ignored.
Although RFC 6066 recommends against it, some servers send a warning
alert prior to ServerHello on SNI mismatch, and, per spec, TLS 1.2
allows it.

We're fine here, but add a test for it. It interacts interestingly with
TLS 1.3 forbidding warning alerts because it happens before version
negotiation.

Change-Id: I0032313c986c835b6ae1aa43da6ee0dad17a97c2
Reviewed-on: https://boringssl-review.googlesource.com/10800
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-09-09 16:20:25 +00:00
David Benjamin
f0e935d7ce Fold stack-allocated types into headers.
Now that we have the extern "C++" trick, we can just embed them in the
normal headers. Move the EVP_CIPHER_CTX deleter to cipher.h and, in
doing so, take away a little bit of boilerplate in defining deleters.

Change-Id: I4a4b8d0db5274a3607914d94e76a38996bd611ec
Reviewed-on: https://boringssl-review.googlesource.com/10804
Reviewed-by: Matt Braithwaite <mab@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-09-07 21:50:05 +00:00
David Benjamin
0fb46d4a35 const-correct a variable.
Change-Id: I00507014c55b2c7fd442a5aa2c3afcbf8c48049b
Reviewed-on: https://boringssl-review.googlesource.com/10741
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-09-06 18:19:37 +00:00
Matt Braithwaite
d17d74d73f Replace Scoped* heap types with bssl::UniquePtr.
Unlike the Scoped* types, bssl::UniquePtr is available to C++ users, and
offered for a large variety of types.  The 'extern "C++"' trick is used
to make the C++ bits digestible to C callers that wrap header files in
'extern "C"'.

Change-Id: Ifbca4c2997d6628e33028c7d7620c72aff0f862e
Reviewed-on: https://boringssl-review.googlesource.com/10521
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-01 22:22:54 +00:00
David Benjamin
57e929f3c8 Enable RSA-PSS in TLS 1.2 by default.
Add a test that RSA-PSS is available in TLS 1.2 by default, both for
signing and verifying. Note that if a custom SSL_PRIVATE_KEY_METHOD is
used and it sets signing preferences, it won't use RSA-PSS if it doesn't
know about it. (See *-Sign-Negotiate-* tests.)

Change-Id: I3776a0c95480188a135795f7ebf31f2b0e0626cc
Reviewed-on: https://boringssl-review.googlesource.com/10723
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-08-30 22:50:05 +00:00
David Benjamin
9962057197 Test that the early callback can affect max_version.
The early callback needs to run before even version negotiation has been
resolved.

Change-Id: Ibb449ccec07dedef19b7827400ef318fa2f422c0
Reviewed-on: https://boringssl-review.googlesource.com/10722
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-08-30 15:56:37 +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
Matt Braithwaite
9c8c418853 Remove RC4 ciphersuites from TLS.
For now, they can be restored by compiling with -DBORINGSSL_RC4_TLS.

Of note, this means that `MEDIUM' is now empty.

Change-Id: Ic77308e7bd4849bdb2b4882c6b34af85089fe3cc
Reviewed-on: https://boringssl-review.googlesource.com/10580
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Matt Braithwaite <mab@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-08-26 19:32:44 +00:00
Adam Langley
a6cd185a54 Set verify_result, even on failure.
If code tries to inspect the verify result in the case of a failure then
it seems reasonable that the error code should be in there.

Change-Id: Ic32ac9d340c2c10a405a7b6580f22a06427f041d
Reviewed-on: https://boringssl-review.googlesource.com/10641
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-08-26 17:47:40 +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
Matt Braithwaite
07e7806177 runner: use 3DES instead of RC4 where possible.
To ease the removal of RC4, use 3DES in cases where RC4 is not required,
but is just a placeholder for "ciphersuite that works in SSLv3."

Change-Id: Ib459173e68a662986235b556f330a7e0e02759d7
Reviewed-on: https://boringssl-review.googlesource.com/10523
Commit-Queue: Matt Braithwaite <mab@google.com>
Reviewed-by: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-08-24 20:27:49 +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
David Benjamin
0fc37ef082 Fix a number of sigalg scope issues.
peer_sigalgs should live on SSL_HANDSHAKE. This both releases a little
bit of memory after the handshake is over and also avoids the bug where
the sigalgs get dropped if SSL_set_SSL_CTX is called at a bad time. See
also upstream's 14e14bf6964965d02ce89805d9de867f000095aa.

This only affects consumers using the old SNI callback and not
select_certificate_cb.

Add a test that the SNI callback works as expected. In doing so, add an
SSL_CTX version of the signing preferences API. This is a property of
the cert/key pair (really just the key) and should be tied to that. This
makes it a bit easier to have the regression test work with TLS 1.2 too.

I thought we'd fixed this already, but apparently not... :-/

BUG=95

Change-Id: I75b02fad4059e6aa46c3b05183a07d72880711b3
Reviewed-on: https://boringssl-review.googlesource.com/10445
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 00:24:34 +00:00
David Benjamin
7c7d8313ab Trim a few extensions when min_version is TLS 1.3.
None of these extensions may be negotiated in TLS 1.3 and are otherwise
on by default. Make the future QUIC/TLS1.3 ClientHello a hair smaller.

Change-Id: I613c339d95470676c78f21fd29e888b7701692c6
Reviewed-on: https://boringssl-review.googlesource.com/10504
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-23 00:28:50 +00:00
David Benjamin
5c4e8571cc Fill in the curve ID for TLS 1.3.
Apparently we forgot to do this.

Change-Id: I348cf6d716ae888fddce69ba4801bf09446f5a72
Reviewed-on: https://boringssl-review.googlesource.com/10503
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-22 18:05:01 +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
Steven Valdez
cb96654404 Adding ARRAY_SIZE macro for getting the size of constant arrays.
Change-Id: Ie60744761f5aa434a71a998f5ca98a8f8b1c25d5
Reviewed-on: https://boringssl-review.googlesource.com/10447
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 19:30:39 +00:00
David Benjamin
04aa694363 Implement BORINGSSL_UNSAFE_FUZZER_MODE for TLS 1.3.
I'll hold on regenerating the transcripts until either the protocol has
stablized more or we're ready to start actually deploying some of this,
but we can get this in now.

Confirmed these #ifdef points are covered by tests:
- BadFinished-*-TLS13
- *-InvalidSignature-*-TLS13

BUG=79

Change-Id: I5f6b9d0f50ac33d5cc79688928fb3fdf6df845ae
Reviewed-on: https://boringssl-review.googlesource.com/10500
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 19:11:34 +00:00
David Benjamin
0e95015aa5 RSA-PSS should work in TLS 1.2.
However, for now, we will only enable it if TLS 1.3 is offered.

BUG=85

Change-Id: I958ae0adeafee553dbffb966a6fa41f8a81cef96
Reviewed-on: https://boringssl-review.googlesource.com/10342
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-08-19 18:44:26 +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