Commit graph

10337 commits

Author SHA1 Message Date
Tobias Nießen
4088b92636 Allow EVP_PKEY_get0_RSA for RSA-PSS keys
RSA-PSS keys use the same internal structure as RSA keys but do not
allow accessing it through EVP_PKEY_get0_RSA. This commit changes that
behavior.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10217)

(cherry picked from commit 465a58b117d5a85623f3998d6fbf2fe8712a5604)
2019-10-28 11:11:34 +00:00
Cesar Pereida Garcia
85728d08ae Update control logic for BN_gcd
PR https://github.com/openssl/openssl/pull/10122 introduced changes to
the BN_gcd function and the control logic inside it accessed `g->d[0]`
irrespective of `g->top`.

When BN_add is called, in case the result is zero, `BN_zero` is called.
The latter behaves differently depending on the API compatibility level
flag: normally `g->d[0]` is cleared but in `no-deprecated` builds only
`g->top` is set to zero.

This commit uses bitwise logic to ensure that `g` is treated as zero if
`g->top` is zero, irrespective of `g->d[0]`.

Co-authored-by: Nicola Tuveri <nic.tuv@gmail.com>

(cherry picked from commit 8aca4bfe8213402c80abc06fe25121461f79128d)

Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10232)
2019-10-23 12:16:13 +03:00
Dr. Matthias St. Pierre
7b18d1a53f Move random-related defines from e_os.h to rand_unix.c
Fixes #10049

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10050)

(cherry picked from commit 01036e2afbe116d608be048ed15930fc885ab2a8)
2019-10-19 00:08:07 +02:00
Dr. Matthias St. Pierre
10cb54d75b rand_unix.c: correct include guard comments
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10050)

(cherry picked from commit 2a7e6ed86be20bd472696a3eafe5d20ec9579dab)
2019-10-19 00:08:04 +02:00
Cesar Pereida Garcia
ef59960504 Constant-time GCD function.
This commit replaces the current `BN_gcd` function with a constant-time
GCD implementation.

(cherry picked from commit f3c4adfc7eb13e9eff514039b4c60b457bdba433)

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10122)
2019-10-17 14:31:40 +03:00
Cesar Pereida Garcia
d63332a5f2 Unify BN_rshift design
This commit aims at refactoring the `BN_rshift` by making it a wrapper
around `bn_rshift_fixed_top`, in order to match the current design of
`BN_lshift`, as suggested in the discussion at
https://github.com/openssl/openssl/pull/10122#discussion_r332474277 .

As described in the code, by refactoring this function, `BN_rshift`
provides a constant-time behavior for sufficiently[!] zero-padded inputs
under the following assumptions: `|n < BN_BITS2|` or `|n / BN_BITS2|`
being non-secret.

Notice that `BN_rshift` returns a canonical representation of the
BIGNUM, if a `fixed_top` representation is required, the caller should
call `bn_rshift_fixed_top` instead.

(cherry picked from commit 8eba6de59e2b06f23c214344423a5a618d1c9ffd)

Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10196)
2019-10-17 14:31:28 +03:00
Cesar Pereida Garcia
bf585d60fc [crypto/asn1/x_bignum.c] Explicit test against NULL
As a fixup to https://github.com/openssl/openssl/pull/9779 to better
conform to the project code style guidelines, this commit amends the
original changeset to explicitly test against NULL, i.e. writing

```
if (p != NULL)
```

rather than

```
if (!p)
```

Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9881)
2019-10-16 15:41:17 +03:00
David Benjamin
44301079c8 Document and add macros for additional DSA options
EVP_PKEY_CTRL_DSA_PARAMGEN_Q_BITS and EVP_PKEY_CTRL_DSA_PARAMGEN_MD are only
exposed from EVP_PKEY_CTX_ctrl, which means callers must write more error-prone
code (see also issue #1319). Add the missing wrapper macros and document them.

Reviewed-by: Matt Caswell <matt@openssl.org>

(cherry picked from commit a97faad76a)

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/10094)
2019-10-15 16:04:46 +02:00
Nicola Tuveri
ac8881e160 [ec_asn1.c] Avoid injecting seed when built-in matches
An unintended consequence of https://github.com/openssl/openssl/pull/9808
is that when an explicit parameters curve is matched against one of the
well-known builtin curves we automatically inherit also the associated
seed parameter, even if the input parameters excluded such
parameter.

This later affects the serialization of such parsed keys, causing their
input DER encoding and output DER encoding to differ due to the
additional optional field.

This does not cause problems internally but could affect external
applications, as reported in
https://github.com/openssl/openssl/pull/9811#issuecomment-536153288

This commit fixes the issue by conditionally clearing the seed field if
the original input parameters did not include it.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/10140)

(cherry picked from commit f97a8af2f3f3573f0759693117c9d33d2a63c27e)
2019-10-15 15:19:38 +03:00
Viktor Szakats
4a8392e203 Fix unused goto label gcc warning
On systems with undefined AI_ADDRCONFIG and AI_NUMERICHOST:

x86_64-w64-mingw32-gcc  -I. -Icrypto/include -Iinclude -m64 -Wall -O3 -fno-ident ...
crypto/bio/b_addr.c: In function 'BIO_lookup_ex':
crypto/bio/b_addr.c:699:7: warning: label 'retry' defined but not used [-Wunused-label]
       retry:
       ^~~~~

Regression from: 3f91ede9ae

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/9856)

(cherry picked from commit be66a15cc1a4c3cc68fa854ceea321ca57f96304)
2019-10-10 19:09:28 +02:00
Richard Levitte
61cc715240 Define AESNI_ASM if AESNI assembler is included, and use it
Because we have cases where basic assembler support isn't present, but
AESNI asssembler support is, we need a separate macro that indicates
that, and use it.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/10080)
2019-10-03 15:07:26 +02:00
Dr. Matthias St. Pierre
1a9a56865c rsa: replace magic number '11' by RSA_PKCS1_PADDING_SIZE
Suggested by Matt Hart

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10084)

(cherry picked from commit f1d1903dd3dd1d68a5eae190b8c2a88bfe0a68ac)
2019-10-03 14:38:57 +02:00
Bernd Edlinger
6f3ccda84f Fix iOS simulator build
Fixes #9999

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/10002)
2019-10-03 08:48:17 +02:00
Michael Osipov
9e2747646d Fix long name of some Microsoft objects
CLA: trivial

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/10029)

(cherry picked from commit 648b53b88ea55b4c2f2c8c57d041075731db5f95)
2019-10-03 08:26:27 +10:00
Kurt Roeckx
eee565ec4b Add defines for __NR_getrandom for all Linux architectures
Fixes: #10015

Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
GH: #10044
(cherry picked from commit 4dcb150ea30f9bbfa7946e6b39c30a86aca5ed02)
2019-09-30 22:29:45 +02:00
Paul Yang
f5517d9521 Fix a bundle of mischecks of return values
Several EVP_PKEY_xxxx functions return 0 and a negative value for
indicating errors. Some places call these functions with a zero return
value check only, which misses the check for the negative scenarios.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10055)

(cherry picked from commit 7e3ae24832e0705583b1471febf3dc0eb1cc021f)
2019-09-30 17:19:05 +08:00
Dr. Matthias St. Pierre
fbbfd128c9 Fix header file include guard names
Make the include guards consistent by renaming them systematically according
to the naming conventions below

The public header files (in the 'include/openssl' directory) are not changed
in 1.1.1, because it is a stable release.

For the private header files files, the guard names try to match the path
specified in the include directives, with all letters converted to upper case
and '/' and '.' replaced by '_'. An extra 'OSSL_' is added as prefix.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9681)
2019-09-27 23:58:12 +02:00
Dr. Matthias St. Pierre
b5acbf9148 Reorganize local header files
Apart from public and internal header files, there is a third type called
local header files, which are located next to source files in the source
directory. Currently, they have different suffixes like

  '*_lcl.h', '*_local.h', or '*_int.h'

This commit changes the different suffixes to '*_local.h' uniformly.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9681)
2019-09-27 23:58:06 +02:00
Dr. Matthias St. Pierre
0c994d54af Reorganize private crypto header files
Currently, there are two different directories which contain internal
header files of libcrypto which are meant to be shared internally:

While header files in 'include/internal' are intended to be shared
between libcrypto and libssl, the files in 'crypto/include/internal'
are intended to be shared inside libcrypto only.

To make things complicated, the include search path is set up in such
a way that the directive #include "internal/file.h" could refer to
a file in either of these two directoroes. This makes it necessary
in some cases to add a '_int.h' suffix to some files to resolve this
ambiguity:

  #include "internal/file.h"      # located in 'include/internal'
  #include "internal/file_int.h"  # located in 'crypto/include/internal'

This commit moves the private crypto headers from

  'crypto/include/internal'  to  'include/crypto'

As a result, the include directives become unambiguous

  #include "internal/file.h"       # located in 'include/internal'
  #include "crypto/file.h"         # located in 'include/crypto'

hence the superfluous '_int.h' suffixes can be stripped.

The files 'store_int.h' and 'store.h' need to be treated specially;
they are joined into a single file.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9681)
2019-09-27 23:57:58 +02:00
Kurt Roeckx
c8d66837ad Use the correct maximum indent
Found by OSS-Fuzz

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>

GH: #9959
(cherry picked from commit a6105ef40d65b35818f2b8ae8ca9e57ca6956d1d)
2019-09-21 10:59:13 +02:00
Bernd Edlinger
8dcd574619 Fix building statically without any dso support
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9889)
2019-09-16 18:27:50 +02:00
ManishPatidar1
94ae5d8283 clearing the ecx private key memory
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9830)

(cherry picked from commit 6ef03ea98fac501e6d6e33bac6ad3c92ea074712)
2019-09-16 14:40:26 +01:00
Bernd Edlinger
515c728dba Fix potential memory leaks with BN_to_ASN1_INTEGER
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9833)

(cherry picked from commit f28bc7d386b25fb75625d0c62c6b2e6d21de0d09)
2019-09-13 13:23:42 +02:00
Tomas Mraz
86ed78676c BIO_f_zlib: Properly handle BIO_CTRL_PENDING and BIO_CTRL_WPENDING calls.
There can be data to write in output buffer and data to read that were
not yet read in the input stream.

Fixes #9866

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9877)

(cherry picked from commit 6beb8b39ba8e4cb005c1fcd2586ba19e17f04b95)
2019-09-12 16:38:46 +02:00
Dr. Matthias St. Pierre
a44e1b2f20 crypto/threads_win.c: fix preprocessor indentation
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9860)
2019-09-11 10:40:18 +02:00
Dr. Matthias St. Pierre
c3656cc594 crypto/threads_none.c: fix syntax error in openssl_get_fork_id()
Fixes #9858

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9860)
2019-09-11 10:32:03 +02:00
Matt Caswell
1cb7eff45b Update copyright year
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9847)
2019-09-10 13:56:40 +01:00
Bernd Edlinger
08229ad838 Fix a padding oracle in PKCS7_dataDecode and CMS_decrypt_set1_pkey
An attack is simple, if the first CMS_recipientInfo is valid but the
second CMS_recipientInfo is chosen ciphertext. If the second
recipientInfo decodes to PKCS #1 v1.5 form plaintext, the correct
encryption key will be replaced by garbage, and the message cannot be
decoded, but if the RSA decryption fails, the correct encryption key is
used and the recipient will not notice the attack.

As a work around for this potential attack the length of the decrypted
key must be equal to the cipher default key length, in case the
certifiate is not given and all recipientInfo are tried out.

The old behaviour can be re-enabled in the CMS code by setting the
CMS_DEBUG_DECRYPT flag.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9777)

(cherry picked from commit 5840ed0cd1e6487d247efbc1a04136a41d7b3a37)
2019-09-10 11:41:20 +01:00
Bernd Edlinger
79f5e2f4b9 Fix a potential crash in rand_unix.c
Due to the dynamic allocation that was added to rand_pool_add_begin
this function could now return a null pointer where it was previously
guaranteed to succeed. But the return value of this function does
not need to be checked by design.

Move rand_pool_grow from rand_pool_add_begin to rand_pool_bytes_needed.
Make an allocation error persistent to avoid falling back to less secure
or blocking entropy sources.

Fixes: a6a66e4511 ("Make rand_pool buffers more dynamic in their sizing.")

Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9687)

(cherry picked from commit fa3eb248e29ca8031e6a14e8a2c6f3cd58b5450e)
2019-09-10 10:03:04 +01:00
Bernd Edlinger
1d36536457 Fix a strict warnings error in rand_pool_acquire_entropy
There was a warning about unused variables in this config:
./config --strict-warnings --with-rand-seed=rdcpu

Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9687)

(cherry picked from commit e301c147a763f67dcc5ba63eb7e2ae40d83a68aa)
2019-09-10 10:03:04 +01:00
Dr. Matthias St. Pierre
63180182ec drbg: fix issue where DRBG_CTR fails if NO_DF is used (2nd attempt)
Since commit 7c226dfc43 a chained DRBG does not add additional
data anymore when reseeding from its parent. The reason is that
the size of the additional data exceeded the allowed size when
no derivation function was used.

This commit provides an alternative fix: instead of adding the
entire DRBG's complete state, we just add the DRBG's address
in memory, thereby providing some distinction between the different
DRBG instances.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9802)
2019-09-09 17:09:06 +01:00
Dr. Matthias St. Pierre
5520695c73 drbg: add fork id to additional data on UNIX systems
Provides a little extra fork-safety on UNIX systems, adding to the
fact that all DRBGs reseed automatically when the fork_id changes.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9802)
2019-09-09 17:09:06 +01:00
Dr. Matthias St. Pierre
1b0fe00e27 drbg: ensure fork-safety without using a pthread_atfork handler
When the new OpenSSL CSPRNG was introduced in version 1.1.1,
it was announced in the release notes that it would be fork-safe,
which the old CSPRNG hadn't been.

The fork-safety was implemented using a fork count, which was
incremented by a pthread_atfork handler. Initially, this handler
was enabled by default. Unfortunately, the default behaviour
had to be changed for other reasons in commit b5319bdbd0, so
the new OpenSSL CSPRNG failed to keep its promise.

This commit restores the fork-safety using a different approach.
It replaces the fork count by a fork id, which coincides with
the process id on UNIX-like operating systems and is zero on other
operating systems. It is used to detect when an automatic reseed
after a fork is necessary.

To prevent a future regression, it also adds a test to verify that
the child reseeds after fork.

CVE-2019-1549

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9802)
2019-09-09 17:09:06 +01:00
Bernd Edlinger
827eab4cd7 Fix build with VS2008
crypto/rand/rand_win.c(70) : error C2065: 'BCRYPT_USE_SYSTEM_PREFERRED_RNG' : undeclared identifier

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9827)

(cherry picked from commit d3a1128bc25ec8bf835c81821e1be68fba39ab4b)
2019-09-09 16:45:22 +02:00
Bernd Edlinger
4bf9781adc Use BN_clear_free in DH_set0_key
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9796)

(cherry picked from commit fa01370f7dc8f0a379483bbe74de11225857e5fe)
2019-09-09 14:47:08 +02:00
Bernd Edlinger
8003138fa9 DH_check_pub_key_ex was accidentally calling DH_check,
so results were undefined.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9796)

(cherry picked from commit 2b95e8efcf8b99892106070d9ac745a0a369f503)
2019-09-09 14:46:30 +02:00
Bernd Edlinger
1f9dc86b55 Change DH_generate_parameters back to order 2q subgroup
For for G=2 and 5 DH_generate_parameters will continue to generate
the order 2q subgroup for compatibility with previous versions.

For G=3 DH_generate_parameters generates an order q subgroup, but it
will not pass the check in DH_check with previous OpenSSL versions.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9820)
2019-09-09 13:58:03 +02:00
Nicola Tuveri
9a43a73380 [ec] Match built-in curves on EC_GROUP_new_from_ecparameters
Description
-----------

Upon `EC_GROUP_new_from_ecparameters()` check if the parameters match any
of the built-in curves. If that is the case, return a new
`EC_GROUP_new_by_curve_name()` object instead of the explicit parameters
`EC_GROUP`.

This affects all users of `EC_GROUP_new_from_ecparameters()`:
- direct calls to `EC_GROUP_new_from_ecparameters()`
- direct calls to `EC_GROUP_new_from_ecpkparameters()` with an explicit
  parameters argument
- ASN.1 parsing of explicit parameters keys (as it eventually
  ends up calling `EC_GROUP_new_from_ecpkparameters()`)

A parsed explicit parameter key will still be marked with the
`OPENSSL_EC_EXPLICIT_CURVE` ASN.1 flag on load, so, unless
programmatically forced otherwise, if the key is eventually serialized
the output will still be encoded with explicit parameters, even if
internally it is treated as a named curve `EC_GROUP`.

Before this change, creating any `EC_GROUP` object using
`EC_GROUP_new_from_ecparameters()`, yielded an object associated with
the default generic `EC_METHOD`, but this was never guaranteed in the
documentation.
After this commit, users of the library that intentionally want to
create an `EC_GROUP` object using a specific `EC_METHOD` can still
explicitly call `EC_GROUP_new(foo_method)` and then manually set the
curve parameters using `EC_GROUP_set_*()`.

Motivation
----------

This has obvious performance benefits for the built-in curves with
specialized `EC_METHOD`s and subtle but important security benefits:
- the specialized methods have better security hardening than the
  generic implementations
- optional fields in the parameter encoding, like the `cofactor`, cannot
  be leveraged by an attacker to force execution of the less secure
  code-paths for single point scalar multiplication
- in general, this leads to reducing the attack surface

Check the manuscript at https://arxiv.org/abs/1909.01785 for an in depth
analysis of the issues related to this commit.

It should be noted that `libssl` does not allow to negotiate explicit
parameters (as per RFC 8422), so it is not directly affected by the
consequences of using explicit parameters that this commit fixes.
On the other hand, we detected external applications and users in the
wild that use explicit parameters by default (and sometimes using 0 as
the cofactor value, which is technically not a valid value per the
specification, but is tolerated by parsers for wider compatibility given
that the field is optional).
These external users of `libcrypto` are exposed to these vulnerabilities
and their security will benefit from this commit.

Related commits
---------------

While this commit is beneficial for users using built-in curves and
explicit parameters encoding for serialized keys, commit
b783beeadf6b80bc431e6f3230b5d5585c87ef87 (and its equivalents for the
1.0.2, 1.1.0 and 1.1.1 stable branches) fixes the consequences of the
invalid cofactor values more in general also for other curves
(CVE-2019-1547).

The following list covers commits in `master` that are related to the
vulnerabilities presented in the manuscript motivating this commit:

- d2baf88c43 [crypto/rsa] Set the constant-time flag in multi-prime RSA too
- 311e903d84 [crypto/asn1] Fix multiple SCA vulnerabilities during RSA key validation.
- b783beeadf [crypto/ec] for ECC parameters with NULL or zero cofactor, compute it
- 724339ff44 Fix SCA vulnerability when using PVK and MSBLOB key formats

Note that the PRs that contributed the listed commits also include other
commits providing related testing and documentation, in addition to
links to PRs and commits backporting the fixes to the 1.0.2, 1.1.0 and
1.1.1 branches.

This commit includes a partial backport of
https://github.com/openssl/openssl/pull/8555
(commit 8402cd5f75)
for which the main author is Shane Lontis.

Responsible Disclosure
----------------------

This and the other issues presented in https://arxiv.org/abs/1909.01785
were reported by Cesar Pereida García, Sohaib ul Hassan, Nicola Tuveri,
Iaroslav Gridin, Alejandro Cabrera Aldaya and Billy Bob Brumley from the
NISEC group at Tampere University, FINLAND.

The OpenSSL Security Team evaluated the security risk for this
vulnerability as low, and encouraged to propose fixes using public Pull
Requests.

_______________________________________________________________________________

Co-authored-by: Shane Lontis <shane.lontis@oracle.com>

(Backport from https://github.com/openssl/openssl/pull/9808)

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9809)
2019-09-09 14:16:18 +03:00
Bernd Edlinger
87bea6550a Remove x86/x86_64 BSAES and AES_ASM support
This leaves VPAES and AESNI support.
The VPAES performance is comparable but BSAES is not
completely constant time. There are table lookups
using secret key data in AES_set_encrypt/decrypt_key
and in ctr mode short data uses the non-constant
time AES_encrypt function instead of bit-slicing.
Furthermore the AES_ASM is by far outperformed
by recent GCC versions.
Since BSAES calls back to AES_ASM for short
data blocks the performance on those is also
worse than the pure software implementaion.

Fixes: #9640

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9675)
2019-09-07 10:26:48 +02:00
Billy Brumley
30c22fa8b1 [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)
2019-09-07 03:57:52 +03:00
Nicola Tuveri
ed0ac11950 [ec/ecp_nistp*.c] restyle: use {} around else too
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9511)

(cherry picked from commit 4fe2ee3a449a8ca2886584e221f34ff0ef5de119)
2019-09-07 02:21:13 +03:00
Nicola Tuveri
61387fd3a5 [ec/ecp_nistp*.c] remove flip_endian()
Replace flip_endian() by using the little endian specific
BN_bn2lebinpad() and BN_lebin2bn().

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9511)

(cherry picked from commit e0b660c27d8d97b4ad9e2098cc957de26872c0ef)
2019-09-07 02:21:09 +03:00
Nicola Tuveri
2432e1291d Uniform BN_bn2binpad() and BN_bn2lebinpad() implementations
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9511)

(cherry picked from commit 1b338abe3abb8c73f004c34d4b8a9272b89dfd5d)
2019-09-07 02:21:03 +03:00
Nicola Tuveri
b9a380f78c Make BN_num_bits() consttime upon BN_FLG_CONSTTIME
This issue was partially addressed by commit
972c87dfc7, which hardened its callee
BN_num_bits_word() to avoid leaking the most-significant word of its
argument via branching and memory access pattern.
The commit message also reported:
> 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.

BN_num_bits() is called directly or indirectly (e.g., through
BN_num_bytes() or BN_bn2binpad() ) in various parts of the `crypto/ec`
code, notably in all the currently supported implementations of scalar
multiplication (in the generic path through ec_scalar_mul_ladder() as
well as in dedicated methods like ecp_nistp{224,256,521}.c and
ecp_nistz256.c).

Under the right conditions, a motivated SCA attacker could retrieve the
secret bitlength of a secret nonce through this vulnerability,
potentially leading, ultimately, to recover a long-term secret key.

With this commit, exclusively for BIGNUMs that are flagged with
BN_FLG_CONSTTIME, instead of accessing only bn->top, all the limbs of
the BIGNUM are accessed up to bn->dmax and bitwise masking is used to
avoid branching.

Memory access pattern still leaks bn->dmax, the size of the lazily
allocated buffer for representing the BIGNUM, which is inevitable with
the current BIGNUM architecture: reading past bn->dmax would be an
out-of-bound read.
As such, it's the caller responsibility to ensure that bn->dmax does not
leak secret information, by explicitly expanding the internal BIGNUM
buffer to a public value sufficient to avoid any lazy reallocation
while manipulating it: this should be already done at the top level
alongside setting the BN_FLG_CONSTTIME.

Thanks to David Schrammel and Samuel Weiser for reporting this issue
through responsible disclosure.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9511)

(cherry picked from commit 8b44198b916015f77bef1befa26edb48ad8a0238)
2019-09-07 02:20:59 +03:00
Nicola Tuveri
083f297a48 Fix a SCA leak using BN_bn2bin()
BN_bn2bin() is not constant-time and leaks the number of bits in the
processed BIGNUM.

The specialized methods in ecp_nistp224.c, ecp_nistp256.c and
ecp_nistp521.c internally used BN_bn2bin() to convert scalars into the
internal fixed length representation.

This can leak during ECDSA/ECDH key generation or handling the nonce
while generating an ECDSA signature, when using these implementations.
The amount and risk of leaked information useful for a SCA attack
varies for each of the three curves, as it depends mainly on the
ratio between the bitlength of the curve subgroup order (governing the
size of the secret nonce/key) and the limb size for the internal BIGNUM
representation (which depends on the compilation target architecture).

To fix this, we replace BN_bn2bin() with BN_bn2binpad(), bounding the
output length to the width of the internal representation buffer: this
length is public.

Internally the final implementation of both BN_bn2binpad() and
BN_bn2bin() already has masking in place to avoid leaking bn->top
through memory access patterns.
Memory access pattern still leaks bn->dmax, the size of the lazily
allocated buffer for representing the BIGNUM, which is inevitable with
the current BIGNUM architecture: reading past bn->dmax would be an
out-of-bound read.
As such, it's the caller responsibility to ensure that bn->dmax does not
leak secret information, by explicitly expanding the internal BIGNUM
buffer to a public value sufficient to avoid any lazy reallocation
while manipulating it: this is already done at the top level alongside
setting the BN_FLG_CONSTTIME.

Finally, the internal implementation of BN_bn2binpad() indirectly calls
BN_num_bits() via BN_num_bytes(): the current implementation of
BN_num_bits() can leak information to a SCA attacker, and is addressed
in the next commit.

Thanks to David Schrammel and Samuel Weiser for reporting this issue
through responsible disclosure.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9511)

(cherry picked from commit 805315d3a20f7274195eed75b06c391dacf3b197)
2019-09-07 02:20:43 +03:00
Bernd Edlinger
9e1403d91a Fix a SCA leak in BN_generate_dsa_nonce
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/9782)

(cherry picked from commit 31ca19403d56ad71d823cf62990518dfc6905bb4)
2019-09-06 18:41:12 +02:00
Cesar Pereida Garcia
1bb2acb998 [crypto/rsa] Set the constant-time flag in multi-prime RSA too
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9779)
2019-09-06 16:15:55 +01:00
Cesar Pereida Garcia
2f18596c32 [crypto/asn1] Fix multiple SCA vulnerabilities during RSA key validation.
This commit addresses multiple side-channel vulnerabilities present
during RSA key validation.
Private key parameters are re-computed using variable-time functions.

This issue was discovered and reported by the NISEC group at TAU Finland.

Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9779)
2019-09-06 16:15:55 +01:00
Bernd Edlinger
5d16346679 Fix error handling in x509_lu.c
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9639)

(cherry picked from commit c70e2ec33943d3bd46d3d9950f774307feda832b)
2019-09-05 08:40:24 +02:00
Bernd Edlinger
ce1ab24163 Cleanup includes in rand_unix.c
Fixes #9757

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9761)

(cherry picked from commit 41ffd2ab09d24692c71850ccd7d5ff154196fe01)
2019-09-05 08:33:48 +02:00