Changes to support ACME, including JWS

[andrewbaxter]

358

[gregparsons]

One test here. Works great. Thanks @andrewbaxter!

[andrewbaxter]

Awesome, and thanks for the comments! Sorry, I didn't quite get to this today. I'll try to do it tomorrow.

[andrewbaxter]

It would also need a README update

I added a (very simple) example to the readme, let me know if there's other additions you think would be good.

[andrewbaxter]

My (limited) real world testing worked.

[Keats]

I will have to read the specs and review the PR properly as I don't really know/use JWK/JWS myself. That's going to take some time but if more people can try that would be great

[randomairborne]

Can I support the work on this somehow? I don't know if I have the technical aptitude to help review, but if I can be of other assistance I would love to.

[Keats]

Ideally maybe we could get one single PR (@andrewbaxter can change the target of other PRs to this one) so we can point at a single point for people to test

[andrewbaxter]

Keep them separate but just change the target? Or should I combine them all? Aside from loading urlsafe base64 hmacs I don't think I have any others incoming

[Keats]

change the target, we review them and then we can do a big review of the whole thing on this PR

[andrewbaxter]

@randomairborne do you have any use cases you could try it with? I looked at various acme libraries and they all seemed to be doing their own jwt stuff (and didn't support EAB). I ended up hacking it into Poem's acme implementation, but I have some reservations about that

[andrewbaxter]

Okay, great!

[andrewbaxter]

@Keats I'm not sure I can re-target MRs in this repo to branches in my fork... sorry, my github ineptitude on full display. AFAICT it only allows me to select branches in this repo.

Edit: I'll try stacking them and recreating the MRs in my fork.

[randomairborne]

I'm attempting to write a new ACME library 😅. If this has a somewhat functional public API, I'll start using it.

[andrewbaxter]

Sorry didn't get to this yesterday. I'll do it now

[andrewbaxter]

Okay I made https://github.com/andrewbaxter/fork-jsonwebtoken/pull/1 https://github.com/andrewbaxter/fork-jsonwebtoken/pull/2 https://github.com/andrewbaxter/fork-jsonwebtoken/pull/3

targeted at this branch, and https://github.com/andrewbaxter/fork-jsonwebtoken/pull/4 with them all merged together if you want to try stuff out.

I think this is everything needed for ACME, but I haven't read through all the different flows in ACME so it's possible some stuff is still missing.

I added you @Keats as a collaborator in case you wanted to merge them into this one. I can close the other MRs in this repo if that would be good.

[Keats]

Ok if there are any JWS/ACME users, can they try that branch?

[Keats]

Has anyone tried it?

[andrewbaxter]

Aside from me I assume... :sweat_smile:

[fuch1m]

I tried to test decoding of a Jws.

I added the following line in my Cargo.toml

jsonwebtoken = { git = "https://github.com/andrewbaxter/fork-jsonwebtoken.git", branch = "branch-integration" }

But I don't know how to transform the string slice containing the Jws into a Jws type which is required by the decode_jws function. I can't hand over just the string slice. This is possible with the regular decode function for jwt.

let jws_token="eyJhbGciOiJSUzI1NiIsIng1dCI6IlptWXlNRFprTlRoa05Ua3lZalkyWW1VM1pEQTFNRE5oWkdSbFl6TTJNamhrTTJWaE1EUTJZUW8ifQ.eyJtZXRhZGF0YS12ZXJzaW9uIjoiMC4wLjEiLCJuYW1lIjoiY29tLmJyLWF1dG9tYXRpb24uYXV0b21hdGlvbnJ1bnRpbWUiLCJ2ZXJzaW9uIjoiNi4wLjAiLCJhcnRpZmFjdHMiOlt7ImZpbGUiOiJhci5lZWI0NGY5ZGRjNWY0MzM2YTEyODJmOGNlZWYwYWI5MzQ0NDJkZTdiNTc0NzJjZGI1YjA1OGZkMDkyNmIzNDc1LnRhciIsInNoYTI1NiI6ImM1NmI3ZGFiYmM3YWE3MzBlZWFiMDc2NjhiZGNiZDdlM2Q0MDg1NTA0N2NhOWEwY2MxYmZlZDIzYTI0ODYxMTEiLCJhcmNoaXRlY3R1cmUiOiJ4ODZfNjQifSx7ImZpbGUiOiJhci4wMTMzMDc5MTg2YzZhYTYwMmNjYjA5ODA5MTU1MzNmMDNiMDFhMDA3MDFiMWYzOWMwMGYyYmYxZjczMDYxZTM2LnRhciIsInNoYTI1NiI6IjU3OGNjODYwYmFjMGM4MTM5MmViNzAyNjJiMmVjZjNlNmM1MjFhYTY0N2JjMjBkNjcyOWNhNTE4NDZhZWEwNWUiLCJhcmNoaXRlY3R1cmUiOiJhYXJjaDY0In0seyJmaWxlIjoiY29tcG9zZTEueW1sIiwic2hhMjU2IjoiMjY2NjU5Njg2NzY3MDY5YmY2NDI5NmNiNGJkYjkxNDZkNzM3Mzg0MTM4NzVjYzEyNTcyNDgwN2M1NGZhNTFhMiIsImFyY2hpdGVjdHVyZSI6Ing4Nl82NCJ9LHsiZmlsZSI6ImNvbXBvc2UyLnltbCIsInNoYTI1NiI6IjE5YTljNTdkMWQwNjVlNGZjNTdkODMxY2IzNmI4Y2U1ZTU3ODk0MDFkM2Q1N2JhNTUyZTJkNjM0YTg4NjkwYjAiLCJhcmNoaXRlY3R1cmUiOiJhYXJjaDY0In1dfQ.PMuAg_M-IqiGY8s6KeiUra30mxUwulgrGkI4YaEMIwsOUE-cI2oqOxWkV4OUJ9NNVnUGWuuBO6E4VFCsabWL3fsIPyl_7mrGToNPAka7OxaadQXhFhZwzaGuBi879-pWVm5NNtjjCO1inDw6dDBLVj89HWeBQxgEgVwYWA1P6lbCLFrstlb_2Qp8yY44KAQDV3bVudmlZn_VaxKlsFSc8_8DOPaeFwbWjqG1XT4WrKAiKFnQCqnzaSyH4O-0bPiZ7rq04mfA6-Nm7ZNnrST3dbG6v9f5Ku9qHu6Nfs0uMGIclLN8Krzk365IuCnLp_W3zL7bTwJ3VGMj6AHjH5U4Jw";
let public_key = "-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA5JX/bCT4jVCtzesSMHd8
Wu7ksIs3tFhw/8oYB+4cLik5sjvc1HiWWA50UK7TIERIIuXq0SAulV9xbHLrZjZn
iU2hbN83+7LySfzQxQRwfIVLFRsUqcm23ZZFMCHxuzEfwl/m+61AkDzARmoUVCyh
LXlomjKzN45RZZqSHVkvpJWidqKcVCtdjsGnVZeeX4TG6QI/hbzRm4y6oxLwriwJ
vl2bz+lJUN57XxhtCgYtdhUa8PfD+5mC26xlLvMfru3Ic1+NKgcZi1g5dmOMQOpY
PnLPN2VKbA9Nx+RxlQK1C+R/mmEM6lGZbSqxHUa3WuB725gVNN+tHTUfsprjax7b
PQIDAQAB
-----END PUBLIC KEY-----";

let decoding_key = DecodingKey::from_rsa_pem(public_key.as_bytes()).unwrap_or_else(|err| {
    println!("Error creating decoding key: {}", err);
    panic!("Decoding key creation failed");
});
let validation = Validation::new(Algorithm::RS256);
let decoded_token: TokenData<serde_json::Value> = match decode_jws(&jws_token, &decoding_key, &validation) {
    Ok(token) => token,
    Err(err) => {
        println!("Error decoding token: {}", err);
        panic!("Token decoding failed");
    }
};

[andrewbaxter]

Ah yeah, so it looks like you have a JWS in the "JWS Compact Serialization" form which is .-concatenated and base64 encoded.

There are two representations discussed in the RFC: JWS JSON Serialization and JWS Compact Serialization.

The JWS support I added here doesn't provide a convenience method for the compact serialization representation. It implements Serialize and Deserialize for the "JWS JSON Serialization" representation. In ACME the JSON is embedded in another structure and de/serialized together with that.

In order to use this with the Compact Serialization I think you'd need to split it by . and put the parts in a JWS object first... something like:

let mut parts = jws_token.split(".");
let jws = JWS<serde_json::Value>{
   protected: parts.next()?,
   payload: parts.next()?,
   signature: parts.next()?,
   _pd: Default::default(),
};
decode_jws(&jws, &decoding_key, &validation)...

Assuming that works, convenience methods might be a good addition but I'm not in a great position to switch gears and add it atm.

[fuch1m]

Thank you, I will try it in the next days!

[fuch1m]

Decoding my JWS in compact form works now for me. I had to disable also the default validation of "exp" via validation.set_required_spec_claims(&[""]); Thanks!

let jws_token="eyJhbGciOiJSUzI1NiIsIng1dCI6IlptWXlNRFprTlRoa05Ua3lZalkyWW1VM1pEQTFNRE5oWkdSbFl6TTJNamhrTTJWaE1EUTJZUW8ifQ.eyJtZXRhZGF0YS12ZXJzaW9uIjoiMC4wLjEiLCJuYW1lIjoiY29tLmJyLWF1dG9tYXRpb24uYXV0b21hdGlvbnJ1bnRpbWUiLCJ2ZXJzaW9uIjoiNi4wLjAiLCJhcnRpZmFjdHMiOlt7ImZpbGUiOiJhci5lZWI0NGY5ZGRjNWY0MzM2YTEyODJmOGNlZWYwYWI5MzQ0NDJkZTdiNTc0NzJjZGI1YjA1OGZkMDkyNmIzNDc1LnRhciIsInNoYTI1NiI6ImM1NmI3ZGFiYmM3YWE3MzBlZWFiMDc2NjhiZGNiZDdlM2Q0MDg1NTA0N2NhOWEwY2MxYmZlZDIzYTI0ODYxMTEiLCJhcmNoaXRlY3R1cmUiOiJ4ODZfNjQifSx7ImZpbGUiOiJhci4wMTMzMDc5MTg2YzZhYTYwMmNjYjA5ODA5MTU1MzNmMDNiMDFhMDA3MDFiMWYzOWMwMGYyYmYxZjczMDYxZTM2LnRhciIsInNoYTI1NiI6IjU3OGNjODYwYmFjMGM4MTM5MmViNzAyNjJiMmVjZjNlNmM1MjFhYTY0N2JjMjBkNjcyOWNhNTE4NDZhZWEwNWUiLCJhcmNoaXRlY3R1cmUiOiJhYXJjaDY0In0seyJmaWxlIjoiY29tcG9zZTEueW1sIiwic2hhMjU2IjoiMjY2NjU5Njg2NzY3MDY5YmY2NDI5NmNiNGJkYjkxNDZkNzM3Mzg0MTM4NzVjYzEyNTcyNDgwN2M1NGZhNTFhMiIsImFyY2hpdGVjdHVyZSI6Ing4Nl82NCJ9LHsiZmlsZSI6ImNvbXBvc2UyLnltbCIsInNoYTI1NiI6IjE5YTljNTdkMWQwNjVlNGZjNTdkODMxY2IzNmI4Y2U1ZTU3ODk0MDFkM2Q1N2JhNTUyZTJkNjM0YTg4NjkwYjAiLCJhcmNoaXRlY3R1cmUiOiJhYXJjaDY0In1dfQ.PMuAg_M-IqiGY8s6KeiUra30mxUwulgrGkI4YaEMIwsOUE-cI2oqOxWkV4OUJ9NNVnUGWuuBO6E4VFCsabWL3fsIPyl_7mrGToNPAka7OxaadQXhFhZwzaGuBi879-pWVm5NNtjjCO1inDw6dDBLVj89HWeBQxgEgVwYWA1P6lbCLFrstlb_2Qp8yY44KAQDV3bVudmlZn_VaxKlsFSc8_8DOPaeFwbWjqG1XT4WrKAiKFnQCqnzaSyH4O-0bPiZ7rq04mfA6-Nm7ZNnrST3dbG6v9f5Ku9qHu6Nfs0uMGIclLN8Krzk365IuCnLp_W3zL7bTwJ3VGMj6AHjH5U4Jw";
let public_key = "-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA5JX/bCT4jVCtzesSMHd8
Wu7ksIs3tFhw/8oYB+4cLik5sjvc1HiWWA50UK7TIERIIuXq0SAulV9xbHLrZjZn
iU2hbN83+7LySfzQxQRwfIVLFRsUqcm23ZZFMCHxuzEfwl/m+61AkDzARmoUVCyh
LXlomjKzN45RZZqSHVkvpJWidqKcVCtdjsGnVZeeX4TG6QI/hbzRm4y6oxLwriwJ
vl2bz+lJUN57XxhtCgYtdhUa8PfD+5mC26xlLvMfru3Ic1+NKgcZi1g5dmOMQOpY
PnLPN2VKbA9Nx+RxlQK1C+R/mmEM6lGZbSqxHUa3WuB725gVNN+tHTUfsprjax7b
PQIDAQAB
-----END PUBLIC KEY-----";

let decoding_key = DecodingKey::from_rsa_pem(public_key.as_bytes()).unwrap_or_else(|err| {
    println!("Error creating decoding key: {}", err);
    panic!("Decoding key creation failed");
});
let validation = Validation::new(Algorithm::RS256);
validation.set_required_spec_claims(&[""]);

let mut parts = jws_token.split(".");
let jws: Jws<serde_json::Value> = Jws {
    protected: parts.next().unwrap().to_string(),
    payload: parts.next().unwrap().to_string(),
    signature: parts.next().unwrap().to_string(),
    _pd: Default::default(),
};

let decoded_token: TokenData<serde_json::Value> = match decode_jws(&jws, &decoding_key, &validation) {
    Ok(token) => token,
    Err(err) => {
        println!("Error decoding token: {}", err);
        panic!("Token decoding failed");
    }
};

[andrewbaxter]

Oh excellent!

[WANG-lp]

any updates on this MR?

[Keats]

Just need some usage feedback, I'm not knowledgeable about it so I can't really have a good opinion on it

[andrewbaxter]

I was thinking about this, and I wonder if it wouldn't be fine to merge as is? It's new functionality, so it won't break existing users, and if there are design or implementation issues that make it unusable making changes (even ones that are nominally breaking) it won't affect anyone because there wouldn't be any users in the first place.

The main risks are that this 1. misses some use cases and the design prevents modification to allow those use cases, but that wouldn't necessarily be found by light usage feedback anyway or 2. isn't ergonomic, but I think it's a simple enough interface that that's not a large risk.