Commit Graph

104 Commits

Author SHA1 Message Date
David Benjamin
87909c0445 Add tests for version negotiation failure alerts.
Ensure that both the client and the server emit a protocol_version alert
(except in SSLv3 where it doesn't exist) with a record-layer version which the
peer will recognize.

Change-Id: I31650a64fe9b027ff3d51e303711910a00b43d6f
2014-12-13 15:23:28 -08:00
David Benjamin
82c9e90a58 Merge SSLv23_method and DTLS_ANY_VERSION.
This makes SSLv23_method go through DTLS_ANY_VERSION's version negotiation
logic. This allows us to get rid of duplicate ClientHello logic. For
compatibility, SSL_METHOD is now split into SSL_PROTOCOL_METHOD and a version.
The legacy version-locked methods set min_version and max_version based this
version field to emulate the original semantics.

As a bonus, we can now handle fragmented ClientHello versions now.

Because SSLv23_method is a silly name, deprecate that too and introduce
TLS_method.

Change-Id: I8b3df2b427ae34c44ecf972f466ad64dc3dbb171
2014-12-13 15:22:21 -08:00
David Benjamin
accb454e44 Add min_version tests.
These tests use both APIs. This also modifies the inline version negotiation's
error codes (currently only used for DTLS) to align with SSLv23's error codes.
Note: the peer should send a protocol_version alert which is currently untested
because it's broken.

Upstream would send such an alert if TLS 1.0 was supported but not otherwise,
which is somewhat bizarre. We've actually regressed and never send the alert in
SSLv23. When version negotiation is unified, we'll get the alerts back.

Change-Id: I4c77bcef3a3cd54a039a642f189785cd34387410
Reviewed-on: https://boringssl-review.googlesource.com/2584
Reviewed-by: Adam Langley <agl@google.com>
2014-12-13 23:00:02 +00:00
David Benjamin
1eb367c03e Add min_version and max_version APIs.
Amend the version negotiation tests to test this new spelling of max_version.
min_version will be tested in a follow-up.

Change-Id: Ic4bfcd43bc4e5f951140966f64bb5fd3e2472b01
Reviewed-on: https://boringssl-review.googlesource.com/2583
Reviewed-by: Adam Langley <agl@google.com>
2014-12-13 22:48:20 +00:00
David Benjamin
f080ecd86d Don't infinite loop on garbage server input.
else block got lost in a rewrite of this code.

Change-Id: I51f1655474ec8bbd4eccb4297124e8584329444e
Reviewed-on: https://boringssl-review.googlesource.com/2560
Reviewed-by: Adam Langley <agl@google.com>
2014-12-11 23:55:38 +00:00
David Benjamin
226a872d2f Don't set client_version to the ServerHello version.
The client_version needs to be preserved, both for the RSA key exchange and
(when this codepath is used for TLS) for the SChannel renego workaround. Fix
the tests to enforce this so the cipher suite version tests catch this.

Change-Id: I0c42dc3ec4830f3724026b400e5066e7a7f1ee97
Reviewed-on: https://boringssl-review.googlesource.com/2551
Reviewed-by: Adam Langley <agl@google.com>
2014-12-11 18:49:42 +00:00
David Benjamin
1e29a6b7c5 Add assertions on the initial record version number.
The record-layer version of the ServerHello should match the final version. The
record-layer version of the ClientHello should be the advertised version, but
clamped at TLS 1.0. This is to ensure future rewrites do not regress this.

Change-Id: I96f1f0674944997ff38b562453a322ce61652635
Reviewed-on: https://boringssl-review.googlesource.com/2540
Reviewed-by: Adam Langley <agl@google.com>
2014-12-11 00:04:37 +00:00
Feng Lu
41aa325c6a ClientHello Padding for Fast Radio Opening in 3G.
The ClientHello record is padded to 1024 bytes when
fastradio_padding is enabled. As a result, the 3G cellular radio
is fast forwarded to DCH (high data rate) state. This mechanism
leads to a substantial redunction in terms of TLS handshake
latency, and benefits mobile apps that are running on top of TLS.

Change-Id: I3d55197b6d601761c94c0f22871774b5a3dad614
2014-12-04 14:30:16 -08:00
David Benjamin
d8a3e78223 Shush a MSVC bool/int comparison warning.
MSVC doesn't like it when you compare the two.

Change-Id: I03c5ff2e2668ac2e536de8278e3a7c98a3dfd117
Reviewed-on: https://boringssl-review.googlesource.com/2460
Reviewed-by: Adam Langley <agl@google.com>
2014-12-04 00:22:31 +00:00
David Benjamin
0105ece171 Fix standalone Windows build.
Don't link with dl, except on Linux where we have malloc tests.

Change-Id: I7b23acc854172e64628a55acecfaa9a661f74f77
Reviewed-on: https://boringssl-review.googlesource.com/2453
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 20:27:57 +00:00
David Benjamin
94d701b7e8 Left-pad a V2ClientHello's random, not right-pad.
The comment has it right, but the rewritten code was wrong.

Change-Id: I450193c39fb62eae32aae090a3834dd83db53421
Reviewed-on: https://boringssl-review.googlesource.com/2444
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:44:12 +00:00
David Benjamin
128dbc30f6 Factor out the client max-version logic into a helper function.
Replace the comment with a clearer one and reimplement it much more tidily. The
mask thing was more complicated than was needed.

This slightly changes behavior on the DTLS_ANY_VERSION side in that, if only
one method is enabled, we no longer short-circuit to the version-locked method
early. This "optimization" seems unnecessary.

Change-Id: I571c8b60ed16bd4357c67d65df0dd1ef9cc5eb57
Reviewed-on: https://boringssl-review.googlesource.com/2451
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:42:39 +00:00
David Benjamin
cde8abae14 Merge client/server SSL_METHODs into the generic one.
Supporting both schemes seems pointless. Now that s->server and s->state are
set appropriately late and get_ssl_method is gone, the only difference is that
the client/server ones have non-functional ssl_accept or ssl_connect hooks. We
can't lose the generic ones, so let's unify on that.

Note: this means a static linker will no longer drop the client or server
handshake code if unused by a consumer linking statically. However, Chromium
needs the server half anyway for DTLS and WebRTC, so that's probably a lost
cause. Android also exposes server APIs.

Change-Id: I290f5fb4ed558f59fadb5d1f84e9d9c405004c23
Reviewed-on: https://boringssl-review.googlesource.com/2440
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:35:15 +00:00
David Benjamin
8c88153465 Remove a place where SSL_clear cleans up after client/server confusion.
SSL_clear sets s->state and dtls1_clear sets cookie_len on the server. Setting
cookie_len on the server seems to serve no purpose but to let the callback know
how large the buffer is. This can be done just before calling the callback.

It also avoids a bug where the cookie check can be bypassed, should the server
not specify an app_verify_cookie_cb, by supplying a cookie of all zeros of the
maximum size. (Zero is fine because an empty cookie is rejected.)

The goal here is to avoid needing the SSL_clear calls in the handshake
functions. They are currently needed to fix the cookie_len setting when using
the generic method. (They get set wrong and then flipped back.)

Change-Id: I5095891bc0f7df62d83a9c84312fcf0b84826faa
Reviewed-on: https://boringssl-review.googlesource.com/2435
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:31:57 +00:00
David Benjamin
c44b1df459 Add test for renego client_version quirk.
In upstream's f4e1169341ad1217e670387db5b0c12d680f95f4, the client_version was
made constant across renegotiations, even if the server negotiated a lower
version. NSS has the same quirk, reportedly for SChannel:

https://code.google.com/p/chromium/codesearch#chromium/src/net/third_party/nss/ssl/ssl3con.c&sq=package:chromium&l=5103

Add a test to ensure we do not regress this.

Change-Id: I214e062463c203b86a9bab00f8503442e1bf74fe
Reviewed-on: https://boringssl-review.googlesource.com/2405
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:29:23 +00:00
David Benjamin
8b8c006564 Fix DTLS_ANY_VERSION and add tests.
This fixes bugs that kept the tests from working:

- Resolve DTLS version and cookie before the session.

- In DTLS_ANY_VERSION, ServerHello should be read with first_packet = 1. This
  is a regression from f2fedefdca. We'll want to
  do the same for TLS, but first let's change this to a boolean has_version in a
  follow-up.

Things not yet fixed:

- DTLS code is not EVP_AEAD-aware. Those ciphers are disabled for now.

- On the client, DTLS_ANY_VERSION creates SSL_SESSIONs with the wrong
  ssl_version. The tests pass because we no longer enforce the match as of
  e37216f56009fbf48c3a1e733b7a546ca6dfc2af. (In fact, we've gone from the server
  ignoring ssl_version and client enforcing to the client mostly ignoring
  ssl_version and the server enforcing.)

- ssl3_send_client_hello's ssl_version check checks for equality against
  s->version rather than >.

Change-Id: I5a0dde221b2009413df9b9443882b9bf3b29519c
Reviewed-on: https://boringssl-review.googlesource.com/2403
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:27:54 +00:00
David Benjamin
65ea8ff84c Debug resumption connections with -debug too.
Change-Id: Ib33cceed561698310f369d63de602123af146a45
Reviewed-on: https://boringssl-review.googlesource.com/2402
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:27:33 +00:00
David Benjamin
61f95277d4 Add tests for OCSP stapling and SCT lists.
We forgot to add those when we implemented the features. (Also relevant because
they will provide test coverage later for configuring features when using the
generic method tables rather than *_client_method.)

Change-Id: Ie08b27de893095e01a05a7084775676616459807
Reviewed-on: https://boringssl-review.googlesource.com/2410
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:26:01 +00:00
David Benjamin
fe8eb9a603 Add tests for session-ID-based resumption.
This implements session IDs in client and server in runner.go.

Change-Id: I26655f996b7b44c7eb56340ef6a415d3f2ac3503
Reviewed-on: https://boringssl-review.googlesource.com/2350
Reviewed-by: Adam Langley <agl@google.com>
2014-11-21 21:35:39 +00:00
David Benjamin
ae3e487d51 Fix a couple more malloc test crashes.
The ex_data index may fail to be allocated. Also don't leave a dangling pointer
in handshake_dgst if EVP_DigestInit_ex fails and check a few more init function
failures.

Change-Id: I2e99a89b2171c9d73ccc925a2f35651af34ac5fb
Reviewed-on: https://boringssl-review.googlesource.com/2342
Reviewed-by: Adam Langley <agl@google.com>
2014-11-19 22:17:50 +00:00
Adam Langley
69a01608f3 Add malloc failure tests.
This commit fixes a number of crashes caused by malloc failures. They
were found using the -malloc-test=0 option to runner.go which runs tests
many times, causing a different allocation call to fail in each case.

(This test only works on Linux and only looks for crashes caused by
allocation failures, not memory leaks or other errors.)

This is not the complete set of crashes! More can be found by collecting
core dumps from running with -malloc-test=0.

Change-Id: Ia61d19f51e373bccb7bc604642c51e043a74bd83
Reviewed-on: https://boringssl-review.googlesource.com/2320
Reviewed-by: Adam Langley <agl@google.com>
2014-11-19 01:24:46 +00:00
David Benjamin
000800a306 Add tests for signature algorithm negotiation.
Change-Id: I5a263734560997b774014b5742877aa4b2940664
Reviewed-on: https://boringssl-review.googlesource.com/2289
Reviewed-by: Adam Langley <agl@google.com>
2014-11-18 22:30:03 +00:00
David Benjamin
ca6c82643a Add DTLS-SRTP tests.
Just the negotiation portion as everything else is external. This feature is
used in WebRTC.

Change-Id: Iccc3983ea99e7d054b59010182f9a56a8099e116
Reviewed-on: https://boringssl-review.googlesource.com/2310
Reviewed-by: Adam Langley <agl@google.com>
2014-11-18 22:16:53 +00:00
Alex Chernyakhovsky
4cd8c43e73 Remove support for processing fragmented alerts
Prior to this change, BoringSSL maintained a 2-byte buffer for alerts,
and would support reassembly of fragmented alerts.

NSS does not support fragmented alerts, nor would any reasonable
implementation produce them. Remove fragmented alert handling and
produce an error if a fragmented alert has ever been encountered.

Change-Id: I31530ac372e8a90b47cf89404630c1c207cfb048
Reviewed-on: https://boringssl-review.googlesource.com/2125
Reviewed-by: Adam Langley <agl@google.com>
2014-11-13 22:58:30 +00:00
David Benjamin
bdf5e72f50 Don't resume sessions if the negotiated version doesn't match.
All of NSS, upstream OpenSSL, SChannel, and Secure Transport require, on the
client, that the ServerHello version match the session's version on resumption.
OpenSSL's current behavior is incompatible with all of these. Fall back to a
full handshake on the server instead of mismatch.

Add a comment on the client for why we are, as of
30ddb434bf, not currently enforcing the same in
the client.

Change-Id: I60aec972d81368c4ec30e2fd515dabd69401d175
Reviewed-on: https://boringssl-review.googlesource.com/2244
Reviewed-by: Adam Langley <agl@google.com>
2014-11-13 22:05:12 +00:00
David Benjamin
e18d821dfc runner: Refuse to resume sessions on mismatching versions.
Clients all consistently reject mismatches. If a different version was
negotiated, a server should ignore the resumption. This doesn't actually affect
current tests.  We really want to be making this change in BoringSSL (and then
upstream), but get the Go half into shape first.

Change-Id: Ieee7e141331d9e08573592e661889bd756dccfa9
Reviewed-on: https://boringssl-review.googlesource.com/2243
Reviewed-by: Adam Langley <agl@google.com>
2014-11-11 18:25:28 +00:00
David Benjamin
ca6554b133 Add tests for client-initiated renegotiation.
These'll get removed once most of renego support is gone, but this is to prove
removing the warning alert from the previous commit still prevents legacy
renegotiations.

Change-Id: I7d9d95e1d4c5d23d3b6d170938a5499a65f2d5ea
Reviewed-on: https://boringssl-review.googlesource.com/2236
Reviewed-by: Adam Langley <agl@google.com>
2014-11-11 00:00:49 +00:00
David Benjamin
9114fae39e Add a test for RSA ServerKeyExchange.
Ensure that the client rejects it with UNEXPECTED_MESSAGE, not by attempting to
decode it.

Change-Id: Ifc5613cf1152e0f7dcbee73e05df1ef367dfbfd5
Reviewed-on: https://boringssl-review.googlesource.com/2232
Reviewed-by: Adam Langley <agl@google.com>
2014-11-11 00:00:39 +00:00
David Benjamin
5e961c1ff1 Add DTLS replay tests.
At the record layer, DTLS maintains a window of seen sequence numbers to detect
replays. Add tests to cover that case. Test both repeated sequence numbers
within the window and sequence numbers past the window's left edge. Also test
receiving sequence numbers far past the window's right edge.

Change-Id: If6a7a24869db37fdd8fb3c4b3521b730e31f8f86
Reviewed-on: https://boringssl-review.googlesource.com/2221
Reviewed-by: Adam Langley <agl@google.com>
2014-11-10 23:58:56 +00:00
Adam Langley
cf2d4f4033 Test renegotiation with BoringSSL as the client.
This also contains a test for the issue fixed in
88333ef7d7.

Change-Id: Id705a82cee34c018491dc301eba8b5097b9c83d5
Reviewed-on: https://boringssl-review.googlesource.com/2083
Reviewed-by: Adam Langley <agl@google.com>
2014-11-04 01:25:31 +00:00
Adam Langley
2ae77d2784 Test server-side renegotiation.
This change adds support to the Go code for renegotiation as a client,
meaning that we can test BoringSSL's renegotiation as a server.

Change-Id: Iaa9fb1a6022c51023bce36c47d4ef7abee74344b
Reviewed-on: https://boringssl-review.googlesource.com/2082
Reviewed-by: Adam Langley <agl@google.com>
2014-11-03 23:18:58 +00:00
David Benjamin
2af684fa92 Add tests for ECDHE_PSK.
pskKeyAgreement is now a wrapper over a base key agreement.

Change-Id: Ic18862d3e98f7513476f878b8df5dcd8d36a0eac
Reviewed-on: https://boringssl-review.googlesource.com/2053
Reviewed-by: Adam Langley <agl@google.com>
2014-10-29 20:33:09 +00:00
David Benjamin
48cae08563 Add tests for PSK cipher suites.
Only the three plain PSK suites for now. ECDHE_PSK_WITH_AES_128_GCM_SHA256 will
be in a follow-up.

Change-Id: Iafc116a5b2798c61d90c139b461cf98897ae23b3
Reviewed-on: https://boringssl-review.googlesource.com/2051
Reviewed-by: Adam Langley <agl@google.com>
2014-10-29 20:32:21 +00:00
Adam Langley
7571292eac Extended master secret support.
This change implements support for the extended master secret. See
https://tools.ietf.org/html/draft-ietf-tls-session-hash-01
https://secure-resumption.com/

Change-Id: Ifc7327763149ab0894b4f1d48cdc35e0f1093b93
Reviewed-on: https://boringssl-review.googlesource.com/1930
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-10-24 21:19:44 +00:00
David Benjamin
7f7882f1a8 Remove obsolete TODO
Change-Id: I5b02f57615d4ab01efbf7199474ce4e43c6956b6
Reviewed-on: https://boringssl-review.googlesource.com/1994
Reviewed-by: Adam Langley <agl@google.com>
2014-10-20 19:19:07 +00:00
Adam Langley
3831173740 Fix memory leak when decoding corrupt tickets.
This is CVE-2014-3567 from upstream. See
https://www.openssl.org/news/secadv_20141015.txt

Change-Id: I9aad422bf1b8055cb251c7ff9346cf47a448a815
Reviewed-on: https://boringssl-review.googlesource.com/1970
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-10-20 19:05:48 +00:00
Ben Laurie
eba2384e53 Missing includes for FreeBSD.
Change-Id: I4ea02a41ed614047ecda156d0c572b04baa174e6
Reviewed-on: https://boringssl-review.googlesource.com/1852
Reviewed-by: Adam Langley <agl@google.com>
2014-09-30 19:15:15 +00:00
David Benjamin
01fe820ab9 Add tests for client version negotiation and session resumption.
BUG=chromium:417134

Change-Id: If5914be98026d899000fde267b2d329861ca3136
Reviewed-on: https://boringssl-review.googlesource.com/1822
Reviewed-by: Adam Langley <agl@google.com>
2014-09-25 22:09:18 +00:00
David Benjamin
b0c8db7347 runner: don't resume sessions if SessionTicketsDisabled is true.
Change-Id: I1cf4a11d66871fff71a5fa93e39471ffb40d3132
Reviewed-on: https://boringssl-review.googlesource.com/1821
Reviewed-by: Adam Langley <agl@google.com>
2014-09-24 23:56:03 +00:00
David Benjamin
fc7b086305 Test that ALPN is preferred over NPN.
Change-Id: Ia9d10f672c8a83f507b46f75869b7c00fe1a4fda
Reviewed-on: https://boringssl-review.googlesource.com/1755
Reviewed-by: Adam Langley <agl@google.com>
2014-09-15 21:10:59 +00:00
David Benjamin
ae2888fbdc Add tests for ALPN support.
Both as client and as server. Also tests that ALPN causes False Start to kick
in.

Change-Id: Ib570346f3c511834152cd2df2ef29541946d3ab4
Reviewed-on: https://boringssl-review.googlesource.com/1753
Reviewed-by: Adam Langley <agl@google.com>
2014-09-15 21:10:46 +00:00
David Benjamin
fa055a2b77 Implement ALPN in runner.go.
Imported from upstream's https://codereview.appspot.com/108710046.

Change-Id: I66c879dcc9fd09446ac1a8380f796b1d68c89e4e
Reviewed-on: https://boringssl-review.googlesource.com/1751
Reviewed-by: Adam Langley <agl@google.com>
2014-09-15 21:09:45 +00:00
David Benjamin
812152aa3b Don't deadlock if a resume test fails the first half.
Otherwise the child is busy waiting for its second handshake.

Change-Id: Ic613eeb04c5d6c1ec1e1bbcb13946d3ac31d05f1
Reviewed-on: https://boringssl-review.googlesource.com/1752
Reviewed-by: Adam Langley <agl@google.com>
2014-09-15 21:08:11 +00:00
David Benjamin
e78bfded9f Improve test coverage for server_name extension.
Notably, this would have caught ed8270a55c
(although, apart from staring at code coverage, knowing to set resumeSession on
the server test isn't exactly obvious). Perhaps we should systematically set it
on all extension server tests; ClientHello extension parsing happens after
resumption has been determined and is often sensitive to it.

Change-Id: Ie83f294a26881a6a41969e9dbd102d0a93cb68b5
Reviewed-on: https://boringssl-review.googlesource.com/1750
Reviewed-by: Adam Langley <agl@google.com>
2014-09-15 21:07:57 +00:00
David Benjamin
f7768e43b2 Test SHA-256 and SHA-384 CBC-mode cipher suites.
These were added in TLS 1.2. They are like the standard AES-CBC cipher suites,
but use different HMACs.

Change-Id: Ib89ddebd1aa398b1347f8285f5d827068b1bd181
Reviewed-on: https://boringssl-review.googlesource.com/1730
Reviewed-by: Adam Langley <agl@google.com>
2014-09-06 00:17:08 +00:00
David Benjamin
39ebf53dd3 Check the server did not use a TLS 1.2 cipher suite pre-TLS 1.2.
This check got refactored in OpenSSL 1.0.2 and broke in the process. Fix this
and add a test. Otherwise things like client auth can get slightly confused; it
will try to sign the MD5/SHA-1 hash, but the TLS 1.2 cipher suite may not use
SSL_HANDSHAKE_MAC_DEFAULT, so those digests won't be available.

Based on upstream's 226751ae4a1f3e00021c43399d7bb51a99c22c17.

Change-Id: I5b864d3a696f3187b849c53b872c24fb7df27924
Reviewed-on: https://boringssl-review.googlesource.com/1696
Reviewed-by: Adam Langley <agl@google.com>
2014-09-02 23:41:34 +00:00
David Benjamin
5c24a1d6b6 Add a test for SSL_OP_TLS_D5_BUG.
If this is part of SSL_OP_ALL, we should have a test for it.

Change-Id: Ia72422beb2da6434726e78e174f3416f90f7c897
Reviewed-on: https://boringssl-review.googlesource.com/1695
Reviewed-by: Adam Langley <agl@google.com>
2014-09-02 22:43:54 +00:00
David Benjamin
e098ec2460 Test client auth under TLS 1.2 hash mismatch and SSL 3.
Maintain a handshake buffer in prf.go to implement TLS 1.2 client auth. Also
use it for SSL 3. This isn't strictly necessary as we know the hash functions,
but Go's hash.Hash interface lacks a Copy method.

Also fix the server-side tests which failed to test every TLS version.

Change-Id: I98492c334fbb9f2f0f89ee9c5c8345cafc025600
Reviewed-on: https://boringssl-review.googlesource.com/1664
Reviewed-by: Adam Langley <agl@google.com>
2014-08-29 00:23:50 +00:00
David Benjamin
e58c4f5321 Add a test to ensure False Start occurs.
This adds the missing test coverage for
7e3305eebd.

Change-Id: I8c9f1dc998afa9bb1f6fb2a7872a651037bb4844
Reviewed-on: https://boringssl-review.googlesource.com/1610
Reviewed-by: Adam Langley <agl@google.com>
2014-08-26 17:41:53 +00:00
David Benjamin
a08e49d17a Add basic TLS Channel ID tests.
Change-Id: I7ccf2b8282dfa8f3985775e8b67edcf3c2949752
Reviewed-on: https://boringssl-review.googlesource.com/1606
Reviewed-by: Adam Langley <agl@google.com>
2014-08-26 17:40:36 +00:00