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

nodejs distributions are not affected the same way by security releases #1187

Closed
kapouer opened this issue Mar 14, 2022 · 13 comments
Closed

Comments

@kapouer
Copy link

kapouer commented Mar 14, 2022

Hi,
as an example, in Debian we have Node.js depending on system-installed OpenSSL,
thus security issues affecting that dependency are, most of the time, dealt with in the corresponding dependency, not in Node.js.

Thus a sentence like "if you're using a Node.js version not distributed by official upstream channel, this security issue might not concern you, please check with their security team", etc...

@DanielRuf
Copy link

Hi @kapouer,

I'm not sure I fully understand what you mean. Do you mean an improvement for security release notes? Isn't this handled by the release WG? What is the current sentence like and where can we see it?

https://github.com/nodejs/release

They also define what gets backported:

Define the policy for what gets backported to release streams.

Sure, most distros and the package maintainers there who package the software for the target distros most often recompile, repackage and backport upstream fixes.

@mhdawson
Copy link
Member

mhdawson commented Mar 14, 2022

That is true for RHEL distributions as well. I'm going to move this into the TSC repo for discussion to see what we might be willing to add to the pre-release announce. The pre-announce is handled by the security stewards and I think we'd want the TSC to be aware if we are going to start adding a disclaimer like that.

@mhdawson mhdawson transferred this issue from nodejs/security-wg Mar 14, 2022
@mcollina
Copy link
Member

I'm +1 to add such a disclaimer. It seemed obvious to me but likely it's not for most devs.

@mhdawson
Copy link
Member

mhdawson commented Mar 14, 2022

TSC Question:

Any concerns with by default adding

If you're using a Node.js version which is part of a linux distribution which uses a system installed OpenSSL, this security upate might not concern you, please check with their security team

to the pre-announce for any OpenSSL only security releases?

@richardlau
Copy link
Member

No concerns from me, although I think Homebrew might also be dynamically linking openssl so it's not just Linux distributions.

@ljharb
Copy link
Member

ljharb commented Mar 14, 2022

Considering (i believe) node isn't officially distributed anywhere without openssl, adding such a message seems "fine", but also seems like the sort of thing users sign up for when they choose to use an unofficial installation mechanism.

@mhdawson
Copy link
Member

Discussion in TSC meeting, might want to say that you may need to update your openssl instead.

@ChALkeR
Copy link
Member

ChALkeR commented Mar 23, 2022

  1. Perhaps the text should be changed to include a reminder to update OpenSSL if needed, as the current one suggests that user might not to have to anything at all. And they likely should update OpenSSL for the security update.

  2. I have concerns with adding this "by default" as this could lead to accidentally adding it in case where it is not applicable, e.g. where there could be other changes included. Forgetting it won't cause much damage (as would just keep the current state of things), and accidentally adding it where it's not applicable could significantly break the trust chain and result in rush updates and/or security issues for users.

So, perhaps we should decide on the text, and add this as a suggested wording for those cases when we are totally sure there won't be any other changes except for the deps update, but leave it at the releaser judgment on whether to include it or not, without specifically suggesting to do that "by default". It won't hurt much if this will be missing in some announcements.

cc @MylesBorins @BethGriggs (as is related to what has been discussed on the meeting) -- does that sound good?

@mhdawson
Copy link
Member

mhdawson commented Mar 23, 2022

@ChALkeR so maybe adding the following in the security release process in this section under the Pre-release announcement to node.js org blog, add the following additional text.

If the security release is planned to only contain an OpenSSL update consider adding the following to the pre-release announcement:

Since this security release will only includes updates for OpenSSL, if you're using a Node.js version which is part of a linux distribution which uses a system installed OpenSSL, this security update might not concern you. You may instead need to update your system OpenSSL libraries, please check the security announcements for the distribution.

@ChALkeR
Copy link
Member

ChALkeR commented Mar 23, 2022

@mhdawson s/is planned to/will/ perhaps (and minor fixes):

If the security release will only contain an OpenSSL update consider adding the following to the pre-release announcement:

Since this security release will only include updates for OpenSSL, if you're using a Node.js version which is part of a Linux distribution which uses a system installed OpenSSL, this Node.js security update might not concern you. You may instead need to update your system OpenSSL libraries, please check the security announcements for the distribution.

@mhdawson
Copy link
Member

@ChALkeR looks good to me.

@mhdawson
Copy link
Member

I'll open a PR to add that and we can have any additional discussion/concerns raised there.

@mhdawson
Copy link
Member

PR - nodejs/node#42456

juanarbol pushed a commit to juanarbol/node that referenced this issue Apr 5, 2022
Fixes: nodejs/TSC#1187

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs#42456
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
juanarbol pushed a commit to nodejs/node that referenced this issue Apr 6, 2022
Fixes: nodejs/TSC#1187

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #42456
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
xtx1130 pushed a commit to xtx1130/node that referenced this issue Apr 25, 2022
Fixes: nodejs/TSC#1187

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs#42456
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
juanarbol pushed a commit to nodejs/node that referenced this issue May 31, 2022
Fixes: nodejs/TSC#1187

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #42456
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
danielleadams pushed a commit to nodejs/node that referenced this issue Jun 27, 2022
Fixes: nodejs/TSC#1187

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #42456
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
targos pushed a commit to nodejs/node that referenced this issue Jul 11, 2022
Fixes: nodejs/TSC#1187

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #42456
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
targos pushed a commit to nodejs/node that referenced this issue Jul 11, 2022
Fixes: nodejs/TSC#1187

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #42456
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
targos pushed a commit to nodejs/node that referenced this issue Jul 31, 2022
Fixes: nodejs/TSC#1187

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #42456
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
Fixes: nodejs/TSC#1187

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs/node#42456
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants