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

[v6.x backport] src: turn key length exception into CHECK #16585

Closed

Conversation

bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis commented Oct 29, 2017

shigeki and others added 4 commits October 29, 2017 11:33
`crypto.createCipher()` sets the fixed IV derived from password and it
leads to a security risk of nonce reuse when counter mode is used.
A warning is emitted when CTR, GCM or CCM is used in
`crypto.createCipher()` to notify users to avoid nonce reuse.

Fixes: nodejs#13801
PR-URL: nodejs#13821
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
The previous commit is a back-port of pull request nodejs#13821 to v6.x.
Its regression test does not apply to the v6.x branch (depends on
semver-major pull request nodejs#9405) so this commit adds a new test.

Refs: nodejs#13821
Refs: nodejs#9405
EVP_CIPHER_CTX_FLAG_WRAP_ALLOW flag needs to be set in using wrap mode
ciphers. In `crypto.createCipher()`, AES key wrap mode does not use a
default IV defined in RFC3394 but a generated IV with
`EVP_BytesToKey()` to be consistent API behaviors with other ciphers.

The built-in AES wrap mode in OpenSSL is not supported in FIPS mode as
http://openssl.6102.n7.nabble.com/AES-Key-Wrap-in-FIPS-Mode-td50238.html
so its tests in FIPS mode are skipped.

Fixes: nodejs#15009
PR-URL: nodejs#15037
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
This exception can logically never happen because of the key stretching
that takes place first.  Failure must therefore be a bug in Node.js and
not in the executing script.

PR-URL: nodejs#15183
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@nodejs-github-bot 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. v6.x labels Oct 29, 2017
MylesBorins pushed a commit that referenced this pull request Nov 14, 2017
This exception can logically never happen because of the key stretching
that takes place first.  Failure must therefore be a bug in Node.js and
not in the executing script.

Backport-PR-URL: #16585
PR-URL: #15183
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

landed in 8b70ad1

MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
This exception can logically never happen because of the key stretching
that takes place first.  Failure must therefore be a bug in Node.js and
not in the executing script.

Backport-PR-URL: #16585
PR-URL: #15183
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
This exception can logically never happen because of the key stretching
that takes place first.  Failure must therefore be a bug in Node.js and
not in the executing script.

Backport-PR-URL: #16585
PR-URL: #15183
Reviewed-By: Colin Ihrig <cjihrig@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
c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants