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>
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>
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>
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 e78dc7e279ed98e1ab9845a70d14dafdfdc88f58)
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>
Partially fixes build with -Wmissing-prototypes -Wmissing-declarations.
Change-Id: I51209c30f532899f57cfdd9a50cff0a8ee3da5b5
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/7512
Reviewed-by: David Benjamin <davidben@google.com>
Partially fixes build with -Wmissing-prototypes -Wmissing-declarations.
Change-Id: I6048f5b7ef31560399b25ed9880156bc7d8abac2
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/7511
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I44aa9be26ad9aea6771cb46a886a721b4bc28fde
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/7510
Reviewed-by: David Benjamin <davidben@google.com>
(Imported from upstream's 2460c7f13389d766dd65fa4e14b69b6fbe3e4e3b.)
This is a no-op for us, but avoid a diff with upstream.
Change-Id: I6e875704a38dcd9339371393a4dd523647aeef44
Reviewed-on: https://boringssl-review.googlesource.com/7491
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
(Imported from upstream's 01c32b5e448f6d42a23ff16bdc6bb0605287fa6f.)
Change-Id: Ib52278dbbac1ed1ad5c80f0ad69e34584d411cec
Reviewed-on: https://boringssl-review.googlesource.com/7461
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Not all assemblers of "gas" flavour handle binary constants, e.g.
seasoned MacOS Xcode doesn't, so give them a hand.
(Imported from upstream's ba26fa14556ba49466d51e4d9e6be32afee9c465.)
Change-Id: I35096dc8035e06d2fbef2363b869128da206ff9d
Reviewed-on: https://boringssl-review.googlesource.com/7459
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I6267c9bfb66940d0b6fe5368514210a058ebd3cc
Reviewed-on: https://boringssl-review.googlesource.com/7494
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Having a different API for this case than upstream is more trouble than is
worth it. This is sad since the new API avoids incomplete EC_GROUPs at least,
but I don't believe supporting this pair of functions will be significantly
more complex than supporting EC_GROUP_new_arbitrary even when we have static
EC_GROUPs.
For now, keep both sets of APIs around, but we'll be able to remove the scar
tissue once Conscrypt's complex dependencies are resolved.
Make the restored EC_GROUP_set_generator somewhat simpler than before by
removing the ability to call it multiple times and with some parameters set to
NULL. Keep the test.
Change-Id: I64e3f6a742678411904cb15c0ad15d56cdae4a73
Reviewed-on: https://boringssl-review.googlesource.com/7432
Reviewed-by: David Benjamin <davidben@google.com>
I messed up a few of these.
ASN1_R_UNSUPPORTED_ALGORITHM doesn't exist. X509_R_UNSUPPORTED_ALGORITHM does
exist as part of X509_PUBKEY_set, but the SPKI parser doesn't emit this. (I
don't mind the legacy code having really weird errors, but since EVP is now
limited to things we like, let's try to keep that clean.) To avoid churn in
Conscrypt, we'll keep defining X509_R_UNSUPPORTED_ALGORITHM, but not actually
do anything with it anymore. Conscrypt was already aware of
EVP_R_UNSUPPORTED_ALGORITHM, so this should be fine. (I don't expect
EVP_R_UNSUPPORTED_ALGORITHM to go away. The SPKI parsers we like live in EVP
now.)
A few other ASN1_R_* values didn't quite match upstream, so make those match
again. Finally, I got some of the rsa_pss.c values wrong. Each of those
corresponds to an (overly specific) RSA_R_* value in upstream. However, those
were gone in BoringSSL since even the initial commit. We placed the RSA <-> EVP
glue in crypto/evp (so crypto/rsa wouldn't depend on crypto/evp) while upstream
placed them in crypto/rsa.
Since no one seemed to notice the loss of RSA_R_INVALID_SALT_LENGTH, let's undo
all the cross-module errors inserted in crypto/rsa. Instead, since that kind of
specificity is not useful, funnel it all into X509_R_INVALID_PSS_PARAMETERS
(formerly EVP_R_INVALID_PSS_PARAMETERS, formerly RSA_R_INVALID_PSS_PARAMETERS).
Reset the error codes for all affected modules.
(That our error code story means error codes are not stable across this kind of
refactoring is kind of a problem. Hopefully this will be the last of it.)
Change-Id: Ibfb3a0ac340bfc777bc7de6980ef3ddf0a8c84bc
Reviewed-on: https://boringssl-review.googlesource.com/7458
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This reverts commit ba70118d8e. Reverting this
did not resolve the regression and the cause is now known.
BUG=593963
Change-Id: Ic5e24b74e8f16b01d9fdd80f267a07ef026c82cf
Reviewed-on: https://boringssl-review.googlesource.com/7454
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
People seem to condition on these a lot. Since this code has now been moved
twice, just make them all cross-module errors rather than leave a trail of
renamed error codes in our wake.
Change-Id: Iea18ab3d320f03cf29a64a27acca119768c4115c
Reviewed-on: https://boringssl-review.googlesource.com/7431
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
If the values of any of the coordinates in the output point |r| were
negative during nistz256 multiplication, then the calls to
|bn_set_word| would result in the wrong coordinates being returned
(the negatives of the correct coordinates would be returned instead).
Fix that.
Change-Id: I6048e62f76dca18f625650d11ef5a051c9e672a4
Reviewed-on: https://boringssl-review.googlesource.com/7442
Reviewed-by: David Benjamin <davidben@google.com>
The (internal) constant-time callers of this function already do a
constant-time reduction before calling. And, nobody should be calling
this function with out-of-range coordinates anyway. So, just require
valid coordinates as input.
Further, this function is rarely called, so don't bother with the
optimization to avoid encoding Montgomery encoding of 1 for the Z
coordinate.
Change-Id: I637ffaf4d39135ca17214915b9a8582ea052eea8
Reviewed-on: https://boringssl-review.googlesource.com/7441
Reviewed-by: David Benjamin <davidben@google.com>
Don't try to fix a bad |x| coordinate by reducing it. Instead, just
fail. This also makes the code clearer; in particular, it was confusing
why |x_| was used for some calculations when it seems like |x| was just
as good or better.
Change-Id: I9a6911f0d2bd72852a26b46f3828eb5ba3ef924f
Reviewed-on: https://boringssl-review.googlesource.com/7440
Reviewed-by: David Benjamin <davidben@google.com>
We never heap-allocate a GCM128_CONTEXT.
Change-Id: I7e89419ce4d81c1598a4b3a214c44dbbcd709651
Reviewed-on: https://boringssl-review.googlesource.com/7430
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Although exactly one iteration of cmov_cached will always initialize selected,
it ends up messing with uninitialized memory. Initialize |selected| before the
loop.
BUG=593540
Change-Id: I5921843f68c6dd1dc7f752538825bc43ba75df4a
Reviewed-on: https://boringssl-review.googlesource.com/7415
Reviewed-by: Arnar Birgisson <arnarb@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The points are only converted to affine form when there are at least
three points being multiplied (in addition to the generator), but there
never is more than one point, so this is all dead code.
Also, I doubt that the comments "...point at infinity (which normally
shouldn't happen)" in the deleted code are accurate. And, the
projective->affine conversions that were removed from p224-64.c and
p256-64.c didn't seem to properly account for the possibility that any of
those points were at infinity.
Change-Id: I611d42d36dcb7515eabf3abf1857e52ff3b45c92
Reviewed-on: https://boringssl-review.googlesource.com/7100
Reviewed-by: David Benjamin <davidben@google.com>
If the function returns early due to an error, then the coordinates of the
result will have their |top| value set to a value beyond what has actually
been been written. Fix that, and make it easier to avoid such issues in the
future by refactoring the code.
As a bonus, avoid a false positive MSVC 64-bit opt build "potentially
uninitialized value used" warning.
Change-Id: I8c48deb63163a27f739c8797962414f8ca2588cd
Reviewed-on: https://boringssl-review.googlesource.com/6579
Reviewed-by: David Benjamin <davidben@google.com>
Having |Z_is_one| be out of sync with |Z| could potentially be a very
bad thing, and in the past there have been multiple bugs of this sort,
including one currently in p256-x86_64.c (type confusion: Montgomery-
encoded vs unencoded). Avoid the issue entirely by getting rid of
|Z_is_one|.
Change-Id: Icb5aa0342df41d6bc443f15f952734295d0ee4ba
Reviewed-on: https://boringssl-review.googlesource.com/6576
Reviewed-by: David Benjamin <davidben@google.com>
This could live in decrepit, but it's tiny and having it makes the
interface more uniform that what we have for MD5 so I put it in the main
code. This is to more easily support nmap.
Change-Id: Ia098cc7ef6e00a90d2f3f56ee7deba8329c9a82e
Reviewed-on: https://boringssl-review.googlesource.com/7400
Reviewed-by: David Benjamin <davidben@google.com>
This reverts commit b944882f26.
Recent Chrome canaries show a visible jump in ERR_SSL_PROTOCOL_ERROR which
coincided with a DEPS roll that included this change. Speculatively revert it
to see if they go back down afterwards.
Change-Id: I067798db144c348d666985986dfb9720d1153b7a
Reviewed-on: https://boringssl-review.googlesource.com/7391
Reviewed-by: David Benjamin <davidben@google.com>
This removes a hard dependency on |BN_mod_exp|, which will allow the
linker to drop it in programs that don't use other features that
require it.
Also, remove the |mont| member of |bn_blinding_st| in favor of having
callers pass it when necssaary. The |mont| member was a weak reference,
and weak references tend to be error-prone.
Finally, reduce the scope of some parts of the blinding code to
|static|.
Change-Id: I16d8ccc2d6d950c1bb40377988daf1a377a21fe6
Reviewed-on: https://boringssl-review.googlesource.com/7111
Reviewed-by: David Benjamin <davidben@google.com>
See also 1b0c438e1a.
Change-Id: Ifcfe15caa4d0db8ef725f8dacd0e8c5c94b00a09
Reviewed-on: https://boringssl-review.googlesource.com/7390
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Instead of crashing when an empty key is passed to
EVP_marshal_public_key(), return with an
EVP_R_UNSUPPORTED_ALGORITHM_ERROR. This brings e.g. X509_PUBKEY_set()
closer to how it behaved before 68772b31 (previously, it returned an
error on an empty public key rather than dereferencing pkey->ameth).
Change-Id: Ieac368725adb7f22329c035d9d0685b44b885888
Reviewed-on: https://boringssl-review.googlesource.com/7351
Reviewed-by: David Benjamin <davidben@google.com>
I went with NID_x25519 to match NID_sha1 and friends in being lowercase.
However, upstream seems to have since chosen NID_X25519. Match their
name.
Change-Id: Icc7b183a2e2dfbe42c88e08e538fcbd242478ac3
Reviewed-on: https://boringssl-review.googlesource.com/7331
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
C is still kind of unsure about the whole two's complement thing and leaves
left-shifting of negative numbers undefined. Sadly, some sanitizers believe in
teaching the controversy and complain when code relies on the theory of two's
complement.
Shushing these sanitizers in this case is easier than fighting with build
configuration, so replace the shifts with masks. (This is equivalent as the
left-shift was of a value right-shifted by the same amount. Instead, we store
the unshifted value in carry0, etc., and mask off the bottom bits.) A few other
places get casts to unsigned types which, by some miracle, C compilers are
forbidden from miscompiling.
This is imported from upstream's b95779846dc876cf959ccf96c49d4c0a48ea3082 and
5b7af0dd6c9315ca76fba16813b66f5792c7fe6e.
Change-Id: I6bf8156ba692165940c0c4ea1edd5b3e88ca263e
Reviewed-on: https://boringssl-review.googlesource.com/7320
Reviewed-by: Adam Langley <agl@google.com>
The coverage tool revealed that we weren't testing all codepaths of the ChaCha
assembly. Add a standalone test as it's much easier to iterate over all lengths
when there isn't the entire AEAD in the way.
I wasn't able to find a really long test vector, so I generated a random one
with the Go implementation we have in runner.
This test gives us full coverage on the ChaCha20_ssse3 variant. (We'll see how
it fares on the other codepaths when the multi-variant test harnesses get in. I
certainly hope there isn't a more novel way to call ChaCha20 than this...)
Change-Id: I087e421c7351f46ea65dacdc7127e4fbf5f4c0aa
Reviewed-on: https://boringssl-review.googlesource.com/7299
Reviewed-by: Adam Langley <agl@google.com>
Only the 32-bit AVX2 code path needs this, but upstream choose to harmonize all
vector code paths.
RT#4346
(Imported from 1ea8ae5090f557fea2e5b4d5758b10566825d74b.)
Tested the new code manually on arm and aarch64, NEON and non-NEON. Steven
reports that all variants pass on x86 and x86-64 too.
I've left the 32-bit x86 AVX2 code disabled since valgrind can't measure the
code coverage, but this avoids diff with upstream. We can enable it if we ever
end up caring.
Change-Id: Id9becc2adfbe44b84764f8e9c1fb5e8349c4d5a8
Reviewed-on: https://boringssl-review.googlesource.com/7295
Reviewed-by: Adam Langley <agl@google.com>
The |fprintf| dependency is quite heavyweight for small targets. Also,
using |fprintf| on a closed file dsecriptor is undefined behavior, and
there's no way that this code can know whether |stderr| has already
been closed. So, just don't do it.
Change-Id: I1277733afe0649ae1324d11cac84826a1056e308
Reviewed-on: https://boringssl-review.googlesource.com/6812
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
If running the stack through a fuzzer, we would like execution to be
completely deterministic. This is gated on a
BORINGSSL_UNSAFE_FUZZER_MODE #ifdef.
For now, this just uses the zero ChaCha20 key and a global counter. As
needed, we can extend this to a thread-local counter and a separate
ChaCha20 stream and counter per input length.
Change-Id: Ic6c9d8a25e70d68e5dc6804e2c234faf48e51395
Reviewed-on: https://boringssl-review.googlesource.com/7286
Reviewed-by: Adam Langley <agl@google.com>
There was some uncertainty about what the code is doing with |$end0|
and whether it was necessary for |$len| to be a multiple of 16 or 96.
Hopefully these added comments make it clear that the code is correct
except for the caveat regarding low memory addresses.
Change-Id: Iea546a59dc7aeb400f50ac5d2d7b9cb88ace9027
Reviewed-on: https://boringssl-review.googlesource.com/7194
Reviewed-by: Adam Langley <agl@google.com>
This was dropped in d27441a9cb due to lack
of use, but node.js now needs it.
Change-Id: I1e207d4b46fc746cfae309a0ea7bbbc04ea785e8
Reviewed-on: https://boringssl-review.googlesource.com/7270
Reviewed-by: David Benjamin <davidben@google.com>
See also upstream's dc22d6b37e8058a4334e6f98932c2623cd3d8d0d. (Though I'm not
sure why they didn't need to fix cmov.)
Change-Id: I2a194a8aea1734d4c1e7f6a0536a636379381627
Reviewed-on: https://boringssl-review.googlesource.com/7280
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 64333004a41a9f4aa587b8e5401420fb70d00687.)
RT#4284.
This case should be impossible to hit because |EC_POINT_add| doesn't use
this function and trying to add equal inputs should never occur during a
multiplication. Support for this exists because the pattern has been
copied from the first 64-bit P-224 and P-256 work that Emilia, Bodo and
I did. There it seemed like a reasonable defense-in-depth in case the
code changed in the future.
Change-Id: I7ff138669c5468b7d7a5153429bec728cb67e338
Reviewed-on: https://boringssl-review.googlesource.com/7246
Reviewed-by: David Benjamin <davidben@google.com>
(Imported from upstream's 515f3be47a0b58eec808cf365bc5e8ef6917266b)
Additional hardening following on from CVE-2016-0702.
Change-Id: I19a6739b401887a42eb335fe5838379dc8d04100
Reviewed-on: https://boringssl-review.googlesource.com/7245
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 25d14c6c29b53907bf614b9964d43cd98401a7fc.)
At the same time remove miniscule bias in final subtraction. Performance
penalty varies from platform to platform, and even with key length. For
rsa2048 sign it was observed to be 4% for Sandy Bridge and 7% on
Broadwell.
(This is part of the fix for CVE-2016-0702.)
Change-Id: I43a13d592c4a589d04c17c33c0ca40c2d7375522
Reviewed-on: https://boringssl-review.googlesource.com/7244
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 08ea966c01a39e38ef89e8920d53085e4807a43a)
Performance penalty is 2%.
(This is part of the fix for CVE-2016-0702.)
Change-Id: Id3b6262c5d3201dd64b93bdd34601a51794a9275
Reviewed-on: https://boringssl-review.googlesource.com/7243
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's ef98503eeef5c108018081ace902d28e609f7772.)
Performance penalty is 2% on Linux and 5% on Windows.
(This is part of the fix for CVE-2016-0702.)
Change-Id: If82f95131c93168282a46ac5a35e2b007cc2bd67
Reviewed-on: https://boringssl-review.googlesource.com/7242
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 708dc2f1291e104fe4eef810bb8ffc1fae5b19c1.)
Performance penalty varies from platform to platform, and even key
length. For rsa2048 sign it was observed to reach almost 10%.
This is part of the fix for CVE-2016-0702.
Change-Id: Ie0860bf3e531196f03102db1bc48eeaf30ab1d58
Reviewed-on: https://boringssl-review.googlesource.com/7241
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 10c639a8a56c90bec9e332c7ca76ef552b3952ac)
Change-Id: Ia8203eeae9d274249595a6e352ec2f77a97ca5d5
Reviewed-on: https://boringssl-review.googlesource.com/7227
Reviewed-by: David Benjamin <davidben@google.com>
If d2i_PrivateKey hit the PKCS#8 codepath, it didn't enforce that the key was
of the specified type.
Note that this requires tweaking d2i_AutoPrivateKey slightly. A PKCS #8
PrivateKeyInfo may have 3 or 4 elements (optional attributes), so we were
relying on this bug for d2i_AutoPrivateKey to work.
Change-Id: If50b7a742f535d208e944ba37c3a585689d1da43
Reviewed-on: https://boringssl-review.googlesource.com/7253
Reviewed-by: Adam Langley <agl@google.com>
They fail the newly-added in-place tests. Since we don't have bots for them
yet, verified manually that the arm and aarch64 code is fine.
Change-Id: Ic6f4060f63e713e09707af05e6b7736b7b65c5df
Reviewed-on: https://boringssl-review.googlesource.com/7252
Reviewed-by: Adam Langley <agl@google.com>
Cases where the input and output buffers overlap are always a little
odd. This change adds a test to ensures that the (generic) AEADs
function in these situations.
Change-Id: I6f1987a5e10ddef6b2b8f037a6d50737a120bc99
Reviewed-on: https://boringssl-review.googlesource.com/7195
Reviewed-by: David Benjamin <davidben@google.com>
Forgot to delete a line.
Change-Id: Ia1fb2904398816d495045dc237337f0be5b09272
Reviewed-on: https://boringssl-review.googlesource.com/7250
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
(Imported from upstream's 04f2a0b50d219aafcef2fa718d91462b587aa23d)
Change-Id: Ie840edeb1fc9d5a4273f137467e3ef16528c9668
Reviewed-on: https://boringssl-review.googlesource.com/7234
Reviewed-by: David Benjamin <davidben@google.com>
This isn't a problem when called from EVP, since the buffer is
aligned in the EVP_CIPHER_CTX. The increment counter code is also
fixed to deal with overflow.
(Imported from upstream's 6533a0b8d1ed12aa5f7dfd7a429eec67c5486bb5)
Change-Id: I8d7191c3d3873db254a551085d2358d90bc8397a
Reviewed-on: https://boringssl-review.googlesource.com/7233
Reviewed-by: David Benjamin <davidben@google.com>
While we're here, may as well test others.
Change-Id: I711528641a3f7dd035c696c3c1d6b035437c91cc
Reviewed-on: https://boringssl-review.googlesource.com/7239
Reviewed-by: Adam Langley <agl@google.com>
The high bits of the type get used for the V_ASN1_NEG bit, so when used with
ASN1_ANY/ASN1_TYPE, universal tags become ambiguous. This allows one to create
a negative zero, which should be impossible. Impose an upper bound on universal
tags accepted by crypto/asn1 and add a test.
BUG=590615
Change-Id: I363e01ebfde621c8865101f5bcbd5f323fb59e79
Reviewed-on: https://boringssl-review.googlesource.com/7238
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's e9cf5f03666bb82f0184e4f013702d0b164afdca and
29305f4edc886db349f2beedb345f9dd93311c09)
Change-Id: I0fa019e9d337676a84a7a6c103d2c4e14e18aede
Reviewed-on: https://boringssl-review.googlesource.com/7240
Reviewed-by: David Benjamin <davidben@google.com>
(Imported from upstream's 3661bb4e7934668bd99ca777ea8b30eedfafa871.)
Fix bug where i2c_ASN1_INTEGER mishandles zero if it is marked as
negative.
Thanks to Huzaifa Sidhpurwala <huzaifas@redhat.com> and Hanno Böck
<hanno@hboeck.de> for reporting this issue.
BUG=590615
Change-Id: I8959e8ae01510a5924862a3f353be23130eee554
Reviewed-on: https://boringssl-review.googlesource.com/7199
Reviewed-by: David Benjamin <davidben@google.com>
Reduce the maximum RSA exponent size to 33 bits, regardless of modulus
size, for public key operations.
Change-Id: I88502b1033d8854696841531031298e8ad96a467
Reviewed-on: https://boringssl-review.googlesource.com/6901
Reviewed-by: Adam Langley <agl@google.com>
It's not used anywhere else, in the library or consumers (Google ones or
ones I could find on Debian codesearch). This is a sufficiently
specialized function that the risk of a third-party library newly
depending on it is low. This removes the last include of asn1.h or
x509.h in crypto/evp.
(This is almost entirely cosmetic because it wasn't keeping the static linker
from doing the right thing anyway. But if we were want to separate the legacy
ASN.1 stack into its own decrepit-like target, we'll need to be pickier about
separation.)
Change-Id: I9be97c9321572e3a2ed093e1d50036b7654cff41
Reviewed-on: https://boringssl-review.googlesource.com/7080
Reviewed-by: Adam Langley <agl@google.com>
A number of values have fallen off now that code's been shuffled
around.
Change-Id: I5eac1d3fa4a9335c6aa72b9876d37bb9a9a029ac
Reviewed-on: https://boringssl-review.googlesource.com/7029
Reviewed-by: Adam Langley <agl@google.com>
Functions which lose object reuse and need auditing:
- d2i_PrivateKey
This removes evp_asn1.c's dependency on the old stack. (Aside from
obj/.) It also takes old_priv_decode out of EVP_ASN1_METHOD in favor of
calling out to the new-style function. EVP_ASN1_METHOD no longer has any
old-style type-specific serialization hooks, only the PKCS#8 and SPKI
ones.
BUG=499653
Change-Id: Ic142dc05a5505b50e4717c260d3893b20e680194
Reviewed-on: https://boringssl-review.googlesource.com/7027
Reviewed-by: Adam Langley <agl@google.com>
This is kind of a ridiculous function. It would be nice to lose it, but
SSL_use_PrivateKey_file actually calls into it (by way of
d2i_PrivateKey_bio).
BUG=499653
Change-Id: I83634f6982b15f4b877e29f6793b7e00a1c10450
Reviewed-on: https://boringssl-review.googlesource.com/7026
Reviewed-by: Adam Langley <agl@google.com>
EVP_PKEY_asn1_find can already be private. EVP_PKEY_asn1_find_str is used
only so the PEM code can get at legacy encoders. Since this is all
legacy non-PKCS8 stuff, we can just explicitly list out the three cases
in the two places that need it. If this changes, we can later add a
table in crypto/pem mapping string to EVP_PKEY type.
With this, EVP_PKEY_ASN1_METHOD is no longer exposed in the public API
and nothing outside of EVP_PKEY reaches into it. Unexport all of that.
Change-Id: Iab661014247dbdbc31e5e9887364176ec5ad2a6d
Reviewed-on: https://boringssl-review.googlesource.com/6871
Reviewed-by: Adam Langley <agl@google.com>
Every key type which has a legacy PEM encoding also has a PKCS#8
encoding. The fallback codepath is never reached.
This removes the only consumer of pem_str, so that may be removed from
EVP_PKEY_ASN1_METHOD.
Change-Id: Ic680bfc162e1dc76db8b8016f6c10f669b24f5aa
Reviewed-on: https://boringssl-review.googlesource.com/6870
Reviewed-by: Adam Langley <agl@google.com>
This allows the static linker to drop it in consumers which don't need this
stuff (i.e. all sane ones), once crypto/x509 falls off. This cuts down
on a number of dependencies from the core crypto bits on crypto/asn1 and
crypto/x509.
BUG=499653
Change-Id: I76a10a04dcc444c1ded31683df9f87725a95a4e6
Reviewed-on: https://boringssl-review.googlesource.com/5660
Reviewed-by: Adam Langley <agl@google.com>
All the signature algorithm logic depends on X509_ALGOR. This also
removes the X509_ALGOR-based EVP functions which are no longer used
externally. I think those APIs were a mistake on my part. The use in
Chromium was unnecessary (and has since been removed anyway). The new
X.509 stack will want to process the signatureAlgorithm itself to be
able to enforce policies on it.
This also moves the RSA_PSS_PARAMS bits to crypto/x509 from crypto/rsa.
That struct is also tied to crypto/x509. Any new RSA-PSS code would
have to use something else anyway.
BUG=499653
Change-Id: I6c4b4573b2800a2e0f863d35df94d048864b7c41
Reviewed-on: https://boringssl-review.googlesource.com/7025
Reviewed-by: Adam Langley <agl@google.com>
This is in preparation for moving the logic itself to crypto/x509, so
the lower-level functions will not be as readily available.
Change-Id: I6507b895317df831ab11d0588c5b09bbb2aa2c24
Reviewed-on: https://boringssl-review.googlesource.com/7023
Reviewed-by: Adam Langley <agl@google.com>
It's only used by crypto/x509, and we don't even support DSA in
crypto/x509 anymore since the EVP_PKEY_CTX hooks aren't wired up.
Change-Id: I1b8538353eb51df353cf9171b1cbb0bb47a879a3
Reviewed-on: https://boringssl-review.googlesource.com/7024
Reviewed-by: Adam Langley <agl@google.com>
gcm_test.c includes a test case that does a 'malloc(0)'. This test case
currently fails if malloc(0) returns NULL. According to the standard,
malloc's behavior with a size of 0is implementation specific and may
either be NULL or another pointer suitable to be passed to free(). This
change modifies gcm_test.c to handle a return value of NULL. It has
been tested with a custom allocator on an experimental branch.
Change-Id: I35514ec9735cedffc621f7dfae42b4c6664a1766
Reviewed-on: https://boringssl-review.googlesource.com/7122
Reviewed-by: Adam Langley <agl@google.com>
The C implementation is still our existing C implementation, but slightly
tweaked to fit with upstream's init/block/emits convention.
I've tested this by looking at code coverage in kcachegrind and
valgrind --tool=callgrind --dump-instr=yes --collect-jumps=yes
(NB: valgrind 3.11.0 is needed for AVX2. And even that only does 64-bit AVX2,
so we can't get coverage for the 32-bit code yet. But I had to disable that
anyway.)
This was paired with a hacked up version of poly1305_test that would repeat
tests with different ia32cap and armcap values. This isn't checked in, but we
badly need a story for testing all the different variants.
I'm not happy with upstream's code in either the C/asm boundary or how it
dispatches between different versions, but just debugging the code has been a
significant time investment. I'd hoped to extract the SIMD parts and do the
rest in C, but I think we need to focus on testing first (and use that to
guide what modifications would help). For now, this version seems to work at
least.
The x86 (not x86_64) AVX2 code needs to be disabled because it's broken. It
also seems pretty unnecessary.
https://rt.openssl.org/Ticket/Display.html?id=4346
Otherwise it seems to work and buys us a decent performance improvement.
Notably, my Nexus 6P is finally faster at ChaCha20-Poly1305 than my Nexus 4!
bssl speed numbers follow:
x86
---
Old:
Did 1554000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000536us (1553167.5 ops/sec): 24.9 MB/s
Did 136000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1003947us (135465.3 ops/sec): 182.9 MB/s
Did 30000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1022990us (29325.8 ops/sec): 240.2 MB/s
Did 1888000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000206us (1887611.2 ops/sec): 30.2 MB/s
Did 173000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1003036us (172476.4 ops/sec): 232.8 MB/s
Did 30000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1027759us (29189.7 ops/sec): 239.1 MB/s
New:
Did 2030000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000507us (2028971.3 ops/sec): 32.5 MB/s
Did 404000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1000287us (403884.1 ops/sec): 545.2 MB/s
Did 83000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1001258us (82895.7 ops/sec): 679.1 MB/s
Did 2018000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000006us (2017987.9 ops/sec): 32.3 MB/s
Did 360000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1001962us (359295.1 ops/sec): 485.0 MB/s
Did 85000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1002479us (84789.8 ops/sec): 694.6 MB/s
x86_64, no AVX2
---
Old:
Did 2023000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000258us (2022478.2 ops/sec): 32.4 MB/s
Did 466000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1002619us (464782.7 ops/sec): 627.5 MB/s
Did 90000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1001133us (89898.1 ops/sec): 736.4 MB/s
Did 2238000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000175us (2237608.4 ops/sec): 35.8 MB/s
Did 483000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1001348us (482349.8 ops/sec): 651.2 MB/s
Did 90000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1003141us (89718.2 ops/sec): 735.0 MB/s
New:
Did 2558000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000275us (2557296.7 ops/sec): 40.9 MB/s
Did 510000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1001810us (509078.6 ops/sec): 687.3 MB/s
Did 115000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1006457us (114262.2 ops/sec): 936.0 MB/s
Did 2818000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000187us (2817473.1 ops/sec): 45.1 MB/s
Did 418000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1001140us (417524.0 ops/sec): 563.7 MB/s
Did 91000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1002539us (90769.5 ops/sec): 743.6 MB/s
x86_64, AVX2
---
Old:
Did 2516000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000115us (2515710.7 ops/sec): 40.3 MB/s
Did 774000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1000300us (773767.9 ops/sec): 1044.6 MB/s
Did 171000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1004373us (170255.5 ops/sec): 1394.7 MB/s
Did 2580000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000144us (2579628.5 ops/sec): 41.3 MB/s
Did 769000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1000472us (768637.2 ops/sec): 1037.7 MB/s
Did 169000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1000320us (168945.9 ops/sec): 1384.0 MB/s
New:
Did 3240000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000114us (3239630.7 ops/sec): 51.8 MB/s
Did 932000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1000059us (931945.0 ops/sec): 1258.1 MB/s
Did 217000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1003282us (216290.1 ops/sec): 1771.8 MB/s
Did 3187000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000100us (3186681.3 ops/sec): 51.0 MB/s
Did 926000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1000071us (925934.3 ops/sec): 1250.0 MB/s
Did 215000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1000479us (214897.1 ops/sec): 1760.4 MB/s
arm, Nexus 4
---
Old:
Did 430248 ChaCha20-Poly1305 (16 bytes) seal operations in 1000153us (430182.2 ops/sec): 6.9 MB/s
Did 115250 ChaCha20-Poly1305 (1350 bytes) seal operations in 1000549us (115186.8 ops/sec): 155.5 MB/s
Did 27000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1030124us (26210.4 ops/sec): 214.7 MB/s
Did 451750 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000549us (451502.1 ops/sec): 7.2 MB/s
Did 118000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1001557us (117816.6 ops/sec): 159.1 MB/s
Did 27000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1024263us (26360.4 ops/sec): 215.9 MB/s
New:
Did 553644 ChaCha20-Poly1305 (16 bytes) seal operations in 1000183us (553542.7 ops/sec): 8.9 MB/s
Did 126000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1000396us (125950.1 ops/sec): 170.0 MB/s
Did 27000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1000336us (26990.9 ops/sec): 221.1 MB/s
Did 559000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1001465us (558182.3 ops/sec): 8.9 MB/s
Did 124000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1000824us (123897.9 ops/sec): 167.3 MB/s
Did 28000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1034854us (27057.0 ops/sec): 221.7 MB/s
aarch64, Nexus 6P
---
Old:
Did 358000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000358us (357871.9 ops/sec): 5.7 MB/s
Did 45000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1022386us (44014.7 ops/sec): 59.4 MB/s
Did 8657 ChaCha20-Poly1305 (8192 bytes) seal operations in 1063722us (8138.4 ops/sec): 66.7 MB/s
Did 350000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000074us (349974.1 ops/sec): 5.6 MB/s
Did 44000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1007907us (43654.8 ops/sec): 58.9 MB/s
Did 8525 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1042644us (8176.3 ops/sec): 67.0 MB/s
New:
Did 713000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000190us (712864.6 ops/sec): 11.4 MB/s
Did 180000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1004249us (179238.4 ops/sec): 242.0 MB/s
Did 41000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1005811us (40763.1 ops/sec): 333.9 MB/s
Did 775000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000719us (774443.2 ops/sec): 12.4 MB/s
Did 182000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1003529us (181360.0 ops/sec): 244.8 MB/s
Did 41000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1010576us (40570.9 ops/sec): 332.4 MB/s
Change-Id: Iaa4ab86ac1174b79833077963cc3616cfb08e686
Reviewed-on: https://boringssl-review.googlesource.com/7226
Reviewed-by: Adam Langley <agl@google.com>
Some consumers of connect BIOs connect them explicitly, and we already have the
BIO_ctrl hooked up.
Change-Id: Ie6b14f8ceb272b560e2b534e0b6c32fae050475b
Reviewed-on: https://boringssl-review.googlesource.com/7217
Reviewed-by: Adam Langley <agl@google.com>
Commit a3d9528e9e has a bug that could
cause counters to be reused if |$avx=2| were set in the AES-NI AES-GCM
assembly code, if the EVP interface were used with certain coding
patterns, as demonstrated by the test cases added in
a5ee83f67e.
This changes the encryption code in the same way the decryption code
was changed in a3d9528e9e.
This doesn't have any effect currently since the AES-NI AES-GCM code
has |$avx=0| now, so |aesni_gcm_encrypt| doesn't change the counter.
Change-Id: Iba69cb4d2043d1ea57c6538b398246af28cba006
Reviewed-on: https://boringssl-review.googlesource.com/7193
Reviewed-by: Adam Langley <agl@google.com>
Taken from 6b2ebe4332e22b4eb7dd6fadf418e3da7b926ca4. These don't do anything
right now but are checked in unmodified to make diffs easier to see.
Change-Id: I4f5bdb7b16f4ac27e7ef175f475540c481b8d593
Reviewed-on: https://boringssl-review.googlesource.com/7224
Reviewed-by: Adam Langley <agl@google.com>
OpenSSL upstream's SIMD assembly is rather complex. This pattern of update
calls should be sufficient to stress all the codepaths.
Change-Id: I50dea8351e4203b6b2cd9b23456eb4a592d31b5e
Reviewed-on: https://boringssl-review.googlesource.com/7223
Reviewed-by: Adam Langley <agl@google.com>
Also avoid using "i" in X509_cert_verify as a loop counter, trust
outcome and as an error ordinal.
(Imported from upstream's a3baa171053547488475709c7197592c66e427cf)
Change-Id: I4b0b542ffacf7fa861c93c8124b334c0aacc3c17
Reviewed-on: https://boringssl-review.googlesource.com/7222
Reviewed-by: David Benjamin <davidben@google.com>
(Imported from upstream's 402fb1896b2aab5cf887127bbce964554b9c8113)
Change-Id: I80c1f952085c8fc9062d3395f211a525151c404d
Reviewed-on: https://boringssl-review.googlesource.com/7219
Reviewed-by: David Benjamin <davidben@google.com>
Also avoid using "i" in X509_cert_verify as a loop counter, trust
outcome and as an error ordinal.
(Imported from upstream's a3baa171053547488475709c7197592c66e427cf)
Change-Id: I492afdbaa5017bcf00a0412033cf99fca3fe9401
Reviewed-on: https://boringssl-review.googlesource.com/7218
Reviewed-by: David Benjamin <davidben@google.com>
It is only used by |bn_div_rem_words|.
Change-Id: I57627091d8db5890d7fea34d8560897717008646
Reviewed-on: https://boringssl-review.googlesource.com/7128
Reviewed-by: David Benjamin <davidben@google.com>
Create a |bn_div_rem_words| that is used for double-word/single-word
divisions and division-with-remainder. Remove all implementations of
|bn_div_words| except for the implementation needed for 64-bit MSVC.
This allows more code to be shared across platforms and also removes
an instance of the dangerous pattern wherein the |div_asm| macro
modified a variable that wasn't passed as a parameter.
Also, document the limitations of the compiler-generated code for the
non-asm code paths more fully. Compilers indeed have not improved in
this respect.
Change-Id: I5a36a2edd7465de406d47d72dcd6bf3e63e5c232
Reviewed-on: https://boringssl-review.googlesource.com/7127
Reviewed-by: David Benjamin <davidben@google.com>
Callers of this function are not checking for the -1 result. Change
the semantics to match their expectations and to match the common
semantics of most other parts of BoringSSL.
Change-Id: I4ec537d7619e20e8ddfee80c72125e4c02cfaac1
Reviewed-on: https://boringssl-review.googlesource.com/7125
Reviewed-by: David Benjamin <davidben@google.com>
Note that this structure has a weak pointer to the key, which was a
problem corrected in the AES-GCM code in
0f8bfdeb33. Also, it uses |void *|
instead of |const AES_KEY *| to refer to that key.
Change-Id: I70e632e3370ab27eb800bc1c0c64d2bd36b7cafb
Reviewed-on: https://boringssl-review.googlesource.com/7123
Reviewed-by: David Benjamin <davidben@google.com>
BIO_FLAGS_MEM_RDONLY keeps the invariant.
(Imported from upstream's a38a159bfcbc94214dda00e0e6b1fc6454a23b78)
Change-Id: I4cb35615d76b77929915e370dbb7fec1455da069
Reviewed-on: https://boringssl-review.googlesource.com/7214
Reviewed-by: David Benjamin <davidben@google.com>
