Also switch them to accepting a u16 length prefix. We appear not to have
any such tests right now, but RSA-2048 would involve modulus well larger
and primes just a hair larger than a u8 length prefix alows.
Change-Id: Icce8f1d976e159b945302fbba732e72913c7b724
Reviewed-on: https://boringssl-review.googlesource.com/22284
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 really need to resurrect the CL to make them entirely static
(https://crbug.com/boringssl/20), but, in the meantime, to make
replacing the EC_METHOD pointer in EC_POINT with EC_GROUP not
*completely* insane, make them refcounted.
OpenSSL did not do this because their EC_GROUPs are mutable
(EC_GROUP_set_asn1_flag and EC_GROUP_set_point_conversion_form). Ours
are immutable but for the two-function dance around custom curves (more
of OpenSSL's habit of making their objects too complex), which is good
enough to refcount.
Change-Id: I3650993737a97da0ddcf0e5fb7a15876e724cadc
Reviewed-on: https://boringssl-review.googlesource.com/22244
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This reverts commit 75d43b5785. Chatting
with EKR, there is some reason to believe that doing this might cause
more middlebox issues. Since we're still in the middle of working
towards viable deployment in the first place, revert this.
We can experiment with this later. I should have arranged for this to be
controlled more carefully anyway.
Change-Id: I0c8bf578f9d7364e913894e1bf3c2b8123dfd770
Reviewed-on: https://boringssl-review.googlesource.com/22204
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>
Both of these changes have stuck in Chrome for quite a while now. Let's
clear them.
Change-Id: I13094451be2584ecaaf6b60eedefb7212b7bcde2
Reviewed-on: https://boringssl-review.googlesource.com/22226
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 is an OpenSSL thing to support platforms where BN_ULONG is not
actually the size it claims to be. We define BN_ULONG to uint32_t and
uint64_t which are guaranteed by C to implement arithemetic modulo 2^32
and 2^64, respectively. Thus there is no need for any of this.
Change-Id: I098cd4cc050a136b9f2c091dfbc28dd83e01f531
Reviewed-on: https://boringssl-review.googlesource.com/21784
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>
This reverts commit f6942f0d22.
Reason for revert: This doesn't actually work in clang-cl. I
forgot we didn't have the clang-cl try bots enabled! :-( I
believe __asm__ is still okay, but I'll try it by hand
tomorrow.
Original change's description:
> Use uint128_t and __asm__ in clang-cl.
>
> clang-cl does not define __GNUC__ but is still a functioning clang. We
> should be able to use our uint128_t and __asm__ code in it on Windows.
>
> Change-Id: I67310ee68baa0c0c947b2441c265b019ef12af7e
> Reviewed-on: https://boringssl-review.googlesource.com/22184
> 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>
TBR=agl@google.com,davidben@google.com
Change-Id: I5c7e0391cd9c2e8cc0dfde37e174edaf5d17db22
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://boringssl-review.googlesource.com/22224
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>
clang-cl does not define __GNUC__ but is still a functioning clang. We
should be able to use our uint128_t and __asm__ code in it on Windows.
Change-Id: I67310ee68baa0c0c947b2441c265b019ef12af7e
Reviewed-on: https://boringssl-review.googlesource.com/22184
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>
We never implemented psk_ke, so there's no need to define the constant.
Change-Id: I6e52596e1a2cf0b3db5e7cd96db6836f4290bf0b
Reviewed-on: https://boringssl-review.googlesource.com/22144
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 is also no need to make the struct public. Also tidy up includes a
bit.
Change-Id: I188848dfd8f9ed42925b2c55da8dc4751c29f146
Reviewed-on: https://boringssl-review.googlesource.com/22126
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>
Some of the complaints seem a bit questionable or their replacements
problematic, but not using strcat, strcpy, and strncpy is easy and
safer.
Change-Id: I64faf24b4f39d1ea410e883f026350094975a9b5
Reviewed-on: https://boringssl-review.googlesource.com/22125
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>
This doesn't matter in so far as runner is not a real TLS
implementation, but it should enforce what there is to enforce just to
keep BoringSSL honest.
Bug: 80
Change-Id: I68940c33712d34a2437dc4dee31342e7f0f57c23
Reviewed-on: https://boringssl-review.googlesource.com/22069
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>
This does not affect TLS 1.2 (beyond Channel ID or NPN) but, in TLS 1.3,
we send several encrypted handshake messages in a row. For the server,
this means 66 wasted bytes in TLS 1.3. Since OpenSSL has otherwise used
one record per message since the beginning and unencrypted overhead is
less interesting, leave that behavior as-is for the time being. (This
isn't the most pressing use of the breakage budget.) But TLS 1.3 is new,
so get this tight from the start.
Change-Id: I64dbd590a62469d296e1f10673c14bcd0c62919a
Reviewed-on: https://boringssl-review.googlesource.com/22068
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 enforce that servers don't send bogus ALPN values, so consumers may
assume that SSL_get0_alpn_selected won't have anything terribly weird.
To maintain that invariant in the face of folks whose ALPN preferences
change (consider a persisted session cache), we should decline to offer
0-RTT if early_alpn would have been rejected by the check anyway.
Change-Id: Ic3a9ba4041d5d4618742eb05e27033525d96ade1
Reviewed-on: https://boringssl-review.googlesource.com/22067
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 in preparation for giving DTLS_STATE one.
Change-Id: I3dfeeaad2d20c547d8e65d739bd0ad5bc1acf74a
Reviewed-on: https://boringssl-review.googlesource.com/22065
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>
new_*_len can just be computed rather than maintained as state.
Change-Id: If097ee9e68d8791fcfeb69052151faf0134c7c52
Reviewed-on: https://boringssl-review.googlesource.com/21948
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>
This finally clears most of the SSL_clear special-cases.
Change-Id: I00fc240ccbf13f4290322845f585ca6f5786ad80
Reviewed-on: https://boringssl-review.googlesource.com/21947
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>
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>
