Skip to content

GitLab

and though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here

Switch branch/tag
This project is mirrored from https://github.com/openssl/openssl.git. Pull mirroring failed .
Last successful update .
  1. 20 Dec, 2019 2 commits
  3. 19 Dec, 2019 1 commit
  5. 05 Dec, 2019 1 commit
  7. 04 Dec, 2019 2 commits
  9. 28 Oct, 2019 1 commit
  11. 16 Oct, 2019 1 commit
  13. 15 Oct, 2019 1 commit
  15. 13 Sep, 2019 1 commit
  17. 12 Sep, 2019 1 commit
  19. 10 Sep, 2019 8 commits
  21. 09 Sep, 2019 3 commits
    • Nicola Tuveri's avatar
      [ec] Match built-in curves on EC_GROUP_new_from_ecparameters · 66fd724d
      Nicola Tuveri authored 
      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
b783beea (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:

- d2baf88c [crypto/rsa] Set the constant-time flag in multi-prime RSA too
- 311e903d [crypto/asn1] Fix multiple SCA vulnerabilities during RSA key validation.
- b783beea [crypto/ec] for ECC parameters with NULL or zero cofactor, compute it
- 724339ff 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 8402cd5f)
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: default avatarShane Lontis <shane.lontis@oracle.com>

(Backport from https://github.com/openssl/openssl/pull/9808)
Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9811)
      66fd724d
    • Billy Brumley's avatar
      [crypto/ec] for ECC parameters with NULL or zero cofactor, compute it · 21c856b7
      Billy Brumley authored 
      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: default avatarNicola Tuveri <nic.tuv@gmail.com>
Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9799)
      21c856b7
    • Cesar Pereida Garcia's avatar
      [crypto/rsa] Fix multiple SCA vulnerabilities during RSA key validation. · adaebd81
      Cesar Pereida Garcia authored 
      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: default avatarBernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9785)
      adaebd81
  23. 07 Sep, 2019 1 commit
  25. 06 Sep, 2019 5 commits
    • Nicola Tuveri's avatar
      [ec/ecp_nistp*.c] restyle: use {} around `else` too · 920e37e3
      Nicola Tuveri authored 
      Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
Reviewed-by: default avatarBernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9793)
      920e37e3
    • Nicola Tuveri's avatar
      [ec/ecp_nistp*.c] remove flip_endian() · e3679b15
      Nicola Tuveri authored 
      Replace flip_endian() by using the little endian specific
bn_bn2lebinpad() and bn_lebin2bn().
Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
Reviewed-by: default avatarBernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9793)
      e3679b15
    • Nicola Tuveri's avatar
      Uniform bn_bn2binpad() and bn_bn2lebinpad() implementations · bde4a001
      Nicola Tuveri authored 
      Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
Reviewed-by: default avatarBernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9793)
      bde4a001
    • Nicola Tuveri's avatar
      Make BN_num_bits() consttime upon BN_FLG_CONSTTIME · 853950f7
      Nicola Tuveri authored 
      This issue was partially addressed by commit
972c87df, 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: default avatarMatt Caswell <matt@openssl.org>
Reviewed-by: default avatarBernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9793)
      853950f7
    • Nicola Tuveri's avatar
      Fix a SCA leak using BN_bn2bin() · 2e9d2934
      Nicola Tuveri authored 
      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: default avatarMatt Caswell <matt@openssl.org>
Reviewed-by: default avatarBernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9793)
      2e9d2934
  27. 27 Aug, 2019 1 commit
  29. 16 Aug, 2019 1 commit
  31. 25 Jul, 2019 1 commit
    • Richard Levitte's avatar
      Document issue with default installation paths on diverse Windows targets · d333ebaf
      Richard Levitte authored 
      For all config targets (except VMS, because it has a completely different
set of scripts), '/usr/local/ssl' is the default prefix for installation
of programs and libraries, as well as the path for OpenSSL run-time
configuration.

For programs built to run in a Windows environment, this default is
unsafe, and the user should set a different prefix.  This has been hinted
at in some documentation but not all, and the danger of leaving the
default as is hasn't been documented at all.

This change documents the issue as a caveat lector, and all configuration
examples now include an example --prefix.

CVE-2019-1552
Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9456)
      d333ebaf
  33. 21 Jul, 2019 1 commit
  35. 19 Jul, 2019 1 commit
  37. 11 Jun, 2019 2 commits
  39. 07 Jun, 2019 1 commit
  41. 28 May, 2019 4 commits