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.

Sign up for GitHub

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

Jump to bottom

deps: float two (more) OpenSSL patches for DSA vulnerabilities #23965

Closed
wants to merge 2 commits into from
Closed

deps: float two (more) OpenSSL patches for DSA vulnerabilities #23965

wants to merge 2 commits into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Oct 30, 2018

Build on from #23950 we have two more issues surrounding DSA.

One has a CVE, CVE-2018-0734 @ https://www.openssl.org/news/secadv/20181030.txt

Severity: Low

The OpenSSL DSA signature algorithm has been shown to be vulnerable to a
timing side channel attack. An attacker could use variations in the signing
algorithm to recover the private key.

Due to the low severity of this issue we are not issuing a new release
of OpenSSL 1.1.1, 1.1.0 or 1.0.2 at this time. The fix will be included
in OpenSSL 1.1.1a, OpenSSL 1.1.0j and OpenSSL 1.0.2q when they become
available.

The other runs into OpenSSL's severity-level policy for CVE assignment and doesn't quite make it so we don't have a CVE for it. openssl/openssl#7487

There is a side channel attack against the division used to calculate one of
the modulo inverses in the DSA algorithm. This change takes advantage of the
primality of the modulo and Fermat's little theorem to calculate the inverse
without leaking information.

If this is accepted I'll put in a PR for 6 & 8 since they have different patches (for 1.0.2).

FWIW I don't believe any of these rise to much of a meaningful level of severity. We're seeing an expected wave of timing attack vulnerabilities being discovered because this is the hottest area for research right now (for good reason, it's fascinating!). But a lot of them are more academic in nature in that they require very specific circumstances to be able to build a successful attack. And in these cases I don't believe exploits have been published anywhere.

Still worth floating on our releases I reckon though. Erring on the side of security is what the vast majority of our users want to see us do.

/cc @nodejs/crypto @nodejs/security

rvagg added 2 commits October 30, 2018 13:17
@rvagg
deps: float 415c3356 from openssl (DSA vulnerability)
f9c336d 
Low severity timing vulnerability in the DSA signature algorithm

Publicly disclosed but unreleased, pending OpenSSL 1.1.0j, not deemed
severe enough to be assigned a CVE #.

Ref: openssl/openssl#7487
PR-URL: https://github.com/nodejs/node/pull/???
Upstream: openssl/openssl@415c3356

Original commit message:

    DSA mod inverse fix

    There is a side channel attack against the division used to calculate one of
    the modulo inverses in the DSA algorithm.  This change takes advantage of the
    primality of the modulo and Fermat's little theorem to calculate the inverse
    without leaking information.

    Thanks to Samuel Weiser for finding and reporting this.

    Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
    Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
    (Merged from openssl/openssl#7487)
@rvagg
deps: float a9cfb8c2 from openssl (CVE-2018-0734)
082e301 
Low severity timing vulnerability in the DSA signature algorithm

Publicly disclosed but unreleased, pending OpenSSL 1.1.0j

Ref: openssl/openssl#7486
Ref: https://www.openssl.org/news/secadv/20181030.txt
PR-URL: https://github.com/nodejs/node/pull/???
Upstream: openssl/openssl@a9cfb8c2

Original commit message:

    Avoid a timing attack that leaks information via a side channel that
    triggers when a BN is resized.  Increasing the size of the BNs
    prior to doing anything with them suppresses the attack.

    Thanks due to Samuel Weiser for finding and locating this.

    Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
    (Merged from openssl/openssl#7486)
@nodejs-github-bot
Copy link
Collaborator

@rvagg build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1413/pipeline

@nodejs-github-bot nodejs-github-bot added the openssl Issues and PRs related to the OpenSSL dependency. label Oct 30, 2018
@rvagg rvagg mentioned this pull request Oct 30, 2018
Closed
@Trott
Copy link
Member

Trott commented Nov 4, 2018

CI: https://ci.nodejs.org/job/node-test-pull-request/18222/

@Trott
Copy link
Member

Trott commented Nov 4, 2018

Landed in c1e6703...213c7d2

@Trott Trott closed this Nov 4, 2018
Trott pushed a commit to Trott/io.js that referenced this pull request Nov 4, 2018
@rvagg @Trott
deps: float 415c3356 from openssl (DSA vulnerability)
e2260e9 
Low severity timing vulnerability in the DSA signature algorithm

Publicly disclosed but unreleased, pending OpenSSL 1.1.0j, not deemed
severe enough to be assigned a CVE #.

Ref: openssl/openssl#7487
PR-URL: https://github.com/nodejs/node/pull/???
Upstream: openssl/openssl@415c3356

Original commit message:

    DSA mod inverse fix

    There is a side channel attack against the division used to calculate one of
    the modulo inverses in the DSA algorithm.  This change takes advantage of the
    primality of the modulo and Fermat's little theorem to calculate the inverse
    without leaking information.

    Thanks to Samuel Weiser for finding and reporting this.

    Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
    Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
    (Merged from openssl/openssl#7487)

PR-URL: nodejs#23965
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Trott pushed a commit to Trott/io.js that referenced this pull request Nov 4, 2018
@rvagg @Trott
deps: float a9cfb8c2 from openssl (CVE-2018-0734)
213c7d2 
Low severity timing vulnerability in the DSA signature algorithm

Publicly disclosed but unreleased, pending OpenSSL 1.1.0j

Ref: openssl/openssl#7486
Ref: https://www.openssl.org/news/secadv/20181030.txt
PR-URL: https://github.com/nodejs/node/pull/???
Upstream: openssl/openssl@a9cfb8c2

Original commit message:

    Avoid a timing attack that leaks information via a side channel that
    triggers when a BN is resized.  Increasing the size of the BNs
    prior to doing anything with them suppresses the attack.

    Thanks due to Samuel Weiser for finding and locating this.

    Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
    (Merged from openssl/openssl#7486)

PR-URL: nodejs#23965
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@rvagg rvagg deleted the rvagg/openssl-CVE-2018-0734 branch November 6, 2018 09:10
@rvagg rvagg mentioned this pull request Nov 14, 2018
Merged
rvagg added a commit to rvagg/io.js that referenced this pull request Nov 14, 2018
@rvagg
deps: float 43e6a58d from openssl (CVE-2018-0734)
56af407 
Low severity timing vulnerability in the DSA signature algorithm

Publicly disclosed but unreleased, pending OpenSSL 1.0.2q

Ref: openssl/openssl#7486
Ref: openssl/openssl#7513
Ref: https://www.openssl.org/news/secadv/20181030.txt
Ref: nodejs#23965
Upstream: openssl/openssl@a9cfb8c2
Upstream: openssl/openssl@43e6a58d

Original commit message:

    Avoid a timing attack that leaks information via a side channel that
    triggers when a BN is resized.  Increasing the size of the BNs
    prior to doing anything with them suppresses the attack.

    Thanks due to Samuel Weiser for finding and locating this.

    Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
    (Merged from openssl/openssl#7486)

Original backport commit message:

    Merge DSA reallocation timing fix CVE-2018-0734.

    Reviewed-by: Richard Levitte <levitte@openssl.org>
    (Merged from openssl/openssl#7513)
rvagg added a commit to rvagg/io.js that referenced this pull request Nov 14, 2018
@rvagg
deps: float ebf65dbe from openssl (DSA vulnerability)
114ab64 
Low severity timing vulnerability in the DSA signature algorithm

Publicly disclosed but unreleased, pending OpenSSL 1.0.2q, not deemed
severe enough to be assigned a CVE #.

Ref: openssl/openssl#7487
Ref: openssl/openssl#7512
Ref: nodejs#23965
Upstream: openssl/openssl@415c3356
Upstream: openssl/openssl@ebf65dbe

Original commit message:

    DSA mod inverse fix

    There is a side channel attack against the division used to calculate one of
    the modulo inverses in the DSA algorithm.  This change takes advantage of the
    primality of the modulo and Fermat's little theorem to calculate the inverse
    without leaking information.

    Thanks to Samuel Weiser for finding and reporting this.

    Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
    Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
    (Merged from openssl/openssl#7487)

Original backport commit message:

    Reviewed-by: Richard Levitte <levitte@openssl.org>
    (Merged from openssl/openssl#7512)
@rvagg rvagg mentioned this pull request Nov 14, 2018
Closed
BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
@rvagg @BridgeAR
deps: float 415c3356 from openssl (DSA vulnerability)
89f447d 
Low severity timing vulnerability in the DSA signature algorithm

Publicly disclosed but unreleased, pending OpenSSL 1.1.0j, not deemed
severe enough to be assigned a CVE #.

Ref: openssl/openssl#7487
PR-URL: https://github.com/nodejs/node/pull/???
Upstream: openssl/openssl@415c3356

Original commit message:

    DSA mod inverse fix

    There is a side channel attack against the division used to calculate one of
    the modulo inverses in the DSA algorithm.  This change takes advantage of the
    primality of the modulo and Fermat's little theorem to calculate the inverse
    without leaking information.

    Thanks to Samuel Weiser for finding and reporting this.

    Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
    Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
    (Merged from openssl/openssl#7487)

PR-URL: #23965
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
@rvagg @BridgeAR
deps: float a9cfb8c2 from openssl (CVE-2018-0734)
d2916e9 
Low severity timing vulnerability in the DSA signature algorithm

Publicly disclosed but unreleased, pending OpenSSL 1.1.0j

Ref: openssl/openssl#7486
Ref: https://www.openssl.org/news/secadv/20181030.txt
PR-URL: https://github.com/nodejs/node/pull/???
Upstream: openssl/openssl@a9cfb8c2

Original commit message:

    Avoid a timing attack that leaks information via a side channel that
    triggers when a BN is resized.  Increasing the size of the BNs
    prior to doing anything with them suppresses the attack.

    Thanks due to Samuel Weiser for finding and locating this.

    Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
    (Merged from openssl/openssl#7486)

PR-URL: #23965
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@rvagg rvagg mentioned this pull request Nov 14, 2018
Closed
@targos
Copy link
Member

targos commented Nov 18, 2018

@rvagg IIUC this will be part of the next OpenSSL release, so I'm adding the dont-land-on label. Please correct me if I'm wrong.

This was referenced Apr 23, 2019
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryzokuken ryzokuken ryzokuken approved these changes

@tniessen tniessen tniessen approved these changes

@jasnell jasnell jasnell approved these changes

@fhinkel fhinkel fhinkel approved these changes

Assignees
No one assigned
Labels
openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@rvagg @nodejs-github-bot @Trott @targos @fhinkel @jasnell @tniessen @ryzokuken @BridgeAR