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

OpenSSL sec releases upcoming #29445

Closed
sam-github opened this issue Sep 5, 2019 · 10 comments · Fixed by nodejs/nodejs.org#2572
Closed

OpenSSL sec releases upcoming #29445

sam-github opened this issue Sep 5, 2019 · 10 comments · Fixed by nodejs/nodejs.org#2572

Comments

@sam-github
Copy link
Contributor

The OpenSSL project team would like to announce the forthcoming release
of OpenSSL versions 1.1.1d, 1.1.0l and 1.0.2t.

These releases will be made available on 10th September 2019 between
approximately 1200-1600 UTC.

These are security fix releases. The highest severity security issue fixed by
these releases is rated as LOW.

FYI. This will affect all release lines. Since the highest severity is LOW, I propose we handle these not as security releases, but as patch updates to the relevant release lines, using the normal release process.

@nodejs/tsc @nodejs/security @nodejs/security-release @nodejs/security-triage and @rvagg: opinions?

8, 10, and 12.x of Node.js will all need updating.

@sam-github sam-github added the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Sep 5, 2019
@rvagg
Copy link
Member

rvagg commented Sep 5, 2019

@sam-github reasonable proposal.

FYI, though in the past, for this kind of thing, we've announced that we will wait for the releases, make an impact assessment within 24 (or 48?) hours, and decide how to proceed from there. Flag that a likely outcome is that they will be rolled into the normal release process, but depending on the impact assessment we might choose to push out dedicated security releases.

The OpenSSL designations can be a bit sketchy as far as our use of OpenSSL goes, so it might be best not to read too much into "LOW" (as far as Node is concerned it may range from "zero impact cause we don't touch any of these things" to "concerning enough that users will probably want this asap").

@sam-github
Copy link
Contributor Author

@rvagg in that case, should we make a blog post in the sec category to give a heads up that this is coming, and we'll make an evaluation when we get the releases?

shigeki added a commit to shigeki/node that referenced this issue Sep 5, 2019
According to CVE-2019-1552(*), it is encouraged to change OPENSSLDIR
from the default of /usr/local/ssl to a privileged directory on
Windows. "C:\Program Files\Common Files\SSL" is set as it is the
default path in OpenSSL-1.1.1.

(*) https://www.openssl.org/news/secadv/20190730.txt

Fixes: nodejs#29445
@shigeki
Copy link
Contributor

shigeki commented Sep 5, 2019

Node-v8 would be affected by CVE-2019-1552 and I submitted #29455 for the fix.

@mhdawson
Copy link
Member

mhdawson commented Sep 5, 2019

+1 to trying to use regular releases first, and agree we should put out note to nodejs-sec mailing list to give people a heads up.

@addaleax
Copy link
Member

addaleax commented Sep 6, 2019

If we do bring out security releases, could we add #29399 and #29459 to the releases? They address regressions from the August security releases, so I think that would make sense.

@rvagg
Copy link
Member

rvagg commented Sep 9, 2019

sorry, was offline for the weekend. all looks good @sam-github and +1 on those two http/2 cleanups from @addaleax, loose ends aren't happy.

@shigeki
Copy link
Contributor

shigeki commented Sep 11, 2019

Security Advisory was announced in https://www.openssl.org/news/secadv/20190910.txt.

Here are my assessments.

  • ECDSA remote timing attack (CVE-2019-1547)
    Not affected. Node supports only named curves for ECDSA signing.

  • Fork Protection (CVE-2019-1549)
    Affected. We do not have the crypto initialization of OPENSSL_INIT_ATFORK. Child processes would have the same random state as the parent. But I think it would be hard to attack it.

  • Padding Oracle in PKCS7_dataDecode and CMS_decrypt_set1_pkey (CVE-2019-1563)
    Not affected. Node does not support PCKS7 and CMS.

@sam-github
Copy link
Contributor Author

I don't think the fork protection affects us, even theoretically. We do fork, but we always exec after fork. We don't fork and continue running and using the OpenSSL PRNG's forked state.

/to @bnoordhuis @rvagg, thoughts?

@Trott Trott removed the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Sep 11, 2019
@rvagg
Copy link
Member

rvagg commented Sep 12, 2019

hm, I guess you're right @sam-github, I can't think of a way you could reuse OpenSSL state at all in a forked process but I'm no expert in the code that touches fork / uv_spawn / whatever.

Unless someone comes up with a more definitive agreement either way, I think you could just put out a statement saying something to the effect of

Here's the issues they announced, we're not impacted by 1547, 1563 and we don't believe there's a path to usefully exploit 1549 with Node.js. So we won't be doing dedicated releases but will get this update out with the next version for each release line.

@bnoordhuis
Copy link
Member

Sam is right, we only fork-then-exec so no problem there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants