RSA Signing & Verification Support

99designs / http-signatures-php

Sign and verify PSR-7 HTTP messages in PHP.

MIT License

44 stars 32 forks source link

RSA Signing & Verification Support #35

Closed liamdennehy closed 5 years ago

liamdennehy commented 6 years ago

RSA signing is largely similar to hmac, both requiring a secret (shared in the hmac case), but the difference in key material for signing is significant. Verification is very different, so not trivial to extend.

This PR adds:

Explicitly typing RSA private keys as distinct from secrets (hmac)
Importing keys and certificates into the KeyStore
Detecting whether to use the RSA signing flow depending on the value of algorithm field, and selecting the right hash algorithm (could be replaced on detection of supplied key?)
Detecting whether to use the RSA verification flow depending on the type of key retrieved with keyId and use a X.509 certificate instead of a secret/privateKey.

liamdennehy commented 6 years ago

OK, that's it. I think this is ready for a thorough review, after handling importing RSA keys & certificates for singing and verification respectively, with a good amount unit tests for all sorts of variations.

liamdennehy commented 6 years ago

Any further comments here?

mtibben commented 5 years ago

Can you rebase and remove the merge commits?

mtibben commented 5 years ago

Hey @liamdennehy I'm not sure you're following my request regarding the rebase, so will try be more clear :) We follow a "feature branch" or "Github Flow" workflow. So:

We don't want merge commits. Rebase them out
We want your branch to be from the most recent master commit. Rebase on master

liamdennehy commented 5 years ago

@mtibben There's a strong possibility I have no idea what I'm doing ☺️ Thanks for the clarification.

liamdennehy commented 5 years ago

@mtibben I'm going to overhaul the Key class to make the flow easier to track, eg

    static function hasX509Certificate($object)

...to explicitly test if an object has a certificate instead of try..catch blocks all over the place.

This will also make a good base to extend the class for #18 (Key Rotation). Will ping you when it's ready.

liamdennehy commented 5 years ago

@mtibben Hopefully much clearer Key class in b9ae276fe63a6bfe3d27fa4d2eb6fa77437746fb, and quicker than I thought.

liamdennehy commented 5 years ago

Any further questions, or updates on open conversations in this PR?

lahaxearnaud commented 5 years ago

Hello,

Can you please merge and tag this PR ?

Regards,

Arnaud

liamdennehy commented 5 years ago

Apologies, accidentally deleted the branch for this PR in my source.

However, I have integrated this feature in my own project along with a number of others, and published this in packagist. Documentation for the entire library is published at Read the Docs: http-signatures-php - incomplete but being expanded regularly.

liamdennehy commented 5 years ago

Closing PR due to inactivity.