This is to make sure all of libssl's consumers' have sufficiently
reasonable toolchains. Once this bakes, we can go about moving
libssl to C++.
This is just starting with libssl for now because libcrypto has more
consumers and libssl would benefit more from C++ than libcrypto (though
libcrypto also has code that would benefit).
BUG=132
Change-Id: Ie02f7b0a8a95defd289cc7e62451d4b16408ca2a
Reviewed-on: https://boringssl-review.googlesource.com/13161
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Measured on a SkyLake processor:
Before:
Did 11373750 AES-128-GCM (16 bytes) seal operations in 1016000us (11194635.8 ops/sec): 179.1 MB/s
Did 2253000 AES-128-GCM (1350 bytes) seal operations in 1016000us (2217519.7 ops/sec): 2993.7 MB/s
Did 453750 AES-128-GCM (8192 bytes) seal operations in 1015000us (447044.3 ops/sec): 3662.2 MB/s
Did 10753500 AES-256-GCM (16 bytes) seal operations in 1016000us (10584153.5 ops/sec): 169.3 MB/s
Did 1898750 AES-256-GCM (1350 bytes) seal operations in 1015000us (1870689.7 ops/sec): 2525.4 MB/s
Did 374000 AES-256-GCM (8192 bytes) seal operations in 1016000us (368110.2 ops/sec): 3015.6 MB/s
After:
Did 11074000 AES-128-GCM (16 bytes) seal operations in 1015000us (10910344.8 ops/sec): 174.6 MB/s
Did 3178250 AES-128-GCM (1350 bytes) seal operations in 1016000us (3128198.8 ops/sec): 4223.1 MB/s
Did 734500 AES-128-GCM (8192 bytes) seal operations in 1016000us (722933.1 ops/sec): 5922.3 MB/s
Did 10394750 AES-256-GCM (16 bytes) seal operations in 1015000us (10241133.0 ops/sec): 163.9 MB/s
Did 2502250 AES-256-GCM (1350 bytes) seal operations in 1016000us (2462844.5 ops/sec): 3324.8 MB/s
Did 544500 AES-256-GCM (8192 bytes) seal operations in 1015000us (536453.2 ops/sec): 4394.6 MB/s
Change-Id: If058935796441ed4e577b9a72d3aa43422edba58
Reviewed-on: https://boringssl-review.googlesource.com/7273
Reviewed-by: Adam Langley <alangley@gmail.com>
This was removed in a00cafc50c because
none of the assembly actually appeared to need it. However, we found the
assembly the uses it: the MOVBE-based, x86-64 code.
Needing H seems silly since Htable is there, but rather than mess with
the assembly, it's easier to put H back in the structure—now with a
better comment.
Change-Id: Ie038cc4482387264d5e0821664fb41f575826d6f
Reviewed-on: https://boringssl-review.googlesource.com/13122
Reviewed-by: Adam Langley <alangley@gmail.com>
This function is only called twice per ECDH or ECDSA operation, and
it only saves a few scalar multiplications and additions compared to
the alternative, so it doesn't need to be specialized.
As the TODO comment above the callers notes, the two calls can be
reduced to one. Implementing |ecp_nistz256_from_mont| in terms of
|ecp_nistz256_mul_mont| helps show that that change is safe.
This also saves a small amount of code size and improves testing and
verification efficiency.
Note that this is already how the function is implemented for targets
other than x86-64 in OpenSSL.
Change-Id: If1404951f1a787d2618c853afd1f0e99a019e012
Reviewed-on: https://boringssl-review.googlesource.com/13021
Reviewed-by: Adam Langley <alangley@gmail.com>
There is no AVX implementation for x86. Previously on x86 the code
checked to see if AVX and MOVBE are available, and if so, then it
uses the CLMUL implementation. Otherwise it fell back to the same
CLMUL implementation. Thus, there is no reason to check if AVX + MOVBE
are enabled on x86.
Change-Id: Id4983d5d38d6b3269a40e288bca6cc51d2d13966
Reviewed-on: https://boringssl-review.googlesource.com/13024
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
BoringSSL can assume that MMX, SSE, and SSE2 is always supported so
there is no need for a runtime check and there's no need for this
fallback code. Removing the code improves coverage analysis and shrinks
code size.
Change-Id: I782a1bae228f700895ada0bc56687e53cd02b5df
Reviewed-on: https://boringssl-review.googlesource.com/13022
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
This re-applies 3f3358ac15 which was
reverted in c7fe3b9ac5 because the field
operations did not fully-reduce operands. This was fixed in
2f1482706fadf51610a529be216fde0721709e66.
Change-Id: I3913af4b282238dbc21044454324123f961a58af
Reviewed-on: https://boringssl-review.googlesource.com/12227
Reviewed-by: Adam Langley <agl@google.com>
Mercifully, PKCS#12 does not actually make ContentInfo and SafeBag
mutually recursive. The top-level object in a PKCS#12 is a SEQUENCE of
data or encrypted data ContentInfos. Their payloads are a SEQUENCE of
SafeBags (aka SafeContents).
SafeBag is a similar structure to ContentInfo but not identical (it has
attributes in it which we ignore) and actually carries the objects.
There is only recursion if the SafeContents bag type is used, which we
do not process.
This means we don't need to manage recursion depth. This also no longer
allows trailing data after the SEQUENCE and removes the comment about
NSS. The test file still passes, so I'm guessing something else was
going on?
Change-Id: I68e2f8a5cc4b339597429d15dc3588bd39267e0a
Reviewed-on: https://boringssl-review.googlesource.com/13071
Reviewed-by: Adam Langley <agl@google.com>
Resolving the TODO here will be messier than the other implementations
but, to start with, remove this 'pivot element' thing. All that is just
to free some array contents without having to memset the whole thing to
zero.
Change-Id: Ifd6ee0b3815006d4f1f19c9db085cb842671c6dc
Reviewed-on: https://boringssl-review.googlesource.com/13057
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
BN_FLG_CONSTTIME is a ridiculous API and easy to mess up
(CVE-2016-2178). Instead, code that needs a particular algorithm which
preserves secrecy of some arguemnt should call into that algorithm
directly.
This is never set outside the library and is finally unused within the
library! Credit for all this goes almost entirely to Brian Smith. I just
took care of the last bits.
Note there was one BN_FLG_CONSTTIME check that was still reachable, the
BN_mod_inverse in RSA key generation. However, it used the same code in
both cases for even moduli and φ(n) is even if n is not a power of two.
Traditionally, RSA keys are not powers of two, even though it would make
the modular reductions a lot easier.
When reviewing, check that I didn't remove a BN_FLG_CONSTTIME that led
to a BN_mod_exp(_mont) or BN_mod_inverse call (with the exception of the
RSA one mentioned above). They should all go to functions for the
algorithms themselves like BN_mod_exp_mont_consttime.
This CL shows the checks are a no-op for all our tests:
https://boringssl-review.googlesource.com/c/12927/
BUG=125
Change-Id: I19cbb375cc75aac202bd76b51ca098841d84f337
Reviewed-on: https://boringssl-review.googlesource.com/12926
Reviewed-by: Adam Langley <alangley@gmail.com>
Avoid the X509_ALGOR dependency entirely. The public API is still using
the legacy ASN.1 structures for now, but the conversions are lifted to
the API boundary. Once we resolve that and the OID table dependency,
this module will no longer block unshipping crypto/asn1 and friends from
Chromium.
This changes the calling convention around the two kinds of PBE suites
we support. Each PBE suite provides a free-form encrypt_init function to
setup an EVP_CIPHER_CTX and write the AlgorithmIdentifer to a CBB. It
then provides a common decrypt_init function which sets up an
EVP_CIPHER_CTX given a CBS of the parameter. The common encrypt code
determines how to call which encrypt_init function. The common decrypt
code parses the OID out of the AlgorithmIdentifer and then dispatches to
decrypt_init.
Note this means the encryption codepath no longer involves parsing back
out a AlgorithmIdentifier it just serialized. We don't have a good story
to access an already serialized piece of a CBB in progress (reallocs can
invalidate the pointer in a CBS), so it's easier to cut this step out
entirely.
Also note this renames the "PBES1" schemes from PKCS#5 to PKCS#12. This
makes it easier to get at the PKCS#12 key derivation hooks. Although
PKCS#12 claims these are variants of PKCS#5's PBES1, they're not very
related. PKCS#12 swaps out the key derivation and even defines its own
AlgorithmIdentifier parameter structure (identical to the PKCS#5 PBES1
one). The only thing of PBES1 that survives is the CBC mode padding
scheme, which is deep in EVP_CIPHER for us. (Of course, all this musing
on layering is moot because we don't implement non-PKCS#12 PBES1 schemes
anyway.)
This also moves some of the random API features (default iteration
count, default salt generation) out of the PBE suites and into the
common code.
BUG=54
Change-Id: Ie96924c73a229be2915be98eab680cadd17326db
Reviewed-on: https://boringssl-review.googlesource.com/13069
Reviewed-by: Adam Langley <alangley@gmail.com>
libcrypto can now be split in two, with everything that depends on
crypto/asn1 in a separate library. That said, Chromium still needs
crypto/pkcs8 to be implemented with CBS/CBB first. (Also libssl and
anything which uses X509* directly.)
BUG=54
Change-Id: Iec976ae637209882408457e94a1eb2465bce8d56
Reviewed-on: https://boringssl-review.googlesource.com/13059
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
They're not called externally. Unexporting these will make it easier to
rewrite the PKCS{5,8,12} code to use CBS/CBB rather than X509_ALGOR.
Getting rid of those callers in Chromium probably won't happen for a
while since it's in our on-disk formats. (And a unit test for some NSS
client cert glue uses it.)
BUG=54
Change-Id: Id4148a2ad567484782a6e0322b68dde0619159fc
Reviewed-on: https://boringssl-review.googlesource.com/13062
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
The motiviation is that M2Crypto passes an ASN1_GENERALIZEDTIME to
this function. This is not distinct from ASN1_UTCTIME (both are
asn1_string_st), but ASN1_GENERALIZEDTIME uses a 4-digit year in its
string representation, whereas ASN1_UTCTIME uses a 2-digit year.
ASN1_UTCTIME_print previously did not return an error on such inputs.
So, stricten (?) the function, ensuring that it checks for trailing
data, and rejects values that are invalid for their place. Along the
way, clean it up and add tests.
Change-Id: Ia8298bed573f2acfdab96638ea69c78b5bba4e4b
Reviewed-on: https://boringssl-review.googlesource.com/13082
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
08b65f4e31 introduced a memory leak and
also got enums confused. Also fix a codepath that was missing an error
code.
Thanks to OSS-Fuzz which appears to have found it in a matter of hours.
Change-Id: Ia9e926c28a01daab3e6154d363d0acda91209a22
Reviewed-on: https://boringssl-review.googlesource.com/13104
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Towards an eventual goal of opaquifying BoringSSL structs, we want
our consumers -- in this case, Android's libcore -- to not directly
manipulate BigNums; and it would be convenient for them if we would
perform the appropriate gymnastics to interpret little-endian byte
streams.
It also seems a priori a bit strange to have only big-endian varieties
of BN byte-conversions.
This CL provides little-endian equivalents of BN_bn2bin_padded
and BN_bin2bn.
BUG=97
Change-Id: I0e92483286def86d9bd71a46d6a967a3be50f80b
Reviewed-on: https://boringssl-review.googlesource.com/12641
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This is a remnant of signature EVP_MDs. Detach them from
EVP_get_digestby{nid,obj}. Nothing appears to rely on this for those two
functions. Alas, Node.js appears to rely on it for EVP_get_digestbyname,
so keep that working.
This avoids causing every consumer's parsing to be unintentionally lax.
It also means fewer OIDs to transcribe when detaching the last of
libcrypto from the legacy ASN.1 stack and its giant OID table.
Note this is an externally visible change. There was one consumer I had
to fix, but otherwise everything handled things incorrectly due to this
quirk, so it seemed better to just fix the API rather than fork off a
second set.
Change-Id: I705e073bc05d946e71cd1c38acfa5e3c6b0a22b4
Reviewed-on: https://boringssl-review.googlesource.com/13058
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Instead, use BN_mod_exp_mont_consttime of p - 2. This removes two more
call sites sensitive to BN_FLG_CONSTTIME. We're down to just that last
BN_mod_inverse modulo φ(n). (Sort of. It's actually not sensitive
because even mod inverses always hit the other codepath. Perhaps we
should just leave it alone.)
Note this comes with a slight behavior change. The BN_MONT_CTXs are
initialized a little earlier. If a caller calls RSA_generate_* and then
reaches into the struct to scrap all the fields on it, they'll get
confused. Before, they had to perform an operation on it to get
confused. This is a completely ridiculous thing to do.
Since we do this a lot, this introduces some convenience functions for
doing the Fermat's Little Theorem mod inverse and fixes a leak in the
DSA code should computing kinv hit a malloc error.
BUG=125
Change-Id: Iafcae2fc6fd379d161f015c90ff7050e2282e905
Reviewed-on: https://boringssl-review.googlesource.com/12925
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Adam Langley