jsonwebtoken icon indicating copy to clipboard operation
jsonwebtoken copied to clipboard

Decoupled crypto backends

Open sidrubs opened this issue 1 year ago • 20 comments

Introduction

This is a POC at abstracting the signing and verifying algorithms behind the JwtSigner and JwtVerifier traits whilst not breaking the encode and decode public interfaces that people know and love.

This allows any crypto backend to be used, as long as it implements the JwtSigner and JwtVerifier traits. Allowing developers to provide their own backend dependent on their use case. This should also enable more flexibility for the jsonwebtoken crate to prepackage different backends (e.g. ring, aws_lc_rs, and RustCrypto).

Implementation

JwtEncoderand JwtDecoder builder style clients were added to the public API allow the developer to use the included algorithms or provide their own.

The original encode and decode public interfaces call this builder under-the-hood with a factory function choosing the correct algorithm backend.

**Note: ** This POC only implements the HMAC family of algorithms as I didn't want to spend a bunch of time on something that was not going to be accepted.

Advantages

  • The builder-style API provides compile time checks to the algorithm and secret's compatibility
  • A developer is able to provide their own backend so does not need to rely on the jsonwebtoken crate maintainers to add something for them.
  • Allows the crate to be flexible, thus hopefully reduce fragmentation in the ecosystem.

Disadvantages

  • I have tried to make this backwards compatible, but this introduces some little idiosyncrasies, especially on the validation side (making use of the original Validation struct). Everything works, it would just seem a little weird to someone looking at parts of the codebase without context.

Wrapping Up

I understand that this a a pretty big change architecturally to the jsonwebtoken crate, so fully understand if you don't want to go this route. I do feel, however, that it makes the project more flexible and maintainable going forward.

Any suggestions are welcome.

sidrubs avatar Oct 03 '24 18:10 sidrubs

I like the look of it but would like both RustCrypto and aws-lc support to make sure it works well. I'm not too sure about the builders but I guess we can discuss that later

Oh yeah sure thing. I'll add aws_lc_rs JwtSigner and JwtVerifier trait implementations (selected by a feature flag).

With regards to the builder, here was my thought process:

The JwtSigner and JwtVerifier traits have an algorithm method so that we know which algorithm the JwtSigner supports. I felt that most people will only ever use the a default header (with the correct algorithm), so why not automatically set it. But to be backwards compatible with the current encode function we would need to be able to set a fully custom header if needs be.

We could avoid the builder pattern by creating a function like the following and calling it from the current encode function (and initializing the signing_provider based on the header and the key). This however would not check the algorithm and secret at compile time, but that is the current behavior, so no problem.

Would this work better for you?

pub fn generic_encode<S: JwtSigner, T: Serialize>(
    signing_provider: S,
    header: &Header,
    claims: T,
    key: &EncodingKey
) -> Result<String> {
    ...
}

sidrubs avatar Oct 03 '24 22:10 sidrubs

I think we can have both the builder and the function?

Keats avatar Oct 04 '24 12:10 Keats

I think we can have both the builder and the function?

I just want to understand correctly as there are two functions I have talked about and I feel that we might be talking past each other.

  1. There are the original encode and decode functions that are currently part of the public API in the jsonwebtoken crate. In my POC, I kept these function signatures as they were (so as not to break current the current public API), but they actually called the builder under-the-hood.
  2. The second function came about when you mentioned that maybe the builder might not be ideal. So instead of using a builder, I suggested that we could use a function that allowed us to make use of arbitrary crypto backends. This would be called under-the-hood of the original encode and decode functions.

I both cases I was visualizing that we keep the same functionality of the original functions [1], but the internal business logic of these functions would either be provided by a builder or a function [2] style interface. So in addition to the original functions [1], we would also expose a method that an arbitrary backend could be used (this would either be a builder or another function).

Does this clear things up?

sidrubs avatar Oct 05 '24 12:10 sidrubs

I have added an aws-lc-rs implementation behind a feature flag (again, only HMAC for now until we agree on a way going forward).

Currently there are rust_crypto and aws_lc_rs feature flags (more can be added easily). They will cause a descriptive compile error if enabled at the same time, but I can change this behavior to a run-time selection if needed.

rust_crypto is the default so can be tested with the following.

cargo test

aws_lc_rs needs to be enabled, so can be tested with the following.

cargo test --no-default-features --features aws_lc_rs

sidrubs avatar Oct 05 '24 13:10 sidrubs

I both cases I was visualizing that we keep the same functionality of the original functions [1], but the internal business logic of these functions would either be provided by a builder or a function [2] style interface. So in addition to the original functions [1], we would also expose a method that an arbitrary backend could be used (this would either be a builder or another function). Does this clear things up?

I think that's fine, as long as we can keep the keys in once_cell/lazy_static and use them without having to clone anything

Keats avatar Oct 06 '24 19:10 Keats

It seems that this POC might be used. So I'll go ahead and add the rest of the algorithms, keeping in mind some of the comments above, and get everything up to scratch to pass CI.

Unless there are any other concerns?

sidrubs avatar Oct 07 '24 23:10 sidrubs

Go ahead, we can play with the DX once it's implemented. Maybe use macros to implement them so it's easy to make changes if needed rather than copy/paste

Keats avatar Oct 10 '24 11:10 Keats

I have converted the builder to _encode and _decode functions that are called by the original public facing encode and decode functions. I think it does simplify things a bit.

I am about half done adding the RSA algorithms functionality.

I did however recently notice that the original signand verify functions in the crypto module are pub. The "new" trait based architecture does not make use of them, should I recreate their functionality using the traits based approach so that we don't break the current public facing API? Or would you envision this being a major version bump and removing those functions?

sidrubs avatar Oct 20 '24 21:10 sidrubs

They are used by some people who only care about the crypto primitives so I would re-implement them and keep them pub

Keats avatar Oct 21 '24 10:10 Keats

Hi @sidrubs will you have time to finish that PR?

Keats avatar Dec 10 '24 12:12 Keats

Hi @sidrubs will you have time to finish that PR?

Hey @Keats we have slightly changed our focus, so am not going to finish the PR anytime soon :-/

sidrubs avatar Dec 10 '24 17:12 sidrubs

@Keats

**Note: ** This POC only implements the HMAC family of algorithms as I didn't want to spend a bunch of time on something that was not going to be accepted.

Would this scope still be acceptable or do you need them all to be implemented in 1 PR?

GlenDC avatar Jan 10 '25 22:01 GlenDC

I think it would be easier to implement all of it in one PR

Keats avatar Jan 11 '25 11:01 Keats

I think it would be easier to implement all of it in one PR

Perfect. Thanks for verifying!

GlenDC avatar Jan 11 '25 11:01 GlenDC

@GlenDC any updates?

Keats avatar Mar 07 '25 10:03 Keats

The situation with the ring crate is a bit worrying from the outside: in the last days it went to unmaintained status, changed hands, changed hands back.

So big :+1: for the direction this pr is going into. Personally I will be using the aws-lc backend because rustls uses it anyways.

kamulos avatar Mar 07 '25 10:03 kamulos

@GlenDC any updates?

It's still on my backlog, someone can do it if they have the bandwidth, otherwise I'll get around it eventually.

GlenDC avatar Mar 07 '25 11:03 GlenDC

Hello @Keats what is the status of the PR? That is to say what would be needed in order to merge it? It would seem like hmac and rsa support was added for aws-lc-rs but only hmac support was added for rust-crypto. And in order to help, would it be better to open a new PR basing the work on this? Thanks.

pauliyobo avatar Apr 09 '25 17:04 pauliyobo

The status is what you see in this PR. This is definitely the way forward and if people want to do PRs on that PR to finish it, please go ahead!

Keats avatar Apr 13 '25 11:04 Keats

I've just picked up this branch and implemented the remaining algorithms for both AWS-LC & RustCrypto: https://github.com/Keats/jsonwebtoken/compare/master...sulami:jsonwebtoken:decoupled-crypto-backends?expand=1

I've also added back the crypto::sign and crypto::verify functions, built on top of the JWT machinery.

I don't know how you'd want to go about this, I've opened a new PR under #428.

sulami avatar May 10 '25 12:05 sulami