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

Replace ring by argonautica, rand, hmac and sha2 #84

Closed
wants to merge 8 commits into from

Conversation

thespooler
Copy link
Contributor

This replaces all usages of ring by their RustCrypto equivalent, namely rand, hmac and sha2.
This PR also includes #81, replacing pbkdf2 by argonautica, so as to remove ring from Cargo.toml and libs.rs, completely removing any ring dependency.

@thespooler
Copy link
Contributor Author

thespooler commented Apr 12, 2020

Take a look at primitives/generator.rs. Since there is no equivalent to SecretKey in hmac, I did couple the key and hashing selection mechanism in a few places (new, ephemeral, extract and signature). Meaning that adding another option beside Hmac<Sha256> isn't as easy as the signature of Assertion::new(AssertionKind, key) might lead you to believe.
If you have any idea on how to neatly wrap this, let me know.

@HeroicKatora
Copy link
Owner

Would you mind making the changes independent of the bpkdf2 removal? I still haven't made up my mind on whether introducing a native dependency (libclang) is good, and want to find out why it is even necessary in the used library. Yes, this means that ring won't be fully removed by this PR but that's fine. Better to go slow and steady than rush.

oxide-auth/src/code_grant/extensions/pkce.rs Outdated Show resolved Hide resolved
oxide-auth/src/primitives/generator.rs Outdated Show resolved Hide resolved
oxide-auth/src/primitives/generator.rs Outdated Show resolved Hide resolved
oxide-auth/src/primitives/generator.rs Show resolved Hide resolved
oxide-auth/src/primitives/generator.rs Outdated Show resolved Hide resolved
oxide-auth/src/primitives/generator.rs Outdated Show resolved Hide resolved
@thespooler

This comment has been minimized.

@thespooler
Copy link
Contributor Author

I will open a new PR to split argonautica and the rest.

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

Successfully merging this pull request may close these issues.

2 participants