Change-Id: I683481b12e66966729297466748f1869de0b913b
Reviewed-on: https://boringssl-review.googlesource.com/17584
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 would be unfamiliar to anyone coming from Chromium.
Change-Id: If9fbdbbadfd874c25dc6ff447ab4af36de0dcd22
Reviewed-on: https://boringssl-review.googlesource.com/17544
Reviewed-by: Adam Langley <agl@google.com>
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>
These are re-recorded with the new fuzzer format.
Bug: 104
Change-Id: I00798f8f2026ae4570ffdcdae4a47999fd277212
Reviewed-on: https://boringssl-review.googlesource.com/17535
Reviewed-by: David Benjamin <davidben@google.com>
This was done by prepending each file with kDataTag, or 0x0000. This
causes them to behave as they did before the fuzzer updates.
Bug: 104
Change-Id: Ic768606911e1310fb59bed647990c237fe15776b
Reviewed-on: https://boringssl-review.googlesource.com/17534
Reviewed-by: David Benjamin <davidben@google.com>
So long as the code is there, it should be fuzzed.
Bug: 104
Change-Id: Iffaa832cc50c2d3c064eb511ba3a133d7f5758f2
Reviewed-on: https://boringssl-review.googlesource.com/17533
Reviewed-by: 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>
This has come up a few times and our docs aren't great. This hopefully
describes the sharp edges better.
Change-Id: I5d4044449f74ec116838fd1bba629cd90dc0d1ac
Reviewed-on: https://boringssl-review.googlesource.com/17504
Reviewed-by: Adam Langley <agl@google.com>
This imports bf5b8ff17dd7039b15cbc6468cd865cbc219581d and
a696708ae6bbe42f409748b3e31bb2f3034edbf3 from upstream. I missed them at
some point.
Change-Id: I882d995868e4c0461b7ca51a854691cf4faa7260
Reviewed-on: https://boringssl-review.googlesource.com/17384
Reviewed-by: Adam Langley <agl@google.com>
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>
Change-Id: I819a5b565e4380f3d816a2e4a68572935c612eae
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/17564
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 document what versions of everything we're using as the .sha1 files
don't say.
Change-Id: I2d496c86761f6df6acd20e1af62094b7d89e5c1d
Reviewed-on: https://boringssl-review.googlesource.com/17485
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
efa4339adde7e627370ed7c46ed00fed5d23310007ef0334ae17510d00e22b8d sde-external-8.5.0-2017-06-08-lin.tar.bz2
Change-Id: I201ca78cbbb3c769ed45705f87b6013758b68349
Reviewed-on: https://boringssl-review.googlesource.com/17484
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
When testing against a browser, multiple connections will be made in
parallel. Keeping the same listening socket lets the other connections
queue up at least rather than fail with ECONNREFUSED. Of course, this is
still far from a realistic server.
Change-Id: I984fb29da4bf8808eb40938b12782dc1730f2e19
Reviewed-on: https://boringssl-review.googlesource.com/17405
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>
The __clang__-guarded #defines cause gas to complain if clang is passed
-fno-integrated-as. Emitting .syntax unified when those are used fixes
this. This matches the change made to ghash-armv4.pl in upstream's
6cf412c473d8145562b76219ce3da73b201b3255.
See also https://github.com/openssl/openssl/pull/3694. This fixes the
build with the latest Android NDK (use the NDK-supplied toolchain file)
with the armeabi ABI.
Bug: chromium:732066
Change-Id: Ic6ca633a58edbe8ae8c7d501bd9515c2476fd7c2
Reviewed-on: https://boringssl-review.googlesource.com/17404
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>
There's a |tag_len| in the generic AEAD context now so keeping a second
copy only invites confusion.
Change-Id: I029d8a8ee366e3af7f61408177c950d5b1a740a9
Reviewed-on: https://boringssl-review.googlesource.com/17424
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@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>
Consumers should now all be using a pattern that allows us to remove
unset fields from the struct.
Change-Id: Ia3cf4941589d624cf25c5173501bedeab73fb7b8
Reviewed-on: https://boringssl-review.googlesource.com/17326
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>
Public and private RSA keys have the same type in OpenSSL, so it's
probably prudent for us to catch this case with an error rather than
crash. (As we do if you, say, configure RSA-PSS parameters on an Ed25519
EVP_PKEY.) Bindings libraries, in particular, tend to hit this sort of
then when their callers do silly things.
Change-Id: I2555e9bfe716a9f15273abd887a8459c682432dd
Reviewed-on: https://boringssl-review.googlesource.com/17325
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>
Our old redirectors were emitting code to call their target functions normally.
However, the PPC ABI expects callers to set up parameter save areas for their
callees, notably if the target is a varargs function.
Instead, mimic the pattern used when calling an external function or function
pointer and avoid touching the stack.
Change-Id: Ia28c9d2b82fcd99c4a2f70f5f587d0e0463a6f0e
Reviewed-on: https://boringssl-review.googlesource.com/17284
Reviewed-by: Adam Langley <agl@google.com>
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>
There was a typo there. Also the document's title capitalizes "64-Bit"
and "V2" funny.
Change-Id: I38a7f8d575ce2bb48dcc2ce5a4d683a7a170db87
Reviewed-on: https://boringssl-review.googlesource.com/17268
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
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>
Change-Id: Ic2eae037d50de4af67f6cbe888e16d507ab674d8
Reviewed-on: https://boringssl-review.googlesource.com/17224
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>
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>
Although Microsoft does not support building with /Wall, starting
VS2017, they provide a way to suppress warnings only in STL code. This
lets us keep some warnings active on our code while disabling them in
the STL.
https://blogs.msdn.microsoft.com/vcblog/2017/02/06/stl-fixes-in-vs-2017-rtm/
We currently still support VS2015, so we can't switch most of our
suppressions to this, but anything which applies only to VS2017 and up
will work.
Change-Id: I5f6d621dd1dbc060e09bded776d1714785a63147
Reviewed-on: https://boringssl-review.googlesource.com/17245
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>
