We have at least three different external build definitions for the
fuzzers. That's enough that requiring each of them account for the split
fuzzer mode is probably too much turbulence.
Change-Id: I96dbb12a2b4f70bfa1b04cd0d15fda918bbf51d6
Reviewed-on: https://boringssl-review.googlesource.com/12183
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This change renames |peer| to |x509_peer| and |cert_chain| to
|x509_chain| in |SSL_SESSION|. It also renames |x509| to |x509_leaf| and
|chain| to |x509_chain| in |CERT|. (All with an eye to maybe making
them lazily initialised in the future).
This a) catches anyone who might be accessing these members directly and
b) makes space for |CRYPTO_BUFFER|-based values to take the unprefixed
names.
Change-Id: I10573304fb7d6f1ea03f9e645f7fc0acdaf71ac2
Reviewed-on: https://boringssl-review.googlesource.com/12162
Reviewed-by: David Benjamin <davidben@google.com>
In transition to removing it altogether, set SSL_MODE_NO_AUTO_CHAIN by
default. If we find some consumer was relying on it, this will allow
them to revert locally with SSL_(CTX_)clear_mode, but hopefully this was
just unused.
BUG=42
Change-Id: Iaf70a436a3324ce02e02dfb18213b6715c034ff2
Reviewed-on: https://boringssl-review.googlesource.com/12180
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Add missing includes of stdio.h, and prefer |IN6ADDR_ANY_INIT| to
|in6addr_any|.
Change-Id: Ia6663ecd6f87008cb82979ef65620a55d8c9405b
Reviewed-on: https://boringssl-review.googlesource.com/11626
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>
3c6a1ea674 switched what layer handled
the DTLS version mapping but forgot to correct the HelloVerifyRequest
logic to account for this.
Thanks to Jed Davis for noticing this.
Change-Id: I94ea18fc43a7ba15dd7250bfbcf44dbb3361b3ce
Reviewed-on: https://boringssl-review.googlesource.com/11984
Reviewed-by: David Benjamin <davidben@google.com>
This is already manually released at the end of the handshake. With this
change, it can happen implicitly, and SSL3_STATE shrinks further by
another pointer.
Change-Id: I94b9f2e4df55e8f2aa0b3a8799baa3b9a34d7ac1
Reviewed-on: https://boringssl-review.googlesource.com/12121
Reviewed-by: Adam Langley <agl@google.com>
Fuzzer mode explores the handshake, but at the cost of losing coverage
on the record layer. Add a separate build flag and client/server
corpora for this mode.
Note this requires tweaks in consumers' fuzzer build definitions.
BUG=111
Change-Id: I1026dc7301645e165a761068a1daad6eedc9271e
Reviewed-on: https://boringssl-review.googlesource.com/12108
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I33a9cb2cc13f2ed64a6bf2728cd3fcc980e1408f
Reviewed-on: https://boringssl-review.googlesource.com/12161
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>
In https://boringssl-review.googlesource.com/#/c/11920/2, I addressed a
number of comments but then forgot to upload the change before
submitting it. This change contains the changes that should have been
included in that commit.
Change-Id: Ib70548e791f80abf07a734e071428de8ebedb907
Reviewed-on: https://boringssl-review.googlesource.com/12160
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>
The distinction for full handshakes is not meaningful (the timestamp is
currently the start of the handshake), but for renewed sessions, we
currently retain the timestamp of the original issuance.
Instead, when minting or receiving tickets, adjust session->time and
session->timeout so that session->time is the ticket issuance time.
This is still not our final TLS 1.3 behavior (which will need a both
renewable and non-renewable times to honor the server ticket lifetime),
but it gets us closer and unblocks handling ticket_age_add from TLS 1.3
draft 18 and sends the correct NewSessionTicket lifetime.
This fixes the ticket lifetime hint which we emit on the server to
mirror the true ticket lifetime. It also fixes the TLS 1.3 server code
to not set the ticket lifetime hint. There is no need to waste ticket
size with it, it is no longer a "hint" in TLS 1.3, and even in the TLS
1.3 code we didn't fill it in on the server.
Change-Id: I140541f1005a24e53e1b1eaa90996d6dada1c3a1
Reviewed-on: https://boringssl-review.googlesource.com/12105
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 there is a malloc failure while assembling the ticket, call
CBB_cleanup. Also return -1 instead of 0; zero means EOF in the old
state machine and -1 means error. (Except enough of the stack gets it
wrong that consumers handle both, but we should fix this.)
Change-Id: I98541a9fa12772ec159f9992d1f9f53e5ca4cc5a
Reviewed-on: https://boringssl-review.googlesource.com/12104
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
There's no sense in flushing twice in one flight. This means when
writing a message is finally synchronous, we don't need the intermediate
state at all.
Change-Id: Iaca60d64917f82dce0456a8b15de4ee00f2d557b
Reviewed-on: https://boringssl-review.googlesource.com/12103
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
TLS 1.3 clarifies that a ticket lifetime of zero means the session is
unusable. We don't currently pay attention to that field (to be fixed in
later changes) but, in preparation for this, switch the >= to a >.
Change-Id: I0e67a0d97bc8def04914f121e84d3e7a2d640d2c
Reviewed-on: https://boringssl-review.googlesource.com/12102
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
These don't make sense and mean some SSL_SESSIONs serialize and
deserialize as different values. If we ever managed to create an
SSL_SESSION without a time, it would never expire because time always
gets set to time(NULL). If we ever created an SSL_SESSION with a zero
timeout, the timeout would be... three? Once we start adjusting
time/timeout to issuance time, driving timeout to zero is actually
plausible, so it should work properly.
Instead, make neither field optional. We always fill both out, so this
shouldn't have any effects. If it does, the only effect would be to
decline to resume some existing tickets which must have been so old that
we'd want them to have expired anyway.
Change-Id: Iee3620658c467dd6d96a2b695fec831721b03b5b
Reviewed-on: https://boringssl-review.googlesource.com/12101
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The values are long, so check for negative numbers.
Change-Id: I8fc7333edbed50dc058547a4b53bc10b234071b4
Reviewed-on: https://boringssl-review.googlesource.com/12100
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This business with |ok| is unnecessary. This function is still rather a
mess, but this is a small improvement.
Change-Id: I28fdf1a3687fe6a9d58d81a22cf2f8e7ce5b9b2c
Reviewed-on: https://boringssl-review.googlesource.com/12080
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>
A renewed session does not refresh the timeout. Add tests for this in
preparation for future changes which will revise this logic.
Specifically, TLS 1.3 draft 18's ticket_age_add logic will require some
tweaks in lifetime tracking to record when the ticket was minted. We'll
also likely wish to tweak the parameters for 1.3 to account for (a)
ECDHE-PSK means we're only worried about expiring a short-circuited
authentication rather than forward secrecy and (b) two hours is too
short for a QUIC 0-RTT replacement.
Change-Id: I0f1edd09151e7fcb5aee2742ef8600fbd7080df6
Reviewed-on: https://boringssl-review.googlesource.com/12002
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 function allows callers to unpack an Ed25519 “seed” value, which is
a 32 byte value that contains sufficient information to build a public
and private key from.
Change-Id: Ie5d8212a73e5710306314b4f8a93b707665870fd
Reviewed-on: https://boringssl-review.googlesource.com/12040
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The naming breaks layering, but it seems we're stuck with it. We don't
seem to have bothered making first-party code call it BIO_print_errors
(I found no callers of BIO_print_errors), so let's just leave it at
ERR_print_errors.
Change-Id: Iddc22a6afc2c61d4b94ac555be95079e0f477171
Reviewed-on: https://boringssl-review.googlesource.com/11960
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This is only used in one place where we don't take advantage of it being
sorted anyway.
Change-Id: If6f0d04e975db903e8a93c57c869ea4964c0be37
Reviewed-on: https://boringssl-review.googlesource.com/12062
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This is the last blocker within BoringSSL itself to opaquifying SSL.
(There are still blockers in consumers, of course.)
BUG=6
Change-Id: Ie3b8dcb78eeaa9aea7311406c5431a8625d60401
Reviewed-on: https://boringssl-review.googlesource.com/12061
Reviewed-by: Adam Langley <agl@google.com>
TLS 1.3 ciphers are now always enabled and come with a hard-coded
preference order.
BUG=110
Change-Id: Idd9cb0d75fb6bf2676ecdee27d88893ff974c4a3
Reviewed-on: https://boringssl-review.googlesource.com/12025
Reviewed-by: Adam Langley <agl@google.com>
HTTP/2 requires TLS 1.2 so the negotiated version should be available
during the ALPN callback.
Change-Id: Iea332808b531a6e5c917de5b8c8917c0aa7428a1
Reviewed-on: https://boringssl-review.googlesource.com/12060
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>
They will get very confused about which key they're using. Any caller
using exporters must either (a) leave renegotiation off or (b) be very
aware of when renegotiations happen anyway. (You need to somehow
coordinate with the peer about which epoch's exporter to use.)
Change-Id: I921ad01ac9bdc88f3fd0f8283757ce673a47ec75
Reviewed-on: https://boringssl-review.googlesource.com/12003
Reviewed-by: Adam Langley <agl@google.com>
The existing tests for this codepath require us to reconfigure the shim.
This will not work when TLS 1.3 cipher configuration is detached from
the old cipher language. It also doesn't hit codepaths like sessions
containing a TLS 1.3 version but TLS 1.2 cipher.
Instead, add some logic to the runner to rewrite tickets and build tests
out of that.
Change-Id: I57ac5d49c3069497ed9aaf430afc65c631014bf6
Reviewed-on: https://boringssl-review.googlesource.com/12024
Reviewed-by: Adam Langley <agl@google.com>
It is not ignored.
Change-Id: I2e607a6d6f7444838fc6fa65cd18e9aa142f139f
Reviewed-on: https://boringssl-review.googlesource.com/12023
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
HTTP/2 places requirements on the cipher suite. So that servers can
decline HTTP/2 when these requirements aren't met, defer ALPN
negotiation.
See also b/32553041.
Change-Id: Idbcf049f9c8bda06a8be52a0154fe76e84607268
Reviewed-on: https://boringssl-review.googlesource.com/11982
Reviewed-by: Adam Langley <agl@google.com>
We were only testing one side.
Change-Id: Ieb755e27b235aaf1317bd2c8e5fb374cb0ecfdb3
Reviewed-on: https://boringssl-review.googlesource.com/12001
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: I87cbc12aeb399646c6426b7a099dbf13aafc2532
Reviewed-on: https://boringssl-review.googlesource.com/11983
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
d2i_X509_from_buffer parses an |X509| from a |CRYPTO_BUFFER| but ensures
that the |X509_CINF.enc| doesn't make a copy of the encoded
TBSCertificate. Rather the |X509| holds a reference to the given
|CRYPTO_BUFFER|.
Change-Id: I38a4e3d0ca69fc0fd0ef3e15b53181844080fcad
Reviewed-on: https://boringssl-review.googlesource.com/11920
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Trim a few more bytes from the future QUIC ClientHello.
Change-Id: If23c5cd078889a9a26cf2231b51b17c2615a38ea
Reviewed-on: https://boringssl-review.googlesource.com/12000
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>
Get some of the duplicate logic out of the way.
Change-Id: Iee7c64577e14d1ddfead7e1e32c42c5c9f2a310d
Reviewed-on: https://boringssl-review.googlesource.com/11981
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>
TLS 1.3 also uses this extension and doesn't use any EC-based suites.
Always offering the extension is simpler. Also this gets an
SSL_get_ciphers call out of the way (that function is somewhat messy in
semantics).
Change-Id: I2091cb1046e0aea85caa76e73f50e8416e6ed94c
Reviewed-on: https://boringssl-review.googlesource.com/11980
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>
This change is based on interpreting TLS 1.3 draft 18.
Change-Id: I727961aff2f7318bcbbc8bf6d62b7d6ad3e62da9
Reviewed-on: https://boringssl-review.googlesource.com/11921
Reviewed-by: David Benjamin <davidben@google.com>
Bazel has moved their primary site to bazel.build.
(Thanks to Damien Martin-guillerez for the change.)
Change-Id: Ifb29dbb79f1e1d9611f40992a3e75e0fb5a3722a
Reviewed-on: https://boringssl-review.googlesource.com/11961
Reviewed-by: David Benjamin <davidben@google.com>
This should never happen, but the SSL_AEAD_CTX_new layer should enforce
key sizes as it's not locally obvious at the call site the caller didn't
get confused. There's still a mess of asserts below, but those should be
fixed by cutting the SSL_CIPHER/SSL_AEAD_CTX boundary differently.
(enc_key_len is validated by virtue of being passed into EVP_AEAD.)
BUG=chromium:659593
Change-Id: I8c91609bcef14ca1509c87aab981bbad6556975f
Reviewed-on: https://boringssl-review.googlesource.com/11940
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: I0aab9c94fcfa58b9cd46eaf716d9317f532f79a2
Reviewed-on: https://boringssl-review.googlesource.com/11850
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: Adam Langley <agl@google.com>
These were forward-declared for SSL3_STATE but with that hidden, it's no
longer necessary.
Change-Id: I8c548822f56f6172b4033b2fa89c038adcec2caa
Reviewed-on: https://boringssl-review.googlesource.com/11860
Reviewed-by: Adam Langley <agl@google.com>
Later work is going to cause some turbulence here.
Change-Id: Iba98bcf56e81492ec0dca54a381b38d1c115247a
Reviewed-on: https://boringssl-review.googlesource.com/11843
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>
Tagging non-pointer return types const doesn't do anything and makes
some compilers grumpy. Thanks to Daniel Hirche for the report.
Change-Id: I157ddefd8f7e604b4d8317ffa2caddb8f0dd89de
Reviewed-on: https://boringssl-review.googlesource.com/11849
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Neither branch of the |if| statement is expected to touch |data_len|.
Clarify this by moving |data_len| after the |if| statement.
Change-Id: Ibbc81e5b0c006882672df18442a6e7987064ca6d
Reviewed-on: https://boringssl-review.googlesource.com/11880
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This is still rather a mess with how it's tied to SSL_AEAD_CTX_new
(probably these should get encapsulated in an SSL_AEAD struct), but this
avoids running the TLS 1.3 nonce logic on fake AEADs. This is impossible
based on cipher version checks, but we shouldn't need to rely on it.
It's also a little tidier since out_mac_secret_len is purely a function
of algorithm_mac.
BUG=chromium:659593
Change-Id: Icc24d43c54a582bcd189d55958e2d232ca2db4dd
Reviewed-on: https://boringssl-review.googlesource.com/11842
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>
This patch changes the urandom PRNG to read one byte from the
getrandom(2) Linux syscall on initialization in order to find any
unexpected behavior.
Change-Id: I8ef676854dc361e4f77527b53d1a14fd14d449a8
Reviewed-on: https://boringssl-review.googlesource.com/8681
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
These structures allow for blobs of data (e.g. certificates) to be
deduplicated in memory.
Change-Id: Iebfec90b85d55565848a178b6951562b4ccc083e
Reviewed-on: https://boringssl-review.googlesource.com/11820
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This shouldn't happen, but it is good to check to avoid the potential
underflow in ssl_session_is_time_valid.
This required tweaking the mock clock in bssl_shim to stop going back in
time.
Change-Id: Id3ab8755139e989190d0b53d4bf90fe1ac203022
Reviewed-on: https://boringssl-review.googlesource.com/11841
Reviewed-by: David Benjamin <davidben@google.com>
David Benjamin
Adam Langley
Matthew Braithwaite
Kenneth Geisshirt
Ladar Levison
Adam Langley
Steven Valdez
Brian Smith
Alessandro Ghedini