Skip to content
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

feat: Add PSS signing and verifying #46

Merged
merged 4 commits into from
May 14, 2019
Merged

feat: Add PSS signing and verifying #46

merged 4 commits into from
May 14, 2019

Conversation

Andrew-Lees11
Copy link
Contributor

Description

This pull request adds usePSS flag to sign and verify. This allows you to use RSA-PSS for signing and verifying. If the flag is set to true the functions will use RSA_PKCS1_PSS_PADDING, a salt the length of the digest and MGF1 mask generation function to sign or verify.

Motivation and Context

This change is required because "In general, RSA-PSS should be used as a replacement for RSA-PKCS#1 v1.5".

It also allows us to support PS256 and PS384 signing/verifying with JWTs as requested by SwiftJWT issue #55

How Has This Been Tested?

Tests have been added covering the new algorithms and the produced signatures have been tested against JWT.io both that one we create is valid and we can verify a valid one created by JWT.io.

Checklist:

  • I have submitted a CLA form
  • If applicable, I have updated the documentation accordingly.
  • If applicable, I have added tests to cover my changes.

@Andrew-Lees11 Andrew-Lees11 requested review from billabt and djones6 May 7, 2019 15:28
Copy link
Contributor

@djones6 djones6 left a comment

Choose a reason for hiding this comment

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

This looks great!

Have you checked for memory leaks?

@Andrew-Lees11
Copy link
Contributor Author

@billabt Would you be able to have a look at this and if you are happy squash merge and tag the change?

@Andrew-Lees11
Copy link
Contributor Author

We tested for Memory leaks and there aren't any in the new PSS paths. We did find one in make keys and have added a Deallocate to the getPEMString function which solves this.

Copy link
Collaborator

@billabt billabt left a comment

Choose a reason for hiding this comment

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

👍

@billabt billabt merged commit 41cce36 into master May 14, 2019
@billabt billabt deleted the pssRSA branch May 14, 2019 12:54
@billabt
Copy link
Collaborator

billabt commented May 14, 2019

@Andrew-Lees11 Had to change your available statements for using PSS. The correct iOS version is 11.0 not 10. It showed up here when I built the package for iOS something we don't do for CI. I fixed it after the merge. Just an FYI.

@billabt
Copy link
Collaborator

billabt commented May 14, 2019

@Andrew-Lees11 Had to make another change to the tests. They were generating warning on Swift 4.0.3.

@Andrew-Lees11
Copy link
Contributor Author

PSS is only supported from Swift 4.1 onwards which must be iOS 11. Thank you for spotting and fixing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants