-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
(v10.x backport) Backport 12 crypto commits #20706
(v10.x backport) Backport 12 crypto commits #20706
Conversation
This change allows users to restrict accepted GCM authentication tag lengths to a single value. PR-URL: nodejs#20039 Fixes: nodejs#17523 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yihong Wang <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
This commit extracts the common code from the Cipher/Cipheriv and Decipher/Decipheriv constructors into a separate function to avoid code duplication. PR-URL: nodejs#20164 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
This commit adds a function named addCipherPrototypeFunctions to avoid code duplication. PR-URL: nodejs#20164 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
This commit renames rsaPublic and removes the rsaPrivate function as the code in these two functions are identical. PR-URL: nodejs#20164 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Currently the following compiler warnings are issued by clang: ../src/node_crypto.cc:2801:56: warning: '&&' within '||' [-Wlogical-op-parentheses] return tag_len == 4 || tag_len == 8 || tag_len >= 12 && tag_len <= 16; ~~ ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~ ../src/node_crypto.cc:2801:56: note: place parentheses around the '&&' expression to silence this warning return tag_len == 4 || tag_len == 8 || tag_len >= 12 && tag_len <= 16; ^ ../src/node_crypto.cc:2925:51: warning: '&&' within '||' [-Wlogical-op-parentheses] if (cipher->auth_tag_len_ != kNoAuthTagLength && ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~ ../src/node_crypto.cc:2925:51: note: place parentheses around the '&&' expression to silence this warning if (cipher->auth_tag_len_ != kNoAuthTagLength && ^ This commit adds parenthesis around these expressions to silence the warnings. PR-URL: nodejs#20216 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
This commit removes the usage of the deprectated crypto.DEFAULT_ENCODING. PR-URL: nodejs#20221 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
PR-URL: nodejs#20225 Refs: nodejs#20039 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
PR-URL: nodejs#20225 Refs: nodejs#20039 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
The authTagLength option can now be used to produce GCM authentication tags with a specific length. PR-URL: nodejs#20235 Refs: nodejs#20039 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yihong Wang <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
For key wrapping algorithms, calling EVP_CipherUpdate() with null output could obtain the size for the ciphertext. Then use the returned size to allocate output buffer. Also add a test case to verify des3-wrap. Signed-off-by: Yihong Wang <[email protected]> PR-URL: nodejs#20370 Fixes: nodejs#19655 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Prefer custom smart pointers fitted to the OpenSSL data structures over more manual memory management and lots of `goto`s. PR-URL: nodejs#20238 Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
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 <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
CI is running at https://ci.nodejs.org/job/node-test-commit/18453/ (Edit: all tests passed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM backport-wise – thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This change allows users to restrict accepted GCM authentication tag lengths to a single value. Backport-PR-URL: #20706 PR-URL: #20039 Fixes: #17523 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yihong Wang <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
This commit extracts the common code from the Cipher/Cipheriv and Decipher/Decipheriv constructors into a separate function to avoid code duplication. Backport-PR-URL: #20706 PR-URL: #20164 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
This commit adds a function named addCipherPrototypeFunctions to avoid code duplication. Backport-PR-URL: #20706 PR-URL: #20164 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
This commit renames rsaPublic and removes the rsaPrivate function as the code in these two functions are identical. Backport-PR-URL: #20706 PR-URL: #20164 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Currently the following compiler warnings are issued by clang: ../src/node_crypto.cc:2801:56: warning: '&&' within '||' [-Wlogical-op-parentheses] return tag_len == 4 || tag_len == 8 || tag_len >= 12 && tag_len <= 16; ~~ ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~ ../src/node_crypto.cc:2801:56: note: place parentheses around the '&&' expression to silence this warning return tag_len == 4 || tag_len == 8 || tag_len >= 12 && tag_len <= 16; ^ ../src/node_crypto.cc:2925:51: warning: '&&' within '||' [-Wlogical-op-parentheses] if (cipher->auth_tag_len_ != kNoAuthTagLength && ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~ ../src/node_crypto.cc:2925:51: note: place parentheses around the '&&' expression to silence this warning if (cipher->auth_tag_len_ != kNoAuthTagLength && ^ This commit adds parenthesis around these expressions to silence the warnings. Backport-PR-URL: #20706 PR-URL: #20216 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
This commit removes the usage of the deprectated crypto.DEFAULT_ENCODING. Backport-PR-URL: #20706 PR-URL: #20221 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
Backport-PR-URL: #20706 PR-URL: #20225 Refs: #20039 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
Backport-PR-URL: #20706 PR-URL: #20225 Refs: #20039 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
The authTagLength option can now be used to produce GCM authentication tags with a specific length. Backport-PR-URL: #20706 PR-URL: #20235 Refs: #20039 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yihong Wang <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
For key wrapping algorithms, calling EVP_CipherUpdate() with null output could obtain the size for the ciphertext. Then use the returned size to allocate output buffer. Also add a test case to verify des3-wrap. Signed-off-by: Yihong Wang <[email protected]> Backport-PR-URL: #20706 PR-URL: #20370 Fixes: #19655 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Prefer custom smart pointers fitted to the OpenSSL data structures over more manual memory management and lots of `goto`s. Backport-PR-URL: #20706 PR-URL: #20238 Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
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 <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
Landed in b6ea5df...cecec46, thank you a lot! |
This is a manual backport of all commits listed in #20691. Full list of commits:
It would be great if @danbev, @yhwang, @addaleax could have a look at their respective commits (FYI, Anna's commit landed without conflicts).
Fixes: #20691