FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[v6.x backport] crypto: add sign/verify support for RSASSA-PSS by tniessen · Pull Request #14376 · nodejs/node · GitHub

/ node Public

[v6.x backport] crypto: add sign/verify support for RSASSA-PSS - #14376

Closed
tniessen wants to merge 6 commits into
nodejs:v6.x-stagingfrom
tniessen:backport-11705-to-v6.x
Closed

[v6.x backport] crypto: add sign/verify support for RSASSA-PSS#14376
tniessen wants to merge 6 commits into
nodejs:v6.x-stagingfrom
tniessen:backport-11705-to-v6.x

Conversation

Copy link
Copy Markdown
Member

Backport of #11705 to v6.x @nodejs/lts

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

tniessen added crypto Issues and PRs related to the crypto subsystem. v6.x labels Jul 19, 2017
tniessen requested a review from gibfahn July 19, 2017 20:49
nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. v6.x labels Jul 19, 2017
jasnell added the semver-minor PRs that contain new features and should be released in the next minor version. label Jul 19, 2017

gibfahn commented Jul 20, 2017

Copy link
Copy Markdown
Member

@tniessen could you pull in #12405 as well?

Copy link
Copy Markdown
Member Author

@gibfahn Do you want a separate PR, separate commit, or can I just adopt and force-push that one-character fix?

Copy link
Copy Markdown
Member Author

Copy link
Copy Markdown
Contributor

same PR, you can amend or cherry-pick, as you wish. I'd probably amend.

tniessen force-pushed the backport-11705-to-v6.x branch from ef0394b to da2b6b8 Compare July 20, 2017 22:21

gibfahn commented Jul 21, 2017

Copy link
Copy Markdown
Member

Separate commit (cherry-pick it onto your backport-11705-to-v6.x branch). Please don't amend, that makes working out what has been backported harder.

tniessen force-pushed the backport-11705-to-v6.x branch from da2b6b8 to e1feaf2 Compare July 21, 2017 07:56

Copy link
Copy Markdown
Member Author

@gibfahn Done.

gibfahn 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This LGTM, although I'd appreciate a quick look from @nodejs/crypto

Copy link
Copy Markdown
Contributor

LGTM, this basically lands clean, the conflicts are tiny, and just due to sign/verify being given names in master, and being anonymous in v6.x.

Copy link
Copy Markdown
Contributor

This would have landed clean if #8993 had been landed, I'll comment there.

Comment thread doc/api/crypto.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

key: {string} - PEM encoded public key (required)

MylesBorins force-pushed the v6.x-staging branch 5 times, most recently from 22fa484 to f9419c2 Compare August 16, 2017 18:42

Copy link
Copy Markdown
Member Author

@gibfahn Status on this? Should I rebase?

gibfahn commented Aug 17, 2017

Copy link
Copy Markdown
Member

See #11705 (comment), this will have to wait till we review semver-minor backports before 6.12.0. I wouldn't bother rebasing yet.

You can subscribe to nodejs/Release#228 if you want to know when LTS will decide on backporting.

MylesBorins force-pushed the v6.x-staging branch 2 times, most recently from aaf4e13 to 31f572c Compare September 5, 2017 16:50

Copy link
Copy Markdown
Contributor

We've agreed to land this.
Can you please rebase

Copy link
Copy Markdown
Contributor

@MylesBorins #8993 will be needed as well to land clean

Copy link
Copy Markdown
Contributor

@tniessen do you want to take another pass at this now that #8993 (comment) has landed its dependent PR?

If not, ping me, and I will.

targos and others added 6 commits September 20, 2017 15:10
PR-URL: nodejs#14107
Fixes: nodejs#14105
Reviewed-By: Refael Ackermann <refack@gmail.com>
Use try/catch to instead of threw.

PR-URL: nodejs#10534
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Simplify the BSD list by defining OPENSSL_BSD if using a matching
BSD platform.
Add NetBSD to the list and update documentation.

PR-URL: nodejs#14313
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
As an example, `curl https://nodejs.org/dist/v8.4.0/SHASUM256.txt` will
return a 404 right now.

PR-URL: nodejs#15101
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Create one file for testing each function of the path module.
Keep general error tests and tests for constant properties in
test-path.js.

PR-URL: nodejs#15093
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Adds support for the PSS padding scheme. Until now, the sign/verify
functions used the old EVP_Sign*/EVP_Verify* OpenSSL API, making it
impossible to change the padding scheme. Fixed by first computing the
message digest and then signing/verifying with a custom EVP_PKEY_CTX,
allowing us to specify options such as the padding scheme and the PSS
salt length.

Fixes: nodejs#1127
PR-URL: nodejs#11705
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
tniessen force-pushed the backport-11705-to-v6.x branch from e1feaf2 to 61d1d20 Compare September 20, 2017 21:53

Copy link
Copy Markdown
Member Author

Hope I did not miss anything. CI: https://ci.nodejs.org/job/node-test-pull-request/10172/

Copy link
Copy Markdown
Contributor

landed in 1213f38

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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. lib / src Issues and PRs related to general changes in the lib or src directory. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.


Back | FazBrowse Home | New Git URL