Clang for Windows does not like OPENSSL_COMPILE_ASSERT inside a function
in C++. It complains that the struct is unused. I think we worked around
this in C previously by making it expand to C11 _Static_assert when
available.
But libssl is now C++ and assumes a C++11-capable compiler. Use real
static_assert.
Bug: 132
Change-Id: I6aceb95360244bd2c80d194b80676483abb60519
Reviewed-on: https://boringssl-review.googlesource.com/17924
Reviewed-by: Adam Langley <agl@google.com>
This plumbs EVP_AEAD_CTX_seal_scatter all the way through to
tls_record.c, so we can add a new zero-copy record sealing method on top
of the existing code.
Change-Id: I01fdd88abef5442dc16605ea31b29b4b1231c073
Reviewed-on: https://boringssl-review.googlesource.com/17684
Reviewed-by: Adam Langley <agl@google.com>
Due to SSL 3.0 legacy, TLS 1.0 through 1.2 allow ClientHello and
ServerHello messages to omit the extensions field altogether, rather
than write an empty field. We broke this in
https://boringssl-review.googlesource.com/c/17704/ when we needed to a
second ServerHello parsing path.
Fix this and add some regression tests to explicitly test both the
omitted and empty extensions ClientHello and ServerHello cases.
Bug: chromium:743218
Change-Id: I8297ba608570238e19f12ea44a9fe2fe9d881d28
Reviewed-on: https://boringssl-review.googlesource.com/17904
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This is in an attempt to debug the Mac flakiness. The timestamps will
hopefully help narrow down the order of operations here.
Bug: 199
Change-Id: I8b8dd7222e3a57a8b055b8bc1b7731334e0fcdf0
Reviewed-on: https://boringssl-review.googlesource.com/17886
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This was broken when we added the API to SSL.
Change-Id: I92d4330b0d70f655c9a9ad33898d6b84704e915c
Reviewed-on: https://boringssl-review.googlesource.com/17884
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This implements PR #1051
(https://github.com/tlswg/tls13-spec/pull/1051).
Local experiments were not able to replicate the claims in the PR, but
implement this anyway for comparison purposes.
Change-Id: Ic9baf5e671f9a44565020466a553dd08f5ec0f1b
Reviewed-on: https://boringssl-review.googlesource.com/17844
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
I suspect this won't actually tell us much useful w.r.t. the Mac test
flakes, but we may as well print what we can get.
Change-Id: I4931f6000648c4bd955a132b54351ff83d6b6273
Reviewed-on: https://boringssl-review.googlesource.com/17804
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
And, with that, stage one is complete. ssl/internal.h may include C++.
Bug: 132
Change-Id: I0cb89f0ed5f4be36632a50744a80321595dc921c
Reviewed-on: https://boringssl-review.googlesource.com/17768
Reviewed-by: Steven Valdez <svaldez@google.com>
This leaves just the TLS 1.3 handshake code.
Bug: 132
Change-Id: I2bd87b0ecd0ae7d6ea1302bc62c67aec5ca1dccb
Reviewed-on: https://boringssl-review.googlesource.com/17767
Reviewed-by: Steven Valdez <svaldez@google.com>
Bug: 132
Change-Id: I2b0c87262a5a529ea264ea8ce2d11c2dba0ec1c8
Reviewed-on: https://boringssl-review.googlesource.com/17766
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
In the process, merge the old canary function back in.
Bug: 132
Change-Id: Ib455320ecea67c839d0b4ac3882669d24f832b74
Reviewed-on: https://boringssl-review.googlesource.com/17765
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Bug: 132
Change-Id: I0b83bb05082aa6dad8c15f906cebc2d4f2d5216b
Reviewed-on: https://boringssl-review.googlesource.com/17764
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
The EC_POINT munging is sufficiently heavy on the goto err that I went
ahead and tidied it up.
Bug: 132
Change-Id: I7a3b3b3f166e39e4559acec834dd8e1ea9ac8620
Reviewed-on: https://boringssl-review.googlesource.com/17747
Reviewed-by: 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>
ssl_cipher required fixing the types of the cipher masks.
Bug: 132
Change-Id: I0428d853b25fe4674ac3cad87a8eb92c6c8659e3
Reviewed-on: https://boringssl-review.googlesource.com/17746
Reviewed-by: 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: 132
Change-Id: Ic68252de7b3a8f90d60f052a3cb707730d5a2b16
Reviewed-on: https://boringssl-review.googlesource.com/17744
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
http://i1.kym-cdn.com/photos/images/original/000/242/631/382.gif
In the first step, switch C files to C++ individually, keeping
everything in internal.h C-compatible. We'll make minimal changes needed
to get things compiling (notably a lot of goto errs will need to turn to
bssl::UniquePtr right away), but more aggressive changes will happen in
later steps.
(To avoid a rebase, I'm intentionally avoiding files that would conflict
with CLs in flight right now.)
Bug: 132
Change-Id: Id4cfd722e7b57d1df11f27236b4658b5d39b5fd2
Reviewed-on: https://boringssl-review.googlesource.com/17667
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: I3de3c48a1de59c2b8de348253ce62a648aa6d6eb
Reviewed-on: https://boringssl-review.googlesource.com/17724
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: Ie8216ab9de2edf37ae3240a5cb97d974e8252d93
Reviewed-on: https://boringssl-review.googlesource.com/17709
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>
Adding it to tlsVersions is sort of pointless when we don't test it.
Change-Id: Ie0c0167cef887aee54e5be90bf7fc98619c1a6fb
Reviewed-on: https://boringssl-review.googlesource.com/17708
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
TLS 1.3 deployment is currently blocked by buggy middleboxes
throughout the ecosystem. As an experiment to better understand these bugs
and the problems they are causing, implement TLS 1.3 variants with
alternate encodings. These are still the same protocol, only encoded
slightly differently. We will use what we learn from these experiments to
guide the TLS 1.3 deployment strategy and proposals to the IETF, if any.
These experiments only target the basic 1-RTT TLS 1.3 handshake. Based on
what we learn from this experiment, we may try future variations to
explore 0-RTT and HelloRetryRequest.
When enabled, the server supports all TLS 1.3 variants while the client
is configured to use a particular variant.
Change-Id: I532411d1abc41314dc76acce0246879b754b4c61
Reviewed-on: https://boringssl-review.googlesource.com/17327
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>
Not sure why it was expanded out like that.
Change-Id: I6899dbd23130ed7196c45c2784330b2a4fe9bdba
Reviewed-on: https://boringssl-review.googlesource.com/17666
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Like other handshake properties, when in 0-RTT on the client,
SSL_version should report the predicted version. This used to work on
accident because of how ssl->version got set in handshake_client.c early
(and that TLS 1.4 does not exist), but we no longer do that.
Change-Id: Ifb63a22b795fe8964ac553844a46040acd5d7323
Reviewed-on: https://boringssl-review.googlesource.com/17664
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We were missing AES256 and 3DES. Though this test dates to the old
record-splitting code which was much scarier than the new one.
Change-Id: Ia84a8c1a2bbd79fa70941f80cf6393013e4f13d5
Reviewed-on: https://boringssl-review.googlesource.com/17543
Reviewed-by: David Benjamin <davidben@google.com>
The in_group check is redundant and test an extremely absurd corner of
the syntax.
Change-Id: Ia54bcd7cda7ba05415d3a250ee93e1acedcc43d6
Reviewed-on: https://boringssl-review.googlesource.com/17542
Reviewed-by: David Benjamin <davidben@google.com>
This was a workaround for triple handshake put in way back, before
extended master secret.
Change-Id: Ie0112fa6323522b17c90a833d558f7182586d2c3
Reviewed-on: https://boringssl-review.googlesource.com/17541
Reviewed-by: David Benjamin <davidben@google.com>
Each of these cases should be rejected before we get to negotiating
anything. Save us a little bit of trouble.
Change-Id: I18cb66be1040dff7f25532da7e4c7d9c5ecd2748
Reviewed-on: https://boringssl-review.googlesource.com/17540
Reviewed-by: David Benjamin <davidben@google.com>
Also mirror the structure of the TLS 1.2 and TLS 1.3 code a bit.
Change-Id: I7b34bf30de63fa0bd47a39a90570846fb2314ad5
Reviewed-on: https://boringssl-review.googlesource.com/17539
Reviewed-by: David Benjamin <davidben@google.com>
We've never actually written tests for equipreference groups at the
BoringSSL level.
Change-Id: I571c081534efbfa8e7b84846fafed0b772721da1
Reviewed-on: https://boringssl-review.googlesource.com/17538
Reviewed-by: David Benjamin <davidben@google.com>
This function isn't used in TLS 1.3.
Change-Id: Icb6209396a36f243a84f0675b8f0c2435b08ad6c
Reviewed-on: https://boringssl-review.googlesource.com/17537
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I90286da596d5822d4cfedf40995d80cf76adaf97
Reviewed-on: https://boringssl-review.googlesource.com/17536
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Otherwise the fuzzer gets stuck at renegotiations.
Bug: 104
Change-Id: If37f9ab165d06e37bfc5c423fba35edaabed293b
Reviewed-on: https://boringssl-review.googlesource.com/17532
Reviewed-by: David Benjamin <davidben@google.com>
This allows us to fill in holes in our fuzzer coverage, notably client
resumption (and thus early data) and server client certificates. The
corpora are not refreshed yet. This will be done in upcoming changes.
Also add an option for debugging fuzzers. It's very useful to test it on
transcripts and make sure that fuzzer mode successfully makes things
compatible.
Bug: 104
Change-Id: I02f0be4045d1baf68efc9a4157f573df1429575d
Reviewed-on: https://boringssl-review.googlesource.com/17531
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>
Once passed to the outside world, an SSL_SESSION is immutable. It is not
thread-safe to set not_resumable. In most cases, the session is already
expired anyway. In other cases, making all this remove session be unlink rather than
destroy is sound and consistent with how we treat sessions elsewhere.
In particular, SSL_CTX_free calls SSL_CTX_flush_sessions(0), and
bulk-invalidating everything like this is interfering with some
follow-up changes to improve the fuzzer.
Change-Id: I2a19b8ce32d9effc1efaa72e934e015ebbbfbf9a
Reviewed-on: https://boringssl-review.googlesource.com/17530
Reviewed-by: David Benjamin <davidben@google.com>
This is in preparation for supporting multiple TLS 1.3 variants.
Change-Id: Ia2caf984f576f1b9e5915bdaf6ff952c8be10417
Reviewed-on: https://boringssl-review.googlesource.com/17526
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
SSL_set_max_proto_version(TLS1_3_DRAFT_VERSION) worked unintentionally.
Fix that. Also add an error when it fails.
Change-Id: I1048fede7b163e1c170e17bf4370b468221a7077
Reviewed-on: https://boringssl-review.googlesource.com/17525
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 upcoming experiments which will require
supporting multiple experimental versions of TLS 1.3 with, on the
server, the ability to enable multiple variants at once. This means the
version <-> wire bijection no longer exists, even when limiting to a
single SSL*. Thus version_to_wire is removed and instead we treat the
wire version as the canonical version value.
There is a mapping from valid wire versions to protocol versions which
describe the high-level handshake protocol in use. This mapping is not
injective, so uses of version_from_wire are rewritten differently.
All the version-munging logic is moved to ssl_versions.c with a master
preference list of all TLS and DTLS versions. The legacy version
negotiation is converted to the new scheme. The version lists and
negotiation are driven by the preference lists and a
ssl_supports_version API.
To simplify the mess around SSL_SESSION and versions, version_from_wire
is now DTLS/TLS-agnostic, with any filtering being done by
ssl_supports_version. This is screwy but allows parsing SSL_SESSIONs to
sanity-check it and reject all bogus versions in SSL_SESSION. This
reduces a mess of error cases.
As part of this, the weird logic where ssl->version is set early when
sending the ClientHello is removed. The one place where we were relying
on this behavior is tweaked to query hs->max_version instead.
Change-Id: Ic91b348481ceba94d9ae06d6781187c11adc15b0
Reviewed-on: https://boringssl-review.googlesource.com/17524
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Like the write half, rather than allocating the maximum size needed and
relying on the malloc implementation to pool this sanely, allocate the
size the TLS record-layer code believes it needs.
As currently arranged, this will cause us to alternate from a small
allocation (for the record header) and then an allocation sized to the
record itself. Windows is reportedly bad at pooling large allocations,
so, *if the server sends us smaller records*, this will avoid hitting
the problem cases.
If the server sends us size 16k records, the maximum allowed by ther
protocol, we simply must buffer up to that amount and will continue to
allocate similar sizes as before (although slightly smaller; this CL
also fixes small double-counting we did on the allocation sizes).
Separately, we'll gather some metrics in Chromium to see what common
record sizes are to determine if this optimization is sufficient. This
is intended as an easy optimization we can do now, in advance of ongoing
work to fix the extra layer of buffering between Chromium and BoringSSL
with an in-place decrypt API.
Bug: chromium:524258
Change-Id: I233df29df1212154c49fee4285ccc37be12f81dc
Reviewed-on: https://boringssl-review.googlesource.com/17329
Reviewed-by: Adam Langley <agl@google.com>
These broke at some point. Add a test for them.
Change-Id: Ie45869e07d9615ae33aae4613f6d9b996af39528
Reviewed-on: https://boringssl-review.googlesource.com/17330
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>
WatchGuard's bug is very distinctive. Report a dedicated error code out
of BoringSSL so we can better track this.
Bug: chromium:733223
Change-Id: Ia42abd8654e7987b1d43c63a4f454f35f6aa873b
Reviewed-on: https://boringssl-review.googlesource.com/17328
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>
Both Conscrypt and Netty have a lot of logic to map between the two
kinds of names. WebRTC needed an SSL_CIPHER_get_rfc_name for something.
Just have both in the library. Also deprecate SSL_CIPHER_get_rfc_name
in favor of SSL_CIPHER_standard_name, which matches upstream if built
with enable-ssl-trace. And, unlike SSL_CIPHER_get_rfc_name, this does
not require dealing with the malloc.
(Strangely this decreases bssl's binary size, even though we're carrying
more strings around. It seems the old SSL_CIPHER_get_rfc_name was
somewhat large in comparison. Regardless, a consumer that disliked 30
short strings probably also disliked the OpenSSL names. That would be
better solved by opaquifying SSL_CIPHER and adding a less stringy API
for configuring cipher lists. That's something we can explore later if
needed.)
I also made the command-line tool print out the standard names since
they're more standard. May as well push folks towards those going
forward.
Change-Id: Ieeb3d63e67ef4da87458e68d130166a4c1090596
Reviewed-on: https://boringssl-review.googlesource.com/17324
Reviewed-by: Robert Sloan <varomodt@google.com>
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>
The pointer and length fields should always be kept in sync. Other code
already assumes this anyway.
Change-Id: I62bc77b805cd4d81f2caa4aa49ad3e9d96faa25e
Reviewed-on: https://boringssl-review.googlesource.com/17306
Reviewed-by: 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>
786793411a only got applied to one of the
setters way back.
Change-Id: Ib798002d5ab7a3d0599b6520af25897949fb0071
Reviewed-on: https://boringssl-review.googlesource.com/17305
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Avoid dealing with that function call everywhere.
Change-Id: I7de64b59c8d17e8286c18a6b20c704e8ba8b9ebe
Reviewed-on: https://boringssl-review.googlesource.com/17267
Reviewed-by: Steven Valdez <svaldez@google.com>
These are not the true version filters due to SSL_OP_NO_* filters.
Change-Id: I4c2db967d885f7c1875a3e052c5b02ea8d612fe1
Reviewed-on: https://boringssl-review.googlesource.com/17266
Reviewed-by: Steven Valdez <svaldez@google.com>
The original motivation behind the sign/complete split was to avoid
needlessly hashing the input on each pass through the state machine, but
we're payload-based now and, in all cases, the payload is either cheap
to compute or readily available. (Even the hashing worry was probably
unnecessary.)
Tweak ssl_private_key_{sign,decrypt} to automatically call
ssl_private_key_complete as needed and take advantage of this in the
handshake state machines:
- TLS 1.3 signing now computes the payload each pass. The payload is
small and we're already allocating a comparable-sized buffer each
iteration to hold the signature. This shouldn't be a big deal.
- TLS 1.2 decryption code still needs two states due to reading the
message (fixed in new state machine style), but otherwise it just
performs cheap idempotent tasks again. The PSK code is reshuffled to
guarantee the callback is not called twice (though this was impossible
anyway because we don't support RSA_PSK).
- TLS 1.2 CertificateVerify signing is easy as the transcript is readily
available. The buffer is released very slightly later, but it
shouldn't matter.
- TLS 1.2 ServerKeyExchange signing required some reshuffling.
Assembling the ServerKeyExchange parameters is moved to the previous
state. The signing payload has some randoms prepended. This is cheap
enough, but a nuisance in C. Pre-prepend the randoms in
hs->server_params.
With this change, we are *nearly* rid of the A/B => same function
pattern.
BUG=128
Change-Id: Iec4fe0be7cfc88a6de027ba2760fae70794ea810
Reviewed-on: https://boringssl-review.googlesource.com/17265
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
It does not appear removing support for these is feasible right now. :-(
Change-Id: I99521ba6c141855b5140d98bce445d7e62415661
Reviewed-on: https://boringssl-review.googlesource.com/17251
Reviewed-by: David Benjamin <davidben@google.com>
We've got an asynchronous ServerKeyExchange state in the middle that
complicates things a bit, but this is still a little tighter.
BUG=128
Change-Id: I4ee2e3b85e677c9555d2fbddd387c12d41ab2b54
Reviewed-on: https://boringssl-review.googlesource.com/17250
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We can take advantage of our flight-by-flight model.
BUG=128
Change-Id: If27a5b6d88055da71199ef672d9c71969925aca9
Reviewed-on: https://boringssl-review.googlesource.com/17249
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
