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

Add RustCrypto feature #21

Open
martinjlowm opened this issue Mar 22, 2022 · 7 comments
Open

Add RustCrypto feature #21

martinjlowm opened this issue Mar 22, 2022 · 7 comments

Comments

@martinjlowm
Copy link

I have a PoC of this crate without the need for OpenSSL and instead use rsa, x509-parser and sha2 to do request signing and verification.

I'd imagine, continuing to support openssl would be beneficial for backwards compatibility - how does a "rust-crypto" feature flag sound? I'm just missing some type-safety when it comes to the Certificate-type change in that branch, but other than that I'd be happy to contribute the changes and open a PR.

PoC: https://github.com/martinjlowm/samael/tree/rust-native

@D1plo1d
Copy link

D1plo1d commented Apr 27, 2022

If a rust-crypto feature is added please continue to support OpenSSL - US organisations that require FIPS certification cannot use rust-crypto (and will not be able to for the foreseeable future AFAIK).

@njaremko
Copy link
Owner

Hi @D1plo1d!

Thanks for your input, we'll definitely continue to support openssl.

@njaremko
Copy link
Owner

Hi @martinjlowm, if you can contribute an equivalent implementation based on safe rust libraries, I'd be happy to merge it.

I'd have to put some thought into defaults and feature flags, but I'm not opposed to it.

@njaremko
Copy link
Owner

Hey @martinjlowm, if you do decide to pick up this work, I wanted to let you know that I've updated the build instructions in the README to lay the groundwork for reproducible builds.

Right now I'm just using it for reproducible dev environments and CI builds, which I hope will make contributing less of a pain in the butt 😜

@martinjlowm
Copy link
Author

martinjlowm commented Jun 13, 2022

Got a branch that we use in production (complete replacement tho) at https://github.com/martinjlowm/samael/tree/bb - although it lacks proper parsing of certificates (right now they are just parsed as strings) - I'll try to find some time to clean it up. In fact, I just happened to be working with this crate again just now and intend to use it against AWS SSO.

In terms of feature flags, I'll come up with something - it should be fairly straightforward to change whichever path We decide to go :)

@martinjlowm martinjlowm changed the title Replace OpenSSL with RustCrypto crates Add RustCrypto feature Sep 5, 2022
@martinjlowm
Copy link
Author

I've been working on #26 - but I kind of hit a road block due to the lifetime declaration of x509_cert's Certificate. There's no owned version as of this moment, so the way certificates are parsed (

.and_then(|decoded| openssl::x509::X509::from_der(&decoded).ok())
) a dereference to an owned type may be necessary. My Rust skills falls a bit short here - I'm open to any tricks that would enable us to have some proper types instead of just storing Vec<u8> on the Service Provider.

Also, error types (in the PR) are just thrown in there without much thought so that still needs some work.

@njaremko
Copy link
Owner

njaremko commented Sep 6, 2022

Hi @martinjlowm, do you have a branch I could look at with the issue you're describing?

Edit: just saw the branch xD. Disregard.

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

No branches or pull requests

3 participants