Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reject excessively large primes in DH key generation. #6457

Closed
wants to merge 1 commit into from

Conversation

guidovranken
Copy link
Contributor

CVE-2018-0732

Signed-off-by: Guido Vranken guidovranken@gmail.com

CVE-2018-0732

Signed-off-by: Guido Vranken <guidovranken@gmail.com>
@mattcaswell mattcaswell added the approval: review pending This pull request needs review by a committer label Jun 11, 2018
@mspncp mspncp added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Jun 11, 2018
BN_MONT_CTX *mont = NULL;
BIGNUM *pub_key = NULL, *priv_key = NULL;

if (BN_num_bits(dh->p) > OPENSSL_DH_MAX_MODULUS_BITS) {
DHerr(DH_F_GENERATE_KEY, DH_R_MODULUS_TOO_LARGE);
goto err;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I would just return 0 rather than goto err. There's nothing to free yet, and below err we inject unnecessary generic error in addition to the more detailed error above.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with viktor. Matt if you want to make that change when you merge, I approve.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with viktor.

Me too.

Copy link

@vdukhovni vdukhovni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall the change is fine. If you prefer to keep the goto err flow, that's OK, but consider the return 0 alternative.

@mattcaswell
Copy link
Member

I made the requested change and pushed this. Thanks!

levitte pushed a commit that referenced this pull request Jun 12, 2018
CVE-2018-0732

Signed-off-by: Guido Vranken <guidovranken@gmail.com>

Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #6457)
@mattcaswell
Copy link
Member

Darn....forgot to add the branches to this. This is for backport to 1.1.0 and 1.0.2...please can someone approve the backport?

@mattcaswell mattcaswell reopened this Jun 12, 2018
Copy link
Member

@t-j-h t-j-h left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved for backport

levitte pushed a commit that referenced this pull request Jun 12, 2018
CVE-2018-0732

Signed-off-by: Guido Vranken <guidovranken@gmail.com>

(cherry picked from commit 91f7361)

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #6457)
levitte pushed a commit that referenced this pull request Jun 12, 2018
CVE-2018-0732

Signed-off-by: Guido Vranken <guidovranken@gmail.com>

(cherry picked from commit 91f7361)

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #6457)
@mattcaswell
Copy link
Member

Pushed to 1.1.0 (ea7abee) and 1.0.2 (3984ef0). Thanks.

rvagg added a commit to rvagg/io.js that referenced this pull request Jun 12, 2018
Pending OpenSSL 1.1.0i release.

Original message:

    Reject excessively large primes in DH key generation.

    CVE-2018-0732

    Signed-off-by: Guido Vranken <guidovranken@gmail.com>

    (cherry picked from commit 91f7361f47b082ae61ffe1a7b17bb2adf213c7fe)

    Reviewed-by: Tim Hudson <tjh@openssl.org>
    Reviewed-by: Matt Caswell <matt@openssl.org>
    (Merged from openssl/openssl#6457)
bob-beck pushed a commit to openbsd/src that referenced this pull request Jun 12, 2018
by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

"During key agreement in a TLS handshake using a DH(E) based ciphersuite a
malicious server can send a very large prime value to the client. This will
cause the client to spend an unreasonably long period of time generating a key
for this prime resulting in a hang until the client has finished. This could be
exploited in a Denial Of Service attack."
makr pushed a commit to makr/openssl that referenced this pull request Jun 13, 2018
CVE-2018-0732

Signed-off-by: Guido Vranken <guidovranken@gmail.com>

(cherry picked from commit 91f7361)

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl#6457)
busterb pushed a commit to libressl/openbsd that referenced this pull request Jun 13, 2018
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
busterb pushed a commit to libressl/openbsd that referenced this pull request Jun 13, 2018
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
busterb pushed a commit to libressl/openbsd that referenced this pull request Jun 13, 2018
by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

"During key agreement in a TLS handshake using a DH(E) based ciphersuite a
malicious server can send a very large prime value to the client. This will
cause the client to spend an unreasonably long period of time generating a key
for this prime resulting in a hang until the client has finished. This could be
exploited in a Denial Of Service attack."
rvagg added a commit to nodejs/node that referenced this pull request Jun 15, 2018
Pending OpenSSL 1.1.0i release.

PR-URL: #21282
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Upstream: openssl/openssl@ea7abee

Original commit message:

    Reject excessively large primes in DH key generation.

    CVE-2018-0732

    Signed-off-by: Guido Vranken <guidovranken@gmail.com>

    (cherry picked from commit 91f7361f47b082ae61ffe1a7b17bb2adf213c7fe)

    Reviewed-by: Tim Hudson <tjh@openssl.org>
    Reviewed-by: Matt Caswell <matt@openssl.org>
    (Merged from openssl/openssl#6457)
targos pushed a commit to nodejs/node that referenced this pull request Jun 15, 2018
Pending OpenSSL 1.1.0i release.

PR-URL: #21282
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Upstream: openssl/openssl@ea7abee

Original commit message:

    Reject excessively large primes in DH key generation.

    CVE-2018-0732

    Signed-off-by: Guido Vranken <guidovranken@gmail.com>

    (cherry picked from commit 91f7361f47b082ae61ffe1a7b17bb2adf213c7fe)

    Reviewed-by: Tim Hudson <tjh@openssl.org>
    Reviewed-by: Matt Caswell <matt@openssl.org>
    (Merged from openssl/openssl#6457)
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jun 22, 2018
Changelog:
2.7.4
We have released LibreSSL 2.7.4, a security update for the
2.7.x series. It contains the following changes:

  * Avoid a timing side-channel leak when generating DSA and ECDSA
    signatures. This is caused by an attempt to do fast modular
    arithmetic, which introduces branches that leak information
    regarding secret values. Issue identified and reported by Keegan
    Ryan of NCC Group.

  * Reject excessively large primes in DH key generation. Problem
    reported by Guido Vranken to OpenSSL
    (openssl/openssl#6457) and based on his
    diff.

2.7.3
We have released LibreSSL 2.7.3, which will be arriving in the LibreSSL
directory of your local OpenBSD mirror soon. This is the first bugfix
release from the 2.7 series, which includes the following changes from 2.7.2:

 * Removed incorrect NULL checks in DH_set0_key(). Reported by Ondrej Sury.

 * Limited tls_config_clear_keys() to only clear private keys.
   This was inadvertently clearing the keypair, which includes the OCSP staple
   and pubkey hash - if an application called tls_configure() followed by
   tls_config_clear_keys(), this would prevent OCSP staples from working.

 * Fixed an issue normalizing CPU architecture in the configure script,
   which disabled assembly optimizations on platforms that get detected
   as 'amd64', opposed to 'x86_64'.

2.7.2
ve released LibreSSL 2.7.2, which will be arriving in the LibreSSL
directory of your local OpenBSD mirror soon. This is the first stable release
from the 2.7 series, which is also included with OpenBSD 6.3.

It includes the following changes from 2.7.1

 * Updated and added extensive new HISTORY sections to API manuals.

 * Added support for shared library builds with CMake on all supported
   platforms. Note that some of the CMake options have changed, consult
   the README for details.

LibreSSL 2.7.2 also includes:

 * Added support for many OpenSSL 1.0.2 and 1.1 APIs, based on
   observations of real-world usage in applications. These are
   implemented in parallel with existing OpenSSL 1.0.1 APIs - visibility
   changes have not been made to existing structs, allowing code written
   for older OpenSSL APIs to continue working.

 * Extensive corrections, improvements, and additions to the
   API documentation, including new public APIs from OpenSSL that had
   no pre-existing documentation.

 * Added support for automatic library initialization in libcrypto,
   libssl, and libtls. Support for pthread_once or a compatible
   equivalent is now required of the target operating system. As a
   side-effect, minimum Windows support is Vista or higher.

 * Converted more packet handling methods to CBB, which improves
   resiliency when generating TLS messages.

 * Completed TLS extension handling rewrite, improving consistency of
   checks for malformed and duplicate extensions.

 * Rewrote ASN1_TYPE_{get,set}_octetstring() using templated ASN.1.
   This removes the last remaining use of the old M_ASN1_* macros
   (asn1_mac.h) from API that needs to continue to exist.

 * Added support for client-side session resumption in libtls.
   A libtls client can specify a session file descriptor (a regular
   file with appropriate ownership and permissions) and libtls will
   manage reading and writing of session data across TLS handshakes.

 * Improved support for strict alignment on ARMv7 architectures,
   conditionally enabling assembly in those cases.

 * Fixed a memory leak in libtls when reusing a tls_config.

 * Merged more DTLS support into the regular TLS code path, removing
   duplicated code.

 * Many improvements to Windows Cmake-based builds and tests,
   especially when targeting Visual Studio.

2.7.1
We have released LibreSSL 2.7.1, which will be arriving in the
LibreSSL directory of your local OpenBSD mirror soon. This is the second
release from the 2.7 series, which will be part of OpenBSD 6.3.

It includes the following changes from 2.7.0

 * Fixed a bug in int_x509_param_set_hosts, calling strlen() if name
   length provided is 0 to match the OpenSSL behaviour. Issue noticed
   by Christian Heimes <christian@python.org>

 * Fixed builds macOS 10.11 and older.

LibreSSL 2.7.1 also includes:

 * Added support for many OpenSSL 1.0.2 and 1.1 APIs, based on
   observations of real-world usage in applications. These are
   implemented in parallel with existing OpenSSL 1.0.1 APIs - visibility
   changes have not been made to existing structs, allowing code written
   for older OpenSSL APIs to continue working.

 * Extensive corrections, improvements, and additions to the
   API documentation, including new public APIs from OpenSSL that had
   no pre-existing documentation.

 * Added support for automatic library initialization in libcrypto,
   libssl, and libtls. Support for pthread_once or a compatible
   equivalent is now required of the target operating system. As a
   side-effect, minimum Windows support is Vista or higher.

 * Converted more packet handling methods to CBB, which improves
   resiliency when generating TLS messages.

 * Completed TLS extension handling rewrite, improving consistency of
   checks for malformed and duplicate extensions.

 * Rewrote ASN1_TYPE_{get,set}_octetstring() using templated ASN.1.
   This removes the last remaining use of the old M_ASN1_* macros
   (asn1_mac.h) from API that needs to continue to exist.

 * Added support for client-side session resumption in libtls.
   A libtls client can specify a session file descriptor (a regular
   file with appropriate ownership and permissions) and libtls will
   manage reading and writing of session data across TLS handshakes.

 * Improved support for strict alignment on ARMv7 architectures,
   conditionally enabling assembly in those cases.

 * Fixed a memory leak in libtls when reusing a tls_config.

 * Merged more DTLS support into the regular TLS code path, removing
   duplicated code.

 * Many improvements to Windows Cmake-based builds and tests,
   especially when targeting Visual Studio.

2.7.0
We have released LibreSSL 2.7.0, which will be arriving in the
LibreSSL directory of your local OpenBSD mirror soon. This is the first
release from the 2.7 series, which will be part of OpenBSD 6.3.
It includes the following changes:

 * Added support for many OpenSSL 1.0.2 and 1.1 APIs, based on
   observations of real-world usage in applications. These are
   implemented in parallel with existing OpenSSL 1.0.1 APIs - visibility
   changes have not been made to existing structs, allowing code written
   for older OpenSSL APIs to continue working.

 * Extensive corrections, improvements, and additions to the
   API documentation, including new public APIs from OpenSSL that had
   no pre-existing documentation.

 * Added support for automatic library initialization in libcrypto,
   libssl, and libtls. Support for pthread_once or a compatible
   equivalent is now required of the target operating system. As a
   side-effect, minimum Windows support is Vista or higher.

 * Converted more packet handling methods to CBB, which improves
   resiliency when generating TLS messages.

 * Completed TLS extension handling rewrite, improving consistency of
   checks for malformed and duplicate extensions.

 * Rewrote ASN1_TYPE_{get,set}_octetstring() using templated ASN.1.
   This removes the last remaining use of the old M_ASN1_* macros
   (asn1_mac.h) from API that needs to continue to exist.

 * Added support for client-side session resumption in libtls.
   A libtls client can specify a session file descriptor (a regular
   file with appropriate ownership and permissions) and libtls will
   manage reading and writing of session data across TLS handshakes.

 * Improved support for strict alignment on ARMv7 architectures,
   conditionally enabling assembly in those cases.

 * Fixed a memory leak in libtls when reusing a tls_config.

 * Merged more DTLS support into the regular TLS code path, removing
   duplicated code.

 * Many improvements to Windows Cmake-based builds and tests,
   especially when targeting Visual Studio.
timmow pushed a commit to timmow/openssl that referenced this pull request Jun 29, 2018
CVE-2018-0732

Signed-off-by: Guido Vranken <guidovranken@gmail.com>

(cherry picked from commit 91f7361)

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl#6457)

Chery picked from 3984ef0
from
https://github.com/openssl/openssl
in response to
https://usn.ubuntu.com/3692-1/
via
https://www.openssl.org/news/secadv/20180612.txt
drt24 pushed a commit to drt24/openbsd-import that referenced this pull request Jul 18, 2020
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Aug 20, 2020
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Aug 20, 2020
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Aug 25, 2020
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Aug 25, 2020
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Sep 2, 2020
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Sep 2, 2020
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Sep 3, 2020
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Sep 3, 2020
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Sep 17, 2020
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Sep 17, 2020
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Sep 24, 2020
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Sep 24, 2020
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Aug 21, 2021
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Aug 21, 2021
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
bernd-edlinger pushed a commit to bernd-edlinger/openssl that referenced this pull request Nov 11, 2021
CVE-2018-0732

Signed-off-by: Guido Vranken <guidovranken@gmail.com>

(cherry picked from commit 91f7361)

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl#6457)
busterb pushed a commit to libressl/openbsd that referenced this pull request Feb 7, 2022
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
busterb pushed a commit to libressl/openbsd that referenced this pull request Feb 7, 2022
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
busterb pushed a commit to libressl/openbsd that referenced this pull request Feb 9, 2022
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
busterb pushed a commit to libressl/openbsd that referenced this pull request Feb 9, 2022
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
busterb pushed a commit to libressl/openbsd that referenced this pull request Feb 9, 2022
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
busterb pushed a commit to libressl/openbsd that referenced this pull request Feb 9, 2022
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
botovq pushed a commit to libressl/openbsd that referenced this pull request Sep 19, 2022
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
botovq pushed a commit to libressl/openbsd that referenced this pull request Sep 19, 2022
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Jan 12, 2023
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
rbm01 pushed a commit to rbm01/OpenBSD-src that referenced this pull request Jan 12, 2023
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
botovq pushed a commit to libressl/openbsd that referenced this pull request Aug 9, 2023
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
botovq pushed a commit to libressl/openbsd that referenced this pull request Aug 9, 2023
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
keradoracomb pushed a commit to keradoracomb/datasheet that referenced this pull request Sep 19, 2023
…ed by Guido Vranken to OpenSSL (openssl/openssl#6457) and based on his diff. suggestions from tb@, ok tb@ jsing@"
NCommander pushed a commit to NCommander/openbsd-src-with-branches that referenced this pull request May 9, 2024
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
NCommander pushed a commit to NCommander/openbsd-src-with-branches that referenced this pull request May 9, 2024
…ported

by Guido Vranken to OpenSSL (openssl/openssl#6457)
and based on his diff.  suggestions from tb@, ok tb@ jsing@

Original commit by sthen@
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants