tls: use emitWarning() for dhparam < 2048 bits by jasnell · Pull Request #11447 · nodejs/node

jasnell · 2017-02-17T23:02:45Z

When a dhparam less than 2048 bits was used, a warning was being printed directly to console.error using an internalUtil.trace function that was not used anywhere else. This replaces it with a proper process warning and removes the internalUtil.trace function.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Affected core subsystem(s)

tls

jasnell · 2017-02-17T23:03:47Z

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

jasnell · 2017-02-17T23:05:03Z

/cc @nodejs/crypto

shigeki · 2017-02-18T12:56:27Z

test/parallel/test-crypto-dhparam-warning.js

This is 1025 bits dhparam as below.

Diffie-Hellman-Parameters: (1025 bit) prime: 01:4c:00:15:a4:7b:a9:85:b0:02:cb:db:05:2b:2b: a2:73:45:9d:ca:0e:8e:9d:02:c1:30:92:82:9c:94: c4:e0:54:48:17:7a:3d:01:76:0f:c6:01:81:3d:c0: 90:cb:8b:5a:1d:af:84:a9:c7:e8:b3:3c:17:97:57: 98:e5:fa:b1:ad:56:67:75:93:fb:10:91:a5:4f:9b: 88:e2:4f:ab:b1:2f:b9:3a:b5:2a:c5:d7:78:ac:27: 8a:8e:72:87:cd:ce:6b:f2:0b:be:8a:5a:7f:b2:ab: 37:cb:56:6c:94:6a:d2:20:08:b1:26:16:83:2f:ec: 2b:fa:51:c7:97:55:78:63:bb generator: 2 (0x2) -----BEGIN DH PARAMETERS----- MIGHAoGBAUwAFaR7qYWwAsvbBSsronNFncoOjp0CwTCSgpyUxOBUSBd6PQF2D8YB gT3AkMuLWh2vhKnH6LM8F5dXmOX6sa1WZ3WT+xCRpU+biOJPq7EvuTq1KsXXeKwn io5yh83Oa/ILvopaf7KrN8tWbJRq0iAIsSYWgy/sK/pRx5dVeGO7AgEC -----END DH PARAMETERS-----

We already have 1024 bits key in test/fixtures/keys/dh1024.pem. Is there any reason to use 1025 bits dh param for this test? Otherwise, I think this test can be merged by adding common.expectWarning check in test/parallel/test-tls-dhe.js

jasnell · 2017-02-19T23:23:37Z

@shigeki ... updated! I put the warning check into test-tls-dhe.js as suggested! thank you! Please take a look!

When a dhparam less than 2048 bits was used, a warning was being printed directly to console.error using an internalUtil.trace function that was not used anywhere else. This replaces it with a proper process warning and removes the internalUtil.trace function.

shigeki

CI https://ci.nodejs.org/job/node-test-commit/8020/ is all green.

rvagg · 2017-02-20T08:15:57Z

LGTM, but are we doing semver-major for these?

jasnell · 2017-02-20T09:01:02Z

This one is likely obscure enough that it wouldn't matter but there no harm in being careful. I'm good with semver-major

…

On Mon, Feb 20, 2017 at 12:16 AM Rod Vagg ***@***.***> wrote: LGTM, but are we doing semver-major for these? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11447 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAa2ec-sMpJZoo_1OlFBm8FQmfPFp1gfks5reUvBgaJpZM4ME2xk> .

jasnell · 2017-02-20T14:52:33Z

semver-major just to be careful

jasnell · 2017-02-20T14:59:12Z

Landed in 9fb86af

When a dhparam less than 2048 bits was used, a warning was being printed directly to console.error using an internalUtil.trace function that was not used anywhere else. This replaces it with a proper process warning and removes the internalUtil.trace function. PR-URL: #11447 Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Rod Vagg <rod@vagg.org>

jasnell · 2017-02-20T15:01:28Z

Corrected landing metadata in d523eb9

sam-github · 2017-02-20T17:36:02Z

If I read the diff correctly, the message on stderr changed from

WARNING: DH parameter is less than 2048 bits

to

SecurityWarning: DH parameter is less than 2048 bits

So its got to be semver-major.

It could probably be backported with a warning name of 'WARNING', but is that useful?

jasnell · 2017-02-21T09:53:08Z

major it is then. Changing the name to WARNING would not be worthwhile.

* **Async Hooks** * The `async_hooks` module has landed in core [[`4a7233c178`](nodejs@4a7233c178)] [nodejs#12892](nodejs#12892). * **Buffer** * Using the `--pending-deprecation` flag will cause Node.js to emit a deprecation warning when using `new Buffer(num)` or `Buffer(num)`. [[`d2d32ea5a2`](nodejs@d2d32ea5a2)] [nodejs#11968](nodejs#11968). * `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances [[`7eb1b4658e`](nodejs@7eb1b4658e)] [nodejs#12141](nodejs#12141). * Many `Buffer` methods now accept `Uint8Array` as input [[`beca3244e2`](nodejs@beca3244e2)] [nodejs#10236](nodejs#10236). * **Child Process** * Argument and kill signal validations have been improved [[`97a77288ce`](nodejs@97a77288ce)] [nodejs#12348](nodejs#12348), [[`d75fdd96aa`](nodejs@d75fdd96aa)] [nodejs#10423](nodejs#10423). * Child Process methods accept `Uint8Array` as input [[`627ecee9ed`](nodejs@627ecee9ed)] [nodejs#10653](nodejs#10653). * **Console** * Error events emitted when using `console` methods are now supressed. [[`f18e08d820`](nodejs@f18e08d820)] [nodejs#9744](nodejs#9744). * **Dependencies** * The npm client has been updated to 5.0.0 [[`3c3b36af0f`](nodejs@3c3b36af0f)] [nodejs#12936](nodejs#12936). * V8 has been updated to 5.8 with forward ABI stability to 6.0 [[`60d1aac8d2`](nodejs@60d1aac8d2)] [nodejs#12784](nodejs#12784). * **Domains** * Native `Promise` instances are now `Domain` aware [[`84dabe8373`](nodejs@84dabe8373)] [nodejs#12489](nodejs#12489). * **Errors** * We have started assigning static error codes to errors generated by Node.js. This has been done through multiple commits and is still a work in progress. * **File System** * The utility class `fs.SyncWriteStream` has been deprecated [[`7a55e34ef4`](nodejs@7a55e34ef4)] [nodejs#10467](nodejs#10467). * The deprecated `fs.read()` string interface has been removed [[`3c2a9361ff`](nodejs@3c2a9361ff)] [nodejs#9683](nodejs#9683). * **HTTP** * Improved support for userland implemented Agents [[`90403dd1d0`](nodejs@90403dd1d0)] [nodejs#11567](nodejs#11567). * Outgoing Cookie headers are concatenated into a single string [[`d3480776c7`](nodejs@d3480776c7)] [nodejs#11259](nodejs#11259). * The `httpResponse.writeHeader()` method has been deprecated [[`fb71ba4921`](nodejs@fb71ba4921)] [nodejs#11355](nodejs#11355). * New methods for accessing HTTP headers have been added to `OutgoingMessage` [[`3e6f1032a4`](nodejs@3e6f1032a4)] [nodejs#10805](nodejs#10805). * **Lib** * All deprecation messages have been assigned static identifiers [[`5de3cf099c`](nodejs@5de3cf099c)] [nodejs#10116](nodejs#10116). * The legacy `linkedlist` module has been removed [[`84a23391f6`](nodejs@84a23391f6)] [nodejs#12113](nodejs#12113). * **N-API** * Experimental support for the new N-API API has been added [[`56e881d0b0`](nodejs@56e881d0b0)] [nodejs#11975](nodejs#11975). * **Process** * Process warning output can be redirected to a file using the `--redirect-warnings` command-line argument [[`03e89b3ff2`](nodejs@03e89b3ff2)] [nodejs#10116](nodejs#10116). * Process warnings may now include additional detail [[`dd20e68b0f`](nodejs@dd20e68b0f)] [nodejs#12725](nodejs#12725). * **REPL** * REPL magic mode has been deprecated [[`3f27f02da0`](nodejs@3f27f02da0)] [nodejs#11599](nodejs#11599). * **Src** * `NODE_MODULE_VERSION` has been updated to 57 (nodejs@ec7cbaf266)] [nodejs#12995](nodejs#12995). * Add `--pending-deprecation` command-line argument and `NODE_PENDING_DEPRECATION` environment variable [[`a16b570f8c`](nodejs@a16b570f8c)] [nodejs#11968](nodejs#11968). * The `--debug` command-line argument has been deprecated. Note that using `--debug` will enable the *new* Inspector-based debug protocol as the legacy Debugger protocol previously used by Node.js has been removed. [[`010f864426`](nodejs@010f864426)] [nodejs#12949](nodejs#12949). * Throw when the `-c` and `-e` command-line arguments are used at the same time [[`a5f91ab230`](nodejs@a5f91ab230)] [nodejs#11689](nodejs#11689). * Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line arguments are used at the same time. [[`8a7db9d4b5`](nodejs@8a7db9d4b5)] [nodejs#12087](nodejs#12087). * **Stream** * `Stream` now supports `destroy()` and `_destroy()` APIs [[`b6e1d22fa6`](nodejs@b6e1d22fa6)] [nodejs#12925](nodejs#12925). * `Stream` now supports the `_final()` API [[`07c7f198db`](nodejs@07c7f198db)] [nodejs#12828](nodejs#12828). * **TLS** * The `rejectUnauthorized` option now defaults to `true` [[`348cc80a3c`](nodejs@348cc80a3c)] [nodejs#5923](nodejs#5923). * The `tls.createSecurePair()` API now emits a runtime deprecation [[`a2ae08999b`](nodejs@a2ae08999b)] [nodejs#11349](nodejs#11349). * A runtime deprecation will now be emitted when `dhparam` is less than 2048 bits [[`d523eb9c40`](nodejs@d523eb9c40)] [nodejs#11447](nodejs#11447). * **URL** * The WHATWG URL implementation is now a fully-supported Node.js API [[`d080ead0f9`](nodejs@d080ead0f9)] [nodejs#12710](nodejs#12710). * **Util** * `Symbol` keys are now displayed by default when using `util.inspect()` [[`5bfd13b81e`](nodejs@5bfd13b81e)] [nodejs#9726](nodejs#9726). * `toJSON` errors will be thrown when formatting `%j` [[`455e6f1dd8`](nodejs@455e6f1dd8)] [nodejs#11708](nodejs#11708). * Convert `inspect.styles` and `inspect.colors` to prototype-less objects [[`aab0d202f8`](nodejs@aab0d202f8)] [nodejs#11624](nodejs#11624). * The new `util.promisify()` API has been added [[`99da8e8e02`](nodejs@99da8e8e02)] [nodejs#12442](nodejs#12442). * **Zlib** * Support `Uint8Array` in Zlib convenience methods [[`91383e47fd`](nodejs@91383e47fd)] [nodejs#12001](nodejs#12001). * Zlib errors now use `RangeError` and `TypeError` consistently [[`b514bd231e`](nodejs@b514bd231e)] [nodejs#11391](nodejs#11391).

* **Async Hooks** * The `async_hooks` module has landed in core [[`4a7233c178`](4a7233c178)] [#12892](#12892). * **Buffer** * Using the `--pending-deprecation` flag will cause Node.js to emit a deprecation warning when using `new Buffer(num)` or `Buffer(num)`. [[`d2d32ea5a2`](d2d32ea5a2)] [#11968](#11968). * `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances [[`7eb1b4658e`](7eb1b4658e)] [#12141](#12141). * Many `Buffer` methods now accept `Uint8Array` as input [[`beca3244e2`](beca3244e2)] [#10236](#10236). * **Child Process** * Argument and kill signal validations have been improved [[`97a77288ce`](97a77288ce)] [#12348](#12348), [[`d75fdd96aa`](d75fdd96aa)] [#10423](#10423). * Child Process methods accept `Uint8Array` as input [[`627ecee9ed`](627ecee9ed)] [#10653](#10653). * **Console** * Error events emitted when using `console` methods are now supressed. [[`f18e08d820`](f18e08d820)] [#9744](#9744). * **Dependencies** * The npm client has been updated to 5.0.0 [[`3c3b36af0f`](3c3b36af0f)] [#12936](#12936). * V8 has been updated to 5.8 with forward ABI stability to 6.0 [[`60d1aac8d2`](60d1aac8d2)] [#12784](#12784). * **Domains** * Native `Promise` instances are now `Domain` aware [[`84dabe8373`](84dabe8373)] [#12489](#12489). * **Errors** * We have started assigning static error codes to errors generated by Node.js. This has been done through multiple commits and is still a work in progress. * **File System** * The utility class `fs.SyncWriteStream` has been deprecated [[`7a55e34ef4`](7a55e34ef4)] [#10467](#10467). * The deprecated `fs.read()` string interface has been removed [[`3c2a9361ff`](3c2a9361ff)] [#9683](#9683). * **HTTP** * Improved support for userland implemented Agents [[`90403dd1d0`](90403dd1d0)] [#11567](#11567). * Outgoing Cookie headers are concatenated into a single string [[`d3480776c7`](d3480776c7)] [#11259](#11259). * The `httpResponse.writeHeader()` method has been deprecated [[`fb71ba4921`](fb71ba4921)] [#11355](#11355). * New methods for accessing HTTP headers have been added to `OutgoingMessage` [[`3e6f1032a4`](3e6f1032a4)] [#10805](#10805). * **Lib** * All deprecation messages have been assigned static identifiers [[`5de3cf099c`](5de3cf099c)] [#10116](#10116). * The legacy `linkedlist` module has been removed [[`84a23391f6`](84a23391f6)] [#12113](#12113). * **N-API** * Experimental support for the new N-API API has been added [[`56e881d0b0`](56e881d0b0)] [#11975](#11975). * **Process** * Process warning output can be redirected to a file using the `--redirect-warnings` command-line argument [[`03e89b3ff2`](03e89b3ff2)] [#10116](#10116). * Process warnings may now include additional detail [[`dd20e68b0f`](dd20e68b0f)] [#12725](#12725). * **REPL** * REPL magic mode has been deprecated [[`3f27f02da0`](3f27f02da0)] [#11599](#11599). * **Src** * `NODE_MODULE_VERSION` has been updated to 57 (ec7cbaf266)] [#12995](#12995). * Add `--pending-deprecation` command-line argument and `NODE_PENDING_DEPRECATION` environment variable [[`a16b570f8c`](a16b570f8c)] [#11968](#11968). * The `--debug` command-line argument has been deprecated. Note that using `--debug` will enable the *new* Inspector-based debug protocol as the legacy Debugger protocol previously used by Node.js has been removed. [[`010f864426`](010f864426)] [#12949](#12949). * Throw when the `-c` and `-e` command-line arguments are used at the same time [[`a5f91ab230`](a5f91ab230)] [#11689](#11689). * Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line arguments are used at the same time. [[`8a7db9d4b5`](8a7db9d4b5)] [#12087](#12087). * **Stream** * `Stream` now supports `destroy()` and `_destroy()` APIs [[`b6e1d22fa6`](b6e1d22fa6)] [#12925](#12925). * `Stream` now supports the `_final()` API [[`07c7f198db`](07c7f198db)] [#12828](#12828). * **TLS** * The `rejectUnauthorized` option now defaults to `true` [[`348cc80a3c`](348cc80a3c)] [#5923](#5923). * The `tls.createSecurePair()` API now emits a runtime deprecation [[`a2ae08999b`](a2ae08999b)] [#11349](#11349). * A runtime deprecation will now be emitted when `dhparam` is less than 2048 bits [[`d523eb9c40`](d523eb9c40)] [#11447](#11447). * **URL** * The WHATWG URL implementation is now a fully-supported Node.js API [[`d080ead0f9`](d080ead0f9)] [#12710](#12710). * **Util** * `Symbol` keys are now displayed by default when using `util.inspect()` [[`5bfd13b81e`](5bfd13b81e)] [#9726](#9726). * `toJSON` errors will be thrown when formatting `%j` [[`455e6f1dd8`](455e6f1dd8)] [#11708](#11708). * Convert `inspect.styles` and `inspect.colors` to prototype-less objects [[`aab0d202f8`](aab0d202f8)] [#11624](#11624). * The new `util.promisify()` API has been added [[`99da8e8e02`](99da8e8e02)] [#12442](#12442). * **Zlib** * Support `Uint8Array` in Zlib convenience methods [[`91383e47fd`](91383e47fd)] [#12001](#12001). * Zlib errors now use `RangeError` and `TypeError` consistently [[`b514bd231e`](b514bd231e)] [#11391](#11391).

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. tls Issues and PRs related to the tls subsystem. util Issues and PRs related to the built-in util module. labels Feb 17, 2017

jasnell force-pushed the dhparam-emit-warning branch from c76d516 to 59c1cee Compare February 17, 2017 23:04

jasnell changed the title ~~crypto: use emitWarning() for dhparam < 2048 bits~~ tls: use emitWarning() for dhparam < 2048 bits Feb 17, 2017

shigeki reviewed Feb 18, 2017

View reviewed changes

jasnell force-pushed the dhparam-emit-warning branch from 59c1cee to 144ffc5 Compare February 19, 2017 23:22

jasnell force-pushed the dhparam-emit-warning branch from 144ffc5 to d92e13b Compare February 19, 2017 23:30

shigeki approved these changes Feb 20, 2017

View reviewed changes

bnoordhuis approved these changes Feb 20, 2017

View reviewed changes

jasnell added the semver-major PRs that contain breaking changes and should be released in the next major version. label Feb 20, 2017

jasnell closed this Feb 20, 2017

jasnell mentioned this pull request Apr 4, 2017

8.0.0 Release Proposal #12220

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

tls: use emitWarning() for dhparam < 2048 bits#11447

tls: use emitWarning() for dhparam < 2048 bits#11447
jasnell wants to merge 1 commit intonodejs:masterfrom
jasnell:dhparam-emit-warning

jasnell commented Feb 17, 2017

Uh oh!

jasnell commented Feb 17, 2017

Uh oh!

jasnell commented Feb 17, 2017

Uh oh!

shigeki Feb 18, 2017

Uh oh!

jasnell commented Feb 19, 2017

Uh oh!

shigeki left a comment

Uh oh!

rvagg commented Feb 20, 2017

Uh oh!

jasnell commented Feb 20, 2017 via email

Uh oh!

jasnell commented Feb 20, 2017

Uh oh!

jasnell commented Feb 20, 2017

Uh oh!

jasnell commented Feb 20, 2017

Uh oh!

sam-github commented Feb 20, 2017

Uh oh!

jasnell commented Feb 21, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

jasnell commented Feb 17, 2017

Checklist

Affected core subsystem(s)

Uh oh!

jasnell commented Feb 17, 2017

Uh oh!

jasnell commented Feb 17, 2017

Uh oh!

shigeki Feb 18, 2017

Choose a reason for hiding this comment

Uh oh!

jasnell commented Feb 19, 2017

Uh oh!

shigeki left a comment

Choose a reason for hiding this comment

Uh oh!

rvagg commented Feb 20, 2017

Uh oh!

jasnell commented Feb 20, 2017 via email

Uh oh!

jasnell commented Feb 20, 2017

Uh oh!

jasnell commented Feb 20, 2017

Uh oh!

jasnell commented Feb 20, 2017

Uh oh!

sam-github commented Feb 20, 2017

Uh oh!

jasnell commented Feb 21, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants