In honor of CVE-2016-9244. Although that particular bug BoGo was already
testing since it uses 16 bytes here.
The empty session ID case is particularly worth testing to make sure we
don't get confused somewhere. RFC 5077 allows clients to offer tickets
with no session ID. This is absurd since the client then has no way of
detecting resumption except by lookahead. We'll never do this as a
client, but should handle it correctly as a server.
Change-Id: I49695d19f03c4efdef43749c07372d590a010cda
Reviewed-on: https://boringssl-review.googlesource.com/13740
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The Go side (thankfully not the C side) was not fully updated for the
exporter secret derivation being earlier at some point. Also TLS 1.2
upgrades the PRF hash for pre-1.2 ciphers to SHA-256, so make sure we
cover that.
Change-Id: Ibdf50ef500e7e48a52799ac75577822bc304a613
Reviewed-on: https://boringssl-review.googlesource.com/13663
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: I471880d785c38123e038279f67348bf02b47d091
Reviewed-on: https://boringssl-review.googlesource.com/13662
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 SNI extension may be ACKed by the server. This is kind of pointless,
but make sure we cover these codepaths.
Change-Id: I14b25ab865dd6e35a30f11ebc9027a1518bbeed9
Reviewed-on: https://boringssl-review.googlesource.com/13633
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: I878dfb9f5d3736c3ec0d5fa39052cca58932dbb7
Reviewed-on: https://boringssl-review.googlesource.com/12981
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: I38cd04fa40edde4e4dd31fdc16bbf92985430198
Reviewed-on: https://boringssl-review.googlesource.com/12702
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 TLS 1.2 and 1.3 state machines do the exact same thing at the
beginning. Let them process the ClientHello extensions, etc., and
finalize the certificate in common code. Once we start picking
parameters, we begin to diverge. Everything before this point is
arguably part of setting up the configuration, which is
version-agnostic.
BUG=128
Change-Id: I293ea3087ecbc3267bd8cdaa011c98d26a699789
Reviewed-on: https://boringssl-review.googlesource.com/13562
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
In TLS 1.2, resumption's benefits are more-or-less subsumed by False
Start. TLS 1.2 resumption lifetime is bounded by how much traffic we are
willing to encrypt without fresh key material, so the lifetime is short.
Renewal uses the same key, so we do not allow it to increase lifetimes.
In TLS 1.3, resumption unlocks 0-RTT. We do not implement psk_ke, so
resumption incorporates fresh key material into both encrypted traffic
(except for early data) and renewed tickets. Thus we are both more
willing to and more interested in longer lifetimes for tickets. Renewal
is also not useless. Thus in TLS 1.3, lifetime is bound separately by
the lifetime of a given secret as a psk_dhe_ke authenticator and the
lifetime of the online signature which authenticated the initial
handshake.
This change maintains two lifetimes on an SSL_SESSION: timeout which is
the renewable lifetime of this ticket, and auth_timeout which is the
non-renewable cliff. It also separates the TLS 1.2 and TLS 1.3 timeouts.
The old session timeout defaults and configuration apply to TLS 1.3, and
we define new ones for TLS 1.3.
Finally, this makes us honor the NewSessionTicket timeout in TLS 1.3.
It's no longer a "hint" in 1.3 and there's probably value in avoiding
known-useless 0-RTT offers.
BUG=120
Change-Id: Iac46d56e5a6a377d8b88b8fa31f492d534cb1b85
Reviewed-on: https://boringssl-review.googlesource.com/13503
Reviewed-by: Adam Langley <agl@google.com>
Fix this and add a test. Otherwise enabling TLS 1.3 will cause a server
to blow through its session cache.
Change-Id: I67edbc468faedfd94a6c30cf842af085a6543b50
Reviewed-on: https://boringssl-review.googlesource.com/13501
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: I44202457841f06a899e140f78ae8afa7ac720283
Reviewed-on: https://boringssl-review.googlesource.com/12600
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>
Until we've gotten it fully working, we should not mint any of these
SSL_SESSIONs, to avoid constraining future versions of our client code.
Notably, if any of our TLS 1.3 clients today serialized sessions, we
would need to rev the serialization format. Without opting into 0-RTT, a
TLS 1.3 client will create SSL_SESSIONs tagged as 0-RTT-capable but
missing important fields (ALPN, etc.). When that serialized session
makes its way to a future version of our client code, it would disagree
with the server about the ALPN value stored in the ticket and cause
interop failures.
I believe the only client code enabling TLS 1.3 right now is Chrome, and
the window is small, so it should be fine. But fix this now before it
becomes a problem.
Change-Id: Ie2b109f8d158017a6f3b4cb6169050d38a66b31c
Reviewed-on: https://boringssl-review.googlesource.com/13342
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>
Change-Id: Idc93fdca2f1c5c23e4ba48c4efed2edbad1e857b
Reviewed-on: https://boringssl-review.googlesource.com/12521
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>
It's not completely clear to me why select_cetificate_cb behaves the way it
does, however not only is it confusing, but it makes assumptions about the
application using BoringSSL (it's not always possible to implement custom
logic outside of the callbacks provided by libssl), that make this callback
somewhat useless.
Case in point, the callback can be used for changing min/max protocol versions
based on per-site policies, and select_certificate_cb is the only place where
SSL_set_min/max_proto_version() can be used (e.g. you can't call them in
cert_cb because it's too late), but the decision on the specific versions to
use might depend on configuration that needs retrieving asynchronously from
over the network, which requires re-running the callback multiple times.
Change-Id: Ia8e151b163628545373e7fd1f327e9af207478a6
Reviewed-on: https://boringssl-review.googlesource.com/13000
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Before RFC 7539 we had a ChaCha20-Poly1305 cipher suite that had a 64/64
nonce/counter split (as DJB's original ChaCha20 did). RFC 7539 changed
that to 96/32 and we've supported both for some time.
This change removes the old version and the TLS cipher suites that used
it.
BUG=chromium:682816
Change-Id: I2345d6db83441691fe0c1ab6d7c6da4d24777849
Reviewed-on: https://boringssl-review.googlesource.com/13203
Reviewed-by: Adam Langley <agl@google.com>
This reverts commit def9b46801.
(I should have uploaded a new version before sending to the commit queue.)
Change-Id: Iaead89c8d7fc1f56e6294d869db9238b467f520a
Reviewed-on: https://boringssl-review.googlesource.com/13202
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>
Before RFC 7539 we had a ChaCha20-Poly1305 cipher suite that had a 64/64
nonce/counter split (as DJB's original ChaCha20 did). RFC 7539 changed
that to 96/32 and we've supported both for some time.
This change removes the old version and the TLS cipher suites that used
it.
Change-Id: Icd9c2117c657f3aa6df55990c618d562194ef0e8
Reviewed-on: https://boringssl-review.googlesource.com/13201
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
This gives coverage over needing to fragment something over multiple
records.
Change-Id: I2373613608ef669358d48f4e12f68577fa5a40dc
Reviewed-on: https://boringssl-review.googlesource.com/13101
Reviewed-by: Adam Langley <agl@google.com>
TLS 1.3 doesn't support renegotiation in the first place, but so callers
don't report TLS 1.3 servers as missing it, always report it as
(vacuously) protected against this bug.
BUG=chromium:680281
Change-Id: Ibfec03102b2aec7eaa773c331d6844292e7bb685
Reviewed-on: https://boringssl-review.googlesource.com/13046
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>
08b65f4e31 introduced a memory leak and
also got enums confused. Also fix a codepath that was missing an error
code.
Thanks to OSS-Fuzz which appears to have found it in a matter of hours.
Change-Id: Ia9e926c28a01daab3e6154d363d0acda91209a22
Reviewed-on: https://boringssl-review.googlesource.com/13104
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This adds support for setting 0-RTT mode on tickets minted by
BoringSSL, allowing for testing of the initial handshake knowledge.
BUG=76
Change-Id: Ic199842c03b5401ef122a537fdb7ed9e9a5c635a
Reviewed-on: https://boringssl-review.googlesource.com/12740
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>
As long as we still have this code, we should make sure it doesn't
regress.
Change-Id: I0290792aedcf667ec49b251d747ffbc141c0cec4
Reviewed-on: https://boringssl-review.googlesource.com/13053
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The write path for TLS is going to need some work. There are some fiddly
cases when there is a write in progress. Start adding tests to cover
this logic.
Later I'm hoping we can extend this flag so it drains the unfinished
write and thus test the interaction of read/write paths in 0-RTT. (We
may discover 1-RTT keys while we're in the middle of writing data.)
Change-Id: Iac2c417e4b5e84794fb699dd7cbba26a883b64ef
Reviewed-on: https://boringssl-review.googlesource.com/13049
Reviewed-by: Adam Langley <agl@google.com>
Channel ID is already enabled on the SSL. This dates to
49c7af1c42 which converted an instance of
tlsext_channel_id_enabled_new to it, but tlsext_channel_id_enabled_new
meant "if Channel ID is enabled, use the new one", not "enable Channel
ID".
Thanks to Eric Rescorla for catching this.
Change-Id: I2d5a82b930ffcbe5527a62a9aa5605ebb71a6b9f
Reviewed-on: https://boringssl-review.googlesource.com/13042
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Upstream accidentally started rejecting server-sent point formats in
https://github.com/openssl/openssl/issues/2133. Our own test coverage
here is also lacking, so flesh it out.
Change-Id: I99059558bd28d3a540c9687649d6db7e16579d29
Reviewed-on: https://boringssl-review.googlesource.com/12979
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: Ie4c566c29c20faac7a9a5e04c88503fc2e1ff4db
Reviewed-on: https://boringssl-review.googlesource.com/12970
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
This extension will be used to test whether
https://github.com/tlswg/tls13-spec/pull/762 is deployable against
middleboxes. For simplicity, it is mutually exclusive with 0-RTT. If
client and server agree on the extension, TLS 1.3 records will use the
format in the PR rather than what is in draft 18.
BUG=119
Change-Id: I1372ddf7b328ddf73d496df54ac03a95ede961e1
Reviewed-on: https://boringssl-review.googlesource.com/12684
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>
Most C standard library functions are undefined if passed NULL, even
when the corresponding length is zero. This gives them (and, in turn,
all functions which call them) surprising behavior on empty arrays.
Some compilers will miscompile code due to this rule. See also
https://www.imperialviolet.org/2016/06/26/nonnull.html
Add OPENSSL_memcpy, etc., wrappers which avoid this problem.
BUG=23
Change-Id: I95f42b23e92945af0e681264fffaf578e7f8465e
Reviewed-on: https://boringssl-review.googlesource.com/12928
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We were only asserting on the shim-side error code.
Change-Id: Idc08c253a7723a2a7fd489da761a56c72f7a3b96
Reviewed-on: https://boringssl-review.googlesource.com/12923
Reviewed-by: Adam Langley <agl@google.com>
It should probably have a TLS 1.3 in the name to be clear that's what
it's testing.
Change-Id: I50b5f503a8038715114136179bde83e7da064e9b
Reviewed-on: https://boringssl-review.googlesource.com/12961
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>
There are no longer any consumers of these APIs.
These were useful back when the CBC vs. RC4 tradeoff varied by version
and it was worth carefully tuning this cutoff. Nowadays RC4 is
completely gone and there's no use in configuring these anymore.
To avoid invalidating the existing ssl_ctx_api corpus and requiring it
regenerated, I've left the entries in there. It's probably reasonable
for new API fuzzers to reuse those slots.
Change-Id: I02bf950e3828062341e4e45c8871a44597ae93d5
Reviewed-on: https://boringssl-review.googlesource.com/12880
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The loop is getting a little deeply nested and hard to read.
Change-Id: I3a99fba54c2f352850b83aef91ab72d5d9aabfb8
Reviewed-on: https://boringssl-review.googlesource.com/12685
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>
Also fix the error code. It's a missing extension, not an unexpected
one.
Change-Id: I48e48c37e27173f6d7ac5e993779948ead3706f2
Reviewed-on: https://boringssl-review.googlesource.com/12683
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>
So we can report it cleanly out of DevTools, it should behave like
SSL_get_curve_id and be reported on resumption too.
BUG=chromium:658905
Change-Id: I0402e540a1e722e09eaebadf7fb4785d8880c389
Reviewed-on: https://boringssl-review.googlesource.com/12694
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>
Also test that TLS 1.3 can be resumed at a different curve.
Change-Id: Ic58e03ad858c861958b7c934813c3e448fb2829c
Reviewed-on: https://boringssl-review.googlesource.com/12692
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>
Nothing calls this anymore. DHE is nearly gone. This unblocks us from
making key_exchange_info only apply to the curve.
Change-Id: I3099e7222a62441df6e01411767d48166a0729b1
Reviewed-on: https://boringssl-review.googlesource.com/12691
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 currently only works for certificates parsed from the network, but
if making several connections that share certificates, some KB of memory
might be saved.
BUG=chromium:671420
Change-Id: I1c7a71d84e1976138641f71830aafff87f795f9d
Reviewed-on: https://boringssl-review.googlesource.com/12706
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 avoids needing a extra state around client certificates to avoid
calling the callbacks twice. This does, however, come with a behavior
change: configuring both callbacks won't work. No consumer does this.
(Except bssl_shim which needed slight tweaks.)
Change-Id: Ia5426ed2620e40eecdcf352216c4a46764e31a9a
Reviewed-on: https://boringssl-review.googlesource.com/12690
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: Iaac633616a54ba1ed04c14e4778865c169a68621
Reviewed-on: https://boringssl-review.googlesource.com/12703
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: Ie947ab176d10feb709c6e135d5241c6cf605b8e8
Reviewed-on: https://boringssl-review.googlesource.com/12700
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>
Right now the only way to set an OCSP response is SSL_CTX_set_ocsp_response
however this assumes that all the SSLs generated from a SSL_CTX share the
same OCSP response, which is wrong.
This is similar to the OpenSSL "function" SSL_get_tlsext_status_ocsp_resp,
the main difference being that this doesn't take ownership of the OCSP buffer.
In order to avoid memory duplication in case SSL_CTX has its own response,
a CRYPTO_BUFFER is used for both SSL_CTX and SSL.
Change-Id: I3a0697f82b805ac42a22be9b6bb596aa0b530025
Reviewed-on: https://boringssl-review.googlesource.com/12660
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 was useful when we were transitioning NPN off in Chromium, but now
there are no callers remaining.
Change-Id: Ic619613d6d475eea6bc258c4a90148f129ea4a81
Reviewed-on: https://boringssl-review.googlesource.com/12637
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>
We've taken to writing bssl::UniquePtr in full, so it's not buying
us much.
Change-Id: Ia2689366cbb17282c8063608dddcc675518ec0ca
Reviewed-on: https://boringssl-review.googlesource.com/12628
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: Ida26e32a700c68e1899f9f6ccff73e2fa5252313
Reviewed-on: https://boringssl-review.googlesource.com/12633
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This allows a consumer to disable Channel ID (for instance, it may be
enabled on the SSL_CTX and later disabled on the SSL) without reaching
into the SSL struct directly.
Deprecate the old APIs in favor of these.
BUG=6
Change-Id: I193bf94bc1f537e1a81602a39fc2b9a73f44c73b
Reviewed-on: https://boringssl-review.googlesource.com/12623
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>
It's our ClientHello representation. May as well name it accordingly.
Also switch away from calling the variable name ctx as that conflicts
with SSL_CTX.
Change-Id: Iec0e597af37137270339e9754c6e08116198899e
Reviewed-on: https://boringssl-review.googlesource.com/12581
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>
Change-Id: I1e28ba84de59336cab432d1db3dd9c6023909081
Reviewed-on: https://boringssl-review.googlesource.com/12625
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: Ie46d45cdb07c692a789594e13040a1ce9d6cf83d
Reviewed-on: https://boringssl-review.googlesource.com/12640
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>
finishedHash should keep a running secret and incorporate entropy as is
available.
Change-Id: I2d245897e7520b2317bc0051fa4d821c32eeaa10
Reviewed-on: https://boringssl-review.googlesource.com/12586
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>
