Skip to content

src: allow --tls-cipher-list in NODE_OPTIONS#13172

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.

Sign up for GitHub

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

Jump to bottom
Merged
sam-github merged 1 commit into from
May 25, 2017
Merged

src: allow --tls-cipher-list in NODE_OPTIONS #13172

sam-github merged 1 commit into from
May 25, 2017

Conversation

@sam-github
Copy link
Contributor

@sam-githubsam-github commented May 23, 2017•
edited
Loading

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
src, tls

@nodejs-github-botnodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label May 23, 2017
@sam-githubsam-github added the tls Issues and PRs related to the tls subsystem. label May 23, 2017
@refack
Copy link
Contributor

refack commented May 23, 2017

Did you forget https://github.com/nodejs/node/blob/master/doc/api/cli.md#node_optionsoptions? Or it should stay undocumented.

@sam-github
Copy link
ContributorAuthor

sam-github commented May 23, 2017

@refack My update and your noticing the miss passed on the wire :-)

@mscdexmscdex added the cli Issues and PRs related to the Node.js command line interface. label May 23, 2017
mhdawson
mhdawson approved these changes May 23, 2017
View reviewed changes
Copy link
Member

@mhdawsonmhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

refack
refack approved these changes May 24, 2017
View reviewed changes
Copy link
Contributor

@refackrefack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😄

@sam-github
Copy link
ContributorAuthor

sam-github commented May 25, 2017

ci: https://ci.nodejs.org/job/node-test-pull-request/8301/

@sam-github
Copy link
ContributorAuthor

sam-github commented May 25, 2017

@nodejs/build FIPS failure is unrelated, but reproduceable and genuine, from https://ci.nodejs.org/job/node-test-commit-linux-fips/8660/nodes=ubuntu1404-64/console:

+ curl -LO https://openssl.org/source/openssl-fips-2.0.9.tar.gz % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 0 0 0 0 0 0 0 0 --:--:-- 0:00:01 --:--:-- 0 0 0 0 0 0 0 0 0 --:--:-- 0:00:02 --:--:-- 0 100 342 100 342 0 0 126 0 0:00:02 0:00:02 --:--:-- 126 100 4239 100 4239 0 0 1218 0 0:00:03 0:00:03 --:--:-- 1218 + gunzip openssl-fips-2.0.9.tar.gz gzip: openssl-fips-2.0.9.tar.gz: not in gzip format

I did this locally, the file isn't tgz, it is

<!DOCTYPE html> <html lang="en"> <!-- head.inc --> <title> /err404.html </title> <meta charset="utf-8"> <meta name="author" content="OpenSSL Foundation, Inc."> ...

What's up?

@gibfahn
Copy link
Member

gibfahn commented May 25, 2017

Looks like the URL has changed from:

https://www.openssl.org/source/openssl-fips-2.0.9.tar.gz to
https://www.openssl.org/source/old/fips/openssl-fips-2.0.9.tar.gz

Not sure that is the correct new link, maybe @mhdawson can confirm?

@sam-github
Copy link
ContributorAuthor

sam-github commented May 25, 2017

It seems not coincidental that this happened about when OpenSSL released:

https://www.openssl.org/source/ shows a new FIPS 2.0.13, and 2.0.9 has been shuffled to a new location. This is annoying, perhaps we can bring the lack of stable URLs up with OpenSSL? New location:

% curl -LO https://openssl.org/source/old/fips/openssl-fips-2.0.9.tar.gz % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 351 100 351 0 0 323 0 0:00:01 0:00:01 --:--:-- 323 100 1391k 100 1391k 0 0 388k 0 0:00:03 0:00:03 --:--:-- 1022k core/tls-node (env-ciphersuites $ u=) % file openssl-fips-2.0.9.tar.gz openssl-fips-2.0.9.tar.gz: gzip compressed data, last modified: Sat Oct 25 12:37:15 2014, max compression, from Unix

This was referenced May 25, 2017
Closed
Closed
@sam-github
src: allow --tls-cipher-list in NODE_OPTIONS
b659385
PR-URL: nodejs#13172 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
@sam-githubsam-github merged commit b659385 into nodejs:masterMay 25, 2017
jasnell pushed a commit that referenced this pull request May 25, 2017
@sam-github@jasnell
src: allow --tls-cipher-list in NODE_OPTIONS
9b959b3
PR-URL: #13172 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
@sam-githubsam-github deleted the env-ciphersuites branch May 26, 2017 03:28
jasnell pushed a commit that referenced this pull request May 28, 2017
@sam-github@jasnell
src: allow --tls-cipher-list in NODE_OPTIONS
287b11d
PR-URL: #13172 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
@jasnelljasnell mentioned this pull request May 28, 2017
Closed
@sam-githubsam-github mentioned this pull request May 30, 2017
Closed
4 tasks
@sam-github
Copy link
ContributorAuthor

sam-github commented May 30, 2017

See #12677

@gibfahngibfahn mentioned this pull request Jun 15, 2017
Closed
3 tasks
@MylesBorins
Copy link
Contributor

MylesBorins commented Jul 17, 2017

Should this not have been semver minor?

@sam-github
Copy link
ContributorAuthor

sam-github commented Jul 17, 2017

yes, semver minor

@sam-githubsam-github added the semver-minor PRs that contain new features and should be released in the next minor version. label Jul 17, 2017
sam-github added a commit to sam-github/node that referenced this pull request Oct 11, 2017
@sam-github
src: allow --tls-cipher-list in NODE_OPTIONS
1c58992
PR-URL: nodejs#13172 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 16, 2017
@sam-github@MylesBorins
src: allow --tls-cipher-list in NODE_OPTIONS
aa5c58c
Backport-PR-URL: #12677 PR-URL: #13172 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Oct 17, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
@sam-github@MylesBorins
src: allow --tls-cipher-list in NODE_OPTIONS
16f8f9b
Backport-PR-URL: #12677 PR-URL: #13172 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Nov 3, 2017
Merged
MylesBorins added a commit that referenced this pull request Nov 6, 2017
@MylesBorins
2017-11-07, Version 6.12.0 'Boron' (LTS)
c6a2ec5
Notable Changes: * assert: - assert.fail() can now take one or two arguments (Rich Trott) #12293 * crypto: - add sign/verify support for RSASSA-PSS (Tobias Nießen) #11705 * deps: - upgrade openssl sources to 1.0.2m (Shigeki Ohtsu) #16691 - upgrade libuv to 1.15.0 (cjihrig) #15745 - upgrade libuv to 1.14.1 (cjihrig) #14866 - upgrade libuv to 1.13.1 (cjihrig) #14117 - upgrade libuv to 1.12.0 (cjihrig) #13306 * fs: - Add support for fs.write/fs.writeSync(fd, buffer, cb) and fs.write/fs.writeSync(fd, buffer, offset, cb) as documented (Andreas Lind) #7856 * inspector: - enable --inspect-brk (Refael Ackermann) #12615 * process: - add --redirect-warnings command line argument (James M Snell) #10116 * src: - allow CLI args in env with NODE_OPTIONS (Sam Roberts) #12028) - --abort-on-uncaught-exception in NODE_OPTIONS (Sam Roberts) #13932 - allow --tls-cipher-list in NODE_OPTIONS (Sam Roberts) #13172 - use SafeGetenv() for NODE_REDIRECT_WARNINGS (Sam Roberts) #12677 * test: - remove common.fail() (Rich Trott) #12293 PR-URL: #16263
MylesBorins added a commit that referenced this pull request Nov 7, 2017
@MylesBorins
2017-11-07, Version 6.12.0 'Boron' (LTS)
70dc6d5
Notable Changes: * assert: - assert.fail() can now take one or two arguments (Rich Trott) #12293 * crypto: - add sign/verify support for RSASSA-PSS (Tobias Nießen) #11705 * deps: - upgrade openssl sources to 1.0.2m (Shigeki Ohtsu) #16691 - upgrade libuv to 1.15.0 (cjihrig) #15745 - upgrade libuv to 1.14.1 (cjihrig) #14866 - upgrade libuv to 1.13.1 (cjihrig) #14117 - upgrade libuv to 1.12.0 (cjihrig) #13306 * fs: - Add support for fs.write/fs.writeSync(fd, buffer, cb) and fs.write/fs.writeSync(fd, buffer, offset, cb) as documented (Andreas Lind) #7856 * inspector: - enable --inspect-brk (Refael Ackermann) #12615 * process: - add --redirect-warnings command line argument (James M Snell) #10116 * src: - allow CLI args in env with NODE_OPTIONS (Sam Roberts) #12028) - --abort-on-uncaught-exception in NODE_OPTIONS (Sam Roberts) #13932 - allow --tls-cipher-list in NODE_OPTIONS (Sam Roberts) #13172 - use SafeGetenv() for NODE_REDIRECT_WARNINGS (Sam Roberts) #12677 * test: - remove common.fail() (Rich Trott) #12293 PR-URL: #16263
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes

@cjihrigcjihrigcjihrig approved these changes

@mhdawsonmhdawsonmhdawson approved these changes

+2 more reviewers

@refackrefackrefack approved these changes

@gibfahngibfahngibfahn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c++Issues and PRs that require attention from people who are familiar with C++.cliIssues and PRs related to the Node.js command line interface.semver-minorPRs that contain new features and should be released in the next minor version.tlsIssues and PRs related to the tls subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@sam-github@refack@gibfahn@MylesBorins@jasnell@cjihrig@mhdawson@mscdex@nodejs-github-bot