crypto: fix error of createCipher in wrap mode by shigeki · Pull Request #15037 · nodejs/node

shigeki · 2017-08-26T00:21:31Z

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.

Fixes: #15009

Checklist

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

Affected core subsystem(s)

crypto

CC: @nodejs/crypto

shigeki · 2017-08-26T00:23:47Z

CI is running on https://ci.nodejs.org/job/node-test-pull-request/9842/

indutny

LGTM

tniessen

Code LGTM

bnoordhuis · 2017-08-28T20:28:30Z

fips buildbot failure is relevant, you'll need to wrap it in a if (common.hasFipsCrypto) { ... } guard.

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.

shigeki · 2017-08-29T15:33:14Z

@bnoordhuis Thanks for checking CI results.
I found that 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 I fixed its tests are skipped in FIPS mode.

CI is running again in https://ci.nodejs.org/job/node-test-pull-request/9867/

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: #15009 PR-URL: #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>

shigeki · 2017-08-29T16:25:49Z

CI failure in fips mode was resolved. Other failures are due to Jenkins error and CI script error.
Landed in 4218f19. Thanks for reviewing everyone.

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/node#15009 PR-URL: nodejs/node#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>

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>

MylesBorins · 2017-09-10T02:04:07Z

This needs to be manually backported to v8.x-staging. @shigeki would you be able to do so?

bnoordhuis · 2017-09-10T11:25:36Z

@MylesBorins I suspect #13821 needs to be back-ported first, then this PR should apply cleanly.

MylesBorins · 2017-09-10T14:25:06Z

That worked, thanks!

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: #15009 PR-URL: #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>

MylesBorins · 2017-09-20T20:54:31Z

Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label.

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>

bnoordhuis · 2017-10-29T10:51:28Z

v6.x: #16584

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. Backport-PR-URL: #16584 Fixes: #15009 PR-URL: #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>

shigeki added crypto Issues and PRs related to the crypto subsystem. lts-watch-v4.x labels Aug 26, 2017

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. labels Aug 26, 2017

indutny approved these changes Aug 26, 2017

View reviewed changes

tniessen approved these changes Aug 27, 2017

View reviewed changes

bnoordhuis approved these changes Aug 28, 2017

View reviewed changes

jasnell approved these changes Aug 29, 2017

View reviewed changes

shigeki force-pushed the fix_aes_wrap branch from d4db553 to de733a4 Compare August 29, 2017 15:28

shigeki closed this Aug 29, 2017

EternalDeiwos mentioned this pull request Aug 29, 2017

AES-KW anvilresearch/webcrypto#45

Closed

MylesBorins added the backport-requested-v8.x label Sep 10, 2017

bnoordhuis mentioned this pull request Sep 10, 2017

src: turn key length exception into CHECK #15183

Closed

MylesBorins removed the backport-requested-v8.x label Sep 10, 2017

MylesBorins mentioned this pull request Sep 10, 2017

v8.5.0 proposal #15308

Merged

MylesBorins added the backport-requested-v6.x label Sep 20, 2017

bnoordhuis mentioned this pull request Oct 29, 2017

[v6.x backport] crypto: fix error of createCipher in wrap mode #16584

Closed

MylesBorins added backported-to-v6.x and removed backport-requested-v6.x labels Nov 14, 2017

MylesBorins mentioned this pull request Nov 21, 2017

v6.12.1 proposal #17180

Merged

This was referenced Jun 12, 2018

crypto: fix behavior of createCipher in wrap mode #21287

Closed

crypto: handle OpenSSL error queue in CipherBase #21288

Closed

adamlaska mentioned this pull request May 13, 2022

[Snyk] Fix for 1 vulnerabilities adamlaska/node#67

Open

snyk-bot mentioned this pull request Apr 27, 2023

[Snyk] Fix for 20 vulnerabilities aliscco/alisco-node#253

Open

This was referenced Apr 28, 2023

[Snyk] Fix for 16 vulnerabilities aliscco/alisco-node#304

Open

[Snyk] Fix for 18 vulnerabilities aliscco/alisco-node#310

Open

snyk-bot mentioned this pull request Apr 28, 2023

[Snyk] Fix for 20 vulnerabilities aliscco/alisco-node#314

Open

This was referenced Apr 28, 2023

[Snyk] Fix for 9 vulnerabilities aliscco/alisco-node#330

Open

[Snyk] Fix for 19 vulnerabilities aliscco/alisco-node#334

Open

snyk-bot mentioned this pull request Apr 28, 2023

[Snyk] Fix for 114 vulnerabilities aliscco/alisco-node#338

Open

aliscco mentioned this pull request Apr 28, 2023

[Snyk] Fix for 8 vulnerabilities aliscco/alisco-node#342

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

crypto: fix error of createCipher in wrap mode#15037

crypto: fix error of createCipher in wrap mode#15037
shigeki wants to merge 1 commit intonodejs:masterfrom
shigeki:fix_aes_wrap

shigeki commented Aug 26, 2017

Uh oh!

shigeki commented Aug 26, 2017 •

edited

Loading

Uh oh!

indutny left a comment

Uh oh!

tniessen left a comment

Uh oh!

bnoordhuis commented Aug 28, 2017

Uh oh!

shigeki commented Aug 29, 2017

Uh oh!

shigeki commented Aug 29, 2017

Uh oh!

MylesBorins commented Sep 10, 2017

Uh oh!

bnoordhuis commented Sep 10, 2017

Uh oh!

MylesBorins commented Sep 10, 2017

Uh oh!

MylesBorins commented Sep 20, 2017

Uh oh!

bnoordhuis commented Oct 29, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Conversation

shigeki commented Aug 26, 2017

Checklist

Affected core subsystem(s)

Uh oh!

shigeki commented Aug 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

indutny left a comment

Choose a reason for hiding this comment

Uh oh!

tniessen left a comment

Choose a reason for hiding this comment

Uh oh!

bnoordhuis commented Aug 28, 2017

Uh oh!

shigeki commented Aug 29, 2017

Uh oh!

shigeki commented Aug 29, 2017

Uh oh!

MylesBorins commented Sep 10, 2017

Uh oh!

bnoordhuis commented Sep 10, 2017

Uh oh!

MylesBorins commented Sep 10, 2017

Uh oh!

MylesBorins commented Sep 20, 2017

Uh oh!

bnoordhuis commented Oct 29, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

shigeki commented Aug 26, 2017 •

edited

Loading