-
Notifications
You must be signed in to change notification settings - Fork 69
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 support for RSA-PSS signature verification #156
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This looks good to me. 🚀
I rebased this on #155 locally and confirmed there weren't any new clippy findings. It builds, lints, and tests green with that branch's fixes applied.
@@ -0,0 +1,7 @@ | |||
# Generating Test Certificates |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small nit: WDYT about replacing this .md
documenting the commands to run with a .sh
that runs them?
OID_PKCS1_SHA384WITHRSA, OID_PKCS1_SHA512WITHRSA, OID_SHA1_WITH_RSA, OID_SIG_ECDSA_WITH_SHA256, | ||
OID_SIG_ECDSA_WITH_SHA384, OID_SIG_ED25519, | ||
}; | ||
use std::convert::TryFrom; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought this might be superfluous but this crate is still using edition = "2018"
and requires the import 👍 No action req'd, just leaving a note for the next reviewer.
let params = params.as_ref()?; | ||
let params = RsaSsaPssParams::try_from(params).ok()?; | ||
let hash_algo = params.hash_algorithm_oid(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional nit: I think some of the intermediate bindings might not add much value here:
let params = params.as_ref()?; | |
let params = RsaSsaPssParams::try_from(params).ok()?; | |
let hash_algo = params.hash_algorithm_oid(); | |
let hash_algo = RsaSsaPssParams::try_from(params.as_ref()?) | |
.ok()? | |
.hash_algorithm_oid(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally speaking, they are not required, but I think they makes debugging easier, and sometimes also easier fights with the compiler types engine :)
Applied, thanks! |
No description provided.