This is a fairly common operation on an X509.
Change-Id: I1820f20b555f75c98ab7e3283b5530bc1c200e2a
Reviewed-on: https://boringssl-review.googlesource.com/7611
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
They now fuzz a lot more than just the initial flow.
Change-Id: Ib0b7eb66969442e539a937d7d87f5ba031fcbef3
Reviewed-on: https://boringssl-review.googlesource.com/7610
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This was fixed in 93a5b442964d9770b5faa0fb381a8c4f43e65abe, but it wasn't
documented. Now that there are no pre-init functions to call like
CRYPTO_set_neon_capable, one instance of BoringSSL may be safely shared between
multiple consumers. As part of that, multiple consumers need to be able to call
CRYPTO_library_init possibly redundantlyand possibly on different threads
without synchronization.
(Though there is still that static initializer nuisance. It would be nice to
replace this with internal CRYPTO_once_t's and then CRYPTO_library_init need
only be called to prime armcap for a sandbox. But one thing at a time.)
Change-Id: I48430182d3649c8cf19082e34da24dee48e6119e
Reviewed-on: https://boringssl-review.googlesource.com/7571
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Whatever compiler settings AOSP is using warns that this is a GNU extension.
Change-Id: Ife395d2b206b607b14c713cbb5a94d479816dad0
Reviewed-on: https://boringssl-review.googlesource.com/7604
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
They may be spelled with or without underscores. Alas, a lot of C code (adb,
cURL) seems to find it a popular pasttime to #define printf *before* including
external headers. This is completely nonsense and invalid, but working around
it is easy and is what we (and OpenSSL) were doing before
061332f216.
I'll be sending a patch to cURL tomorrow to make them at least do their macro
trickery after external #includes for sanity. adb's sysdeps.h is a lot longer
and consistently #included first so I'll probably leave that be for lack of
time.
Change-Id: I03a0a253f2c902eb45f45faace1e5c5df4335ebf
Reviewed-on: https://boringssl-review.googlesource.com/7605
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This reverts commit 6f0c4db90e except for the
imported assembly files, which are left as-is but unused. Until upstream fixes
https://rt.openssl.org/Ticket/Display.html?id=4483, we shouldn't ship this
code. Once that bug has been fixed, we'll restore it.
Change-Id: I74aea18ce31a4b79657d04f8589c18d6b17f1578
Reviewed-on: https://boringssl-review.googlesource.com/7602
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The documentation in |RSA_METHOD| says that the |ctx| parameter to
|mod_exp| can be NULL, however the default implementation doesn't
handle that case. That wouldn't matter since internally it is always
called with a non-NULL |ctx| and it is static, but an external
application could get a pointer to |mod_exp| by extracting it from
the default |RSA_METHOD|. That's unlikely, but making that impossible
reduces the chances that future refactorings will cause unexpected
trouble.
Change-Id: Ie0e35e9f107551a16b49c1eb91d0d3386604e594
Reviewed-on: https://boringssl-review.googlesource.com/7580
Reviewed-by: David Benjamin <davidben@google.com>
|BN_mod_mul_montgomery| has better constant-time behavior (usually)
than |BN_mod_mul| and |BN_mod_sqr| and on platforms where we have
assembly language optimizations (when |OPENSSL_BN_ASM_MONT| is set in
crypto/bn/montgomery.c) it is faster. While doing so, reorder and
rename the |BN_MONT_CTX| parameters of the blinding functions to match
the order normally used in Montgomery math functions.
As a bonus, remove a redundant copy of the RSA public modulus from the
|BN_BLINDING| structure, which reduces memory usage.
Change-Id: I70597e40246429c7964947a1dc46d0d81c7530ef
Reviewed-on: https://boringssl-review.googlesource.com/7524
Reviewed-by: David Benjamin <davidben@google.com>
In VS2015's debug runtime, the C runtime has been unloaded by the time
DLL_PROCESS_DETACH is called and things crash. Instead, don't run destructors
at that point.
This means we do *not* free memory associated with any remaining thread-locals
on application shutdown, only shutdown of individual threads. This is actually
desirable since it's consistent with pthreads. If an individual thread calls
pthread_exit, destructors are run. If the entire process exits, they are not.
(It's also consistent with thread_none.c which never bothers to free
anything.)
BUG=chromium:595795
Change-Id: I3e64d46ea03158fefff583c1e3e12dfa0c0e172d
Reviewed-on: https://boringssl-review.googlesource.com/7601
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
We reordered extensions some time ago to ensure a non-empty extension was last,
but the comment was since lost (or I forgot to put one in in the first place).
Add one now so we don't regress.
Change-Id: I2f6e2c3777912eb2c522a54bbbee579ee37ee58a
Reviewed-on: https://boringssl-review.googlesource.com/7570
Reviewed-by: David Benjamin <davidben@google.com>
In the case |BN_CTX_get| failed, the function returned without calling
|BN_CTX_end|. Fix that.
Change-Id: Ia24cba3256e2cec106b539324e9679d690048780
Reviewed-on: https://boringssl-review.googlesource.com/7592
Reviewed-by: David Benjamin <davidben@google.com>
It looks like we started reformatting that function and adding curly braces,
etc., but forget to finish it. This is corroborated by the diff. Although git
thinks I removed the EAY-style one and tweaked the #if-0'd one, I actually
clang-formatted the EAY-style one anew and deleted the #if-0'd one after
tweaking the style to match. Only difference is the alignment stuff is
uintptr_t rather than intptr_t since the old logic was using unsigned
arithmetic.
Change-Id: Ia244e4082a6b6aed3ef587d392d171382c32db33
Reviewed-on: https://boringssl-review.googlesource.com/7574
Reviewed-by: David Benjamin <davidben@google.com>
This code is only used in ec_montgomery.c, so |field_encode| and
|field_decode| are never NULL.
Change-Id: I42a3ad5744d4ed6f0be1707494411e7efcf930ff
Reviewed-on: https://boringssl-review.googlesource.com/7585
Reviewed-by: David Benjamin <davidben@google.com>
It is only used in ec_montgomery.c, so move it there.
Change-Id: Ib189d5579d6363bdc1da89b775ad3df824129758
Reviewed-on: https://boringssl-review.googlesource.com/7584
Reviewed-by: David Benjamin <davidben@google.com>
These only affect the tests.
Change-Id: If22d047dc98023501c771787b485276ece92d4a2
Reviewed-on: https://boringssl-review.googlesource.com/7573
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
We do an ad-hoc upper-bound check, but if the version is too low, we also
shouldn't offer the session. This isn't fatal to the connection and doesn't
have issues (we'll check the version later regardless), but offering a session
we're never going to accept is pointless. The check should match what we do in
ServerHello.
Credit to Matt Caswell for noticing the equivalent issue in an OpenSSL pull
request.
Change-Id: I17a4efd37afa63b34fca53f4c9b7ac3ae2fa3336
Reviewed-on: https://boringssl-review.googlesource.com/7543
Reviewed-by: David Benjamin <davidben@google.com>
Patch from https://mta.openssl.org/pipermail/openssl-dev/2016-March/005625.html.
Upstream has yet to make a decision on aliasing requirements for their
assembly. If they choose to go with the stricter aliasing requirement rather
than land this patch, we'll probably want to tweak EVP_AEAD's API guarantees
accordingly and then undiverge.
In the meantime, import this to avoid a regression on x86 from when we had
compiler-vectorized code on GCC platforms. Per our assembly coverage tools and
pending multi-CPU-variant tests, we have good coverage here. Unlike Poly1305
(which is currently waiting on yet another upstream bugfix), where there is
risk of missed carries everywhere, it is much more difficult to accidentally
make a ChaCha20 implementation that fails based on the data passed into it.
This restores a sizeable speed improvement on x86.
Before:
Did 1131000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000205us (1130768.2 ops/sec): 18.1 MB/s
Did 161000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1006136us (160018.1 ops/sec): 216.0 MB/s
Did 28000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1023264us (27363.4 ops/sec): 224.2 MB/s
Did 1166000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000447us (1165479.0 ops/sec): 18.6 MB/s
Did 160000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1004818us (159232.8 ops/sec): 215.0 MB/s
Did 30000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1016977us (29499.2 ops/sec): 241.7 MB/s
After:
Did 2208000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000031us (2207931.6 ops/sec): 35.3 MB/s
Did 402000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1001717us (401310.9 ops/sec): 541.8 MB/s
Did 97000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1005394us (96479.6 ops/sec): 790.4 MB/s
Did 2444000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000089us (2443782.5 ops/sec): 39.1 MB/s
Did 459000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1000563us (458741.7 ops/sec): 619.3 MB/s
Did 97000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1007942us (96235.7 ops/sec): 788.4 MB/s
Change-Id: I976da606dae062a776e0cc01229ec03a074035d1
Reviewed-on: https://boringssl-review.googlesource.com/7561
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I24d0179ca5019e82ca1494c8773f373f8c09ce82
Reviewed-on: https://boringssl-review.googlesource.com/7566
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I0effe99d244c4ccdbb0e34db6e01a59c9463cb15
Reviewed-on: https://boringssl-review.googlesource.com/7572
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The removes the last of OpenSSL's variables that count occurrences of a
function on the stack.
Change-Id: I1722c6d47bedb47b1613c4a5da01375b5c4cc220
Reviewed-on: https://boringssl-review.googlesource.com/7450
Reviewed-by: David Benjamin <davidben@google.com>
This removes the final use of in_handshake. Note that there is still a
rentrant call of read_bytes -> handshake_func when we see a
HelloRequest. That will need to be signaled up to ssl_read_impl
separately out of read_app_data.
Change-Id: I823de243f75e6b73eb40c6cf44157b4fc21eb8fb
Reviewed-on: https://boringssl-review.googlesource.com/7439
Reviewed-by: David Benjamin <davidben@google.com>
This removes one use of in_handshake and consolidates some DTLS and TLS
code.
Change-Id: Ibbdd38360a983dabfb7b18c7bd59cb5e316b2adb
Reviewed-on: https://boringssl-review.googlesource.com/7435
Reviewed-by: David Benjamin <davidben@google.com>
fatal_alert isn't read at all right now, and warn_alert is only checked
for close_notify. We only need three states:
- Not shutdown.
- Got a fatal alert (don't care which).
- Got a warning close_notify.
Leave ssl->shutdown alone for now as it's tied up with SSL_set_shutdown
and friends. To distinguish the remaining two, we only need a boolean.
Change-Id: I5877723af82b76965c75cefd67ec1f981242281b
Reviewed-on: https://boringssl-review.googlesource.com/7434
Reviewed-by: David Benjamin <davidben@google.com>
Some ARMv8 Android devices don't have AT_HWCAP2. This means, when running in
32-bit mode (ARM capability APIs on Linux are different between AArch32 and
AArch64), we can't discover the various nice instructions.
On a Nexus 6P, this gives a, uh, minor performance win when running in 32-bit
mode.
Before:
Did 1085000 AES-128-GCM (16 bytes) seal operations in 1000003us (1084996.7 ops/sec): 17.4 MB/s
Did 60000 AES-128-GCM (1350 bytes) seal operations in 1013416us (59205.7 ops/sec): 79.9 MB/s
Did 11000 AES-128-GCM (8192 bytes) seal operations in 1019778us (10786.7 ops/sec): 88.4 MB/s
Did 1009000 AES-256-GCM (16 bytes) seal operations in 1000650us (1008344.6 ops/sec): 16.1 MB/s
Did 49000 AES-256-GCM (1350 bytes) seal operations in 1015698us (48242.7 ops/sec): 65.1 MB/s
Did 9394 AES-256-GCM (8192 bytes) seal operations in 1071104us (8770.4 ops/sec): 71.8 MB/s
Did 1557000 SHA-1 (16 bytes) operations in 1000317us (1556506.6 ops/sec): 24.9 MB/s
Did 762000 SHA-1 (256 bytes) operations in 1000527us (761598.6 ops/sec): 195.0 MB/s
Did 45000 SHA-1 (8192 bytes) operations in 1013773us (44388.6 ops/sec): 363.6 MB/s
Did 1459000 SHA-256 (16 bytes) operations in 1000271us (1458604.7 ops/sec): 23.3 MB/s
Did 538000 SHA-256 (256 bytes) operations in 1000990us (537467.9 ops/sec): 137.6 MB/s
Did 26000 SHA-256 (8192 bytes) operations in 1008403us (25783.3 ops/sec): 211.2 MB/s
After:
Did 1890000 AES-128-GCM (16 bytes) seal operations in 1000068us (1889871.5 ops/sec): 30.2 MB/s
Did 509000 AES-128-GCM (1350 bytes) seal operations in 1000112us (508943.0 ops/sec): 687.1 MB/s
Did 110000 AES-128-GCM (8192 bytes) seal operations in 1007966us (109130.7 ops/sec): 894.0 MB/s
Did 1960000 AES-256-GCM (16 bytes) seal operations in 1000303us (1959406.3 ops/sec): 31.4 MB/s
Did 460000 AES-256-GCM (1350 bytes) seal operations in 1001873us (459140.0 ops/sec): 619.8 MB/s
Did 97000 AES-256-GCM (8192 bytes) seal operations in 1005337us (96485.1 ops/sec): 790.4 MB/s
Did 1927000 SHA-1 (16 bytes) operations in 1000429us (1926173.7 ops/sec): 30.8 MB/s
Did 1151000 SHA-1 (256 bytes) operations in 1000425us (1150511.0 ops/sec): 294.5 MB/s
Did 87000 SHA-1 (8192 bytes) operations in 1003089us (86732.1 ops/sec): 710.5 MB/s
Did 2357390 SHA-256 (16 bytes) operations in 1000116us (2357116.6 ops/sec): 37.7 MB/s
Did 1410000 SHA-256 (256 bytes) operations in 1000176us (1409751.9 ops/sec): 360.9 MB/s
Did 101000 SHA-256 (8192 bytes) operations in 1007007us (100297.2 ops/sec): 821.6 MB/s
BUG=chromium:596156
Change-Id: Iacc1f8d8a07e991d4615f2e12c5c54923fb31aa2
Reviewed-on: https://boringssl-review.googlesource.com/7507
Reviewed-by: David Benjamin <davidben@google.com>
This removes the thread-unsafe SIGILL-based detection and the
multi-consumer-hostile CRYPTO_set_NEON_capable API. (Changing
OPENSSL_armcap_P after initialization is likely to cause problems.)
The right way to detect ARM features on Linux is getauxval. On aarch64,
we should be able to rely on this, so use it straight. Split this out
into its own file. The #ifdefs in the old cpu-arm.c meant it shared all
but no code with its arm counterpart anyway.
Unfortunately, various versions of Android have different missing APIs, so, on
arm, we need a series of workarounds. Previously, we used a SIGILL fallback
based on OpenSSL's logic, but this is inherently not thread-safe. (SIGILL also
does not tell us if the OS knows how to save and restore NEON state.) Instead,
base the behavior on Android NDK's cpu-features library, what Chromium
currently uses with CRYPTO_set_NEON_capable:
- Android before API level 20 does not provide getauxval. Where missing,
we can read from /proc/self/auxv.
- On some versions of Android, /proc/self/auxv is also not readable, so
use /proc/cpuinfo's Features line.
- Linux only advertises optional features in /proc/cpuinfo. ARMv8 makes NEON
mandatory, so /proc/cpuinfo can't be used without additional effort.
Finally, we must blacklist a particular chip because the NEON unit is broken
(https://crbug.com/341598).
Unfortunately, this means CRYPTO_library_init now depends on /proc being
available, which will require some care with Chromium's sandbox. The
simplest solution is to just call CRYPTO_library_init before entering
the sandbox.
It's worth noting that Chromium's current EnsureOpenSSLInit function already
depends on /proc/cpuinfo to detect the broken CPU, by way of base::CPU.
android_getCpuFeatures also interally depends on it. We were already relying on
both of those being stateful and primed prior to entering the sandbox.
BUG=chromium:589200
Change-Id: Ic5d1c341aab5a614eb129d8aa5ada2809edd6af8
Reviewed-on: https://boringssl-review.googlesource.com/7506
Reviewed-by: David Benjamin <davidben@google.com>
Many of the compatibility issues are described at
https://msdn.microsoft.com/en-us/library/mt612856.aspx. The macros
that suppressed warnings on a per-function basis no longer work in
Update 1, so replace them with #pragmas. Update 1 warns when |size_t|
arguments to |printf| are casted, so stop doing that casting.
Unfortunately, this requires an ugly hack to continue working in
MSVC 2013 as MSVC 2013 doesn't support "%zu". Finally, Update 1 has new
warnings, some of which need to be suppressed.
---
Updated by davidben to give up on suppressing warnings in crypto/x509 and
crypto/x509v3 as those directories aren't changed much from upstream. In each
of these cases, upstream opted just blindly initialize the variable, so do the
same. Also switch C4265 to level 4, per Microsoft's recommendation and work
around a bug in limits.h that happens to get fixed by Google include order
style.
(limits.h is sensitive to whether corecrt.h, pulled in by stddef.h and some
other headers, is included before it. The reason it affected just one file is
we often put the file's header first, which means base.h is pulling in
stddef.h. Relying on this is ugly, but it's no worse than what everything else
is doing and this doesn't seem worth making something as tame as limits.h so
messy to use.)
Change-Id: I02d1f935356899f424d3525d03eca401bfa3e6cd
Reviewed-on: https://boringssl-review.googlesource.com/7480
Reviewed-by: David Benjamin <davidben@google.com>
Conscrypt, thanks to Java's RSAPrivateKeySpec API, must be able to use RSA keys
with only modulus and exponent. This is kind of silly and breaks the blinding
code so they, both in OpenSSL and BoringSSL, had to explicitly turn blinding
off.
Add a test for this as we're otherwise sure to break it on accident.
We may wish to avoid the silly rsa->flags modification, I'm not sure. For now,
keep the requirement in so other consumers do not accidentally rely on this.
(Also add a few missing ERR_clear_error calls. Functions which are expected to
fail should be followed by an ERR_clear_error so later unexpected failures
don't get confused.)
BUG=boringssl:12
Change-Id: I674349821f1f59292b8edd085f21dc37e8bcaa75
Reviewed-on: https://boringssl-review.googlesource.com/7560
Reviewed-by: David Benjamin <davidben@google.com>
In |bn_blinding_update| the condition |b->e != NULL| would never be
true (probably), but the test made reasoning about the correctness of
the code confusing. That confusion was amplified by the circuitous and
unusual way in which |BN_BLINDING|s are constructed. Clarify all this
by simplifying the construction of |BN_BLINDING|s, making it more like
the construction of other structures.
Also, make counter unsigned as it is no longer ever negative.
Change-Id: I6161dcfeae19a80c780ccc6762314079fca1088b
Reviewed-on: https://boringssl-review.googlesource.com/7530
Reviewed-by: David Benjamin <davidben@google.com>
Simplify the code by always caching Montgomery contexts in the RSA
structure, regardless of the |RSA_FLAG_CACHE_PUBLIC| and
|RSA_FLAG_CACHE_PRIVATE| flags. Deprecate those flags.
Now that we do this no more than once per key per RSA exponent, the
private key exponents better because the initialization of the
Montgomery contexts isn't perfectly side-channel protected.
Change-Id: I4fbcfec0f2f628930bfeb811285b0ae3d103ac5e
Reviewed-on: https://boringssl-review.googlesource.com/7521
Reviewed-by: David Benjamin <davidben@google.com>
The function wants the expected value first.
Change-Id: I6d3e21ebfa55d6dd99a34fe8380913641b4f5ff6
Reviewed-on: https://boringssl-review.googlesource.com/7501
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Otherwise it's confusing if you mistype the test name.
Change-Id: Idf32081958f85f3b5aeb8993a07f6975c27644f8
Reviewed-on: https://boringssl-review.googlesource.com/7500
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Imported from patch attached to
https://rt.openssl.org/Ticket/Display.html?id=4439.
But with the extra vs $extra typo fixed.
The root problem appears to be that lazy_reduction tries to use paddd instead
of paddq when they believe the sum will not overflow a u32. In the final call
to lazy_reduction, this is not true. svaldez and I attempted to work through
the bounds, but the bounds derived from the cited paper imply paddd is always
fine. Empirically in a debugger, the bounds are exceeded in the test case.
I requested more comments from upstream on the bug. When upstream lands their
final fix (hopefully with comments), I will update this code. In the meantime,
let's stop carrying known-broken stuff.
(vlazy_reduction is probably something similar, but since we don't enable that
code, we haven't bothered analyzing it.)
Also add the smaller of the two test cases that catch the bug. (The other uses
an update pattern which isn't quite what poly1305_test does.)
Change-Id: I446ed47c21f10b41a0745de96ab119a3f6fd7801
Reviewed-on: https://boringssl-review.googlesource.com/7544
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: If76154c8d255600e925a408acdc674fc7dad0359
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/7526
Reviewed-by: David Benjamin <davidben@google.com>
We failed to correctly parse files that executed from the very start of
the file due to a missing '- line XXX'. We now use the 'Ir' indicator to
recognize the beginning of a file.
Change-Id: I529fae9458ac634bf7bf8af61ef18f080e808535
Reviewed-on: https://boringssl-review.googlesource.com/7542
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Use |size_t| for array indexes. Use |int| for boolean flags. Declare
the variables that had their types changed closer to where they are
used.
Previously, some `for` loops depended on `i` being signed, so their
structure had to be changed to work with the unsigned type.
Change-Id: I247e4f04468419466733b6818d81d28666da0ad3
Reviewed-on: https://boringssl-review.googlesource.com/7468
Reviewed-by: David Benjamin <davidben@google.com>
Maintain the max_len values in foo.options files which ClusterFuzz can process.
Also recompute the recommended client and server lengths as they've since
gotten much more extensive.
Change-Id: Ie87a80d8a4a0c41e215f0537c8ccf82b38c4de09
Reviewed-on: https://boringssl-review.googlesource.com/7509
Reviewed-by: Mike Aizatsky <aizatsky@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Most code already dereferences it directly.
Change-Id: I227fa91ecbf25a19077f7cfba21b0abd2bc2bd1d
Reviewed-on: https://boringssl-review.googlesource.com/7422
Reviewed-by: David Benjamin <davidben@google.com>
There is a potential double free in EVP_DigestInit_ex. This is believed
to be reached only as a result of programmer error - but we should fix it
anyway.
(Imported from upstream's e78dc7e279)
Change-Id: I1da7be7db7afcbe9f30f168df000d64ed73d7edd
Reviewed-on: https://boringssl-review.googlesource.com/7541
Reviewed-by: David Benjamin <davidben@google.com>
We recently gained -Werror=missing-prototypes. (See also, we really need to get
those Android bots...)
Change-Id: I3962d3050bccf5f5a057d029b5cbff1695ca1a03
Reviewed-on: https://boringssl-review.googlesource.com/7540
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The fields of the |bn_blinding_st| are not updated atomically.
Consequently, one field (|A| or |Ai|) might get updated while the
other field (|Ai| or |A|) doesn't get updated, if an error occurs in
the middle of updating. Deal with this by reseting the counter so that
|A| and |Ai| will both get recreated the next time the blinding is
used.
Fix a separate but related issue by resetting the counter to zero after
calling |bn_blinding_create_param| only if |bn_blinding_create_param|
succeeded. Previously, regardless of whether an error occured in
|bn_blinding_create_param|, |b->counter| would get reset to zero. The
consequence of this was that potentially-bad blinding values would get
used 32 times instead of (32 - |b->counter|) times.
Change-Id: I236cdb6120870ef06cba129ed86619f593cbcf3d
Reviewed-on: https://boringssl-review.googlesource.com/7520
Reviewed-by: David Benjamin <davidben@google.com>
Make it match how it is done in p224-64.c. Note in particular that
|size| may be 17, so presumably |pre_comp[16]| is accessed, which one
would not expect when it was declared |precomp[16][3]|.
Change-Id: I54c1555f9e20ccaacbd4cd75a7154b483b4197b7
Reviewed-on: https://boringssl-review.googlesource.com/7467
Reviewed-by: David Benjamin <davidben@google.com>
Since the function doesn't call |BN_CTX_get|, it doesn't need to call
|BN_CTX_start|/|BN_CTX_end|.
Change-Id: I6cb954d3fee2959bdbc81b9b97abc52bb6f7704c
Reviewed-on: https://boringssl-review.googlesource.com/7469
Reviewed-by: David Benjamin <davidben@google.com>
As far as I can tell, this is the last place within libcrypto where
this type of check is missing.
Change-Id: I3d09676abab8c9f6c4e87214019a382ec2ba90ee
Reviewed-on: https://boringssl-review.googlesource.com/7519
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: Ieba81f114483095f3657e87f669c7562ff75b58c
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/7516
Reviewed-by: David Benjamin <davidben@google.com>
David Benjamin
Brian Smith
Piotr Sikora
Matt Mueller
Steven Valdez