Commit Graph

392 Commits

Author SHA1 Message Date
David Benjamin
f2fedefdca Simplify HelloVerifyRequest processing.
Rather than switching the order of the ServerHello and HelloVerifyRequest
states and processing each twice, have the states follow the protocol order.
HelloVerifyRequest reading is optional and ServerHello is strict. Use the
send_cookie bit to determine whether we're expecting a cookie or not.

Fix the dtls1_stop_timer call in these states to consistently hit the end of a
server flight; the previous flight should not be cleared from the retransmit
buffer until the entire next flight is received. That said, OpenSSL doesn't
appear to implement the part where, on receipt of the previous peer flight, the
buffered flight is retransmitted. (With the exception of a SSL3_MT_FINISHED
special-case in dtls1_read_bytes.) So if the peer is also OpenSSL, this doesn't
do anything.

Also fix the DTLS test which wasn't actually asserting that the ClientHello
matched.

Change-Id: Ia542190972dbffabb837d32c9d453a243caa90b2
Reviewed-on: https://boringssl-review.googlesource.com/1551
Reviewed-by: Adam Langley <agl@google.com>
2014-08-18 18:05:10 +00:00
David Benjamin
76d8abe7fd Get SSL 3.0 server tests working.
The missing SSL 3.0 client support in runner.go was fairly minor.

Change-Id: Ibbd440c9b6be99be08a214dec6b93ca358d8cf0a
Reviewed-on: https://boringssl-review.googlesource.com/1516
Reviewed-by: Adam Langley <agl@google.com>
2014-08-14 21:42:36 +00:00
David Benjamin
6fd297bb62 Add initial DTLS tests.
Change-Id: I7407261bdb2d788c879f2e67e617a615d9ff8f8b
Reviewed-on: https://boringssl-review.googlesource.com/1505
Reviewed-by: Adam Langley <agl@google.com>
2014-08-14 16:55:45 +00:00
David Benjamin
5a593af42a Move configuration into a dedicated TestConfig struct.
This removes some duplicate code in parsing command-line flags and, more
importantly, makes configuration available when constructing the SSL_CTX and
avoids a number of globals.

Change-Id: I26e2d2285b732f855a2c82752bc8e0db480c3b30
Reviewed-on: https://boringssl-review.googlesource.com/1502
Reviewed-by: Adam Langley <agl@google.com>
2014-08-13 23:44:50 +00:00
David Benjamin
83c0bc94d7 Test-only DTLS implementation in runner.go.
Run against openssl s_client and openssl s_server. This seems to work for a
start, although it may need to become cleverer to stress more of BoringSSL's
implementation for test purposes.

In particular, it assumes a reliable, in-order channel. And it requires that
the peer send handshake fragments in order. Retransmit and whatnot are not
implemented. The peer under test will be expected to handle a lossy channel,
but all loss in the channel will be controlled. MAC errors, etc., are fatal.

Change-Id: I329233cfb0994938fd012667ddf7c6a791ac7164
Reviewed-on: https://boringssl-review.googlesource.com/1390
Reviewed-by: Adam Langley <agl@google.com>
2014-08-13 23:43:38 +00:00
David Benjamin
98e882ead1 Clean up s23_srvr.c.
ssl23_get_client_hello has lots of remnants of SSLv2 support and remnants of an
even older SSL_OP_NON_EXPORT_FIRST option (see upstream's
d92f0bb6e9ed94ac0c3aa0c939f2565f2ed95935) which complicates the logic.

Split it into three states and move V2ClientHello parsing into its own
function. Port it to CBS and CBB to give bounds checks on the V2ClientHello
parse.

This fixes a minor bug where, if the SSL_accept call in ssl23_get_client_hello
failed, cb would not be NULL'd and SSL_CB_ACCEPT_LOOP would get reported an
extra time.

It also unbreaks the invariant between s->packet, s->packet_length,
s->s3->rbuf.buf, and s->s3->rbuf.offset at the point the switch, although this
was of no consequence because the first ssl3_read_n call passes extend = 0
which resets s->packet and s->packet_length.

It also makes us tolerant to major version bumps in the ClientHello. Add tests
for TLS tolerance of both minor and major version bumps as well as the HTTP
request error codes.

Change-Id: I948337f4dc483f4ebe1742d3eba53b045b260257
Reviewed-on: https://boringssl-review.googlesource.com/1455
Reviewed-by: Adam Langley <agl@google.com>
2014-08-12 21:10:56 +00:00
David Benjamin
1e7f8d70ec Move SendV2ClientHello to handshake coverage tests.
It's a different handshake flow with more state machine coverage. We should
make sure to test the asynchronous version.

Change-Id: I0bb79ca7e6a86bd3cac66bac1f795a885d474909
Reviewed-on: https://boringssl-review.googlesource.com/1454
Reviewed-by: Adam Langley <agl@google.com>
2014-08-08 17:40:18 +00:00
David Benjamin
bed9aae757 Add RenewTicketOnResume tests.
Didn't have coverage for abbreviated handshakes with NewSessionTicket. Also add
some missing resumeSession flags so the tests match the comments.

Change-Id: Ie4d76e8764561f3f1f31e1aa9595324affce0db8
Reviewed-on: https://boringssl-review.googlesource.com/1453
Reviewed-by: Adam Langley <agl@google.com>
2014-08-08 17:39:57 +00:00
David Benjamin
9821454f2b Add tests for CVE-2014-3511.
Also change MaxHandshakeRecordLength to 1 in the handshake coverage tests to
better stress the state machine.

Change-Id: I27fce2c000b3d4818fd2e9a47fb09d3f646dd1bd
Reviewed-on: https://boringssl-review.googlesource.com/1452
Reviewed-by: Adam Langley <agl@google.com>
2014-08-08 17:39:47 +00:00
David Benjamin
7e2e6cf1a0 Add test coverage for TLS version negotiation.
Test all pairs of client and server version, except for the ones that require
SSLv3 client support in runner.go. That is, as yet, still missing.

Change-Id: I601ab49c5526cd2eb4f85d5d535570e32f218d5b
Reviewed-on: https://boringssl-review.googlesource.com/1450
Reviewed-by: Adam Langley <agl@google.com>
2014-08-08 17:39:16 +00:00
David Benjamin
a8e3e0e936 Remove SSL_OP_TLS_ROLLBACK_BUG.
It's not part of SSL_OP_ALL and is unused, so remove it. Add a test that
asserts the version check works.

Change-Id: I917516594ec5a4998a8316782f035697c33d99b0
Reviewed-on: https://boringssl-review.googlesource.com/1418
Reviewed-by: Adam Langley <agl@google.com>
2014-08-08 17:04:53 +00:00
Kenny Root
7fdeaf1101 Retry sending record split fragment when SSL write fails.
When the write size was exactly SSL3_RT_MAX_PLAIN_LENGTH+1 and record
splitting is needed, an extra byte would be added to the max size of the
message to be written. This would cause the requested size to not exceed
the max. If the SSL_WANT_WRITE error were returned, the next packet
would not get the extra byte added to the max packet size since
record_split_done is set. Since a different set of arguments
(SSL3_RT_MAX_PLAIN_LENGTH+1 vs SSL3_RT_MAX_PLAIN_LENGTH) would be passed
to do_ssl3_write, it would return an "SSL3_WRITE_PENDING:bad write
retry" error.

To avoid a failure in the opposite direction, the max variable increment
is removed as well. This can happen when SSL_MODE_ENABLE_PARTIAL_WRITE
is not enabled and the call to ssl3_write_bytes contains, e.g., a buffer
of 2*SSL3_RT_MAX_PLAIN_LENGTH, where the first call into do_ssl3_write
succeeds writing the first SSL3_RT_MAX_PLAIN_LENGTH bytes, but writing
the second SSL3_RT_MAX_PLAIN_LENGTH bytes fails. This means the first
time the the second section of SSL3_RT_MAX_PLAIN_LENGTH bytes has called
do_ssl3_write with "max" bytes, but next call to ssl3_write_bytes in
turn calls into do_ssl3_write with "max+1" bytes.

Change-Id: Icf8453195c1145a54d31b8e8146801118207df03
Reviewed-on: https://boringssl-review.googlesource.com/1420
Reviewed-by: Kenny Root <kroot@google.com>
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 00:08:44 +00:00
David Benjamin
43ec06f705 Test state machine asynchronous behavior.
Add a framework for testing the asynchronous codepath. Move some handshake
state machine coverage tests to cover a range of record-layer and
handshake-layer asynchronicity.

This adds tests for the previous two async bugs fixed.

Change-Id: I422ef33ba3eeb0ad04766871ed8bc59b677b169e
Reviewed-on: https://boringssl-review.googlesource.com/1410
Reviewed-by: Adam Langley <agl@google.com>
2014-08-05 20:41:58 +00:00
David Benjamin
f4e5c4e106 runner: Implement DHE-RSA.
Use it to test DHE-RSA in BoringSSL.

Change-Id: I88f7bfa76507a6f60234d61d494c9f94b7df4e0a
Reviewed-on: https://boringssl-review.googlesource.com/1377
Reviewed-by: Adam Langley <agl@google.com>
2014-08-04 20:12:58 +00:00
David Benjamin
884fdf1616 runner: Take the build directory as flag.
Don't hardcode the directory.

Change-Id: I5c778a4ff16e00abbac2959ca9c9b4f4c40576f7
Reviewed-on: https://boringssl-review.googlesource.com/1376
Reviewed-by: Adam Langley <agl@google.com>
2014-08-04 20:11:07 +00:00
David Benjamin
2bc8e6fc9a runner: Take the number of workers as a flag.
Default to the number of CPUs. Avoids the tests launching 64 valgrinds in
parallel on machines without gobs of memory.

Change-Id: I9eeb365b48aa7407e303d161f90ce69a591a884c
Reviewed-on: https://boringssl-review.googlesource.com/1375
Reviewed-by: Adam Langley <agl@google.com>
2014-08-04 20:10:58 +00:00
David Benjamin
bef270a9cf Add server-side FallbackSCSV tests.
Assert that inappropriate fallbacks are detected, but if the client_version
matches the server's highest version, do not abort the handshake.

Change-Id: I9d72570bce45e1eb23fc2b74a3c5fca10562e573
Reviewed-on: https://boringssl-review.googlesource.com/1373
Reviewed-by: Adam Langley <agl@google.com>
2014-08-04 20:10:43 +00:00
David Benjamin
d86c7671a8 Add a test to assert parsing V2ClientHellos works.
Should have test coverage there as long as we care about supporting it.

Change-Id: Ic67539228b550f2ebd0b543d5a58640913b0474b
Reviewed-on: https://boringssl-review.googlesource.com/1371
Reviewed-by: Adam Langley <agl@google.com>
2014-08-04 20:10:29 +00:00
David Benjamin
7e3305eebd Fix False Start without session tickets.
One of the state transitions wasn't rewritten to CR_CHANGE. Add a test to
exercise this codepath. Also SSL_cutthrough_complete references the state.

Change-Id: Ib2f7ac5ac3f0348864efa93cf13cfd87454572f0
Reviewed-on: https://boringssl-review.googlesource.com/1337
Reviewed-by: Adam Langley <agl@google.com>
2014-07-28 20:31:56 +00:00
David Benjamin
86271ee9f8 Change CCS_OK to EXPECT_CCS.
Now that the flag is set accurately, use it to enforce that the handshake and
CCS synchronization. If EXPECT_CCS is set, enforce that:

(a) No handshake records may be received before ChangeCipherSpec.

(b) There is no pending handshake data at the point EXPECT_CCS is set.

Change-Id: I04b228fe6a7a771cf6600b7d38aa762b2d553f08
Reviewed-on: https://boringssl-review.googlesource.com/1299
Reviewed-by: Adam Langley <agl@google.com>
2014-07-25 17:58:58 +00:00
David Benjamin
d23f412a8a Improve test coverage around NewSessionTicket message.
Test both when the peer doesn't support session tickets and when the server
promises a NewSessionTicket message but doesn't deliver.

Change-Id: I48f338094002beac2e6b80e41851c72822b3b9d5
Reviewed-on: https://boringssl-review.googlesource.com/1300
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:09:55 +00:00
David Benjamin
f3ec83dee0 Add EarlyChangeCipherSpec tests.
Adapted from patch in https://www.imperialviolet.org/2014/06/05/earlyccs.html.

Change-Id: I14bf314d105780e23e6bd09217870deff5744979
Reviewed-on: https://boringssl-review.googlesource.com/1292
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:05:13 +00:00
David Benjamin
1d5c83e063 Add test coverage for session resumption with tickets.
The shim is now passed two file descriptors. In a session resumption test, the
second is used in an abbreviated handshake immediately after the first.

Change-Id: I1f348c05f1a8ff2881fb46fc9e869696f74071c6
Reviewed-on: https://boringssl-review.googlesource.com/1291
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:04:52 +00:00
David Benjamin
b61b4de6df Fix runner.go session ticket support.
Some test code to insert a bogus session ticket was retained. Also,
decryptTicket mutated its input, in turn, mutating the ClientHello,
breaking the Finished hash.

The latter fix should probably be merged upstream.

Change-Id: I6949f842c67e19df8742561fb0b849af9f5f099d
Reviewed-on: https://boringssl-review.googlesource.com/1290
Reviewed-by: Adam Langley <agl@google.com>
2014-07-23 15:31:24 +00:00
David Benjamin
42be6456ca Add SkipChangeCipherSpec-Server-NPN test.
Finished isn't always the first post-CCS message.

Change-Id: I4f70eeed57cf732693d07212b096efb2594c5b3c
Reviewed-on: https://boringssl-review.googlesource.com/1288
Reviewed-by: Adam Langley <agl@google.com>
2014-07-21 19:24:32 +00:00
David Benjamin
de620d9c87 runner: Require the CertificateVerify signature and hash to match.
This change can probably be ported over to upstream crypto/tls. The current Go
TLS implementation ignores the signature and hash algorithm lists in
CertificateVerify and CertificateRequest. Take these into account so that our
tests assert OpenSSL fills them out correctly.

Also fix a bug in the original code where 'err' within the switch block get
shadowed.

Change-Id: I5d9c0b31ebb4662ecc767ed885a20707f0e86216
Reviewed-on: https://boringssl-review.googlesource.com/1253
Reviewed-by: Adam Langley <agl@google.com>
2014-07-21 15:59:48 +00:00
David Benjamin
a0e5223bb8 Add SkipChangeCipherSpec tests.
They pass, but this is an error case that is probably worth a test.

Change-Id: I37b2eec34a1781fa8342eea57ee4f9da81ce17ed
Reviewed-on: https://boringssl-review.googlesource.com/1257
Reviewed-by: Adam Langley <agl@google.com>
2014-07-21 02:14:13 +00:00
David Benjamin
407a10cf43 Fix parsing of CertificateRequests.
Got one of the conditions flipped.

Change-Id: I327a9c13e42865459e8d69a431b0d3a2bc6b54a5
Reviewed-on: https://boringssl-review.googlesource.com/1210
Reviewed-by: Adam Langley <agl@google.com>
2014-07-16 17:07:15 +00:00
David Benjamin
1f5f62b204 Add a server NPN test.
Change-Id: Ib34a24e86bb5de117ecf5609918e130c1ff9532e
Reviewed-on: https://boringssl-review.googlesource.com/1161
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:08:01 +00:00
David Benjamin
67666e7823 Add tests for the server accepting client certificates.
Change-Id: I9acc4363c6b9804d5fe464053393cf16ffb7785c
Reviewed-on: https://boringssl-review.googlesource.com/1159
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 21:45:57 +00:00
David Benjamin
9c651c9ef6 Be strict about requiring ServerKeyExchange.
Missing ServerKeyExchange is handled, but only because it hits an
ERR_R_INTERNAL_ERROR in ssl3_send_client_key_exchange in trying to find the
server ECDH parameters. Be strict about requiring it for ECDHE.

Change-Id: Ifce5b73c8bd14746b8a2185f479d550e9e3f84df
Reviewed-on: https://boringssl-review.googlesource.com/1157
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 21:42:30 +00:00
David Benjamin
e8f3d666cc Be strict about expecting a server Certificate message.
Introduce a ssl_cipher_has_server_public_key to save the repeated
NULL/PSK/RSA_PSK[*] check. Don't allow skipping to ServerKeyExchange when
expecting Certificate; the messages expected are determined by the cipher
suite. The ssl3_get_server_public_key call is already guarded.

As the previous test demonstrates, this is safe because of the
ssl3_check_cert_and_algorithm call, but avoid the looseness in the parsing
there.

[*] NB: we don't implement RSA_PSK, and OpenSSL has never implemented it.

Change-Id: I0571e6bcbeb8eb883f77878bdc98d1aa3a287cf3
Reviewed-on: https://boringssl-review.googlesource.com/1156
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 20:50:30 +00:00
David Benjamin
1c375dda8a Add UnauthenticatedECDH bug test.
This works, but there's enough shared codepaths that it's worth a test to
ensure it stays that way.

Change-Id: I5d5a729811e35832170322957258304213204e3b
Reviewed-on: https://boringssl-review.googlesource.com/1155
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 20:50:09 +00:00
David Benjamin
636293bf25 Add client auth tests.
Change-Id: If3ecae4c97f67085b9880ffa49dd616f1436ce97
Reviewed-on: https://boringssl-review.googlesource.com/1112
Reviewed-by: Adam Langley <agl@google.com>
2014-07-09 21:04:06 +00:00
David Benjamin
7b03051103 Add a test for certificate types parsing.
Change-Id: Icddd39ae183f981f78a65427a4dda34449ca389a
Reviewed-on: https://boringssl-review.googlesource.com/1111
Reviewed-by: Adam Langley <agl@google.com>
2014-07-09 21:03:54 +00:00
David Benjamin
35a7a4492d Check duplicate extensions before processing.
ClientHello and ServerHello are not allowed to include duplicate extensions.
Add a new helper function to check this and call as appropriate. Remove ad-hoc
per-extension duplicate checks which are no unnecessary.

Add runner.go tests to verify such message correctly rejected.

Change-Id: I7babd5b642dfec941459512869e2dd6de26a831c
Reviewed-on: https://boringssl-review.googlesource.com/1100
Reviewed-by: Adam Langley <agl@google.com>
2014-07-08 22:17:59 +00:00
David Benjamin
197b3abaa8 Add a test that server_name extensions are parsed correctly.
Change-Id: Id4025835df49eb498df9a48fc81061541778569b
Reviewed-on: https://boringssl-review.googlesource.com/1092
Reviewed-by: Adam Langley <agl@google.com>
2014-07-02 22:52:59 +00:00
David Benjamin
025b3d3459 Add some basic server tests to runner.go.
client_shim.cc and runner.go are generalized to handle both ends. Plumb a bit
through the test case to control which and add server versions of all the
cipher suite tests.

Change-Id: Iab2640b390f7ed7160c9a9bf6bb34b8bec761b2e
Reviewed-on: https://boringssl-review.googlesource.com/1091
Reviewed-by: Adam Langley <agl@google.com>
2014-07-02 22:52:02 +00:00
David Benjamin
325b5c3667 Fix FallbackSCSV test.
It wasn't actually testing SSL_enable_fallback_scsv, just that not calling it
didn't send an SCSV. Plumb the 'flag' parameter to testCase through and add a
test that asserts it does get sent when expected. (Make it a []string since Go
doesn't distinguish nil string from "" and for flexibility.)

Change-Id: I124c01e045aebbed5c1d87b7196de7c2026f26f3
Reviewed-on: https://boringssl-review.googlesource.com/1071
Reviewed-by: Adam Langley <agl@google.com>
2014-07-02 00:30:29 +00:00
Adam Langley
ac61fa379f Implement TLS_FALLBACK_SCSV support for the client.
With this change, calling SSL_enable_fallback_scsv on a client SSL* will
cause the fallback SCSV to be sent.

This is intended to be set when the client is performing TLS fallback
after a failed connection. (This only happens if the application itself
implements this behaviour: OpenSSL does not do fallback automatically.)

The fallback SCSV indicates to the server that it should reject the
connection if the version indicated by the client is less than the
version supported by the server.

See http://tools.ietf.org/html/draft-bmoeller-tls-downgrade-scsv-02.

Change-Id: I478d6d5135016f1b7c4aaa6c306a1a64b1d215a6
2014-06-23 12:03:11 -07:00
Adam Langley
80842bdb44 Fix test of first of 255 CBC padding bytes.
Thanks to Peter Gijsels for pointing out that if a CBC record has 255
bytes of padding, the first was not being checked.
2014-06-20 13:17:37 -07:00
Adam Langley
95c29f3cd1 Inital import.
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).

(This change contains substantial changes from the original and
effectively starts a new history.)
2014-06-20 13:17:32 -07:00