samael
samael copied to clipboard
njaremko
Add RustCrypto feature
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
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).
Hi @D1plo1d!
Thanks for your input, we'll definitely continue to support openssl.
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.
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 😜
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 :)
I've been working on https://github.com/njaremko/samael/pull/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 (https://github.com/njaremko/samael/blob/842148e49be22b9d817bf458e92cce4f70ace673/src/service_provider/mod.rs#L514) 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.
Hi @martinjlowm, do you have a branch I could look at with the issue you're describing?
Edit: just saw the branch xD. Disregard.