Skip to content
  • Nicola Tuveri's avatar
    [ec] Match built-in curves on EC_GROUP_new_from_ecparameters · 9a43a733
    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
    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 8402cd5f75f8c2f60d8bd39775b24b03dd8b3b38)
    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/9809)
    9a43a733
To find the state of this project's repository at the time of any of these versions, check out the tags.