Commit Graph

159 Commits

Author SHA1 Message Date
David Benjamin
e0e7d0da68 Initialize the record buffers after the handshake check.
The new V2ClientHello sniff asserts, for safety, that nothing else has
initialized the record layer before it runs. However, OpenSSL allows you to
avoid explicitly calling SSL_connect/SSL_accept and instead let
SSL_read/SSL_write implicitly handshake for you. This check happens at a fairly
low-level in the ssl3_read_bytes function, at which point the record layer has
already been initialized.

Add some tests to ensure this mode works.

(Later we'll lift the handshake check to a higher-level which is probably
simpler.)

Change-Id: Ibeb7fb78e5eb75af5411ba15799248d94f12820b
Reviewed-on: https://boringssl-review.googlesource.com/3334
Reviewed-by: Adam Langley <agl@google.com>
2015-02-09 19:49:45 +00:00
David Benjamin
b80168e1b8 Test that False Start fails if the server second leg is omitted.
This works fine, but I believe NSS had a bug here a couple years ago. Also move
all the Skip* bug options next to each other in order.

Change-Id: I72dcb3babeee7ba73b3d7dc5ebef2e2298e37438
Reviewed-on: https://boringssl-review.googlesource.com/3333
Reviewed-by: Adam Langley <agl@google.com>
2015-02-09 19:43:48 +00:00
David Benjamin
3fd1fbd1c8 Add test coverage for normal alert parsing.
We have test coverage for invalid alerts, but not for normal ones on the DTLS
side.

Change-Id: I359dce8d4dc80dfa99b5d8bacd73f48a8e4ac310
Reviewed-on: https://boringssl-review.googlesource.com/3291
Reviewed-by: Adam Langley <agl@google.com>
2015-02-03 21:57:02 +00:00
David Benjamin
ddb9f15e18 Reject all invalid records.
The check on the DTLS side was broken anyway. On the TLS side, the spec does
say to ignore them, but there should be no need for this in future-proofing and
NSS doesn't appear to be lenient here. See also
https://boringssl-review.googlesource.com/#/c/3233/

Change-Id: I0846222936c5e08acdcfd9d6f854a99df767e468
Reviewed-on: https://boringssl-review.googlesource.com/3290
Reviewed-by: Adam Langley <agl@google.com>
2015-02-03 21:55:53 +00:00
David Benjamin
afbc63fc2f Simplify DTLS epoch rewind.
SSL_AEAD_CTX ownership is currently too confusing. Instead, rely on the lack of
renego, so the previous epoch always uses the NULL cipher. (Were we to support
DTLS renego, we could keep track of s->d1->last_aead_write_ctx like
s->d1->last_write_sequence, but it isn't worth it.)

Buffered messages also tracked an old s->session, but this is unnecessary. The
s->session NULL check in tls1_enc dates to the OpenSSL initial commit and is
redundant with the aead NULL check.

Change-Id: I9a510468d95934c65bca4979094551c7536980ae
Reviewed-on: https://boringssl-review.googlesource.com/3234
Reviewed-by: Adam Langley <agl@google.com>
2015-02-03 20:34:06 +00:00
David Benjamin
0ea8dda93e Remove alert_fragment and handshake_fragment.
Nothing recognized through those codepaths is fragmentable in DTLS. Also remove
an unnecessary epoch check. It's not possible to process a record from the
wrong epoch.

Change-Id: I9d0f592860bb096563e2bdcd2c8e50a0d2b65f59
Reviewed-on: https://boringssl-review.googlesource.com/3232
Reviewed-by: Adam Langley <agl@google.com>
2015-02-03 19:10:08 +00:00
David Benjamin
b3774b9619 Add initial handshake reassembly tests.
For now, only test reorderings when we always or never fragment messages.
There's a third untested case: when full messages and fragments are mixed. That
will be tested later after making it actually work.

Change-Id: Ic4efb3f5e87b1319baf2d4af31eafa40f6a50fa6
Reviewed-on: https://boringssl-review.googlesource.com/3216
Reviewed-by: Adam Langley <agl@google.com>
2015-02-03 19:05:30 +00:00
David Benjamin
d660b57208 runner: Refactor handshake fragmenting slightly.
No behavior change. This is in preparation for buffering a flight of handshake
messages to reorder vigorously on flush.

Change-Id: Ic348829b340bf58d28f332027646559cb11046ac
Reviewed-on: https://boringssl-review.googlesource.com/3215
Reviewed-by: Adam Langley <agl@google.com>
2015-02-03 00:43:13 +00:00
David Benjamin
83f9040339 Add DTLS timeout and retransmit tests.
This extends the packet adaptor protocol to send three commands:
  type command =
    | Packet of []byte
    | Timeout of time.Duration
    | TimeoutAck

When the shim processes a Timeout in BIO_read, it sends TimeoutAck, fails the
BIO_read, returns out of the SSL stack, advances the clock, calls
DTLSv1_handle_timeout, and continues.

If the Go side sends Timeout right between sending handshake flight N and
reading flight N+1, the shim won't read the Timeout until it has sent flight
N+1 (it only processes packet commands in BIO_read), so the TimeoutAck comes
after N+1. Go then drops all packets before the TimeoutAck, thus dropping one
transmit of flight N+1 without having to actually process the packets to
determine the end of the flight. The shim then sees the updated clock, calls
DTLSv1_handle_timeout, and re-sends flight N+1 for Go to process for real.

When dropping packets, Go checks the epoch and increments sequence numbers so
that we can continue to be strict here. This requires tracking the initial
sequence number of the next epoch.

The final Finished message takes an additional special-case to test. DTLS
triggers retransmits on either a timeout or seeing a stale flight. OpenSSL only
implements the former which should be sufficient (and is necessary) EXCEPT for
the final Finished message. If the peer's final Finished message is lost, it
won't be waiting for a message from us, so it won't time out anything. That
retransmit must be triggered on stale message, so we retransmit the Finished
message in Go.

Change-Id: I3ffbdb1de525beb2ee831b304670a3387877634c
Reviewed-on: https://boringssl-review.googlesource.com/3212
Reviewed-by: Adam Langley <agl@google.com>
2015-02-03 00:40:58 +00:00
David Benjamin
d9b091b5e2 Revert "Drop retransmits in DTLS tests."
This reverts commit c67a3ae6ba. With a
deterministic clock, we can now go back to being strict about retransmits. Our
tests will now require that the shim only retransmit when we expect it to.

Change-Id: Iab1deb9665dcd294790c8253d920089e83a9140c
Reviewed-on: https://boringssl-review.googlesource.com/3211
Reviewed-by: Adam Langley <agl@google.com>
2015-02-03 00:39:57 +00:00
David Benjamin
6ae7f072e3 Only send sigalgs extension in 1.2-capable ClientHellos.
BUG=https://code.google.com/p/webrtc/issues/detail?id=4223

Change-Id: I88eb036fdc6da17bc6a5179df02f35486abe9add
Reviewed-on: https://boringssl-review.googlesource.com/3030
Reviewed-by: Adam Langley <agl@google.com>
2015-01-26 18:45:04 +00:00
David Benjamin
4189bd943c Test application data and Finished reordering.
This is fatal for TLS but buffered in DTLS. The buffering isn't strictly
necessary (it would be just as valid to drop the record on the floor), but so
long as we want this behavior it should have a test.

Change-Id: I5846bb2fe80d78e25b6dfad51bcfcff2dc427c3f
Reviewed-on: https://boringssl-review.googlesource.com/3029
Reviewed-by: Adam Langley <agl@google.com>
2015-01-26 18:43:02 +00:00
David Benjamin
5fa3eba03d Clear the error queue when dropping a bad DTLS packet.
This regressed in e95d20dcb8. EVP_AEAD will push
errors on the error queue (unlike the EVP_CIPHER codepath which checked
everything internally to ssl/ and didn't bother pushing anything). This meant
that a dropped packet would leave junk in the error queue.

Later, when SSL_read returns <= 0 (EOF or EWOULDBLOCK), the non-empty error
queue check in SSL_get_error kicks in and SSL_read looks to have failed.

BUG=https://code.google.com/p/webrtc/issues/detail?id=4214

Change-Id: I1e5e41c77a3e5b71e9eb0c72294abf0da677f840
Reviewed-on: https://boringssl-review.googlesource.com/2982
Reviewed-by: Adam Langley <agl@google.com>
2015-01-22 22:06:40 +00:00
David Benjamin
6095de8da2 Add tests for certificate mismatch.
Cover another mildly interesting error case.

Change-Id: Ice773af79f5e03f39f0cd2a9e158bae03e065392
Reviewed-on: https://boringssl-review.googlesource.com/2841
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:51:17 +00:00
David Benjamin
c67a3ae6ba Drop retransmits in DTLS tests.
BoringSSL currently retransmits non-deterministically on an internal timer
(rather than one supplied externally), so the tests currently fail flakily
depending on timing. Valgrind is a common source for this. We still assume an
in-order and reliable channel, but drop retransmits silently:

- Handshake messages may arrive with old sequence numbers.

- Retransmitted CCS records arrive from the previous epoch.

- We may receive a retransmitted Finished after we believe the handshake has
  completed. (Aside: even in a real implementation, only Finished is possible
  here. Even with out-of-order delivery, retransmitted or reordered messages
  earlier in the handshake come in under a different epoch.)

Note that because DTLS renego and a Finished retransmit are ambiguous at the
record layer[*], this precludes us writing tests for DTLS renego. But DTLS
renego should get removed anyway. As BoringSSL currently implements renego,
this ambiguity is also a source of complexity in the real implementation. (See
the SSL3_MT_FINISHED check in dtls1_read_bytes.)

[*] As a further fun aside, it's also complex if dispatching renego vs Finished
after handshake message reassembly. The spec doesn't directly say the sequence
number is reset across renegos, but it says "The first message each side
transmits in /each/ handshake always has message_seq = 0". This means that such
an implementation needs the handshake message reassembly logic be aware that a
Finished fragment with high sequence number is NOT an out-of-order fragment for
the next handshake.

Change-Id: I35d13560f82bcb5eeda62f4de1571d28c818cc36
Reviewed-on: https://boringssl-review.googlesource.com/2770
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:13:05 +00:00
David Benjamin
e95d20dcb8 Support EVP_AEAD in DTLS.
This CL removes the last of the EVP_CIPHER codepath in ssl/. The dead code is
intentionally not pruned for ease of review, except in DTLS-only code where
adding new logic to support both, only to remove half, would be cumbersome.

Fixes made:
- dtls1_retransmit_state is taught to retain aead_write_ctx rather than
  enc_write_ctx.
- d1_pkt.c reserves space for the variable-length nonce when echoed into the
  packet.
- dtls1_do_write sizes the MTU based on EVP_AEAD max overhead.
- tls1_change_cipher_state_cipher should not free AEAD write contexts in DTLS.
  This matches the (rather confused) ownership for the EVP_CIPHER contexts.
  I've added a TODO to resolve this craziness.

A follow-up CL will remove all the resultant dead code.

Change-Id: I644557f4db53bbfb182950823ab96d5e4c908866
Reviewed-on: https://boringssl-review.googlesource.com/2699
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:03:40 +00:00
David Benjamin
13be1de469 Add a basic MTU test.
The minimum MTU (not consistently enforced) is just under 256, so it's
difficult to test everything, but this is a basic test. (E.g., without renego,
the only handshake message with encryption is Finished which fits in the MTU.)
It tests the server side because the Certificate message is large enough to
require fragmentation.

Change-Id: Ida11f1057cebae2b800ad13696f98bb3a7fbbc5e
Reviewed-on: https://boringssl-review.googlesource.com/2824
Reviewed-by: Adam Langley <agl@google.com>
2015-01-12 22:37:25 +00:00
David Benjamin
780d6dd0fe Treat handshake_failure in response to ClientHello special.
Add a dedicated error code to the queue for a handshake_failure alert in
response to ClientHello. This matches NSS's client behavior and gives a better
error on a (probable) failure to negotiate initial parameters.

BUG=https://crbug.com/446505

Change-Id: I34368712085a6cbf0031902daf2c00393783d96d
Reviewed-on: https://boringssl-review.googlesource.com/2751
Reviewed-by: Adam Langley <agl@google.com>
2015-01-06 18:31:49 +00:00
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
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
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
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
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
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
David Benjamin
4e99c52bf6 Fix FalseStart-SessionTicketsDisabled tests.
They weren't inheriting async settings.

Change-Id: I5e9c04914926910dce63f93462cce4024627fb26
Reviewed-on: https://boringssl-review.googlesource.com/1605
Reviewed-by: Adam Langley <agl@google.com>
2014-08-25 23:00:59 +00:00
David Benjamin
2561dc335a Introduce a mechanism for base64 options.
We may wish to pass data to the runner that contains NULs.

Change-Id: Id78dad0ad0b5b6d0537481c818e3febdf1740cc9
Reviewed-on: https://boringssl-review.googlesource.com/1603
Reviewed-by: Adam Langley <agl@google.com>
2014-08-25 23:00:44 +00:00
David Benjamin
d30a990850 Implement TLS Channel ID in runner.go
Change-Id: Ia349c7a7cdcfd49965cd0c4d6cf81a76fbffb696
Reviewed-on: https://boringssl-review.googlesource.com/1604
Reviewed-by: Adam Langley <agl@google.com>
2014-08-25 22:48:18 +00:00
David Benjamin
2a0c496ab3 Fix duplicate test name.
Change-Id: I16be575e4a6a13c74bd45a8fe3e1473502a80c86
Reviewed-on: https://boringssl-review.googlesource.com/1600
Reviewed-by: Adam Langley <agl@google.com>
2014-08-25 21:47:25 +00:00
David Benjamin
ede973a89a Tidy up cipher ordering.
To align with what Chrome sends on NSS, remove all 3DES cipher suites except
RSA_WITH_3DES_EDE_CBC_SHA. This avoids having to order a PFS 3DES cipher
against a non-PFS 3DES cipher.

Remove the strength sort which wanted place AES_256_CBC ahead of AES_128_GCM
and is not especially useful (everything under 128 is either 3DES or DES).
Instead, explicitly order all the bulk ciphers. Continue to prefer PFS over
non-PFS and ECDHE over DHE.

This gives the following order in Chromium. We can probably prune it a bit
(DHE_DSS, DH_*) in a follow-up.

TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 (0xcc14)   Forward Secrecy	256
TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (0xcc13)   Forward Secrecy	256
TLS_DHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (0xcc15)   Forward Secrecy	256
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (0xc02f)   Forward Secrecy	128
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 (0xc02b)   Forward Secrecy	128
TLS_DHE_DSS_WITH_AES_128_GCM_SHA256 (0xa2)   Forward Secrecy*	128
TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 (0x9e)   Forward Secrecy	128
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (0xc014)   Forward Secrecy	256
TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA (0xc00a)   Forward Secrecy	256
TLS_DHE_RSA_WITH_AES_256_CBC_SHA (0x39)   Forward Secrecy	256
TLS_DHE_DSS_WITH_AES_256_CBC_SHA (0x38)   Forward Secrecy*	256
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (0xc013)   Forward Secrecy	128
TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA (0xc009)   Forward Secrecy	128
TLS_DHE_RSA_WITH_AES_128_CBC_SHA (0x33)   Forward Secrecy	128
TLS_DHE_DSS_WITH_AES_128_CBC_SHA (0x32)   Forward Secrecy*	128
TLS_ECDHE_RSA_WITH_RC4_128_SHA (0xc011)   Forward Secrecy	128
TLS_ECDHE_ECDSA_WITH_RC4_128_SHA (0xc007)   Forward Secrecy	128
TLS_DH_DSS_WITH_AES_128_GCM_SHA256 (0xa4)	128
TLS_DH_RSA_WITH_AES_128_GCM_SHA256 (0xa0)	128
TLS_RSA_WITH_AES_128_GCM_SHA256 (0x9c)	128
TLS_DH_RSA_WITH_AES_256_CBC_SHA (0x37)	256
TLS_DH_DSS_WITH_AES_256_CBC_SHA (0x36)	256
TLS_RSA_WITH_AES_256_CBC_SHA (0x35)	256
TLS_DH_RSA_WITH_AES_128_CBC_SHA (0x31)	128
TLS_DH_DSS_WITH_AES_128_CBC_SHA (0x30)	128
TLS_RSA_WITH_AES_128_CBC_SHA (0x2f)	128
TLS_RSA_WITH_RC4_128_SHA (0x5)	128
TLS_RSA_WITH_RC4_128_MD5 (0x4)	128
TLS_RSA_WITH_3DES_EDE_CBC_SHA (0xa)	112

BUG=405091

Change-Id: Ib8dd28469414a4eb496788a57a215e7e21f8c37f
Reviewed-on: https://boringssl-review.googlesource.com/1559
Reviewed-by: Adam Langley <agl@google.com>
2014-08-19 19:00:32 +00:00
David Benjamin
8bc38f556a DTLS version negotiation doesn't happen at HelloVerifyRequest.
RFC 6347 changed the meaning of server_version in HelloVerifyRequest. It should
now always be 1.0 with version negotiation not happening until ServerHello. Fix
runner.go logic and remove #if-0'd code in dtls1_get_hello_verify.

Enforce this in the runner for when we get DTLS 1.2 tests.

Change-Id: Ice83628798a231df6bf268f66b4c47b14a519386
Reviewed-on: https://boringssl-review.googlesource.com/1552
Reviewed-by: Adam Langley <agl@google.com>
2014-08-18 18:07:43 +00:00
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