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

blog: aug 2019 security post-release announcement #2412

Merged
merged 1 commit into from
Aug 15, 2019

Conversation

sam-github
Copy link
Contributor

Waiting on actual release to update timestamps and merge.

@richardlau
Copy link
Member

It looks like some of the fixes are revertible (via --security-reverts=). Do we document those?

@richardlau
Copy link
Member

It looks like some of the fixes are revertible (via --security-reverts=). Do we document those?

(Also it looks like the flag is --security-revert= in 8.x and --security-reverts= in 10.x and 12.x which is an annoying inconsistency.)

@sam-github
Copy link
Contributor Author

sam-github commented Aug 15, 2019

In these cases, I would rather not doc the sec reverts. They are there if anyone reports problems, but there weren't any concerns raised about the limits during PR review, they seem pretty generous and unlikely to be hit by normal apps.

The inconsistency is not good. I think it was a typo introduced in https://github.com/addaleax/node/blob/be4a3b658a692d390f4fa9a187189c2ee993de1c/src/node_options.cc#L137 but probably not noticed because there were no revertible CVEs at the time.

I don't think this should be fixed now, but maybe it is worth fixing on master for next time, if there is one? EDIT or perhaps it was intentional because the argument is an array? Not sure

sam-github added a commit to sam-github/node that referenced this pull request Aug 15, 2019
It was called --security-revert prior to 12.x, but changed in
nodejs#22490.

See:
nodejs/nodejs.org#2412 (comment)
@sam-github sam-github marked this pull request as ready for review August 15, 2019 22:41
@sam-github sam-github merged commit 68dddad into master Aug 15, 2019
@sam-github sam-github deleted the 19-08-sec-announce-post branch August 15, 2019 22:50
Trott pushed a commit to nodejs/node that referenced this pull request Aug 17, 2019
It was called --security-revert prior to 12.x, but changed in
#22490.

See:
nodejs/nodejs.org#2412 (comment)

PR-URL: #29153
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to nodejs/node that referenced this pull request Aug 19, 2019
It was called --security-revert prior to 12.x, but changed in
#22490.

See:
nodejs/nodejs.org#2412 (comment)

PR-URL: #29153
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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 this pull request may close these issues.

5 participants