-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
Comments
@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"). |
@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? |
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
Node-v8 would be affected by CVE-2019-1552 and I submitted #29455 for the fix. |
+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. |
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. |
Security Advisory was announced in https://www.openssl.org/news/secadv/20190910.txt. Here are my assessments.
|
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? |
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
|
Sam is right, we only fork-then-exec so no problem there. |
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.
The text was updated successfully, but these errors were encountered: