crypto: add test case for AES key wrapping by yhwang · Pull Request #20587 · nodejs/node · GitHub
Skip to content

crypto: add test case for AES key wrapping#20587

Closed
yhwang wants to merge 1 commit into
nodejs:masterfrom
yhwang:add-aes-keywrap-test
Closed

crypto: add test case for AES key wrapping#20587
yhwang wants to merge 1 commit into
nodejs:masterfrom
yhwang:add-aes-keywrap-test

Conversation

@yhwang

@yhwang yhwang commented May 8, 2018

Copy link
Copy Markdown
Member

Add test cases for AES key wrapping and only detect output length in
cipher case. The reason being is the returned output length is
insufficient in AES key unwrapping case.

Signed-off-by: Yihong Wang yh.wang@ibm.com

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

Add test cases for AES key wrapping and only detect output length in
cipher case. The reason being is the returned output length is
insufficient in AES key unwrapping case.

Signed-off-by: Yihong Wang <yh.wang@ibm.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. labels May 8, 2018
@yhwang

yhwang commented May 8, 2018

Copy link
Copy Markdown
Member Author

@bnoordhuis bnoordhuis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Were you planning to bring this up with the upstream openssl project as well?

Comment thread src/node_crypto.cc
// For key wrapping algorithms, get output size by calling
// EVP_CipherUpdate() with null output.
if (mode == EVP_CIPH_WRAP_MODE &&
if (kind_ == kCipher && mode == EVP_CIPH_WRAP_MODE &&

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this change intended? If so, this likely should be a different commit.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@jasnell this is needed. Sorry the title of this commit is a little misleading but I explained that I need the code change in the description

@yhwang

yhwang commented May 8, 2018

Copy link
Copy Markdown
Member Author

@bnoordhuis I will try it

@yhwang yhwang added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 9, 2018
@targos

targos commented May 12, 2018

Copy link
Copy Markdown
Member

@targos

targos commented May 12, 2018

Copy link
Copy Markdown
Member

Landed in e3ceae7

@targos targos closed this May 12, 2018
targos pushed a commit that referenced this pull request May 12, 2018
Add test cases for AES key wrapping and only detect output length in
cipher case. The reason being is the returned output length is
insufficient in AES key unwrapping case.

PR-URL: #20587
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
tniessen pushed a commit to tniessen/node that referenced this pull request May 13, 2018
Add test cases for AES key wrapping and only detect output length in
cipher case. The reason being is the returned output length is
insufficient in AES key unwrapping case.

PR-URL: nodejs#20587
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
addaleax pushed a commit that referenced this pull request May 14, 2018
Add test cases for AES key wrapping and only detect output length in
cipher case. The reason being is the returned output length is
insufficient in AES key unwrapping case.

Backport-PR-URL: #20706
PR-URL: #20587
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@addaleax addaleax mentioned this pull request May 14, 2018
@targos targos added backported-to-v10.x and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels May 14, 2018
@yhwang yhwang deleted the add-aes-keywrap-test branch May 16, 2018 05:35
@yhwang

yhwang commented May 31, 2018

Copy link
Copy Markdown
Member Author

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.

6 participants