jsonwebtoken icon indicating copy to clipboard operation
jsonwebtoken copied to clipboard

support ring and aws-lc-rs (default = ring)

Open GlenDC opened this issue 1 year ago • 9 comments

Closes #389

Tested using:

cargo hack check --each-feature --no-dev-deps
cargo test
cargo test --features aws_lc_rs
cargo test --examples
cargo test --examples --features aws_lc_rs

Not ran it with wasm / windows enabled though. But I would hope the flags manage that?

Current approach does mean you always pull in ring, but it shouldn't compile if you are not using it. Do not know of an elegant way to do it differently. We can make ring a feature as well that is enabled by default, but it would make it akward for when you have both ring and aws-lc-rs enabled.

rustls fixes that by then just making a runtime error. Bit odd, I would prefer just it to fail then. So that's an option to still add that here, not sure if it's desired/required.

GlenDC avatar Aug 28 '24 11:08 GlenDC

All in all this PR is pretty innocent I think. Only caveat is that you do expose ErrorKind in your API.

This is AFAIK the only place where you do leak the crypto library used... If it wasn't for that it would be an invisible change... I think

GlenDC avatar Aug 28 '24 11:08 GlenDC

ah I thought you meant to do the PR having the 2 rust-crypt/aws-lc-rs backends and not ring/aws-lc

Keats avatar Aug 28 '24 20:08 Keats

That’s fine for me as well. But then we might need that to be merged first?

GlenDC avatar Aug 28 '24 20:08 GlenDC

I think I can create another branch and merge it on that branch if that's ok

Keats avatar Aug 29 '24 11:08 Keats

I've made a mess in https://github.com/Keats/jsonwebtoken/pull/404 commit wise i'll try to clean it when I have time but should be ok to start

Keats avatar Aug 29 '24 11:08 Keats

History on the other PR is fixed

Keats avatar Sep 02 '24 11:09 Keats

Hey @GlenDC,

There seems to be constant refrain of people needing different crypto backends for their use-case.

I recently opened an issue (#409), that suggests the use of traits to define a signer and verifier. If a user ever wanted a non-implemented crypto backend they would then be able to implement the traits for it and make use of it with the jsonwebtoken crate. There could be "standard" implementations for ring, aws-lc-rs, and RustCrypto (all behind appropriate feature flags).

I realize you have a working PR here, but would you mind if I implemented it with traits instead? I feel it would make the crate more flexible going forward. I am also open to other suggestions that you would prefer.

sidrubs avatar Sep 24 '24 00:09 sidrubs

@sidrubs @GlenDC I'll add my vote in this general direction. I could make use of an external signer and verifier implementation for building out external authentication in NATS. NATS expects a variant of ed25519-nkey even though it is essentially EdDSA, meaning it is not possible to use the rest of the functionality in this crate.

abalmos avatar Sep 24 '24 23:09 abalmos

Hey @GlenDC,

There seems to be constant refrain of people needing different crypto backends for their use-case.

I recently opened an issue (#409), that suggests the use of traits to define a signer and verifier. If a user ever wanted a non-implemented crypto backend they would then be able to implement the traits for it and make use of it with the jsonwebtoken crate. There could be "standard" implementations for ring, aws-lc-rs, and RustCrypto (all behind appropriate feature flags).

I realize you have a working PR here, but would you mind if I implemented it with traits instead? I feel it would make the crate more flexible going forward. I am also open to other suggestions that you would prefer.

I have gone ahead and made a draft PR with a POC on a possible method of implementing this.

sidrubs avatar Oct 03 '24 19:10 sidrubs

Implemented in another PR

Keats avatar Sep 29 '25 09:09 Keats