Skip to content

Implement RSASSA-PSS (PS) signing and verifying#21

Merged
omsmith merged 6 commits intoauth0:masterfrom
csprl:master
Jan 25, 2019
Merged

Implement RSASSA-PSS (PS) signing and verifying#21
omsmith merged 6 commits intoauth0:masterfrom
csprl:master

Conversation

@csprl
Copy link
Copy Markdown
Contributor

@csprl csprl commented Apr 24, 2018

This would be useful for auth0/node-jws#47

@omsmith
Copy link
Copy Markdown
Contributor

omsmith commented May 23, 2018

Hi @csprl. Thanks for the contribution.

Would it be possible for you to add some tests to the test suite for this?

@csprl
Copy link
Copy Markdown
Contributor Author

csprl commented May 23, 2018

Hey, I added tests (based on the existing ones for "regular" RSA). It does however seem like support for PSS padding wasn't added until Node.js v6. Function arguments for Verify.verify() were also changed in v7, but back again to the v6 behaviour in v8 which is why the v7 build is failing. Considering these Node.js versions aren't officially supported anymore it might make sense to drop support for them.

@omsmith
Copy link
Copy Markdown
Contributor

omsmith commented May 23, 2018

Perfect thanks. I'll update the support matrix on travis later today. I'm still going to keep the old old versions, as I don't want to do a major bump at the moment. I think what we can do is

  • add this as semver-minor
  • check the node version in tests (I think we're doing this elsewhere already)
  • add a note to the readme that PS* is Node 6+

@csprl
Copy link
Copy Markdown
Contributor Author

csprl commented May 23, 2018

Sounds good. I added checks for Node version and pinpointed 6.12.0 to be the exact release when PSS support was added. I also added a notice in README.md.

@omsmith
Copy link
Copy Markdown
Contributor

omsmith commented May 23, 2018

That's awesome, thanks for being so responsive to feedback and sorry that I took a while to hop on this!

I'll aim to get this out tonight, but I realized that #26 may have a couple issues with https://github.com/crypto-browserify/browserify-sign that I'll want to address first (perhaps by just reverting my change for now in order to get yours out).

@csprl
Copy link
Copy Markdown
Contributor Author

csprl commented May 23, 2018

No worries at all! You absolutely don't have to rush this change, but getting PS support in node-jws in the near future would be great.

@nikolaymatrosov
Copy link
Copy Markdown

It would be great if this PR was merged, so all dependent packages obtain the ability to use PS* algorithms.

@omsmith omsmith merged commit f9dcd6a into auth0:master Jan 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants