| Commit message (Collapse) | Author | Age | Files | Lines |
| |
|
| |
|
|
|
|
|
|
|
| |
A large mechanical diff led to sloppy review and gave coverity an
opportunity to be right for once. First time in a good many weeks.
same diff/ok jsing
|
|
|
|
|
|
|
|
|
|
|
| |
The EC API allows callers to optionally pass in a BN_CTX, which means that
any code needing a BN_CTX has to check if one was provided, allocate one if
not, then free it again. Rather than doing this dance throughout the EC
code, handle the BN_CTX existance at the EC API boundary. This means that
lower level implementation code can simply assume that the BN_CTX is
available.
ok tb@
|
|
|
|
| |
ok jsing
|
| |
|
|
|
|
|
|
|
|
|
|
| |
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@
|
|
|
|
|
|
|
| |
BN_clear_free() is a wrapper that calls BN_free() - call BN_free() directly
instead.
ok tb@
|
|
|
|
|
|
|
|
|
|
| |
Unlike in the affine/compressed/... cases, when setting projective
coordinates of an elliptic curve point, there is no check whether
the point is actually on the curve.
Pointed out by Guido Vranken
ok beck miod
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Libcrypto currently has a mess of *_lcl.h, *_locl.h, and *_local.h names
used for internal headers. Move all these headers we inherited from
OpenSSL to *_local.h, reserving the name *_internal.h for our own code.
Similarly, move dtls_locl.h and ssl_locl.h to dtls_local and ssl_local.h.
constant_time_locl.h is moved to constant_time.h since it's special.
Adjust all .c files in libcrypto, libssl and regress.
The diff is mechanical with the exception of tls13_quic.c, where
#include <ssl_locl.h> was fixed manually.
discussed with jsing,
no objection bcook
|
|
|
|
|
|
| |
Dealing with elliptic curves makes some people think that it would be kind
of neat to multiply types with variable names. Sometimes. Only in function
definitions.
|
|
|
|
|
|
|
|
|
|
|
|
| |
If a private key encoded with EC parameters happens to have
order 1 and is used for ECDSA signatures, this causes an
infinite loop since a random integer x in the interval [0,1)
will be 0, so do ... while (x == 0); will loop indefinitely.
Found and reported with a reproducer by Hanno Boeck.
Helpful comments and analysis from David Benjamin.
ok beck jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Instead of bounding only bounding the group order, also bound the
cofactor using Hasse's theorem. This could probably be made a lot
tighter since all curves of cryptographic interest have small
cofactors, but for now this is good enough.
A timeout found by oss-fuzz creates a "group" with insane parameters
over a 40-bit field: the order is 14464, and the cofactor has 4196223
bits (which is obviously impossible by Hasse's theorem). These led to
running an expensive loop in ec_GFp_simple_mul_ct() millions of times.
Fixes oss-fuzz #46056
Diagnosed and fix joint with jsing
ok inoguchi jsing (previous version)
|
|
|
|
|
|
|
| |
The cofactor we tried to calculate should only be zeroed if we failed
to compute it.
ok inoguchi jsing
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The pre-OpenSSL 1.1.0 default was to use explicit curve parameter
encoding. Most applications want to use named curve parameter encoding
and have to opt into this explicitly.
Stephen Henson changed this default in OpenSSL commit 86f300d3 6 years
ago and provided a new OPENSSL_EC_EXPLICIT_CURVE define to opt back into
the old default. According to Debian's codesearch, no application
currently does this, which indicates that we currently have a bad default.
In the future it is more likely that applications expect the new
default, so we follow OpenSSL to avoid problems.
Prompted by schwarze who noted that OPENSSL_EC_EXPLICIT_CURVE is missing.
ok beck inoguchi jsing
|
|
|
|
| |
ok jsing
|
|
|
|
| |
ok jsing
|
|
|
|
| |
ok jsing
|
|
|
|
| |
ok jsing
|
|
|
|
|
|
| |
Similar to part of OpenSSL commit 8e3cced75fb5fee5da59ebef9605d403a999391b
ok jsing
|
|
|
|
| |
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
There are numerous functions in ec/ that exist with _GF2m and _GFp
variants for no good reason. The code of both variants is the same.
The EC_METHODs contain a pointer to the appropriate version. This
commit hides the _GF2m and _GFp variants from internal use and
provides versions that work for both curve types. These will be made
public in an upcoming library bump.
Similar to part of OpenSSL commit 8e3cced75fb5fee5da59ebef9605d403a999391b
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Bad API design makes it possible to set an EC_KEY public key to
a point not on the curve. As a consequence, it was possible to
have bogus ECDSA signatures validated. In practice, all software
uses either EC_POINT_oct2point*() to unmarshal public keys or
issues a call to EC_KEY_check_key() after setting it. This way,
a point on curve check is performed and the problem is mitigated.
In OpenSSL commit 1e2012b7ff4a5f12273446b281775faa5c8a1858, Emilia
Kasper moved the point-on-curve check from EC_POINT_oct2point to
EC_POINT_set_affine_coordinates_*, which results in more checking.
In addition to this commit, we also check in the currently unused
codepath of a user set callback for setting compressed coordinates,
just in case this will be used at some point in the future.
The documentation of EC_KEY_check_key() is very vague on what it
checks and when checks are needed. It could certainly be improved
a lot. It's also strange that EC_KEY_set_key() performs no checks,
while EC_KEY_set_public_key_affine_coordinates() implicitly calls
EC_KEY_check_key().
It's a mess.
Issue found and reported by Guido Vranken who also tested an earlier
version of this fix.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
try to compute it using Hasse's bound. This works as long as the
cofactor is small enough.
Port of Brumley's fix for CVE-2019-1547 in OpenSSL 1.1.1 (old license)
tests & ok inoguchi
input & ok jsing
commit 30c22fa8b1d840036b8e203585738df62a03cec8
Author: Billy Brumley <bbrumley@gmail.com>
Date: Thu Sep 5 21:25:37 2019 +0300
[crypto/ec] for ECC parameters with NULL or zero cofactor, compute it
The cofactor argument to EC_GROUP_set_generator is optional, and SCA
mitigations for ECC currently use it. So the library currently falls
back to very old SCA-vulnerable code if the cofactor is not present.
This PR allows EC_GROUP_set_generator to compute the cofactor for all
curves of cryptographic interest. Steering scalar multiplication to more
SCA-robust code.
This issue affects persisted private keys in explicit parameter form,
where the (optional) cofactor field is zero or absent.
It also affects curves not built-in to the library, but constructed
programatically with explicit parameters, then calling
EC_GROUP_set_generator with a nonsensical value (NULL, zero).
The very old scalar multiplication code is known to be vulnerable to
local uarch attacks, outside of the OpenSSL threat model. New results
suggest the code path is also vulnerable to traditional wall clock
timing attacks.
CVE-2019-1547
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/9781)
|
|
|
|
| |
ok beck jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Based on OpenSSL commit 875ba8b21ecc65ad9a6bdc66971e50
by Billy Brumley, Sohaib ul Hassan and Nicola Tuveri.
ok beck jsing
commit 875ba8b21ecc65ad9a6bdc66971e50461660fcbb
Author: Sohaib ul Hassan <soh.19.hassan@gmail.com>
Date: Sat Jun 16 17:07:40 2018 +0300
Implement coordinate blinding for EC_POINT
This commit implements coordinate blinding, i.e., it randomizes the
representative of an elliptic curve point in its equivalence class, for
prime curves implemented through EC_GFp_simple_method,
EC_GFp_mont_method, and EC_GFp_nist_method.
This commit is derived from the patch
https://marc.info/?l=openssl-dev&m=131194808413635 by Billy Brumley.
Coordinate blinding is a generally useful side-channel countermeasure
and is (mostly) free. The function itself takes a few field
multiplicationss, but is usually only necessary at the beginning of a
scalar multiplication (as implemented in the patch). When used this way,
it makes the values that variables take (i.e., field elements in an
algorithm state) unpredictable.
For instance, this mitigates chosen EC point side-channel attacks for
settings such as ECDH and EC private key decryption, for the
aforementioned curves.
For EC_METHODs using different coordinate representations this commit
does nothing, but the corresponding coordinate blinding function can be
easily added in the future to extend these changes to such curves.
Co-authored-by: Nicola Tuveri <nic.tuv@gmail.com>
Co-authored-by: Billy Brumley <bbrumley@gmail.com>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6526)
|
|
|
|
|
|
|
| |
from Nicola Tuveri (who spotted the omission of ecp_nist.c from the PR).
discussed with jsing
tested by jsg
|
|
|
|
| |
breakage.
|
|
|
|
|
|
|
|
| |
after the constant time commits various regress tests started failing
on sparc64 ssh t9, libcrypto ec ecdh ecdsa and trying to ssh out
resulted in 'invalid elliptic curve value'
ok tb@
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
the EC module.
From Billy Brumley and his team, via
https://github.com/libressl-portable/openbsd/pull/94
With tweaks from jsing and me.
ok jsing
|
|
|
|
|
|
|
|
|
|
| |
reduces conditional logic (-218, +82).
MOD_EXP_CTIME_MIN_CACHE_LINE_WIDTH cache alignment calculation bn/bn_exp.c
wasn'tt quite right. Two other tricky bits with ASN1_STRING_FLAG_NDEF and
BN_FLG_STATIC_DATA where the condition cannot be collapsed completely.
Passes regress. ok beck
|
|
|
|
|
|
| |
as was done earlier in libssl. Thanks inoguchi@ for noticing
libssl had more reacharounds into this.
ok jsing@ inoguchi@
|
| |
|
|
|
|
| |
ok krw@
|
|
|
|
|
|
| |
actual function. This removes the last ASN1_dup_of usage from the tree.
Feedback from doug@ and miod@
|
|
|
|
| |
ok miod@
|
|
|
|
| |
pointer for NULL the line above; ok doug@
|
|
|
|
|
|
|
|
| |
After calling BN_CTX_start(), there must be a BN_CTX_end() before
returning. There were missing BN_CTX_end() calls in error paths. One diff
chunk was simply removing redundant code related to this.
ok deraadt@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
There are currently cases where the return from each call is checked,
the return from only the last call is checked and cases where it is not
checked at all (including code in bn, ec and engine).
Checking the last return value is valid as once the function fails it will
continue to return NULL. However, in order to be consistent check each
call with the same idiom. This makes it easy to verify.
Note there are still a handful of cases that do not follow the idiom -
these will be handled separately.
ok beck@ doug@
|
|
|
|
|
|
|
| |
Improves readability, keeps the code smaller so that it is warmer in your
cache.
review & ok deraadt@
|
|
|
|
|
|
|
|
|
| |
an OPENSSL_NO_* define. This avoids relying on something else pulling it
in for us, plus it fixes several cases where the #ifndef OPENSSL_NO_XYZ is
never going to do anything, since OPENSSL_NO_XYZ will never defined, due
to the fact that opensslconf.h has not been included.
This also includes some miscellaneous sorting/tidying of headers.
|
|
|
|
|
|
| |
Also remove unused des_ver.h, which exports some of these strings, but is not installed.
ok miod@ tedu@
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
| |
OPENSSL_foo wrappers. This changes:
OPENSSL_malloc->malloc
OPENSSL_free->free
OPENSSL_relloc->realloc
OPENSSL_freeFunc->free
|
|
|
|
|
| |
before attempting to invoke it; trivial one-liner in OpenSSL RT #2569 ignored
for 2.5 years.
|
| |
|