I've left EVP_set_buggy_rsa_parser as a no-op stub for now, but it
shouldn't need to last very long. (Just waiting for a CL to land in a
consumer.)
Bug: chromium:735616
Change-Id: I6426588f84dd0803661a79c6636a0414f4e98855
Reviewed-on: https://boringssl-review.googlesource.com/22124
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>
As with SSLTranscript before, we temporarily need some nastiness in
SSL3_STATE, but this is in preparation of giving SSL3_STATE a
constructor and destructor.
Change-Id: Ifc0ce34fdcd8691d521d8ea03ff5e83dad43b4a3
Reviewed-on: https://boringssl-review.googlesource.com/21944
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>
Our assembly does not use the GOT to reference symbols, which means
references to visible symbols will often require a TEXTREL. This is
undesirable, so all assembly-referenced symbols should be hidden. CPU
capabilities are the only such symbols defined in C.
These symbols may be hidden by doing at least one of:
1. Build with -fvisibility=hidden
2. __attribute__((visibility("hidden"))) in C.
3. .extern + .hidden in some assembly file referencing the symbol.
We have lots of consumers and can't always rely on (1) happening. We
were doing (3) by way of d216b71f90 and
16e38b2b8f, but missed 32-bit x86 because
it doesn't cause a linker error.
Those two patches are not in upstream. Upstream instead does (3) by way
of x86cpuid.pl and friends, but we have none of these files.
Standardize on doing (2). This avoids accidentally getting TEXTRELs on
some 32-bit x86 build configurations. This also undoes
d216b71f90 and
16e38b2b8f. They are no now longer needed
and reduce the upstream diff.
Change-Id: Ib51c43fce6a7d8292533635e5d85d3c197a93644
Reviewed-on: https://boringssl-review.googlesource.com/22064
Commit-Queue: Matt Braithwaite <mab@google.com>
Reviewed-by: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
If you specify any --target which refers to a x86_64-apple-darwin
triple, or a more specific variant derived from it, specifying
-stdlib=libc++ is required, otherwise clang falls back to libstdc++
which didn't include c++11, and fails to compile in very obscure ways
(simply failing to find any c++11 symbols).
Change-Id: I58025cea91eaa0c16d9b5831f9965889b75bbc31
Reviewed-on: https://boringssl-review.googlesource.com/21984
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>
Now that we've gotten everything, test this by just making bssl_shim run
all errors twice. The manual tests added to ssl_test.cc may now be
removed.
Bug: 206
Change-Id: Iefa0eae83ba59b476e6b6c6f0f921d5d1b72cbfb
Reviewed-on: https://boringssl-review.googlesource.com/21886
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
While a fairly small hook, open_close_notify is pretty weird. It
processes things at the record level and not above. Notably, this will
break if it skips past a TLS 1.3 KeyUpdate.
Instead, it can share the core part of SSL_read/SSL_peek, with slight
tweaks to post-handshake processing. Note this does require some tweaks
to that code. Notably, to retain the current semantics that SSL_shutdown
does not call funny callbacks, we suppress tickets.
Change-Id: Ia0cbd0b9f4527f1b091dd2083a5d8c7efb2bac65
Reviewed-on: https://boringssl-review.googlesource.com/21885
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Ideally we'd put this deep in the record layer, but sending alerts
currently awkwardly sets the field early, so we can't quite lock it out
this deep down.
This is mostly a sanity-check, but a later CL will fix SSL_shutdown's
post-handshake message processing, so this will help catch errors there.
Change-Id: I78e627c19547dbcdc85fb168795240d692baf031
Reviewed-on: https://boringssl-review.googlesource.com/21884
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This gets us closer to exposing BIO-free APIs. The next step is probably
to make the experimental bssl::OpenRecord function call a split out core
of ssl_read_impl.
Change-Id: I4acebb43f708df8c52eb4e328da8ae3551362fb9
Reviewed-on: https://boringssl-review.googlesource.com/21865
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
With this change, it should now always be the case that rr->length is
zero on entry to ssl3_read_message. This will let us detach everything
but application data from rr. This pushes some init_buf invariants down
into tls_open_record so we don't need to maintain them everywhere.
Change-Id: I206747434e0a9603eea7d19664734fd16fa2de8e
Reviewed-on: https://boringssl-review.googlesource.com/21524
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Enough were to make record processing idempotent (we either consume a
record or we don't), but some errors would cause us to keep processing
records when we should get stuck.
This leaves errors in the layer between the record bits and the
handshake. I'm hoping that will be easier to resolve once they do not
depend on BIO, at which point the checks added in this CL may move
around.
Bug: 206
Change-Id: I6b177079388820335e25947c5bd736451780ab8f
Reviewed-on: https://boringssl-review.googlesource.com/21366
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
We'll probably want to either move or add additional checks later, but
meanwhile this gets more code on the BIO-free side of the divide.
Change-Id: I3e2b570cdf1d70a262d952c20fd2d76ff4f70dd0
Reviewed-on: https://boringssl-review.googlesource.com/21365
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Ultimately the ssl_buffer_* code will be above SSL_PROTOCOL_METHOD, so
having the processing be analogous is simpler. This also means that DTLS
can surface errors out of dtls_open_record without the caller reading an
extra record.
Bug: 206
Change-Id: Ic1cb3a884763c8e875e1129b1cda226f72bc95b7
Reviewed-on: https://boringssl-review.googlesource.com/21364
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
We only support non-blocking BIOs for DTLS as of
https://boringssl-review.googlesource.com/13945. This logic is a remnant
of that. It should not be necessary. All users of DTLSv1_get_timeout
call DTLSv1_handle_timeout. This gets it out of the way for
dtls_open_record calls which don't use dtls1_get_record.
We can restore it elsewhere if necessary, but I don't think we need it.
Change-Id: Idb737868358e4b59ad3cb2c994c7084ffcdb3709
Reviewed-on: https://boringssl-review.googlesource.com/21349
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This removes the last place where non-app-data hooks leave anything
uncomsumed in rrec. (There is still a place where non-app-data hooks see
a non-empty rrec an entrance. read_app_data calls into read_handshake.
That'll be fixed in a later patch in this series.)
This should not change behavior, though some error codes may change due
to some processing happening in a slightly different order.
Since we do this in a few places, this adds a BUF_MEM_append with tests.
Change-Id: I9fe1fc0103e47f90e3c9f4acfe638927aecdeff6
Reviewed-on: https://boringssl-review.googlesource.com/21345
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Change-Id: I63b9972034fdc85bf2d23e7d46516755855fafbe
Reviewed-on: https://boringssl-review.googlesource.com/22024
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>
Now in Android.bp files, target.linux applies to all targets running a
linux kernel (android, linux_glibc, linux_bionic). So we can now share
sources between android and linux hosts.
Tested with:
https://android-review.googlesource.com/#/c/platform/external/boringssl/+/512517
Change-Id: I9c503f48cea17780e02bb38b419078a457d54f66
Reviewed-on: https://boringssl-review.googlesource.com/22004
Reviewed-by: Robert Sloan <varomodt@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>
RSABadValueTooLong should have the true one as a suffix, not a prefix,
so that the version check still works. Also do the padding manually to
catch a few other bad padding cases. This is sufficient coverage so that
disabling any one comparison in the padding check flags some failure.
Change-Id: Ibcad284e5ecee3e995f43101c09e4cf7694391e9
Reviewed-on: https://boringssl-review.googlesource.com/21904
Reviewed-by: Steven Valdez <svaldez@google.com>
Application records may be packed with other application data records or
with handshake records. We also were never testing CCS and handshake
being packed together. Implement this by moving the packing logic to the
bottom of BoGo's DTLS record layer.
Change-Id: Iabc14ec4ce7b99ed1f923ce9164077efe948c7a0
Reviewed-on: https://boringssl-review.googlesource.com/21844
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>
To make sure I don't break it later on.
Change-Id: I0a326800593cd3196efaf2ec9f4042935ecf8eb8
Reviewed-on: https://boringssl-review.googlesource.com/21864
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
d3868ac4d6f92e57376924a62e8d52f36d7a326561ec0bbd1d5681759a947134 sde-external-8.9.0-2017-08-06-lin.tar.bz2
9852d57e0d6b4509accb4f9faf862327a79c18b630aac4f35c65ce7a270a9230 strawberry-perl-5.26.1.1-32bit-portable.zip
I've left CMake alone for now because that involves building things and
there's some mess between newer CMakes trying to mess with the Android
NDK, so that needs to be tested out a bit.
Change-Id: I6beea9d4774474ac62e843c8668fcef7f6a6fc04
Reviewed-on: https://boringssl-review.googlesource.com/21824
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
See
https://groups.google.com/a/chromium.org/d/msg/infra-announce/A6_zQKzCHDo/ZKqSptzeBgAJ.
This allows us to avoid checking out unnecessary things (right now every
Windows bot downloads clang). We also can maintain the SDE bits in DEPS
rather than having to update the recipe for it.
This is the first half of the change which conditions things on
variables but leaves the defaults as they are. This will be followed up
by a change to the recipe to set the variables, then to switch the
defaults.
Change-Id: Iebcc4d0a146d0b0df94f480e539d70cbf4c862d3
Reviewed-on: https://boringssl-review.googlesource.com/21804
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>
It should be inside the llvm-build directory, otherwise it's not in
.gitignore and things get confused.
Change-Id: I5be31e0b0db69fff9935cbf6dbd9c612fd5a4769
Reviewed-on: https://boringssl-review.googlesource.com/21805
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>
I think that's the last of the ssl3_ prefix being used for common
functions.
Change-Id: Id83e6f2065c3765931250bd074f6ebf1fc251696
Reviewed-on: https://boringssl-review.googlesource.com/21347
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
These are common between TLS and DTLS so should not have the ssl3_
prefix. (TLS-only stuff should really have a tls_ prefix, but we still
have a lot of that one.)
This also fixes a stray reference to ssl3_send_client_key_exchange..
Change-Id: Ia05b360aa090ab3b5f075d5f80f133cbfe0520d4
Reviewed-on: https://boringssl-review.googlesource.com/21346
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
We still have more links to cut for ssl.h to not pull in x509.h (notably
pem.h), but this resolves some easy ones. I've kept the constants the
same just in case, but nowhere are the constants mixed up by callers or
passed from one to the other in the functions' implementations. They're
completely independent.
Change-Id: Ic0896283378b5846afd6422bfe740951ac552f0e
Reviewed-on: https://boringssl-review.googlesource.com/21704
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
It's no longer needed in the public header at all, now that we've hidden
the SSL_CTX struct.
Change-Id: I2fc6ddbeb52f000487627b433b9cdd7a4cde37a8
Reviewed-on: https://boringssl-review.googlesource.com/21684
Reviewed-by: Steven Valdez <svaldez@google.com>
Change-Id: Ifb227675cbc8e60128140768fb7d7f5f94928ac2
Reviewed-on: https://boringssl-review.googlesource.com/21764
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>
Commit 9a4876e193 broke NGINX builds with
BoringSSL due to this missing include (OpenSSL builds work fine):
src/event/ngx_event_openssl.c: In function ‘ngx_ssl_session_ticket_key_callback’:
src/event/ngx_event_openssl.c:3065:13: error: implicit declaration of function ‘HMAC_Init_ex’; did you mean ‘SHA1_Init’? [-Werror=implicit-function-declaration]
if (HMAC_Init_ex(hctx, key[0].hmac_key, size, digest, NULL) != 1) {
^~~~~~~~~~~~
Change-Id: Ie7170f05034d5fd8c85d1948b4ab9c9bb8447d13
Reviewed-on: https://boringssl-review.googlesource.com/21664
Reviewed-by: Adam Langley <agl@google.com>
GCC 7.2.0 (in Release builds) can't figure out that |type| is always
set:
../ssl/tls_record.cc: In function ‘bssl::OpenRecordResult bssl::OpenRecord(SSL*, bssl::Span<unsigned char>*, size_t*, uint8_t*, bssl::Span<unsigned char>)’:
../ssl/tls_record.cc:595:44: error: ‘type’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (type != SSL3_RT_APPLICATION_DATA && type != SSL3_RT_ALERT) {
cc1plus: all warnings being treated as errors
Change-Id: I1ca9683a18d89097288018f48b50991bce185da8
Reviewed-on: https://boringssl-review.googlesource.com/21724
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Thanks to Alex Gaynor for reporting this.
Change-Id: I983ecb33cf017160f82582cc79e71f8ae7b30b99
Reviewed-on: https://boringssl-review.googlesource.com/21744
Reviewed-by: David Benjamin <davidben@google.com>
The handshake should be generic between TLS and DTLS.
Change-Id: I6feb2f013dd5d771f206750653ab9d117d7ea716
Reviewed-on: https://boringssl-review.googlesource.com/21348
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
This frees us up to make SSL_CTX a C++ type and avoids a lot of
protrusions of otherwise private types into the global namespace.
Bug: 6
Change-Id: I8a0624a53a4d26ac4a483fa270c39ecdd07459ee
Reviewed-on: https://boringssl-review.googlesource.com/21584
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>
On some Chrome builds on Windows (including the official builds that we
ship) there are dynamic initializers for kNamedGroups in chrome.dll and
chrome_child.dll. Tagging this array with constexpr is guaranteed to
avoid this.
Bug: chromium:341941
Change-Id: I0e4ea0665b8ed9640b76b709dd300416be49e59e
Reviewed-on: https://boringssl-review.googlesource.com/21564
Reviewed-by: Bruce Dawson <brucedawson@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 doesn't particularly matter but is more consistent with DTLS and
avoids the callback being potentially called from two places.
Change-Id: I2f57ca94d2d532c56f37a0bac7000c15b3b4b520
Reviewed-on: https://boringssl-review.googlesource.com/21344
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
We usually use read/write rather than recv/send to describe the two
sides.
Change-Id: Ie3ac8c52c59ea9a5143f56b894f58cecd351dc7d
Reviewed-on: https://boringssl-review.googlesource.com/21304
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Clients need not accept CertificateRequest. We don't, have no intention
to, and post-handshake auth now requires an extension.
Change-Id: I2160c89e4a6988a7d743052b588d8aa2598ffabf
Reviewed-on: https://boringssl-review.googlesource.com/21305
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
The only difference is whether there's an alert to send back, but we'll
need to allow an "error without alert" in several cases anyway:
1. If the server sees an HTTP request or garbage instead of a
ClientHello, it shouldn't send an alert.
2. Resurfaced errors.
Just make zero signal no alert for now. Later on, I'm thinking we might
just want to put the alert into the outgoing buffer and make it further
uniform.
This also gives us only one error state to keep track of rather than
two.
Bug: 206
Change-Id: Ia821d9f89abd2ca6010e8851220d4e070bc42fa1
Reviewed-on: https://boringssl-review.googlesource.com/21286
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This is analogous to the Go stack's handshakeErr field. Since it's quite
common for callers to run two I/O operations in parallel[*] like
SSL_read and SSL_write (or SSL_read and SSL_do_handshake for client
0-RTT). Accordingly, the new handshake state machine jams itself up on
handshake error, but to fully work with such callers, we should also
replay the error state.
This doesn't yet catch all cases (there are some parts of the read flow
which need to be fixed). Those will be resolved in later changes.
[*] Not actually in parallel, of course, but logically in parallel on a
non-blocking socket.
Bug: 206
Change-Id: I5a4d37a258b9e3fc555b732938b0528b839650f8
Reviewed-on: https://boringssl-review.googlesource.com/21285
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This will be useful for the SSL stack to properly resurface handshake
failures. Leave this in a private header and, along the way, hide the
various types.
(ERR_NUM_ERRORS didn't change in meaning. The old documentation was
wrong.)
Bug: 206
Change-Id: I4c6ca98d162d11ad5e17e4baf439a18fbe371018
Reviewed-on: https://boringssl-review.googlesource.com/21284
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
