BUG=chromium:659593
Change-Id: I73a4751609b85df7cd40f0f60dc3f3046a490940
Reviewed-on: https://boringssl-review.googlesource.com/11861
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>
On the client we'll leave it off by default until the change has made it
through Chrome's release process. For TLS 1.3, there is no existing
breakage risk, so always do it. This saves us the trouble of having to
manually turn it on in servers.
See [0] for a data point of someone getting it wrong.
[0] https://hg.mozilla.org/projects/nss/rev/9dbc21b1c3cc
Change-Id: I74daad9e7efd2040e9d66d72d558b31f145e6c4c
Reviewed-on: https://boringssl-review.googlesource.com/11680
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I18cee423675d6a686f83b4ef4b38696cb618392c
Reviewed-on: https://boringssl-review.googlesource.com/11683
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Nick Harper <nharper@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
BUG=103
Change-Id: I9a49fbaf66af73978ce264d27926f483e1e44766
Reviewed-on: https://boringssl-review.googlesource.com/11620
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>
Channel ID for TLS 1.3 uses the same digest construction as
CertificateVerify. This message is signed with the Channel ID key and
put in the same handshake message (with the same format) as in TLS 1.2.
BUG=103
Change-Id: Ia5b2dffe5a39c39db0cecb0aa6bdc328e53accc2
Reviewed-on: https://boringssl-review.googlesource.com/11420
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>
{sha1, ecdsa} is virtually nonexistent. {sha512, ecdsa} is pointless
when we only accept P-256 and P-384. See Chromium Intent thread here:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/kWwLfeIQIBM/9chGZ40TCQAJ
This tweaks the signature algorithm logic slightly so that sign and
verify preferences are separate.
BUG=chromium:655318
Change-Id: I1097332600dcaa38e62e4dffa0194fb734c6df3f
Reviewed-on: https://boringssl-review.googlesource.com/11621
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>
Some new tests needed to be suppressed.
Change-Id: I4474d752c338a18440efb213e0795ae81ad754fb
Reviewed-on: https://boringssl-review.googlesource.com/11583
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This should land in the same group of revisions as the two parent
commits.
Change-Id: Id9d769b890b3308ea70b705e7241c73cb1930ede
Reviewed-on: https://boringssl-review.googlesource.com/11581
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
We'll never send cookies, but we'll echo them on request. Implement it
in runner as well and test.
BUG=98
Change-Id: Idd3799f1eaccd52ac42f5e2e5ae07c209318c270
Reviewed-on: https://boringssl-review.googlesource.com/11565
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This doesn't currently honor the required KeyUpdate response. That will
be done in a follow-up.
BUG=74
Change-Id: I750fc41278736cb24230303815e839c6f6967b6a
Reviewed-on: https://boringssl-review.googlesource.com/11412
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
These too must be rejected. Test both unknown extensions and extensions
in the wrong context.
Change-Id: I54d5a5060f9efc26e5e4d23a0bde3c0d4d302d09
Reviewed-on: https://boringssl-review.googlesource.com/11501
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>
This is part of TLS 1.3 draft 16 but isn't much of a wire format change,
so go ahead and add it now. When rolling into Chromium, we'll want to
add an entry to the error mapping.
Change-Id: I8fd7f461dca83b725a31ae19ef96c890d603ce53
Reviewed-on: https://boringssl-review.googlesource.com/11563
Reviewed-by: David Benjamin <davidben@google.com>
We need to retain a pair of Finished messages for renegotiation_info.
SSL 3.0's is actually larger than TLS 1.2's (always 12 bytes). Take
renegotiation out in preparation for trimming them to size.
Change-Id: I2e238c48aaf9be07dd696bc2a6af75e9b0ead299
Reviewed-on: https://boringssl-review.googlesource.com/11570
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: Ifcdbeab9291d1141605a09a1960702c792cffa86
Reviewed-on: https://boringssl-review.googlesource.com/11561
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>
Change-Id: I5d4fc0d3204744e93d71a36923469035c19a5b10
Reviewed-on: https://boringssl-review.googlesource.com/11560
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The server acknowledging a non-existent session is a particularly
interesting case since getting it wrong means a NULL crash.
Change-Id: Iabde4955de883595239cfd8e9d84a7711e60a886
Reviewed-on: https://boringssl-review.googlesource.com/11500
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>
BUG=77
Change-Id: If568412655aae240b072c29d763a5b17bb5ca3f7
Reviewed-on: https://boringssl-review.googlesource.com/10840
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
BUG=77
Change-Id: Id8c45e98c4c22cdd437cbba1e9375239e123b261
Reviewed-on: https://boringssl-review.googlesource.com/10763
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>
EnableAllCiphers is problematic since some (version, cipher)
combinations aren't even defined and crash. Instead, use the
SendCipherSuite bug to mask the true cipher (which is becomes arbitrary)
for failure tests. The shim should fail long before we get further.
This lets us remove a number of weird checks in the TLS 1.3 code.
This also fixes the UnknownCipher tests which weren't actually testing
anything. EnableAllCiphers is now AdvertiseAllConfiguredCiphers and
does not filter out garbage values.
Change-Id: I7102fa893146bb0d096739e768c5a7aa339e51a8
Reviewed-on: https://boringssl-review.googlesource.com/11481
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>
This is another case where the specification failed to hammer things
down and OpenSSL messed it up as a result. Also fix the SCT test in TLS
1.3.
Change-Id: I47541670447d1929869e1a39b2d9671a127bfba0
Reviewed-on: https://boringssl-review.googlesource.com/11480
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>
The client/server split didn't actually make sense. We're interested in
whether the client will notice the bad version before anything else, so
ignore peer cipher preferences so all combinations work.
Change-Id: I52f84b932509136a9b39d93e46c46729c3864bfd
Reviewed-on: https://boringssl-review.googlesource.com/11413
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
ConflictingVersionNegotiation really should be about, say 1.1 and 1.2
since those may be negotiated via either mechanism. (Those two cases are
actually kinda weird and we may wish to change the spec. But, in the
meantime, test that we have the expected semantics.)
Also test that we ignore true TLS 1.3's number for now, until we use it,
and that TLS 1.3 suitably ignores ClientHello.version.
Change-Id: I76c660ddd179313fa68b15a6fda7a698bef4d9c9
Reviewed-on: https://boringssl-review.googlesource.com/11407
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
They weren't updated for the new version negotiation. (Though right now
they're just testing that we *don't* implement the downgrade detection
because it's a draft version.)
Change-Id: I4c983ebcdf3180d682833caf1e0063467ea41544
Reviewed-on: https://boringssl-review.googlesource.com/11406
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Otherwise we panic. Thanks to EKR for reporting.
Change-Id: Ie4b6c2e18e1c77c7b660ca5d4c3bafb38a82cb6a
Reviewed-on: https://boringssl-review.googlesource.com/11405
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
OpenSSL recently had a regression here (CVE-2016-6309). We're fine,
but so that we stay that way, add some tests.
Change-Id: I244d7ff327b7aad550f86408c5e5e65e6d1babe5
Reviewed-on: https://boringssl-review.googlesource.com/11321
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>
BUG=106
Change-Id: Iaa12aeb67627f3c22fe4a917c89c646cb3dc1843
Reviewed-on: https://boringssl-review.googlesource.com/11325
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>
Change-Id: I73f9fd64b46f26978b897409d817b34ec9d93afd
Reviewed-on: https://boringssl-review.googlesource.com/11080
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>
This mirror's 2dc0204603 on the C side.
BUG=90
Change-Id: Iebb72df5a5ae98cb2fd8db519d973cd734ff05ea
Reviewed-on: https://boringssl-review.googlesource.com/11320
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>
This is in preparation for implementing the version extension and is
probably what we should have done from the beginning as it makes
intolerance bugs simpler.
This means knobs like SendClientVersion and SendServerVersion deal with
the wire values while knobs like NegotiateVersion and MaxVersion deal
with logical versions. (This matches how the bugs have always worked.
SendFoo is just a weird post-processing bit on the handshake messages
while NegotiateVersion actually changes how BoGo behaves.)
BUG=90
Change-Id: I7f359d798d0899fa2742107fb3d854be19e731a4
Reviewed-on: https://boringssl-review.googlesource.com/11300
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>
This GREASEs cipher suites, groups, and extensions. For now, we'll
always place them in a hard-coded position. We can experiment with more
interesting strategies later.
If we add new ciphers and curves, presumably we prefer them over current
ones, so place GREASE values at the front. This prevents implementations
from parsing only the first value and ignoring the rest.
Add two new extensions, one empty and one non-empty. Place the empty one
in front (IBM WebSphere can't handle trailing empty extensions) and the
non-empty one at the end.
Change-Id: If2e009936bc298cedf2a7a593ce7d5d5ddbb841a
Reviewed-on: https://boringssl-review.googlesource.com/11241
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>
Get us a little bit more room here.
BUG=79
Change-Id: Ifadad94ead7794755a33f02d340111694b3572af
Reviewed-on: https://boringssl-review.googlesource.com/11228
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
That is an extremely confusing name. It should be NPN-Declined-TLS13.
Change-Id: I0e5fa50a3ddb0b80e88a8bc10d0ef87d0fff0a54
Reviewed-on: https://boringssl-review.googlesource.com/11227
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We recently added a three-connection option, but the transcripts were
still assuming just -Normal and -Resume.
Change-Id: I8816bce95dd7fac779af658e3eb86bc78bb95c91
Reviewed-on: https://boringssl-review.googlesource.com/11226
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Both the C and Go code were sampling the real clock. With this, two
successive iterations of runner transcripts give the same output.
Change-Id: I4d9e219e863881bf518c5ac199dce938a49cdfaa
Reviewed-on: https://boringssl-review.googlesource.com/11222
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>
We want to ensure -fuzzer passes tests, except for the tests it
intentionally fails on. This ensures that we don't lose our ability to
refresh the fuzzer transcripts.
Change-Id: I761856c30379a3934fd46a24627ef8415b136f93
Reviewed-on: https://boringssl-review.googlesource.com/11221
Reviewed-by: Adam Langley <agl@google.com>
Apparently we never wrote one of those. Also send a decrypt_error alert
to be consistent with all the other signature checks.
Change-Id: Ib5624d098d1e3086245192cdce92f5df26005064
Reviewed-on: https://boringssl-review.googlesource.com/11180
Reviewed-by: David Benjamin <davidben@google.com>
SSL_peek works fine for us, but OpenSSL 1.1.0 regressed this
(https://github.com/openssl/openssl/issues/1563), and we don't have
tests either. Fix this.
SSL_peek can handle all weird events that SSL_read can, so use runner
and tell bssl_shim to do a SSL_peek + SSL_peek + SSL_read instead of
SSL_read. Then add tests for all the events we may discover.
Change-Id: I9e8635e3ca19653a02a883f220ab1332d4412f98
Reviewed-on: https://boringssl-review.googlesource.com/11090
Reviewed-by: Adam Langley <agl@google.com>
The old numbers violate a MUST-level requirement in TLS 1.2 to not
advertise anonymous (0x0700 ends in 0x00). The spec has been updated
with new allocations which avoid these.
BUG=webrtc:6342
Change-Id: Ia5663ada98fa1ebf0f8a7f50fe74a0e9206c4194
Reviewed-on: https://boringssl-review.googlesource.com/11131
Reviewed-by: Adam Langley <agl@google.com>
Found by libFuzzer and then one more mistake caught by valgrind. Add a
test for this case.
Change-Id: I92773bc1231bafe5fc069e8568d93ac0df4c8acb
Reviewed-on: https://boringssl-review.googlesource.com/11129
Reviewed-by: David Benjamin <davidben@google.com>
This is in preparation for using the supported_versions extension to
experiment with draft TLS 1.3 versions, since we don't wish to restore
the fallback. With versions begin opaque values, we will want
version_from_wire to reject unknown values, not attempt to preserve
order in some way.
This means ClientHello.version processing needs to be separate code.
That's just written out fully in negotiate_version now. It also means
SSL_set_{min,max}_version will notice invalid inputs which aligns us
better with upstream's versions of those APIs.
This CL doesn't replace ssl->version with an internal-representation
version, though follow work should do it once a couple of changes land
in consumers.
BUG=90
Change-Id: Id2f5e1fa72847c823ee7f082e9e69f55e51ce9da
Reviewed-on: https://boringssl-review.googlesource.com/11122
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>
Passing --quiet makes valgrind only print out errors, so we don't need
to suppress things. Combine that with checking valgrind's dedicated exit
code so we notice errors that happen before the "---DONE---" marker.
This makes that marker unnecessary for valgrind. all_tests.go was not
sensitive to this, but still would do well to have valgrind be silent.
Change-Id: I841edf7de87081137e38990e647e989fd7567295
Reviewed-on: https://boringssl-review.googlesource.com/11128
Reviewed-by: Adam Langley <agl@google.com>
If the test failed due to non-ASan reasons but ASan also had errors,
output those too.
Change-Id: Id908fe2a823c59255c6a9585dfaa894a4fcd9f59
Reviewed-on: https://boringssl-review.googlesource.com/11127
Reviewed-by: Adam Langley <agl@google.com>
Runner needs to implement fuzzer mode as well so we can record
transcripts from it. A bunch of tests were failing:
- C and Go disagreed on what fuzzer mode did to TLS 1.3 padding. So we
fuzz more code, align Go with C. Fuzzer mode TLS 1.3 still pads but
just skips the final AEAD.
- The deterministic RNG should be applied per test, not per exchange. It
turns out, if your RNG is deterministic, one tends to pick the same
session ID over and over which confuses clients. (Resumption is
signaled by echoing the session ID.)
Now the only failing tests are the ones one would expect to fail.
BUG=79
Change-Id: Ica23881a6e726adae71e6767730519214ebcd62a
Reviewed-on: https://boringssl-review.googlesource.com/11126
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>
If we see garbage in ClientHello.version and then select static RSA,
that garbage is what goes in the premaster.
Change-Id: I65190a44439745e6b5ffaf7669f063da725c8097
Reviewed-on: https://boringssl-review.googlesource.com/11092
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>
Plain PSK omits the ServerKeyExchange when there is no hint and includes
it otherwise (it should have always sent it), while other PSK ciphers
like ECDHE_PSK cannot omit the hint. Having different capabilities here
is odd and RFC 4279 5.2 suggests that all PSK ciphers are capable of
"[not] provid[ing] an identity hint".
Interpret this to mean no identity hint and empty identity hint are the
same state. Annoyingly, this gives a plain PSK implementation two
options for spelling an empty hint. The spec isn't clear and this is not
really a battle worth fighting, so I've left both acceptable and added a
test for this case.
See also https://android-review.googlesource.com/c/275217/. This is also
consistent with Android's PskKeyManager API, our only consumer anyway.
https://developer.android.com/reference/android/net/PskKeyManager.html
Change-Id: I8a8e6cc1f7dd1b8b202cdaf3d4f151bebfb4a25b
Reviewed-on: https://boringssl-review.googlesource.com/11087
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>
access_denied is only used to indicate client cert errors and Chrome
maps it to ERR_SSL_BAD_CLIENT_AUTH_CERT accordingly:
access_denied
A valid certificate was received, but when access control was
applied, the sender decided not to proceed with negotiation. This
message is always fatal.
We don't appear to be the cause of Chrome's recent
ERR_SSL_BAD_CLIENT_AUTH_CERT spike, but we should send these correctly
nonetheless.
If the early callback fails, handshake_failure seems the most
appropriate ("I was unable to find suitable parameters"). There isn't
really an alert that matches DoS, but internal_error seems okay?
internal_error
An internal error unrelated to the peer or the correctness of the
protocol (such as a memory allocation failure) makes it impossible
to continue. This message is always fatal.
There's nothing wrong, per se, with your ClientHello, but I just can't
deal with it right now. Please go away.
Change-Id: Icd1c998c09dc42daa4b309c1a4a0f136b85eb69d
Reviewed-on: https://boringssl-review.googlesource.com/11084
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>
I'm not sure what happened here. These are both the same as
MissingKeyShare-Client.
Change-Id: I6601ed378d8639c1b59034f1e96c09a683bb62ca
Reviewed-on: https://boringssl-review.googlesource.com/11007
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
