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

deps: float 99540ec from openssl (CVE-2018-0735) #23950

Closed
wants to merge 1 commit into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Oct 29, 2018

Low severity timing vulnerability in ECDSA signature generation. Publicly disclosed but unreleased, pending OpenSSL 1.1.0j.

This is for master, 10.x and 11.x, should cherry-pick without problem.

There is a version of this for 1.0.2 @ openssl/openssl#7513 but as yet it's unreviewed so we shouldn't jump the gun.

I don't think we need to rush a release out for this, but it should certainly go out with whatever the next releases are for 10 and 11, security or standard.

/cc @nodejs/crypto @nodejs/release

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

Original commit message:

Timing vulnerability in ECDSA signature generation (CVE-2018-0735)

Preallocate an extra limb for some of the big numbers to avoid a reallocation
that can potentially provide a side channel.

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

@nodejs-github-bot nodejs-github-bot added the openssl Issues and PRs related to the OpenSSL dependency. label Oct 29, 2018
@rvagg
Copy link
Member Author

rvagg commented Oct 29, 2018

ooops, and cc @nodejs/security

@paulidale
Copy link

Please give the reviewers some time to review backports. The 1.0.2 backport wasn't trivial......
I expect it will either be accepted within the next day or changes will be asked for resulting in a couple of days of delay.

@targos
Copy link
Member

targos commented Oct 29, 2018

I can include it in 11.1.0 tomorrow if it is fast-tracked

Low severity timing vulnerability in ECDSA signature generation

Publicly disclosed but unreleased, pending OpenSSL 1.1.0j

Also includes trivial syntax fix from
openssl/openssl#7516

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

Original commit message:

    Timing vulnerability in ECDSA signature generation (CVE-2018-0735)

    Preallocate an extra limb for some of the big numbers to avoid a reallocation
    that can potentially provide a side channel.

    Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
    (Merged from openssl/openssl#7486)
@rvagg rvagg force-pushed the rvagg/openssl-CVE-2018-0735 branch from e5d7aac to 4addbc7 Compare October 29, 2018 09:52
@rvagg
Copy link
Member Author

rvagg commented Oct 29, 2018

Squashed a trivial syntax fix (openssl/openssl#7516) into my commit here, noted in the commit msg

Testing @ https://ci.nodejs.org/job/node-test-pull-request/18201/

@rvagg
Copy link
Member Author

rvagg commented Oct 29, 2018

@targos let's not bother with the fast-track on this one, it's very low severity. Will land in a couple of days.

@rvagg
Copy link
Member Author

rvagg commented Oct 30, 2018

two more related commits @ #23965

@Trott
Copy link
Member

Trott commented Nov 4, 2018

Trott pushed a commit to Trott/io.js that referenced this pull request Nov 4, 2018
Low severity timing vulnerability in ECDSA signature generation

Publicly disclosed but unreleased, pending OpenSSL 1.1.0j

Also includes trivial syntax fix from
openssl/openssl#7516

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

Original commit message:

    Timing vulnerability in ECDSA signature generation (CVE-2018-0735)

    Preallocate an extra limb for some of the big numbers to avoid a reallocation
    that can potentially provide a side channel.

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

PR-URL: nodejs#23950
Refs: https://www.openssl.org/news/secadv/20181029.txt
Refs: openssl/openssl#7486
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott
Copy link
Member

Trott commented Nov 4, 2018

Landed in d8fb81f

@Trott Trott closed this Nov 4, 2018
@rvagg rvagg deleted the rvagg/openssl-CVE-2018-0735 branch November 6, 2018 09:10
@rvagg rvagg mentioned this pull request Nov 14, 2018
BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
Low severity timing vulnerability in ECDSA signature generation

Publicly disclosed but unreleased, pending OpenSSL 1.1.0j

Also includes trivial syntax fix from
openssl/openssl#7516

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

Original commit message:

    Timing vulnerability in ECDSA signature generation (CVE-2018-0735)

    Preallocate an extra limb for some of the big numbers to avoid a reallocation
    that can potentially provide a side channel.

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

PR-URL: #23950
Refs: https://www.openssl.org/news/secadv/20181029.txt
Refs: openssl/openssl#7486
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.