Commit Graph

419 Commits

Author SHA1 Message Date
David Benjamin
639846e5e4 Add tests for trailing data in handshake messages.
It's easy to forget to check those. Unfortunately, it's also easy to
forget to check inner structures, which is going to be harder to stress,
but do these to start with. In doing, so fix up and unify some
error-handling, and add a missing check when parsing TLS 1.2
CertificateRequest.

This was also inspired by the recent IETF posting.

Change-Id: I27fe3cd3506258389a75d486036388400f0a33ba
Reviewed-on: https://boringssl-review.googlesource.com/10963
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-09-12 21:00:50 +00:00
David Benjamin
cd2c806530 Factor per-message test machinery out.
This will let us use the same test scenarios for testing messages with
trailing garbage or skipped messages.

Change-Id: I9f177983e8dabb6c94d3d8443d224b79a58f40b1
Reviewed-on: https://boringssl-review.googlesource.com/10962
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-09-12 19:56:40 +00:00
David Benjamin
5510863fbd Temporary remove the TLS 1.3 anti-downgrade mechanism.
This mechanism is incompatible with deploying draft versions of TLS 1.3.

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

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

Change-Id: Ic3cf72b7c31ba91e5cca0cfd7a3fca830c493a43
Reviewed-on: https://boringssl-review.googlesource.com/11005
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-09-12 18:10:23 +00:00
David Benjamin
c241d79261 Add tests around compression methods.
Not that this matters in the slightest, but the recent IETF mailing
reminded me we don't test this.

Change-Id: I300c96d6a63733d538a7019a7cb74d4e65d0498f
Reviewed-on: https://boringssl-review.googlesource.com/10961
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-09-09 17:29:21 +00:00
David Benjamin
abe94e3b0d Test that SNI warning alerts are ignored.
Although RFC 6066 recommends against it, some servers send a warning
alert prior to ServerHello on SNI mismatch, and, per spec, TLS 1.2
allows it.

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

Change-Id: I0032313c986c835b6ae1aa43da6ee0dad17a97c2
Reviewed-on: https://boringssl-review.googlesource.com/10800
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-09-09 16:20:25 +00:00
David Benjamin
57e929f3c8 Enable RSA-PSS in TLS 1.2 by default.
Add a test that RSA-PSS is available in TLS 1.2 by default, both for
signing and verifying. Note that if a custom SSL_PRIVATE_KEY_METHOD is
used and it sets signing preferences, it won't use RSA-PSS if it doesn't
know about it. (See *-Sign-Negotiate-* tests.)

Change-Id: I3776a0c95480188a135795f7ebf31f2b0e0626cc
Reviewed-on: https://boringssl-review.googlesource.com/10723
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-08-30 22:50:05 +00:00
David Benjamin
163c95691a Forbid EMS from changing during renegotation.
Changing parameters on renegotiation makes all our APIs confusing. This
one has no reason to change, so lock it down. In particular, our
preference to forbid Token Binding + renego may be overridden at the
IETF, even though it's insane. Loosening it will be a bit less of a
headache if EMS can't change.

https://www.ietf.org/mail-archive/web/unbearable/current/msg00690.html
claims that this is already in the specification and enforced by NSS. I
can't find anything to this effect in the specification. It just says
the client MUST disable renegotiation when EMS is missing, which is
wishful thinking. At a glance, NSS doesn't seem to check, though I could
be misunderstanding the code.

Nonetheless, locking this down is a good idea anyway. Accurate or not,
take the email as an implicit endorsement of this from Mozilla.

Change-Id: I236b05991d28bed199763dcf2f47bbfb9d0322d7
Reviewed-on: https://boringssl-review.googlesource.com/10721
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-08-30 15:43:35 +00:00
Matt Braithwaite
9c8c418853 Remove RC4 ciphersuites from TLS.
For now, they can be restored by compiling with -DBORINGSSL_RC4_TLS.

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

Change-Id: Ic77308e7bd4849bdb2b4882c6b34af85089fe3cc
Reviewed-on: https://boringssl-review.googlesource.com/10580
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Matt Braithwaite <mab@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-08-26 19:32:44 +00:00
Matt Braithwaite
07e7806177 runner: use 3DES instead of RC4 where possible.
To ease the removal of RC4, use 3DES in cases where RC4 is not required,
but is just a placeholder for "ciphersuite that works in SSLv3."

Change-Id: Ib459173e68a662986235b556f330a7e0e02759d7
Reviewed-on: https://boringssl-review.googlesource.com/10523
Commit-Queue: Matt Braithwaite <mab@google.com>
Reviewed-by: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-08-24 20:27:49 +00:00
David Benjamin
5c4e8571cc Fill in the curve ID for TLS 1.3.
Apparently we forgot to do this.

Change-Id: I348cf6d716ae888fddce69ba4801bf09446f5a72
Reviewed-on: https://boringssl-review.googlesource.com/10503
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-08-22 18:05:01 +00:00
David Benjamin
0e95015aa5 RSA-PSS should work in TLS 1.2.
However, for now, we will only enable it if TLS 1.3 is offered.

BUG=85

Change-Id: I958ae0adeafee553dbffb966a6fa41f8a81cef96
Reviewed-on: https://boringssl-review.googlesource.com/10342
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-08-19 18:44:26 +00:00
David Benjamin
46662482b8 Test resuming renewed sessions.
In TLS 1.3 draft 14, due to resumption using a different cipher, this
is actually not too hard to mess up. (In fact BoGo didn't quite get it
right.)

Fortunately, the new cipher suite negotiation in draft 15 should make
this reasonable again once we implement it. In the meantime, test it.

Change-Id: I2eb948eeaaa051ecacaa9095b66ff149582ea11d
Reviewed-on: https://boringssl-review.googlesource.com/10442
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-18 23:53:19 +00:00
Steven Valdez
32635b828f Add limit for consecutive KeyUpdate messages.
Change-Id: I2e1ee319bb9852b9c686f2f297c470db54f72279
Reviewed-on: https://boringssl-review.googlesource.com/10370
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-18 23:43:12 +00:00
Steven Valdez
54ed58e806 Forbid PKCS1 in TLS 1.3.
BUG=84

Change-Id: Ie5eaefddd161488996033de28c0ebd1064bb793d
Reviewed-on: https://boringssl-review.googlesource.com/10484
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-08-18 20:51:01 +00:00
Adam Langley
37646838e9 Have |SSL_get_verify_result| return |X509_V_OK| when no client certificate is given.
9498e74 changed the default value of verify_result to an error. This
tripped up NGINX, which depends on a bug[1] in OpenSSL. netty-tcnative
also uses this behavior, though it currently isn't tripped up by 9498e74
because it calls |SSL_set_verify_result|. However, we would like to
remove |SSL_set_verify_result| and with two data points, it seems this
is behavior we must preserve.

This change sets |verify_result| to |X509_V_OK| when a) no client
certificate is requested or b) none is given and it's optional.

[1] See BUGS in https://www.openssl.org/docs/manmaster/ssl/SSL_get_verify_result.html

Change-Id: Ibd33660ae409bfe272963a8c39b7e9aa83c3d635
Reviewed-on: https://boringssl-review.googlesource.com/9067
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-18 20:37:10 +00:00
David Benjamin
ee32bea1d3 Fix TLS 1.2 sigalgs fallback logic for ECDSA.
Also fix up those tests as they were a little confused. It is always the
shim that signs and has a configured certificate in these tests.

BUG=95

Change-Id: I57a6b1bad19986c79cd30aaa6cf3b8ca307ef8b2
Reviewed-on: https://boringssl-review.googlesource.com/10444
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-18 19:41:22 +00:00
David Benjamin
8a8349b53e Request contexts are now illegal during the handshake.
One less thing to keep track of.
https://github.com/tlswg/tls13-spec/pull/549 got merged.

Change-Id: Ide66e547140f8122a3b8013281be5215c11b6de0
Reviewed-on: https://boringssl-review.googlesource.com/10482
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-08-18 15:40:40 +00:00
David Benjamin
e73c7f4281 Flesh out missing TLS 1.3 state machine coverage.
The TLS 1.3 state machine is actually less in need of the aggressive
state machine coverage tests, but nonetheless, we should cover all
handshake shapes. PSK resumption and HelloRetryRequest were missing.

We were also accidentally running "DTLS" versions of the TLS 1.3 tests
but silently running TLS 1.2.

Change-Id: I65db4052b89d770db7e47738e73aaadde9634236
Reviewed-on: https://boringssl-review.googlesource.com/10441
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-08-17 22:49:14 +00:00
David Benjamin
e54af069d8 Configure common config bits in one place.
Right now the logic happens twice which is a nuisance.

Change-Id: Ia8155ada0b4479b2ca4be06152b8cd99816e14e8
Reviewed-on: https://boringssl-review.googlesource.com/10440
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-08-17 19:59:48 +00:00
David Benjamin
405da48900 Improve TLS 1.3 resumption/version tests.
Some version mismatch cases were not being covered due to TLS 1.2 and
TLS 1.3 having very different spellings for tickets resumption. Also
explicitly test that TLS 1.2 tickets aren't offered in the TLS 1.3 slot
and vice versa.

Change-Id: Ibe58386ea2004fb3c1af19342b8d808f13f737a9
Reviewed-on: https://boringssl-review.googlesource.com/10183
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-16 20:50:32 +00:00
Steven Valdez
4aa154e08f Adding code to send session as PSK Identity.
BUG=75

Change-Id: Ied864cfccbc0e68d71c55c5ab563da27b7253463
Reviewed-on: https://boringssl-review.googlesource.com/9043
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-16 20:35:32 +00:00
David Benjamin
eed2401cac Apply SendClientVersion at the end.
Much of the ClientHello logic queries hello.vers. To avoid it getting
confused, do all modifications right at the end, otherwise
SendClientVersion also affects whether the key share is sent.

Change-Id: I8be2a4a9807ef9ad88af03971ea1c37e4ba36b9c
Reviewed-on: https://boringssl-review.googlesource.com/10341
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-08-15 18:33:07 +00:00
David Benjamin
3e51757de2 Enforce the server ALPN protocol was advertised.
The server should not be allowed select a protocol that wasn't
advertised. Callers tend to not really notice and act as if some default
were chosen which is unlikely to work very well.

Change-Id: Ib6388db72f05386f854d275bab762ca79e8174e6
Reviewed-on: https://boringssl-review.googlesource.com/10284
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-11 16:46:34 +00:00
David Benjamin
881f196075 Make ECDHE_PSK + AES_GCM unmatchable.
These are probably a good idea to ship so long as we have the PSK
callbacks at all, but they're not *completely* standard yet and Android
tests otherwise need updating to know about them. We don't care enough
about PSK to be in a rush to ship them, and taking them out is an easier
default action until then.

Change-Id: Ic646053d29b69a114e2efea61d593d5e912bdcd0
Reviewed-on: https://boringssl-review.googlesource.com/10225
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-11 16:00:42 +00:00
David Benjamin
25fe85b38c Insert a state before cert_cb.
If cert_cb runs asynchronously, we end up repeating a large part of very
stateful ClientHello processing. This seems to be mostly fine and there
are few users of server-side cert_cb (it's a new API in 1.0.2), but it's
a little scary.

This is also visible to external consumers because some callbacks get
called multiple times. We especially should try to avoid that as there
is no guarantee that these callbacks are idempotent and give the same
answer each time.

Change-Id: I212b2325eae2cfca0fb423dace101e466c5e5d4e
Reviewed-on: https://boringssl-review.googlesource.com/10224
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-08-11 15:45:00 +00:00
EKR
f71d7ed014 Shim-specific configuration file with suppressions and error translation.
This is more progress in letting other stacks use the test runner.
You can provide a per-shim configuration file that includes:

 - A list of test patterns to be suppressed (presumably because
   they don't work). This setting is ignored if -test is used.
 - A translation table of expected errors to shim-specific errors.

BUG=92

Change-Id: I3c31d136e35c282e05d4919e18ba41d44ea9cf2a
Reviewed-on: https://boringssl-review.googlesource.com/9161
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-09 19:17:42 +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
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
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
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
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
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
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
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
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
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