If get_issuer fails, some of these calls return rather than jumping to common
cleanup code.
Change-Id: Iacd59747fb11e9bfaae86f2eeed88798ee08203e
Reviewed-on: https://boringssl-review.googlesource.com/5711
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 25efcb44ac88ab34f60047e16a96c9462fad39c1 and
56353962e7da7e385c3d577581ccc3015ed6d1dc.)
Change-Id: I2ff22fc9da23868de02e6f31c50a3f1d0c6dec1a
Reviewed-on: https://boringssl-review.googlesource.com/5710
Reviewed-by: Adam Langley <agl@google.com>
The function BN_MONT_CTX_set was assuming that the modulus was non-zero
and therefore that |mod->top| > 0. In an error situation that may not be
the case and could cause a seg fault.
This is a follow on from CVE-2015-1794.
(Imported from upstream's 512368c9ed4d53fb230000e83071eb81bf628b22.)
The CVE itself doesn't affect us as the bit strength check in the DHE logic
excludes zero.
Also add tests to bn_test for a couple of division by zero cases. (This and
BN_div.)
Change-Id: Ibd8ef98d6be48eb95110021c23cd8e278656764d
Reviewed-on: https://boringssl-review.googlesource.com/5690
Reviewed-by: Adam Langley <agl@google.com>
BN_bin2bn takes a size_t as it should, but it passes that into bn_wexpand which
takes unsigned. Switch bn_wexpand and bn_expand to take size_t before they
check bounds against INT_MAX.
BIGNUM itself still uses int everywhere and we may want to audit all the
arithmetic at some point. Although I suspect having bn_expand require that the
number of bits fit in an int is sufficient to make everything happy, unless
we're doing interesting arithmetic on the number of bits somewhere.
Change-Id: Id191a4a095adb7c938cde6f5a28bee56644720c6
Reviewed-on: https://boringssl-review.googlesource.com/5680
Reviewed-by: Adam Langley <agl@google.com>
Move the bn_expand call inside decode_hex; it's an implementation detail of
hex-decoding. decode_dec instead works with BN_mul_word and BN_add_word so it
can just rely on BN internally expanding things and check the return value.
Also clean up the decode_hex loop so it's somewhat more readable and check for
INT_MAX in bn_x2bn. It uses int over size_t rather pervasively, but while I'm
here at least make that function check overflow.
BUG=517474
Change-Id: I4f043973ee43071a02ea5d4313a8fdaf12404e84
Reviewed-on: https://boringssl-review.googlesource.com/5679
Reviewed-by: Adam Langley <agl@google.com>
“!= 0” is implicit in if statements and it looks very weird here.
Change-Id: I7f4e71c479b8ff9821a040f1c542b15af19b8aed
Reviewed-on: https://boringssl-review.googlesource.com/5720
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
RSA_PADDING_NONE is actually the important one for RSA_decrypt since OAEP isn't
used much and RSA_PKCS1_PADDING is unsafe to use due to timing constraints.
(The SSL stack uses RSA_PADDING_NONE and does the padding check separately.)
Change-Id: I5f9d168e7c34796a41bf01fc1878022742b63501
Reviewed-on: https://boringssl-review.googlesource.com/5641
Reviewed-by: Adam Langley <agl@google.com>
Some compilers in some configurations warn about this structure member
not being assigned a value. Since it is never used anywhere, just
remove it.
Change-Id: I46064234961bf449fe5fcb88594ddb3ff390e7d7
Reviewed-on: https://boringssl-review.googlesource.com/5621
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Missed a mention of CRYPTO_have_hwrand.
Change-Id: I9756d80105c2fcee487a7badbf4d82f375b5652d
Reviewed-on: https://boringssl-review.googlesource.com/5640
Reviewed-by: Adam Langley <agl@google.com>
Previously, |x| was reset to the value of the cofactor for no reason,
and there was an unnecessary copy made of |order|.
Change-Id: Ib6b06f651e280838299dff534c38726ebf4ccc97
Reviewed-on: https://boringssl-review.googlesource.com/4447
Reviewed-by: Adam Langley <agl@google.com>
Since the caller must check for CRYPTO_hwrand failures anyway, there's not much
point in doing the CRYPTO_have_hwrand check externally.
(As a bonus, CRYPTO_hwrand no longer compiles to abort() on ARM, so linker
deduplicating won't confuse Chrome's crash reporter...)
Change-Id: I2191d835fbda5b70812f14cd9a873a5e35c30c6d
Reviewed-on: https://boringssl-review.googlesource.com/5630
Reviewed-by: Adam Langley <agl@google.com>
It's not clear why OpenSSL had a union. The comment says something about sizes
of long, since OpenSSL doesn't use stdint.h. But the variable is treated as a
bunch of uint32_t's, not DES_cblocks.
The key schedule is also always used by iterating or indexing into a uint32_t*,
treating the 16 2-word subkeys as a single uint32_t[32]. Instead, index into
them properly shush any picky tools. The compiler should be able to figure out
what's going on and optimize it appropriately.
BUG=517495
Change-Id: I83d0e63ac2c6fb76fac1dceda9f2fd6762074341
Reviewed-on: https://boringssl-review.googlesource.com/5627
Reviewed-by: Adam Langley <agl@google.com>
Probably a remnant of ifdef soup somewhere.
Change-Id: I472f236a2db54a97490b22b0bbcc1701a2dba3b3
Reviewed-on: https://boringssl-review.googlesource.com/5623
Reviewed-by: Adam Langley <agl@google.com>
This is the only test amongst the tests for core crypto functionality
that depends on crypto/bio. This change removes that dependency. This
also factors out the duplicative hexdump logic into a shared function.
Change-Id: Ic280a71d086555a6993c05f183b94e1d38b60932
Reviewed-on: https://boringssl-review.googlesource.com/5622
Reviewed-by: Adam Langley <agl@google.com>
The documentation for |BN_CTX_get| states: "Once |BN_CTX_get| has
returned NULL, all future calls will also return NULL until
|BN_CTX_end| is called." Some code takes advantage of that guarantee
by only checking the return value of the last call to |BN_CTX_get| in a
series of calls. That is correct and the most efficient way of doing
it. However, that pattern is inconsistent with most of the other uses
of |BN_CTX_get|. Also, static analysis tools like Coverity cannot
understand that pattern. This commit removes the instances of that
pattern that Coverity complained about when scanning *ring*.
Change-Id: Ie36d0223ea1caee460c7979547cf5bfd5fb16f93
Reviewed-on: https://boringssl-review.googlesource.com/5611
Reviewed-by: Adam Langley <agl@google.com>
The error condition was checked for, but the return statement was
missing.
Change-Id: I92f89809a7a112fdece49a2a8a8628ff2da8e0da
Reviewed-on: https://boringssl-review.googlesource.com/5610
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
When using CMake to build with MSVC, MSVC complains about unreachable
code in the <xtree> header. This incantation silences that.
Change-Id: I5fc5305dc816a009a4c59501b212fd11e290637d
Reviewed-on: https://boringssl-review.googlesource.com/5552
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Add it to |EVP_get_cipherbynid|, along with |EVP_rc2_40_cbc| and
|EVP_aes_192_cbc|.
Change-Id: Iee7621a91262359d1650684652995884a6cef37a
Reviewed-on: https://boringssl-review.googlesource.com/5590
Reviewed-by: Adam Langley <agl@google.com>
err is a horrible API, but functions should consistently participate or not
participate in it. We were missing a few codepaths.
Change-Id: I762074d5030b8e9d3e5bba9f8fa91fbdccbee25a
Reviewed-on: https://boringssl-review.googlesource.com/5571
Reviewed-by: Adam Langley <agl@google.com>
Replace |()| with |(void)| in some prototypes to avoid compiler
warnings about old-style prototypes when building in some non-default
configurations for ARM.
Change-Id: Id57825084941c997bb7c41ec8ed94962f97ff732
Reviewed-on: https://boringssl-review.googlesource.com/5570
Reviewed-by: Adam Langley <agl@google.com>
MSVC and clang-cl automatically define |_WIN32| but |WIN32| is only
defined if a Windows header file has been included or if -DWIN32 was
passed on the command line. Thus, it is always better to test |_WIN32|
than |WIN32|. The convention in BoringSSL is to test |OPENSSL_WINDOWS|
instead, except for the place where |OPENSSL_WINDOWS| is defined.
Change-Id: Icf3e03958895be32efe800e689d5ed6a2fed215f
Reviewed-on: https://boringssl-review.googlesource.com/5553
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Some compilers warn about this duplicate declaration in some
configurations.
Change-Id: I3ff038a5feb0b2632b140e6632ea48acb0dcc118
Reviewed-on: https://boringssl-review.googlesource.com/5551
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 374fd385c2347b965c3490aa1c10025e1339d265.)
This codepath is only reachable on malloc failure if putting DSA private
keys into a PKCS#8 PrivateKeyInfo.
Change-Id: I88052eab3f477c4cdf5749be525878278d966a69
Reviewed-on: https://boringssl-review.googlesource.com/5543
Reviewed-by: Adam Langley <agl@google.com>
This change mirrors upstream's custom extension API because we have some
internal users that depend on it.
Change-Id: I408e442de0a55df7b05c872c953ff048cd406513
Reviewed-on: https://boringssl-review.googlesource.com/5471
Reviewed-by: Adam Langley <agl@google.com>
These are not in upstream and were probably introduced on accident by stray vim
keystrokes.
Change-Id: I35f51f81fc37e75702e7d8ffc6f040ce71321b54
Reviewed-on: https://boringssl-review.googlesource.com/5490
Reviewed-by: Adam Langley <agl@google.com>
See upstream's 9f0b86c68bb96d49301bbd6473c8235ca05ca06b. Generated by
using upstream's script in 5a3ce86e21715a683ff0d32421ed5c6d5e84234d and
then manually throwing out the false positives. (We converted a bunch of
stuff already in 91157550061d5d794898fe47b95384a7ba5f7b9d.)
This may require some wrestling with depot_tools to land in Chromium due
to Rietveld's encoding bugs, but hopefully that will avoid future
problems; Rietveld breaks if either old or new file is Latin-1.
Change-Id: I26dcb20c7377f92a0c843ef5d74d440a82ea8ceb
Reviewed-on: https://boringssl-review.googlesource.com/5483
Reviewed-by: Adam Langley <agl@google.com>
This means e.g. that a caller can say:
RAND_SSLEay()->bytes(...)
and so on. But in exchange for this convenience, I've changed the
signatures to be more BoringSSL-ish (|size_t| instead of |int|).
That's fine; |RAND_set_rand_method(SSLEay())| still works. And by
works I mean "does nothing".
Change-Id: I35479b5efb759da910ce46e22298168b78c9edcf
Reviewed-on: https://boringssl-review.googlesource.com/5472
Reviewed-by: Adam Langley <agl@google.com>
Fastradio was a trick where the ClientHello was padding to at least 1024
bytes in order to trick some mobile radios into entering high-power mode
immediately. After experimentation, the feature is being dropped.
This change also tidies up a bit of the extensions code now that
everything is using the new system.
Change-Id: Icf7892e0ac1fbe5d66a5d7b405ec455c6850a41c
Reviewed-on: https://boringssl-review.googlesource.com/5466
Reviewed-by: Adam Langley <agl@google.com>
smmintrin.h uses #if __cplusplus rather than #ifdef __cplusplus which
trips up warnings.
Change-Id: Ic2e0a64f0485a6e1d807c1e2d9d7f5bf8c64e296
Reviewed-on: https://boringssl-review.googlesource.com/5450
Reviewed-by: Adam Langley <agl@google.com>
See TODO comment being removed.
Change-Id: I92ce7018f88c24b3e2e61441397fda36b977d3b8
Reviewed-on: https://boringssl-review.googlesource.com/5435
Reviewed-by: Adam Langley <agl@google.com>
Rather, take a leaf out of Chromium's book and use MSVC's __cpuid and
_xgetbv built-in, with an inline assembly emulated version for other
compilers.
This preserves the behavior of the original assembly with the following
differences:
- CPUs without cpuid aren't support. Chromium's base/cpu.cc doesn't
check, and SSE2 support is part of our baseline; the perlasm code
is always built with OPENSSL_IA32_SSE2.
- The clear_xmm block in cpu-x86-asm.pl is removed. This was used to
clear some XMM-using features if OSXSAVE was set but XCR0 reports the
OS doesn't use XSAVE to store SSE state. This wasn't present in the
x86_64 and seems wrong. Section 13.5.2 of the Intel manual, volume 1,
explicitly says SSE may still be used in this case; the OS may save
that state in FXSAVE instead. A side discussion on upstream's RT#2633
agrees.
- The old code ran some AMD CPUs through the "intel" codepath and some
went straight to "generic" after duplicating some, but not all, logic.
The AMD copy didn't clear some reserved bits and didn't query CPUID 7
for AVX2 support. This is moot since AMD CPUs today don't support
AVX2, but it seems they're expected to in the future?
- Setting bit 10 is dropped. This doesn't appear to be queried anywhere,
was 32-bit only, and seems a remnant of upstream's
14e21f863a3e3278bb8660ea9844e92e52e1f2f7.
Change-Id: I0548877c97e997f7beb25e15f3fea71c68a951d2
Reviewed-on: https://boringssl-review.googlesource.com/5434
Reviewed-by: Adam Langley <agl@google.com>
In the ancient times, before ex_data and OpenSSL, SSLeay supported a
single app_data slot in various types. Later app_data begat ex_data, and
app_data was replaced by compatibility macros to ex_data index zero.
Today, app_data is still in use, but ex_data never reserved index zero
for app_data. This causes some danger where, if the first ex_data
registration did not use NULL callbacks, the registration's callbacks
would collide with app_data.
Instead, add an option to the types with app_data to reserve index zero.
Also switch SSL_get_ex_data_X509_STORE_CTX_idx to always return zero
rather than allocate a new one. It used to be that you used
X509_STORE_CTX_get_app_data. I only found one consumer that we probably
don't care about, but, to be safe and since it's easy, go with the
conservative option. (Although SSL_get_ex_data_X509_STORE_CTX_idx wasn't
guaranteed to alias app_data, in practice it always did. No consumer
ever calls X509_STORE_CTX_get_ex_new_index.)
Change-Id: Ie75b279d60aefd003ffef103f99021c5d696a5e9
Reviewed-on: https://boringssl-review.googlesource.com/5313
Reviewed-by: Adam Langley <agl@google.com>
The OR seems to be correct. %r9d previously contains 1 for a non-Intel
CPU. The next line isolates the XOP bit but *also* preserves that 1.
This figures in later in in line 121 to avoid tagging as Intel. (Most
codepaths in the AMD-specific branch go straight to .Lgeneric, but some
go to .Lintel. This probably could be done more clearly as there's some
complicated code that's subtly different...)
Change-Id: Ie7b9566074498333f26416e46478e4f4df68194f
Reviewed-on: https://boringssl-review.googlesource.com/5433
Reviewed-by: Adam Langley <agl@google.com>
Consumers sometimes use ERR_LIB_USER + <favorite number> instead of
ERR_get_next_error_library. To avoid causing them grief, keep ERR_LIB_USER
last.
Change-Id: Id19ae7836c41d5b156044bd20d417daf643bdda2
Reviewed-on: https://boringssl-review.googlesource.com/5290
Reviewed-by: Adam Langley <agl@google.com>
Running make_errors.go every time a function is renamed is incredibly
tedious. Plus we keep getting them wrong.
Instead, sample __func__ (__FUNCTION__ in MSVC) in the OPENSSL_PUT_ERROR macro
and store it alongside file and line number. This doesn't change the format of
ERR_print_errors, however ERR_error_string_n now uses the placeholder
"OPENSSL_internal" rather than an actual function name since that only takes
the uint32_t packed error code as input.
This updates err scripts to not emit the function string table. The
OPENSSL_PUT_ERROR invocations, for now, still include the extra
parameter. That will be removed in a follow-up.
BUG=468039
Change-Id: Iaa2ef56991fb58892fa8a1283b3b8b995fbb308d
Reviewed-on: https://boringssl-review.googlesource.com/5275
Reviewed-by: Adam Langley <agl@google.com>
gyp doesn't allow two files to share the same name to avoid bugs in OS X
libtool, so asn1.c's need to all get a prefix.
Change-Id: I3593597912c49dd02655cae329fb253ed4f6f56d
Reviewed-on: https://boringssl-review.googlesource.com/5431
Reviewed-by: Adam Langley <agl@google.com>
Chromium's NaCl build has _POSIX_SOURCE already defined, so #undef it first.
The compiler used also dislikes static asserts with the same name.
Change-Id: I0283fbad1a2ccf98cdb0ca2a7965b15441806308
Reviewed-on: https://boringssl-review.googlesource.com/5430
Reviewed-by: Adam Langley <agl@google.com>
RFC 7359 includes tests for various edge cases. Also, as
CRYPTO_poly1305_update can be used single-shot and streaming, we should
explicitly stress both.
Change-Id: Ie44c203a77624be10397ad05f06ca98d937db76f
Reviewed-on: https://boringssl-review.googlesource.com/5410
Reviewed-by: Adam Langley <agl@google.com>
