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

Attempt to fallback to pkcs8 if der fails #74

Closed
wants to merge 2 commits into from
Closed

Attempt to fallback to pkcs8 if der fails #74

wants to merge 2 commits into from

Conversation

Jake-Shadle
Copy link
Contributor

@Jake-Shadle Jake-Shadle commented Feb 4, 2019

While attempting to get mozilla/sccache#272 working since we use GCS, I found that the reason it was failing was because jsonwebtoken was only using the from_der constructor for RSAKeyPair, while GCP private keys are stored in pkcs8, and falling back to from_pkcs8 allows the key to be deserialized correctly.

I considered adding a more functions to instead allow someone to pass in the RSAKeyPair from outside the crate instead of this, but seemed like a bit of overkill when it could just be this one-liner, but let me know if you want me to rework it. 🙂

@jamwaffles
Copy link

I got this branch working in my (hacky) code by converting the key from PKCS8 to DER using rustls:

use rustls::pemfile;
use jsonwebtoken::encode;

let private_key = "/* service account JSON private key here */";

let mut certs = pemfile::pkcs8_private_keys(&mut private_key.as_bytes())?;

// Get the first cert, don't care if it errors
let key = certs.remove(0);

let token = encode(
    &header,
    &claims,
    &key.0
)

It seems the conversion step from PKCS8 to DER is still required. Is that the case or am I missing something?

@Jake-Shadle
Copy link
Contributor Author

Jake-Shadle commented Feb 5, 2019

That's odd, for reference I know for sure that this code

// Could also use the pem crate, but that seems overly complicated for just the specific
// case of GCP keys
let key_string = self.sa_key.private_key.splitn(5, "-----").nth(2).ok_or_else(|| "invalid key format")?;
// Skip the leading `\n`
let key_bytes = base64::decode_config(key_string[1..].as_bytes(), base64::MIME)?;

let auth_request_jwt = jwt::encode(
    &jwt::Header::new(jwt::Algorithm::RS256),
    &jwt_claims,
    &key_bytes,
)?;

works correctly when used against the code in this PR, as I go from 100% cache misses due to being unable to acquire a token to it functioning normally, but without the previous dependency on OpenSSL (just to do the conversion to DER).

@Keats Keats changed the base branch from master to next March 22, 2019 08:38
@Keats
Copy link
Owner

Keats commented Mar 22, 2019

Looks like signature::RSAKeyPair is not at the same place anymore in ring 0.14?

@Jake-Shadle Jake-Shadle mentioned this pull request May 15, 2019
@Keats
Copy link
Owner

Keats commented May 25, 2019

Closed in favour of #89

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