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 support for RSA-PSS signature verification #156

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

nthuemmel-scontain
Copy link
Contributor

No description provided.

Copy link
Collaborator

@cpu cpu left a 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
Copy link
Collaborator

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;
Copy link
Collaborator

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.

Comment on lines +100 to +102
let params = params.as_ref()?;
let params = RsaSsaPssParams::try_from(params).ok()?;
let hash_algo = params.hash_algorithm_oid();
Copy link
Collaborator

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:

Suggested change
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();

Copy link
Member

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 :)

@chifflier chifflier merged commit a1ce88a into rusticata:master Apr 4, 2024
6 of 10 checks passed
@chifflier
Copy link
Member

Applied, thanks!

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.

3 participants