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 26d7fce1 from openssl (CVE-2018-0734 follow-on) #24353

Closed

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Nov 14, 2018

The fix for CVE-2018-0734, floated in 213c7d2, failed to include a
constant-time calculation for one of the variables. This introduces
a fix for that.

Ref: openssl/openssl#7549
Upstream: openssl/openssl@26d7fce1

Original commit message:
    Add a constant time flag to one of the bignums to avoid a timing leak.

    Reviewed-by: Tim Hudson <tjh@openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/7549)

    (cherry picked from commit 00496b6423605391864fbbd1693f23631a1c5239)

This is for 1.1.0, so can go in to 11 and 10. I'll do a separate one for 1.0.2.

@nodejs/crypto

The fix for CVE-2018-0734, floated in 213c7d2, failed to include a
constant-time calculation for one of the variables. This introduces
a fix for that.

Ref: openssl/openssl#7549
Upstream: openssl/openssl@26d7fce1

Original commit message:
    Add a constant time flag to one of the bignums to avoid a timing leak.

    Reviewed-by: Tim Hudson <tjh@openssl.org>
    (Merged from openssl/openssl#7549)

    (cherry picked from commit 00496b6423605391864fbbd1693f23631a1c5239)
@nodejs-github-bot nodejs-github-bot added the openssl Issues and PRs related to the OpenSSL dependency. label Nov 14, 2018
@rvagg rvagg mentioned this pull request Nov 14, 2018
rvagg added a commit to rvagg/io.js that referenced this pull request Nov 14, 2018
The fix for CVE-2018-0734, floated in 213c7d2, failed to include a
constant-time calculation for one of the variables. This introduces
a fix for that.

Ref: openssl/openssl#7549
Ref: nodejs#24353
Upstream: openssl/openssl@26d7fce1

Original commit message:
    Add a constant time flag to one of the bignums to avoid a timing leak.

    Reviewed-by: Tim Hudson <tjh@openssl.org>
    (Merged from openssl/openssl#7549)

    (cherry picked from commit 00496b6423605391864fbbd1693f23631a1c5239)
@BridgeAR
Copy link
Member

@rvagg adding the backport-requested labels will prevent these from being pulled into a release automatically. If they can not be backported, the releasing person will add the label to indicate that a manual backport is required.

@Trott
Copy link
Member

Trott commented Nov 16, 2018

@nodejs/crypto @nodejs/security Would be great to get some reviews for this one-liner.

@Trott
Copy link
Member

Trott commented Nov 16, 2018

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 17, 2018
@danbev
Copy link
Contributor

danbev commented Nov 17, 2018

Landed in 323a365.

@danbev danbev closed this Nov 17, 2018
danbev pushed a commit that referenced this pull request Nov 17, 2018
The fix for CVE-2018-0734, floated in 213c7d2, failed to include a
constant-time calculation for one of the variables. This introduces
a fix for that.

Upstream: openssl/openssl@26d7fce1

Original commit message:
  Add a constant time flag to one of the bignums to avoid a timing leak.

  Reviewed-by: Tim Hudson <tjh@openssl.org>
  (Merged from openssl/openssl#7549)

  (cherry picked from commit 00496b6423605391864fbbd1693f23631a1c5239)

PR-URL: #24353
Refs: openssl/openssl#7549
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@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.

@rvagg rvagg deleted the rvagg/openssl-CVE-2018-0734-followup branch November 19, 2018 10:18
@rvagg
Copy link
Member Author

rvagg commented Nov 19, 2018

yes correct @targos, those labels are appropriate thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants