@Keats, the use case is not wider file format support than .DER. This is about handling RFC 7517 - JSON Web Key (JWK). Here is Google's JWKS document, served from https://www.googleapis.com/oauth2/v3/certs:

{
  "keys": [
    {
      "kid": "7978a91347261a291bd71dcab4a464be7d279666",
      "e": "AQAB",
      "kty": "RSA",
      "alg": "RS256",
      "n": "sFlU5LpHUtYIm7B27iiu7c4ZPZk7ULUNmFdMVsTmYJxJqQBKUIKU9ozwF6TlUsECmYUMLpQhX_iHuaZRcpG2YiG7jbmi9HMlonIXX7uUe7PIf8rNHhveX_VI7ZpwPTnab3_7ciy_o8ZFde6KNltkx_DLRO6hXf6z6ow1APFIIriaNlF8niz5cy0fPIv0e_Z2p13Sz3mnAACjBKZGPw2X9GWh5XpRoDEQBcibXpeLuA7ti8zLZuH-9ybXOoou699fr4QHFxUkcd_8fFqmzO5PKnlOnJZ0gtuXCCYYc9XPX-WSqlqbGNMZy0Giu2wHbNbeWdepkgVlGuJonTnMx4gLuQ",
      "use": "sig"
    },
    {
      "kid": "8aad66bdefc1b43d8db27e65e2e2ef301879d3e8",
      "e": "AQAB",
      "kty": "RSA",
      "alg": "RS256",
      "n": "vvAaaSpfr934Qx0ioFiWsopq7UCfLNn0zjYVbq4bvUcGSXU9kowYmQArR7WlIkjk1moffla0UV75QRaQPATva1oD5xQnnW-20haeMWTSsMgUHoN0Np9AD8ffPz-DfMJBOHIo4REL1BFFS33HSZgPl0hxJ-5UScqr4lW1JMy5XGeRho30dnmKTpakU1Oc35hFYKSea_O2SXfmbqiAkWlWkilEzgHq4pzVWiDZe4ZgfMdD4vqkSNrO_PkBFBT1mnBJztQ1h4v1jvUW-zeYYwIcPTaOX-xOTiGH9uQkcNPpe5pBrIZJqR5VNrDl_bJOmvVlhhXZSn4fkxA8kyQcZXGaTw",
      "use": "sig"
    }
  ]
}

We only have access to a base64url encoded modulus and exponent. In order for me to use this library with JWKS, I need to be able to decode tokens using the modulus and exponent tuple.

Another use case that I'm after is taking an RSA private key file from a Google Cloud service account JSON definition. It would be great to see RSA support added instead of having to extract the key using jq and converting it to a DER file. Does this PR address using PEM keys with the encode() method?

@Keats I addressed your requested changes.

To be sure I don't forget anything, RSA keys can be set:

• in DER format: currently the case
• with (n, e) format: this PR
• in PKCS 8: Attempt to fallback to pkcs8 if der fails #74

@jamwaffles I believe #74 fixes your issue no? Looks like it's for Google Cloud services as well

No, PKCS #8 is typically base64 encoded in the standard .pem format:

-----BEGIN ENCRYPTED PRIVATE KEY-----
MIIBrzBJBgkqhkiG9w0BBQ0wPDAbBgkqhkiG9w0BBQwwDgQImQO8S8BJYNACAggA
MB0GCWCGSAFlAwQBKgQQ398SY1Y6moXTJCO0PSahKgSCAWDeobyqIkAb9XmxjMmi
hABtlIJBsybBymdIrtPjtRBTmz+ga40KFNfKgTrtHO/3qf0wSHpWmKlQotRh6Ufk
0VBh4QjbcNFQLzqJqblW4E3v853PK1G4OpQNpFLDLaPZLIyzxWOom9c9GXNm+ddG
LbdeQRsPoolIdL61lYB505K/SXJCpemb1RCHO/dzsp/kRyLMQNsWiaJABkSyskcr
eDJBZWOGQ/WJKl1CMHC8XgjqvmpXXas47G5sMSgFs+NUqVSkMSrsWMa+XkH/oT/x
P8ze1v0RDu0AIqaxdZhZ389h09BKFvCAFnLKK0tadIRkZHtNahVWnFUks5EP3C1k
2cQQtWBkaZnRrEkB3H0/ty++WB0owHe7Pd9GKSnTMIo8gmQzT2dfZP3+flUFHTBs
RZ9L8UWp2zt5hNDtc82hyNs70SETaSsaiygYNbBGlVAWVR9Mp8SMNYr1kdeGRgc3
7r5E
-----END ENCRYPTED PRIVATE KEY-----

That's RFC 5208.

This is to handle RFC 7517.

@Keats Is there a blocker in this? This library isn't usable in a JWKS environment (like those configured by Istio, or Google's JWKS infrastructure, etc.) without the ability to decode based solely on the exponent and modulus.

Not anything blocking this PR in particular, but the next version is blocked on #76 to avoid having to publish multiple major versions

@Keats this should be semver compatible as a "minor" bump, barring API changes right?

It would work as a minor bump but then if the plan in #76 goes ahead would be removed very soon after.
Hopefully I can find some time this weekend to work on #76 to have a clearer idea

@Keats We don't want to pass a PEM encoded or JWK encoded text representation of a binary key as an argument of jsonwebtoken::decode function and decode the key each time the function is called, right?

I'm trying to understand if this feature somehow relates or blocks the feature with ES256 support.

We don't want to pass a PEM encoded or JWK encoded text representation of a binary key as an argument of jsonwebtoken::decode function and decode the key each time the function is called, right?

Have a look at the first post of #76, I think providing a function for each usecase is going to be easier in the end as we can see it's going to be too messy trying to make everything fit in a single function as we can already see.

Have a look at the first post of #76

Ok, I see. Having decoder types seems as a good idea.

Can someone look at the conflicts with after the ES support has been merged in next?

@AaronFriel and @Keats I am using Aaron's branch as i'm working on a keycloak adaptor for a project i'm on. I am able to get things to work, and would like to see this merged. If needed i can look at resolving the merge conflicts.

@mann-ed do you want to take a look first?

@AaronFriel Yes, i'll take a look at it tomorrow. I'll get back to you by Friday April 5th as i have other obligations i need to attend to in the next few days.

@AaronFriel Ok, i have it updated, and i created a pull request to your branch. I don't know if that is what you wanted me to do.
AaronFriel#1

Let me know if you have time to review, or if there is something else i should do.

@mann-ed @AaronFriel I'll pull your changes directly next week unless there is an issue

Close button is too close...

@mann-ed @Keats Sorry, I haven't been doing the best at watching my GitHub notifications. I merged the PR from @mann-ed into my branch, which updated this pull request.

No worries @AaronFriel
Have you seen #89? It looks like a very sane approach to all the various key formats without duplicating all the functions

@Keats would it be acceptable to add a key format for "KeyFormat::RSAModulusExponentPair"? Or what would you call it?

The name seems good to me. What do you think of this approach of using a KeyFormat enum?

I'm not sure, to be honest, because it looks like it's assumed that the Key k must be a [u8], but that's not the case here. I'd have to create an (arbitrary) binary encoding of the modulus and exponent just to unwrap those later on, which seems silly.

I think the trait structure needs reworking, but I haven't dug into it enough to make a specific comment on how. verify taking a &[u8] is still not correct for my use case. I have an exponent and a modulus, which are already correctly encoded byte vectors. Concatenating them is not correct, and it seems very silly to serialize them to a byte array to pass to verify, then deserialize inside the method to recover the key pair.

It seems like instead maybe verify should be generic in a type K that is an instance of Key, and maybe Key has an associated type Data as well as a method that uses it. For PKCS8 and DER, Key::Data = &[u8], for my use case, Key::Data = (&[u8], &[u8])?

It seems like instead maybe verify should be generic in a type K that is an instance of Key, and maybe Key has an associated type Data as well as a method that uses it. For PKCS8 and DER, Key::Data = &[u8], for my use case, Key::Data = (&[u8], &[u8])?

Sounds like a good idea.

I'm thinking an enum would be better than a trait in the end, any reason to prefer a trait other than adding one being a breaking change?

cc @Jake-Shadle

@AaronFriel I pushed an enum version to #91
This way we can have a Key::ModulusExponent { n, e } enum member. I've made a temporary verify_ring_rsa in crypto.rs to extract the bytes from the enum which you can change/remove depending on what's easier.

Another thing I'm thinking is to maybe keep your sign_rsa/verify_rsa since that allows restricting the Key type at the type level rather than having an error at runtime.
Eg you can't verify a token HS512 with a modulus/exponent so it would be nice to error at compilation.

I've integrated that PR in 10105af

Can anyone try the next branch to see if that works for them?

Closing in favour of #91