848113a30b added mitigation for a
side-channel attack. This commit extends approach to all code
paths for consistency.
[It also removes redundant white spaces introduced in last commit.]
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6480)
This module is used only with odd input lengths, i.e. not used in normal
PKI cases, on contemporary processors. The problem was "illuminated" by
fuzzing tests.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6440)
The One&Done attack, which is described in a paper to appear in the
USENIX Security'18 conference, uses EM emanations to recover the values
of the bits that are obtained using BN_is_bit_set while constructing
the value of the window in BN_mod_exp_consttime. The EM signal changes
slightly depending on the value of the bit, and since the lookup of a
bit is surrounded by highly regular execution (constant-time Montgomery
multiplications) the attack is able to isolate the (very brief) part of
the signal that changes depending on the bit. Although the change is
slight, the attack recovers it successfully >90% of the time on several
phones and IoT devices (all with ARM processors with clock rates around
1GHz), so after only one RSA decryption more than 90% of the bits in
d_p and d_q are recovered correctly, which enables rapid recovery of
the full RSA key using an algorithm (also described in the paper) that
modifies the branch-and-prune approach for a situation in which the
exponents' bits are recovered with errors, i.e. where we do not know
a priori which bits are correctly recovered.
The mitigation for the attack is relatively simple - all the bits of
the window are obtained at once, along with other bits so that an
entire integer's worth of bits are obtained together using masking and
shifts, without unnecessarily considering each bit in isolation. This
improves performance somewhat (one call to bn_get_bits is faster than
several calls to BN_is_bit_set), so the attacker now gets one signal
snippet per window (rather than one per bit) in which the signal is
affected by all bits in the integer (rather than just the one bit).
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6276)
Experiments have shown that the lookup table used by BN_GF2m_mod_arr
introduces sufficient timing signal to recover the private key for an
attacker with access to cache timing information on the victim's host.
This only affects binary curves (which are less frequently used).
No CVE is considered necessary for this issue.
The fix is to replace the lookup table with an on-the-fly calculation of
the value from the table instead, which can be performed in constant time.
Thanks to Youngjoo Shin for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6270)
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6070)
Calculating BN_mod_inverse where n is 1 (or -1) doesn't make sense. We
should return an error in that case. Instead we were returning a valid
result with value 0.
Fixes#6004
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6119)
Montgomery multiplication post-conditions in some of code paths were
formally non-constant time. Cache access pattern was result-neutral,
but a little bit asymmetric, which might have produced a signal [if
processor reordered load and stores at run-time].
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6141)
Co-authored-by: Nicola Tuveri <nic.tuv@gmail.com>
Co-authored-by: Cesar Pereida Garcia <cesar.pereidagarcia@tut.fi>
Co-authored-by: Sohaib ul Hassan <soh.19.hassan@gmail.com>
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6009)
Old code replaced in favor of a clearer implementation.
Performances are not penalized.
Updated the copyright end date to 2018.
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5963)
This removes some code because we cannot trace the original contributor
to get their agreement for the licence change (original commit e03ddfae).
After this change there will be numerous failures in the test cases until
someone rewrites the missing code.
All *_free functions should accept a NULL parameter. After this change
the following *_free functions will fail if a NULL parameter is passed:
BIO_ACCEPT_free()
BIO_CONNECT_free()
BN_BLINDING_free()
BN_CTX_free()
BN_MONT_CTX_free()
BN_RECP_CTX_free()
BUF_MEM_free()
COMP_CTX_free()
ERR_STATE_free()
TXT_DB_free()
X509_STORE_free()
ssl3_free()
ssl_cert_free()
SSL_SESSION_free()
SSL_free()
[skip ci]
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/5757)
The make variables LIB_CFLAGS, DSO_CFLAGS and so on were used in
addition to CFLAGS and so on. This works without problem on Unix and
Windows, where options with different purposes (such as -D and -I) can
appear anywhere on the command line and get accumulated as they come.
This is not necessarely so on VMS. For example, macros must all be
collected and given through one /DEFINE, and the same goes for
inclusion directories (/INCLUDE).
So, to harmonize all platforms, we repurpose make variables starting
with LIB_, DSO_ and BIN_ to be all encompassing variables that
collects the corresponding values from CFLAGS, CPPFLAGS, DEFINES,
INCLUDES and so on together with possible config target values
specific for libraries DSOs and programs, and use them instead of the
general ones everywhere.
This will, for example, allow VMS to use the exact same generators for
generated files that go through cpp as all other platforms, something
that has been impossible to do safely before now.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5357)
Output copyright year depends on any input file(s) and the script.
This is not perfect, but better than what we had.
Also run 'make update'
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5350)
BN_from_montgomery_word doesn't have a constant memory access pattern.
Replace the pointer trick with a constant-time select. There is, of
course, still the bn_correct_top leak pervasive in BIGNUM itself.
See also https://boringssl-review.googlesource.com/22904 from BoringSSL.
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/5228)
The exponent here is one of d, dmp1, or dmq1 for RSA. This value and its
bit length are both secret. The only public upper bound is the bit width
of the corresponding modulus (RSA n, p, and q, respectively).
Although BN_num_bits is constant-time (sort of; see bn_correct_top notes
in preceding patch), this does not fix the root problem, which is that
the windows are based on the minimal bit width, not the upper bound. We
could use BN_num_bits(m), but BN_mod_exp_mont_consttime is public API
and may be called with larger exponents. Instead, use all top*BN_BITS2
bits in the BIGNUM. This is still sensitive to the long-standing
bn_correct_top leak, but we need to fix that regardless.
This may cause us to do a handful of extra multiplications for RSA keys
which are just above a whole number of words, but that is not a standard
RSA key size.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5154)
(This patch was written by Andy Polyakov. I only wrote the commit
message. Mistakes in the analysis are my fault.)
BN_num_bits, by way of BN_num_bits_word, currently leaks the
most-significant word of its argument via branching and memory access
pattern.
BN_num_bits is called on RSA prime factors in various places. These have
public bit lengths, but all bits beyond the high bit are secret. This
fully resolves those cases.
There are a few places where BN_num_bits is called on an input where the
bit length is also secret. This does *not* fully resolve those cases as
we still only look at the top word. Today, that is guaranteed to be
non-zero, but only because of the long-standing bn_correct_top timing
leak. Once that is fixed, a constant-time BN_num_bits on such inputs
must count bits on each word.
Instead, those cases should not call BN_num_bits at all. In particular,
BN_mod_exp_mont_consttime uses the exponent bit width to pick windows,
but it should be using the maximum bit width. The next patch will fix
this.
Thanks to Dinghao Wu, Danfeng Zhang, Shuai Wang, Pei Wang, and Xiao Liu
for reporting this issue.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5154)
C preprocessor flags get separated from C flags, which has the
advantage that we don't get loads of macro definitions and inclusion
directory specs when linking shared libraries, DSOs and programs.
This is a step to add support for "make variables" when configuring.
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5177)
Commit 9f9442918a changed the semantics of BN_copy() to additionally
copy the BN_FLG_CONSTTIME flag if it is set. This turns out to be
ill advised as it has unintended consequences. For example calling
BN_mod_inverse_no_branch() can sometimes return a result with the flag
set and sometimes not as a result. This can lead to later failures if we
go down code branches that do not support constant time, but check for
the presence of the flag.
The original commit was made due to an issue in BN_MOD_CTX_set(). The
original PR fixed the problem in that function, but it was changed in
review to fix it in BN_copy() instead. The solution seems to be to revert
the BN_copy() change and go back to the originally proposed way.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/5080)
All exponentiation subroutines but BN_mod_exp_mont_consttime produce
non-negative result for negative input, which is confusing for fuzzer.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/4676)
Performance regression was reported for EC key generation between
1.0.2 and 1.1.x [in GH#2891]. It naturally depends on platform,
values between 6 and 9% were observed.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4743)
Around 138 distinct errors found and fixed; thanks!
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3459)
The function BN_security_bits() uses the values from SP800-57 to assign
security bit values for different FF key sizes. However the value for 192
security bits is wrong. SP800-57 has it as 7680 but the code had it as
7690.
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4546)
Since return is inconsistent, I removed unnecessary parentheses and
unified them.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4541)
Names were not removed.
Some comments were updated.
Replace Andy's address with openssl.org
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4516)
Add functions to return DH parameters using NID and to return the
NID if parameters match a named set. Currently this supports only
RFC7919 parameters but could be expanded in future.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4485)
If BN_FLG_STATIC_DATA is set don't cleanse a->d as it will reside
in read only memory. If BN_FLG_MALLOCED is not set don't modify the
BIGNUM at all.
This change applies to BN_clear_free() and BN_free(). Now the BIGNUM
structure is opaque applications cannot create a BIGNUM structure
without BN_FLG_MALLOCED being set so they are unaffected.
Update internal DH routines so they only copy pointers for read only
parameters.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4485)
RSA_setup_blinding() calls BN_BLINDING_create_param() which later calls
BN_mod_exp() as follows:
BN_mod_exp(ret->A, ret->A, ret->e, ret->mod, ctx)
ret->mod will have BN_FLG_CONSTTIME set, but ret->e does not. In
BN_mod_exp() we only test the third param for the existence of this flag.
We should test all the inputs.
Thanks to Samuel Weiser (samuel.weiser@iaik.tugraz.at) for reporting this
issue.
This typically only happens once at key load, so this is unlikely to be
exploitable in any real scenario.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4477)
Removed e_os.h from all bar three headers (apps/apps.h crypto/bio/bio_lcl.h and
ssl/ssl_locl.h).
Added e_os.h into the files that need it now.
Directly reference internal/nelem.h when required.
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4188)
Movely removal of unnecessary brackets but some could be bugs addressed too.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4202)
return true for characters > 127. I.e. they are allowing extended ASCII
characters through which then cause problems. E.g. marking superscript '2' as
a number then causes the common (ch - '0') conversion to number to fail
miserably. Likewise letters with diacritical marks can also cause problems.
If a non-ASCII character set is being used (currently only EBCDIC), it is
adjusted for.
The implementation uses a single table with a bit for each of the defined
classes. These functions accept an int argument and fail for
values out of range or for characters outside of the ASCII set. They will
work for both signed and unsigned character inputs.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4102)
Add a new global DRBG for private keys used by RAND_priv_bytes.
Add BN_priv_rand() and BN_priv_rand_range() which use RAND_priv_bytes().
Change callers to use the appropriate BN_priv... function.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4076)
If RAND_add wraps around, XOR with existing. Add test to drbgtest that
does the wrap-around.
Re-order seeding and stop after first success.
Add RAND_poll_ex()
Use the DF and therefore lower RANDOMNESS_NEEDED. Also, for child DRBG's,
mix in the address as the personalization bits.
Centralize the entropy callbacks, from drbg_lib to rand_lib.
(Conceptually, entropy is part of the enclosing application.)
Thanks to Dr. Matthias St Pierre for the suggestion.
Various code cleanups:
-Make state an enum; inline RANDerr calls.
-Add RAND_POLL_RETRIES (thanks Pauli for the idea)
-Remove most RAND_seed calls from rest of library
-Rename DRBG_CTX to RAND_DRBG, etc.
-Move some code from drbg_lib to drbg_rand; drbg_lib is now only the
implementation of NIST DRBG.
-Remove blocklength
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4019)
[extended tests]
Original text:
Use BUF_strlcpy() instead of strcpy().
Use BUF_strlcat() instead of strcat().
Use BIO_snprintf() instead of sprintf().
In some cases, keep better track of buffer lengths.
This is part of a large change submitted by Markus Friedl <markus@openbsd.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/3701)
And BN_pseudo_rand_range is really BN_rand_range.
Document that we might deprecate those functions.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3743)
To make it consistent in the code base
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/3749)
As well as a coding style nit is fixed.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/3763)
Run perltidy on util/mkerr
Change some mkerr flags, write some doc comments
Make generated tables "const" when genearting lib-internal ones.
Add "state" file for mkerr
Renerate error tables and headers
Rationalize declaration of ERR_load_XXX_strings
Fix out-of-tree build
Add -static; sort flags/vars for options.
Also tweak code output
Moved engines/afalg to engines (from master)
Use -static flag
Standard engine #include's of errors
Don't linewrap err string tables unless necessary
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3392)
The assembler already knows the actual path to the generated file and,
in other perlasm architectures, is left to manage debug symbols itself.
Notably, in OpenSSL 1.1.x's new build system, which allows a separate
build directory, converting .pl to .s as the scripts currently do result
in the wrong paths.
This also avoids inconsistencies from some of the files using $0 and
some passing in the filename.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3431)
- Mostly missing fall thru comments
- And uninitialized value used in sslapitest.c
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3440)
The second BN_is_zero test can never be true.
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3434)
BN_is_prime_fasttest_ex begins by rejecting if a <= 1. Then it goes to
set A := abs(a), but a cannot be negative at this point.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3275)
Previously, BN_is_prime_fasttest_ex, when doing trial-division, would
check whether the candidate is a multiple of a number of small primes
and, if so, reject it. However, three is a multiple of three yet is
still a prime number.
This change accepts small primes as prime when doing trial-division.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3264)
Fix some comments too
[skip ci]
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3069)
This module is used only with odd input lengths, i.e. not used in normal
PKI cases, on contemporary processors. The problem was "illuminated" by
fuzzing tests.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Commit d5aa14dd simplified the bn_expand_internal() and BN_copy() functions.
Unfortunately it also removed some checks which are still required,
otherwise we call memcpy passing in NULL which is not allowed.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2836)
There are a number of symbols in bn which are internal only and never used
by anything. They should be removed.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2766)
This removes the fips configure option. This option is broken as the
required FIPS code is not available.
FIPS_mode() and FIPS_mode_set() are retained for compatibility, but
FIPS_mode() always returns 0, and FIPS_mode_set() can only be used to
turn FIPS mode off.
Reviewed-by: Stephen Henson <steve@openssl.org>
I don't think this actually affects anything since the cfi_restore
directives aren't strictly needed anyway. (The old values are still in
memory so either will do.)
CLA: trivial
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2582)
- harmonize handlers with guidelines and themselves;
- fix some bugs in handlers;
- add missing handlers in chacha and ecp_nistz256 modules;
Reviewed-by: Rich Salz <rsalz@openssl.org>
The BIGNUM behaviour is supposed to be "consistent" when going into and
out of APIs, where "consistent" means 'top' is set minimally and that
'neg' (negative) is not set if the BIGNUM is zero (which is iff 'top' is
zero, due to the previous point).
The BN_DEBUG testing (make test) caught the cases that this patch
corrects.
Note, bn_correct_top() could have been used instead, but that is intended
for where 'top' is expected to (sometimes) require adjustment after direct
word-array manipulation, and so is heavier-weight. Here, we are just
catching the negative-zero case, so we test and correct for that
explicitly, in-place.
Change-Id: Iddefbd3c28a13d935648932beebcc765d5b85ae7
Signed-off-by: Geoff Thorpe <geoff@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1672)
Couple of updates to make this code work properly again;
* use OPENSSL_assert() instead of assert() (and #include <assert.h>)
* the circular-dependency-avoidance uses RAND_bytes() (not pseudo)
Change-Id: Iefb5a9dd73f71fd81c1268495c54a64378955354
Signed-off-by: Geoff Thorpe <geoff@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1672)
