| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
| |
|
|
|
|
|
| |
This is only testing basic functionality anyway, so 10000 tests are more
than enough.
|
|
|
|
|
|
| |
The runtime is roughly quadratic in N_TESTS. While it only takes 1-2s on
modern machines, this test takes a long time on slow machines. A reduction
of runtime by a factor of ~16 is significant.
|
|
|
|
|
|
|
|
|
| |
This avoids having a slow down when processing test vector files that only
have a single group. Note that the processing of test vector files is in
turn going to be rate limited by the number of concurrent test groups,
which means we do not need variable limits for vectors.
Reduces a Wycheproof regress run down to ~8 seconds on an Apple M1.
|
|
|
|
|
|
|
|
|
|
| |
Rather than sometimes clearing, turn the free functions into ones that
always clear (as we've done elsewhere). Turn the EC_GROUP_clear_free() and
EC_POINT_clear_free() functions into wrappers that call the *_free()
version. Do similar for the EC_METHOD implementations, removing the
group_clear_finish() and point_clear_finish() hooks in the process.
ok tb@
|
|
|
|
|
|
|
| |
Add a basic test coordinator, that allows for Wycheproof test groups to be
run concurrently. This can be further improved (especially for vectors that
have limited test groups), however it already reduces the regress duration
by about half on an Apple M1.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently, if compiled without OPENSSL_BN_ASM_MONT,
EC_GROUP_new_curve_GFp() tries to use EC_GFp_nist_method(), falling back to
EC_GFp_mont_method() if it is not a NIST curve (if OPENSSL_BN_ASM_MONT is
defined we use EC_GFp_mont_method() unconditionally).
Now that we have a reasonable non-assembly Montgomery implementation, the
performance of EC_GFp_nist_method() is either similar or slower than
EC_GFp_mont_method() (the exception being P-521, however if you're using
that you're not doing it for performance reasons anyway).
The EC_GFp_nist_method() uses rather scary BN NIST code (which would
probably already be removed, if not for the BN and EC public APIs), it uses
code paths that are currently less constant time, and there is additional
overhead in checking to see if the curve is actually supported.
Stop trying to use EC_GFp_nist_method() and unconditionally use
EC_GFp_mont_method() in all cases. While here, factor out the common
setup code and call it from both EC_GROUP_new_curve_GFp() and
EC_GROUP_new_curve_GF2m().
ok beck@ tb@
|
|
|
|
|
|
|
| |
This code would need changes to be safe to use concurrently - remove it
since it is somewhat incomplete and needs reworking.
Requested by tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The EC code has an amazing array of function pointer hooks, such that a
method can hook into almost any operation... and then there is the
EC_FLAGS_DEFAULT_OCT flag, which adds a bunch of complex code and #ifdef
so you can avoid setting three of those function pointers!
Remove EC_FLAGS_DEFAULT_OCT, the now unused flags field from EC_METHOD,
along with the various code that was wrapped in EC_FLAGS_DEFAULT_OCT,
setting the three function pointers that need to be set in each of the
EC_METHODs.
ok beck@ tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Rather than calling bn_mul_add_words() twice - once to multiply and once
to reduce - perform the multiplication and reduction in a single pass using
bn_mulw_addw_addw() directly. Also simplify the addition of the resulting
carries, which in turn allows us to avoid zeroing the top half of the
temporary words.
This provides a ~20-25% performance improvement for RSA operations on
aarch64.
ok tb@
|
|
|
|
|
|
|
|
|
| |
Call bn_mulw_addw() rather than doing bn_mulw() follow by bn_addw(). This
simplifies the code slightly, plus on some platforms bn_mulw_addw() can
be optimised (and bn_mulw_addtw() will then benefit from such an
optimisation).
ok tb@
|
|
|
|
|
|
|
| |
BN_clear_free() is a wrapper that calls BN_free() - call BN_free() directly
instead.
ok tb@
|
|
|
|
| |
This should have been included in a previous diff/commit...
|
|
|
|
|
|
|
|
|
|
|
|
| |
The assembly bn_mul_mont() implementations effectively use alloca() to
allocate space for computation (at up to 8x the input size), without
any limitation. This means that sufficiently large inputs lead to the
stack being blown. Prevent this by using the C based implementation
instead.
Thanks to Jiayi Lin <jlin139 at asu dot edu> for reporting this to us.
ok beck@ tb@
|
|
|
|
|
|
|
|
|
|
|
| |
Provide a constant-time-style Montgomery multiplication implementation.
Use this in place of the assembly bn_mul_mont() on platforms that either
do not have an assembly implementation or have not compiled it in.
Also use this as the fallback version for bn_mul_mont(), rather than
falling back to a non-constant time implementation.
ok beck@ tb@
|
|
|
|
|
|
|
|
|
|
| |
Pull out the simplistic implementation (using BN_mul() or BN_sqr()) into a
bn_mod_mul_montgomery_simple() function. Provide bn_mod_mul_montgomery()
with an implementation that changes depending on if the assembly
bn_mul_mont() is available or not. Turn BN_mod_mul_montgomery() and
BN_to_montgomery() into callers of bn_mod_mul_montgomery().
ok beck@ tb@
|
| |
|
| |
|
|
|
|
|
|
| |
This came from bn_asm.c and did not even compile until recently.
ok beck@ tb@
|
| |
|
|
|
|
|
|
|
|
| |
Most of the implemeentation functions for EC_GFp_simple_method() are reused
by other code, hence they cannot be made static. However, this keeps the
pattern consistent.
ok tb@
|
|
|
|
| |
Drop extra parentheses, unwrap some lines, compare pointers against NULL.
|
|
|
|
|
|
|
| |
Move the EC_METHOD to the bottom of the file, which allows implementation
functions to become static. Remove unneeded prototypes.
ok tb@
|
|
|
|
|
|
|
| |
Move the EC_METHOD to the bottom of the file, which allows implementation
functions to become static. Remove unneeded prototypes.
ok tb@
|
|
|
|
|
|
|
| |
Move the EC_METHOD to the bottom of the file, which allows all
implementation functions to become static. Remove unneeded prototypes.
ok tb@
|
| |
|
|
|
|
|
|
|
| |
Use a fang dangled thing (known as a function) to avoid duplicating the
same code in five places.
ok tb@
|
|
|
|
|
|
|
|
| |
These functions are rather similar, so there's no need for the code to
be wildly different. Add a missing NULL check to ndef_prefix_free() since
that will be needed in a subsequent commit.
ok jsing
|
|
|
|
|
|
|
|
|
| |
All the structs are static and we need to reach into them many times.
Having a shorter name is more concise and results in less visual clutter.
It also avoids many overlong lines and we will be able to get rid of some
unfortunate line wrapping down the road.
Discussed with jsing
|
| |
|
| |
|
| |
|
|
|
|
|
|
| |
Loosely based on OpenSSL commit 6692ff77.
Prodded by job
|
|
|
|
|
|
|
|
| |
Flip the logic of NULL checks on out and *out to unindent, use calloc()
instead of malloc() and check on assign. Also drop the newly added len2
again, it isn't needed.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
| |
i2d functions return <= 0 on error, so check for that instead of == 0.
The issue with CMS_SharedInfo_encode() was found by Niels Dossche.
OpenSSL review overlooked that they had turned penclen into a size_t.
In principle the issue with i2d_X509_ALGOR() is purely cosmetic. Why do
a strange check when there is an idiomatic check? Then again this is CMS...
ok jsing
|
|
|
|
|
|
|
|
|
|
| |
RSA_public_decrypt() returns <= 0 on error. Assigning to a size_t and
checking for == 0 is not the right thing to do here. Neither is blindly
turning the check into <= 0...
Found by Niels Dossche
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The i2d API design is: call a function first with a pointer to NULL, get
the length, allocate a buffer, call the function passing the buffer in.
Both calls should be checked since ther are still internal allocations.
At the heart of ASN.1 encoding, this idiom is used and the second call
is assumed to succeed after the length was determined. This is far from
guaranteed. Check that the second call returns the same length and error
otherwise.
ok jsing
|
|
|
|
|
|
| |
This was presumably intended to be OPENSSL_NO_EC_NISTP_64_GCC_128, however
generic code has ended up inside the ifdef (and none of the NISTP code
or prototypes now remain).
|
|
|
|
| |
This code has been deleted, however the prototypes managed to hang around.
|
| |
|
|
|
|
|
| |
These are per-app, so per-file. Most of them already are static, adjust
the rest of them.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
Do not display such ciphers in the usage display and error out if
they are given. As pointed out by Pauli Dale, the current situation
is confusing.
Fixes GH issues #786 and #819
ok jsing
|
|
|
|
|
|
|
|
| |
We have long had expensive checks for DSA domain parameters in
old_dsa_priv_decode(). These were implemented in a more complicated
way than necesary.
ok beck jsing
|
|
|
|
|
|
|
|
| |
This makes sure that the elliptic curve is not completely stupid.
This is conservative enough: the smallest named groups that we support
have an order of 112 bits.
ok beck jsing
|
|
|
|
|
|
|
|
|
|
|
|
| |
ECDSA is essentially the same thing as DSA, except that it is slightly
less stupid. Signing specifies an infinite loop, which is only possible
with arbitrary ECDSA domain parameters. Fortunately, most use of ECDSA
in the wild is based on well-known groups, so it is known a priori that
the loop is not infinite. Still, infinite loops are bad. A retry is
unlikely, 32 retries have a probability of ~2^-8000. So it's pretty
safe to error out.
ok beck jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The DSA standard specifies an infinite loop: if either r or s is zero
in the signature calculation, a new random number k shall be generated
and the whole thing is to be redone. The rationale is that, as the
standard puts it, "[i]t is extremely unlikely that r = 0 or s = 0 if
signatures are generated properly."
The problem is... There is no cheap way to know that the DSA domain
parameters we are handed are actually DSA domain parameters, so even
if all our calculations are carefully done to do all the checks needed,
we cannot know if we generate the signatures properly. For this we would
need to do two primality checks as well as various congruences and
divisibility properties. Doing this easily leads to DoS, so nobody does
it.
Unfortunately, it is relatively easy to generate parameters that pass
all sorts of sanity checks and will always compute s = 0 since g
is nilpotent. Thus, as unlikely as it is, if we are in the mathematical
model, in practice it is very possible to ensure that s = 0.
Read David Benjamin's glorious commit message for more information
https://boringssl-review.googlesource.com/c/boringssl/+/57228
Thanks to Guido Vranken for reporting this issue, also thanks to
Hanno Boeck who apparently found and reported similar problems earlier.
ok beck jsing
|
|
|
|
|
|
| |
Explicitly check against NULL and turn early return into goto err.
ok beck jsing
|
|
|
|
|
|
|
|
| |
We already had some checks on both sides, but they were less precise
and differed between the functions. The code here is messy enough, so
any simplification is helpful...
ok beck jsing
|