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.

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

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

Merged
merged 1 commit into from
May 25, 2017
Merged

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

merged 1 commit into from
May 25, 2017

Conversation

sam-github
Copy link
Contributor

@sam-github sam-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-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label May 23, 2017
@sam-github sam-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
Contributor Author

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

@mscdex mscdex 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

@mhdawson mhdawson 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

@refack refack 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
Contributor Author

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

@sam-github
Copy link
Contributor Author

@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
Contributor Author

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-github sam-github merged commit b659385 into nodejs:master May 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-github sam-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]>
@jasnell jasnell mentioned this pull request May 28, 2017
Closed
@sam-github sam-github mentioned this pull request May 30, 2017
Closed
4 tasks
@sam-github
Copy link
Contributor Author

See #12677

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

Should this not have been semver minor?

@sam-github
Copy link
Contributor Author

yes, semver minor

@sam-github sam-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]>
@MylesBorins MylesBorins 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]>
@MylesBorins MylesBorins 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 free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@refack refack refack approved these changes

@jasnell jasnell jasnell approved these changes

@cjihrig cjihrig cjihrig approved these changes

@mhdawson mhdawson mhdawson approved these changes

@gibfahn gibfahn gibfahn approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. cli Issues and PRs related to the Node.js command line interface. semver-minor PRs that contain new features and should be released in the next minor version. tls Issues 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