Commit Graph

1427 Commits

Author SHA1 Message Date
Martin Kreichgauer
baafa4a653 Undo rename of tlsext_tick_lifetime_hint.
It was renamed to ticket_liftetime_hint in
1e6f11a7ff, which breaks Qt.

Change-Id: I9c6d3097fe96e669f06a4e0880bd4d7d82b03ba8
Reviewed-on: https://boringssl-review.googlesource.com/10181
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-09 17:50:28 +00:00
David Benjamin
e7e36aae25 Test that switching versions on renego is illegal.
We handle this correctly but never wrote a test for it. Noticed this in
chatting about the second ClientHello.version bug workaround with Eric
Rescorla.

Change-Id: I09bc6c995d07c0f2c9936031b52c3c639ed3695e
Reviewed-on: https://boringssl-review.googlesource.com/9154
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-08 17:27:42 +00:00
David Benjamin
b9195402b4 Align SSL_SESSION_up_ref with OpenSSL.
Only X509_up_ref left (it's still waiting on a few external callers).

BUG=89

Change-Id: Ia2aec2bb0a944356cb1ce29f3b58a26bdb8a9977
Reviewed-on: https://boringssl-review.googlesource.com/9141
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-08-05 19:00:33 +00:00
David Benjamin
a9c3bf142e Add TLS_{client,server}_method.
Inch towards OpenSSL 1.1.0 compatibility.

BUG=91

Change-Id: Ia45b6bdb5114d0891fdffdef0b5868920324ecec
Reviewed-on: https://boringssl-review.googlesource.com/9140
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-08-05 18:59:32 +00:00
David Benjamin
4087df92f4 Move more side-specific code out of tls13_process_certificate.
tls13_process_certificate can take a boolean for whether anonymous is
allowed. This does change the error on the client slightly, but I think
this is correct anyway. It is not a syntax error for the server to send
no certificates in so far as the Certificate message allows it. It's
just illegal.

Change-Id: I1af80dacf23f50aad0b1fbd884bc068a40714399
Reviewed-on: https://boringssl-review.googlesource.com/9072
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-04 16:58:46 +00:00
David Benjamin
bb9e36e005 Test client certificates carry over on session resumption.
We have tests for this as a server, but none as a client. Extend the
certificate verification tests here. This is in preparation for ensuring
that TLS 1.3 session resumption works correctly.

Change-Id: I9ab9f42838ffd69f73fbd877b0cdfaf31caea707
Reviewed-on: https://boringssl-review.googlesource.com/9111
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-04 16:43:57 +00:00
David Benjamin
e455e51d85 Push some duplicated code into ssl_verify_cert_chain.
No sense in having it in both the 1.2 and 1.3 code.

Change-Id: Ib3854714afed24253af7f4bcee26d25e95a10211
Reviewed-on: https://boringssl-review.googlesource.com/9071
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-04 16:41:03 +00:00
David Benjamin
56d280da2f Remove the SSL_R_SESSION_ID_CONTEXT_UNINITIALIZED sanity check.
While the sanity check isn't insane (one should arrange for sessions to
be invalidated once client auth settings change, and a sid_ctx is one
way to do it), this check lives in a poor place to enforce configuration
mistakes. To be effective, it needs to happen at the start of the
handshake, independent of the ClientHello from the peer.

But the benefit this check gives is low compared to the trouble it will
be to continually maintain this difference from OpenSSL (our own
ssl_test and bssl_shim forget to set a dummy sid_ctx).  Instead, remove
it so we don't have to duplicate it across TLS 1.2 and TLS 1.3. Also so
we don't have weird failures which only manifest once a resuming client
connects.

Change-Id: Ia7f88711701afde5e26b7782c2264ce78dccc89b
Reviewed-on: https://boringssl-review.googlesource.com/9112
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-03 21:43:50 +00:00
David Benjamin
721e8b79a9 Test that servers enforce session timeouts.
Extend the DTLS mock clock to apply to sessions too and test that
resumption behaves as expected.

Change-Id: Ib8fdec91b36e11cfa032872b63cf589f93b3da13
Reviewed-on: https://boringssl-review.googlesource.com/9110
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-03 21:27:07 +00:00
David Benjamin
a20e535fb1 Add a test for session ID context logic.
We almost forgot to handle this in TLS 1.3, so add a test for it.

Change-Id: I28600325d8fb6c09365e909db607cbace12ecac7
Reviewed-on: https://boringssl-review.googlesource.com/9093
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-03 21:20:33 +00:00
David Benjamin
33dad1b7a1 Stop pretending to ssl_clear_bad_session.
We broke this to varying degrees ages ago.

This is the logic to implement the variations of rules in TLS to discard
sessions after a failed connection, where a failed connection could be
one of:

- A connection that was not cleanly shut down.

- A connection that received a fatal alert.

The first one is nonsense since close_notify does not actually work in
the real world. The second is a vaguely more plausible but...

- A stateless ticket-based server can't drop sessions anyway.

- In TLS 1.3, a client may receive many tickets over the lifetime of a
  single connection. With an external session cache like ours which may,
  in theory, but multithreaded, this will be a huge hassle to track.

- A client may well attempt to establish a connection and reuse the
  session before we receive the fatal alert, so any application state we
  hope to manage won't really work.

- An attacker can always close the connection before the fatal alert, so
  whatever security policy clearing the session gave is easily
  bypassable.

Implementation-wise, this has basically never worked. The
ssl_clear_bad_session logic called into SSL_CTX_remove_session which
relied on the internal session cache. (Sessions not in the internal
session cache don't get removed.) The internal session cache was only
useful for a server, where tickets prevent this mechanism from doing
anything. For a client, we since removed the internal session cache, so
nothing got removed. The API for a client also did not work as it gave
the SSL_SESSION, not the SSL, so a consumer would not know the key to
invalidate anyway.

The recent session state splitting change further broke this.

Moreover, calling into SSL_CTX_remove_session logic like that is
extremely dubious because it mutates the not_resumable flag on the
SSL_SESSION which isn't thread-safe.

Spec-wise, TLS 1.3 has downgraded the MUST to a SHOULD.

Given all that mess, just remove this code. It is no longer necessary to
call SSL_shutdown just to make session caching work.

Change-Id: Ib601937bfc5f6b40436941e1c86566906bb3165d
Reviewed-on: https://boringssl-review.googlesource.com/9091
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-03 21:07:36 +00:00
David Benjamin
cec7344bba Add a CBS version of SSL_early_callback_ctx_extension_get.
Save a little bit of typing at the call site.

Change-Id: I818535409b57a694e5e0ea0e9741d89f2be89375
Reviewed-on: https://boringssl-review.googlesource.com/9090
Reviewed-by: Adam Langley <agl@google.com>
2016-08-03 20:47:05 +00:00
Steven Valdez
1e6f11a7ff Adding NewSessionTicket.
We will now send tickets as a server and accept them as a
client. Correctly offering and resuming them in the handshake will be
implemented in a follow-up.

Now that we're actually processing draft 14 tickets, bump the draft
version.

Change-Id: I304320a29c4ffe564fa9c00642a4ace96ff8d871
Reviewed-on: https://boringssl-review.googlesource.com/8982
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-03 20:03:20 +00:00
David Benjamin
e8e84b9008 Reject warning alerts in TLS 1.3.
As of https://github.com/tlswg/tls13-spec/pull/530, they're gone.
They're still allowed just before the ClientHello or ServerHello, which
is kind of odd, but so it goes.

BUG=86

Change-Id: I3d556ab45e42d0755d23566e006c0db9af35b7b6
Reviewed-on: https://boringssl-review.googlesource.com/9114
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-03 19:58:01 +00:00
Steven Valdez
7259f2fd08 Prefix ext_key_share methods.
Change-Id: Id6a7443246479c62cbe0024e2131a2013959e21e
Reviewed-on: https://boringssl-review.googlesource.com/9078
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-02 23:13:29 +00:00
David Benjamin
a70de147ff Check for trailing data in key_share extension.
Change-Id: I057e19a9547a14b3950395db4318eaf0da01ec13
Reviewed-on: https://boringssl-review.googlesource.com/9079
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-02 21:37:39 +00:00
David Benjamin
ce079fda12 Add SSL_is_dtls.
OpenSSL 1.1.0 added a function to tell if an SSL* is DTLS or not. This
is probably a good idea, especially since SSL_version returns
non-normalized versions.

BUG=91

Change-Id: I25c6cf08b2ebabf0c610c74691de103399f729bc
Reviewed-on: https://boringssl-review.googlesource.com/9077
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-02 20:43:58 +00:00
David Benjamin
da2630c190 Remove redundant SSL_VERIFY_PEER check.
None of the SSL_VERIFY_FAIL_IF_NO_PEER_CERT codepaths will ever be
reached if SSL_VERIFY_PEER is unset. If we've gotten as far as getting a
Certificate message, consider SSL_VERIFY_FAIL_IF_NO_PEER_CERT alone
significant grounds for rejecting no peer certificate.

Change-Id: I2c6be4269d65b2467b86b1fc7d76ac47ca735553
Reviewed-on: https://boringssl-review.googlesource.com/9070
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-02 20:09:04 +00:00
Nick Harper
0b3625bcfd Add support for TLS 1.3 PSK resumption in Go.
Change-Id: I998f69269cdf813da19ccccc208b476f3501c8c4
Reviewed-on: https://boringssl-review.googlesource.com/8991
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-02 19:37:07 +00:00
David Benjamin
afc64dec74 Add tests to ensure our ClientHello does not change.
We'll need to update it on occasion, but we should not update our
default ClientHello without noticing.

Change-Id: I19ca52fdbe10e3ac14413fecd16be2e58af5a1f6
Reviewed-on: https://boringssl-review.googlesource.com/9075
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-02 19:34:18 +00:00
David Benjamin
3ce4389e96 Move some client/server special-cases out of tls13_process_certificate.
Where we can move uncommon logic to the caller, we probably ought to.

Change-Id: I54a09fffffc20290be05295137ccb605d562cad0
Reviewed-on: https://boringssl-review.googlesource.com/9069
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-02 19:20:24 +00:00
David Benjamin
4501bd5118 Align with OpenSSL on SSL_set_bio behavior.
SSL_set_bio is a nightmare.

In f715c42322, we noticed that, among
other problems, SSL_set_bio's actual behavior did not match how
SSL_set_rfd was calling it due to an asymmetry in the rbio/wbio
handling. This resulted in SSL_set_fd/SSL_set_rfd calls to crash.  We
decided that SSL_set_rfd's believed semantics were definitive and
changed SSL_set_bio.

Upstream, in 65e2d672548e7c4bcb28f1c5c835362830b1745b, decided that
SSL_set_bio's behavior, asymmetry and all, was definitive and that the
SSL_set_rfd crash was a bug in SSL_set_rfd. Accordingly, they switched
the fd callers to use the side-specific setters, new in 1.1.0.

Align with upstream's behavior and add tests for all of SSL_set_bio's
insanity. Also export the new side-specific setters in anticipation of
wanting to be mostly compatible with OpenSSL 1.1.0.

Change-Id: Iceac9508711f79750a3cc2ded081b2bb2cbf54d8
Reviewed-on: https://boringssl-review.googlesource.com/9064
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-02 17:50:39 +00:00
Adam Langley
d4aae0f965 Minor typo fixes.
Change-Id: Idf9db184348140972e57b2a8fa30dc9cb8b2e0f2
Reviewed-on: https://boringssl-review.googlesource.com/9065
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-01 19:49:06 +00:00
David Benjamin
4890165509 Empty signature algorithms in TLS 1.3 CertificateRequest is illegal.
In TLS 1.2, this was allowed to be empty for the weird SHA-1 fallback
logic. In TLS 1.3, not only is the fallback logic gone, but omitting
them is a syntactic error.

   struct {
       opaque certificate_request_context<0..2^8-1>;
       SignatureScheme
         supported_signature_algorithms<2..2^16-2>;
       DistinguishedName certificate_authorities<0..2^16-1>;
       CertificateExtension certificate_extensions<0..2^16-1>;
   } CertificateRequest;

Thanks to Eric Rescorla for pointing this out.

Change-Id: I4991e59bc4647bb665aaf920ed4836191cea3a5a
Reviewed-on: https://boringssl-review.googlesource.com/9062
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-01 19:47:26 +00:00
David Benjamin
0c40a96455 Send unsupported_extension on unexpected ServerHello extensions.
We were sending decode_error, but the spec explicitly says (RFC 5246):

   unsupported_extension
      sent by clients that receive an extended server hello containing
      an extension that they did not put in the corresponding client
      hello.  This message is always fatal.

Also add a test for this when it's a known but unoffered extension. We
actually end up putting these in different codepaths now due to the
custom extensions stuff.

Thanks to Eric Rescorla for pointing this out.

Change-Id: If6c8033d4cfe69ef8af5678b873b25e0dbadfc4f
Reviewed-on: https://boringssl-review.googlesource.com/9061
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-01 18:56:31 +00:00
David Benjamin
636ff1cb7e Convert rsa_1024_key.pem to a PKCS#8 PEM blob.
I missed one.

Change-Id: I311776efd1b2e5da7dca4c88b59a4a4c3e7df94b
Reviewed-on: https://boringssl-review.googlesource.com/9042
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-01 18:42:17 +00:00
Adam Langley
9498e74a92 Don't have the default value of |verify_result| be X509_V_OK.
It seems much safer for the default value of |verify_result| to be an
error value.

Change-Id: I372ec19c41d77516ed12d0169969994f7d23ed70
Reviewed-on: https://boringssl-review.googlesource.com/9063
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-01 18:11:39 +00:00
David Benjamin
0d1b0961f9 Fix mixed comment markers.
We managed to mix two comment styles in the Go license headers and
copy-and-paste it throughout the project.

Change-Id: Iec1611002a795368b478e1cae0b53127782210b1
Reviewed-on: https://boringssl-review.googlesource.com/9060
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-01 14:52:39 +00:00
Steven Valdez
1dc53d2840 Adding handling for KeyUpdate post-handshake message.
BUG=74

Change-Id: I72d52c1fbc3413e940dddbc0b20c7f22459da693
Reviewed-on: https://boringssl-review.googlesource.com/8981
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-29 23:06:09 +00:00
Steven Valdez
8e1c7be1a7 Adding Post-Handshake message handling.
Change-Id: I5cc194fc0a3ba8283049078e5671c924ee23036c
Reviewed-on: https://boringssl-review.googlesource.com/8980
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-29 22:34:57 +00:00
Steven Valdez
87eab4902d Splitting SSL session state.
To prevent configuration/established session confusion, the handshake
session state is separated into the configured session (ssl->session)
and the newly created session (ssl->s3->new_session). Upon conclusion of
the handshake, the finalized session is stored
in (ssl->s3->established_session). During the handshake, any requests
for the session (SSL_get_session) return a non-resumable session, to
prevent resumption of a partially filled session. Sessions should only
be cached upon the completion of the full handshake, using the resulting
established_session. The semantics of accessors on the session are
maintained mid-renego.

Change-Id: I4358aecb71fce4fe14a6746c5af1416a69935078
Reviewed-on: https://boringssl-review.googlesource.com/8612
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-29 21:22:46 +00:00
David Benjamin
163f29af07 Move post-handshake message handling out of read_app_data.
This finishes getting rid of ssl_read_bytes! Now we have separate
entry-points for the various cases. For now, I've kept TLS handshake
consuming records partially. When we do the BIO-less API, I expect that
will need to change, since we won't have the record buffer available.

(Instead, the ssl3_read_handshake_bytes and extend_handshake_buffer pair
will look more like the DTLS side or Go and pull the entire record into
init_buf.)

This change opts to make read_app_data drive the message to completion
in anticipation of DTLS 1.3. That hasn't been specified, but
NewSessionTicket certainly will exist. Knowing that DTLS necessarily has
interleave seems something better suited for the SSL_PROTOCOL_METHOD
internals to drive.

It needs refining, but SSL_PROTOCOL_METHOD is now actually a half-decent
abstraction boundary between the higher-level protocol logic and
DTLS/TLS-specific record-layer and message dispatchy bits.

BUG=83

Change-Id: I9b4626bb8a29d9cb30174d9e6912bb420ed45aff
Reviewed-on: https://boringssl-review.googlesource.com/9001
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-29 21:05:49 +00:00
David Benjamin
e97fb48fbe Test that V2ClientHello must be the first record.
Regression tests for upstream's
https://github.com/openssl/openssl/issues/1298.

Also, given that we're now on our third generation of V2ClientHello
handling, I'm sure we'll have a fourth and fifth and one of these days
I'm going to mess this one up. :-)

Change-Id: I6fd8f311ed0939fbbfd370448b637ccc06145021
Reviewed-on: https://boringssl-review.googlesource.com/9040
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-29 19:39:31 +00:00
EKR
173bf93827 Accept the special token 'UNTRANSLATED_ERROR' instead of the expected error code when -loose-errors argument is used. Usable for non-bssl shims
Change-Id: I7e85a2677fe28a22103a975d517bbee900c44ac3
Reviewed-on: https://boringssl-review.googlesource.com/9050
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-29 17:00:24 +00:00
Brian Smith
4edca0b308 Add BN_rand_range_ex and use internally.
There are many cases where we need |BN_rand_range| but with a minimum
value other than 0. |BN_rand_range_ex| provides that.

Change-Id: I564326c9206bf4e20a37414bdbce16a951c148ce
Reviewed-on: https://boringssl-review.googlesource.com/8921
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-29 16:09:26 +00:00
David Benjamin
4792110b2b Forbid interleaving app data in a HelloRequest.
We already forbid renego/app-data interleave. Forbid it within a
HelloRequest too because that's nonsense. No one would ever send:

   [hs:HelloReq-] [app:Hello world] [hs:-uest]

Add tests for this case.

This is in preparation for our more complex TLS 1.3 post-handshake logic
which is going to go through the usual handshake reassembly logic and,
for sanity, will want to enforce this anyway.

BUG=83

Change-Id: I80eb9f3333da3d751f98f25d9469860d1993a97a
Reviewed-on: https://boringssl-review.googlesource.com/9000
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-29 15:44:42 +00:00
David Benjamin
17e1292fe4 Make runner's -test parameter take glob patterns.
Per request from EKR. Also we have a lot of long test names, so this
seems generally a good idea.

Change-Id: Ie463f5367ec7d33005137534836005b571c8f424
Reviewed-on: https://boringssl-review.googlesource.com/9021
Reviewed-by: Adam Langley <agl@google.com>
2016-07-29 00:08:20 +00:00
David Benjamin
4497e58961 Switch finish_handshake to release_current_message.
With the previous DTLS change, the dispatch layer only cares about the
end of the handshake to know when to drop the current message. TLS 1.3
post-handshake messages will need a similar hook, so convert it to this
lower-level one.

BUG=83

Change-Id: I4c8c3ba55ba793afa065bf261a7bccac8816c348
Reviewed-on: https://boringssl-review.googlesource.com/8989
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-07-28 22:59:18 +00:00
David Benjamin
02edcd0098 Reject stray post-Finished messages in DTLS.
This is in preparation for switching finish_handshake to a
release_current_message hook. finish_handshake in DTLS is also
responsible for releasing any memory associated with extra messages in
the handshake.

Except that's not right and we need to make it an error anyway. Given
that the rest of the DTLS dispatch layer already strongly assumes there
is only one message in epoch one, putting the check in the fragment
processing works fine enough. Add tests for this.

This will certainly need revising when DTLS 1.3 happens (perhaps just a
version check, perhaps bringing finish_handshake back as a function that
can fail... which means we need a state just before SSL_ST_OK), but DTLS
1.3 post-handshake messages haven't really been written down, so let's
do the easy thing for now and add a test for when it gets more
interesting.

This removes the sequence number reset in the DTLS code. That reset
never did anything becase we don't and never will renego. We should make
sure DTLS 1.3 does not bring the reset back for post-handshake stuff.
(It was wrong in 1.2 too. Penultimate-flight retransmits and renego
requests are ambiguous in DTLS.)

BUG=83

Change-Id: I33d645a8550f73e74606030b9815fdac0c9fb682
Reviewed-on: https://boringssl-review.googlesource.com/8988
Reviewed-by: Adam Langley <agl@google.com>
2016-07-28 22:53:04 +00:00
David Benjamin
9fd9580137 Remove ssl->s3->message_complete in favor of ssl->init_msg.
This was only used so we knew when we had a current message to discard
and when we didn't. With init_msg being tracked better, we can use that
instead.

As part of this, switch the V2ClientHello hack to not using
reuse_message. Otherwise we have to fill in init_msg and friends in two
places.

The next change will require that we have a better handle on the "is
there a current message" boolean.

BUG=83

Change-Id: I917efacbad10806d492bbe51eda74c0779084d60
Reviewed-on: https://boringssl-review.googlesource.com/8987
Reviewed-by: Adam Langley <agl@google.com>
2016-07-28 22:52:47 +00:00
David Benjamin
a950948962 Use SSL3_HM_HEADER_LENGTH a bit more.
Somewhat clearer what it's for than just 4.

Change-Id: Ie7bb89ccdce188d61741da203acd624b49b69058
Reviewed-on: https://boringssl-review.googlesource.com/8986
Reviewed-by: Adam Langley <agl@google.com>
2016-07-28 22:49:57 +00:00
David Benjamin
481b9d2047 Remove begin_handshake and allocate init_buf lazily.
For TLS 1.3, we will need to process more complex post-handshake
messages. It is simplest if we use the same mechanism. In preparation,
allow ssl3_get_message to be called at any point.

Note that this stops reserving SSL3_RT_MAX_PLAIN_LENGTH in init_buf
right off the bat. Instead it will grow as-needed to accomodate the
handshake. SSL3_RT_MAX_PLAIN_LENGTH is rather larger than we probably
need to receive, particularly as a server, so this seems a good plan.

BUG=83

Change-Id: Id7f4024afc4c8a713b46b0d1625432315594350e
Reviewed-on: https://boringssl-review.googlesource.com/8985
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-07-28 22:07:28 +00:00
David Benjamin
7baf681a8b Convert all of our test private keys to PKCS#8 PEM blobs.
Right now they're RSA PRIVATE KEY or EC PRIVATE KEY which requires a bit
more effort to parse. It means the PEM header is necessary to parse
these. OpenSSL and Go automagically convert the format, but other shims
(namely NSS) may not.

Change-Id: I9fa2767dcf1fe6ceeea546390759e1c364a8f16f
Reviewed-on: https://boringssl-review.googlesource.com/9020
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-28 21:54:02 +00:00
David Benjamin
49054544d3 Clear init_msg/init_num whenever we clear the backing store.
This API needs to be improved but, for the time being, keep the
invariant reasonable.

Change-Id: If94d41e7e7936e44de5ecb36da45f89f80df7784
Reviewed-on: https://boringssl-review.googlesource.com/8984
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-07-28 21:44:39 +00:00
David Benjamin
bd4679d133 Tidy up ssl3_get_message slightly.
Change-Id: Iccd86440bf8721098050fac220dc9bb80bbfc670
Reviewed-on: https://boringssl-review.googlesource.com/8983
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-07-28 21:36:55 +00:00
David Benjamin
21c0028d40 Implement KeyUpdate in Go.
Implemented in preparation for testing the C implementation. Tested
against itself.

BUG=74

Change-Id: Iec1b9ad22e09711fa4e67c97cc3eb257585c3ae5
Reviewed-on: https://boringssl-review.googlesource.com/8873
Reviewed-by: Nick Harper <nharper@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-28 18:43:52 +00:00
David Benjamin
d5a4ecb61a Support accepting TLS 1.3 tickets on the Go client.
We still don't do anything useful with them, but we know not to put them
in the session ticket field.

In doing so, fix a bug in the CorruptTicket option where it would crash
if tickets are exactly 40 byets in length.

BUG=75

Change-Id: Id1039a58ed314a67d0af4f2c7e0617987c2bd6b5
Reviewed-on: https://boringssl-review.googlesource.com/8872
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-28 00:03:30 +00:00
David Benjamin
58104889ad Add support for sending TLS 1.3 tickets in Go.
Also parse out the ticket lifetime which was previously ignored.

BUG=75

Change-Id: I6ba92017bd4f1b31da55fd85d2af529fd592de11
Reviewed-on: https://boringssl-review.googlesource.com/8871
Reviewed-by: Nick Harper <nharper@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-27 22:37:31 +00:00
David Benjamin
4528e2b477 Take DHE ciphers out of 1.3 in Go.
We have no intention of implementing FFDHE and the DHE ciphers currently
don't work in the 1.3 handshake anyway. Cipher suite negotiation is to
be refactored in the spec so these cipher values won't be used for FFDHE
anyway.

Change-Id: I51547761d70a397dc3dd0391b71db98189f1a844
Reviewed-on: https://boringssl-review.googlesource.com/8874
Reviewed-by: Nick Harper <nharper@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-27 22:37:02 +00:00
EKR
842ae6cad0 Support unimplemented tests in test runner.
This change allows the shim to return a magic error code (89) to
indicate that it doesn't implement some of the given flags for a test.
Unimplemented tests are, by default, an error. The --allow-unimplemented
flag to the test runner causes them to be ignored.

This is done in preparation for non-BoringSSL shims.

Change-Id: Iecfd545b9cf44df5e25b719bfd06275c8149311a
Reviewed-on: https://boringssl-review.googlesource.com/8970
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-27 18:54:40 +00:00