Skip to content

AssociatedAlgorithmIdentifier implementation #278

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.

Sign up for GitHub

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

Jump to bottom
Merged
tarcieri merged 2 commits into from
Apr 5, 2023
Merged

AssociatedAlgorithmIdentifier implementation #278

tarcieri merged 2 commits into from
Apr 5, 2023
+197 −15

Conversation

@lumag
Copy link
Contributor

@lumag lumag commented Mar 27, 2023

This is an implementation of a trait defined at RustCrypto/formats#954

@lumag lumag mentioned this pull request Mar 27, 2023
Closed
@rozbb
Copy link

rozbb commented Mar 27, 2023

Thanks! Could you possibly 1) make the ID string consts at the top of the file, and 2) put some links in the comments for where the strings come from?

tarcieri
tarcieri reviewed Mar 27, 2023
View reviewed changes
baloo
baloo reviewed Apr 1, 2023
View reviewed changes
@lumag lumag force-pushed the kai branch 2 times, most recently from 5d63864 to 3ca392b Compare April 2, 2023 22:55
lumag
lumag commented Apr 2, 2023
View reviewed changes
@lumag lumag force-pushed the kai branch 2 times, most recently from 5fb4f89 to ceb637c Compare April 3, 2023 14:30
@lumag lumag marked this pull request as ready for review April 3, 2023 14:31
@lumag
Copy link
Contributor Author

lumag commented Apr 3, 2023

@baloo this PR has support for PKCS1v15. Could you please check whether it works for you?

@baloo
Copy link
Member

baloo commented Apr 3, 2023

Yeah, I was missing the blanket impl for SignatureAlgorithmIdentifier. That works for me.

@baloo baloo mentioned this pull request Apr 3, 2023
Closed
baloo
baloo reviewed Apr 3, 2023
View reviewed changes
src/pkcs1v15.rs
#[cfg(feature = "sha2")]
impl RsaSignatureAssociatedOid for sha2::Sha224 {
const OID: ObjectIdentifier =
const_oid::ObjectIdentifier::new_unwrap("1.2.840.113549.1.1.14");
Copy link
Member

@baloo baloo Apr 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const_oid::ObjectIdentifier::new_unwrap("1.2.840.113549.1.1.14");
const_oid::db::rfc5912::SHA_224_WITH_RSA_ENCRYPTION

Copy link
Contributor Author

@lumag lumag Apr 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this was bugging me too. But I think the overall design now is not to add a dependency on const-oid/db.
@tarcieri ?

Copy link
Member

@tarcieri tarcieri Apr 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point we could probably consider using the db in this crate, though it does add to compile times a little bit.

@tarcieri
Copy link
Member

tarcieri commented Apr 3, 2023

Which of these (#278 vs #284) should I be reviewing? They have many of the same changes.

@lumag
Copy link
Contributor Author

lumag commented Apr 3, 2023

Which of these (#278 vs #284) should I be reviewing? They have many of the same changes.

This ons (#278). #284 was a spin-off w/o pkcs1v15 changes which were controvercial.

tarcieri
tarcieri reviewed Apr 3, 2023
View reviewed changes
@lumag lumag force-pushed the kai branch 2 times, most recently from c389a1e to 4170b19 Compare April 4, 2023 02:14
@lumag
Copy link
Contributor Author

lumag commented Apr 4, 2023

@tarcieri rsa crate doesn't have direct dependency on spki/der. Would it be possible to cut a dummy point release for pkcs8 to bump its internaly dependecy? Or we'd better declare rsa -> spki dependency?

@tarcieri
Copy link
Member

tarcieri commented Apr 4, 2023

@lumag already in progress: RustCrypto/formats#983

@lumag
Copy link
Contributor Author

lumag commented Apr 4, 2023

Thanks! didn't notice it.

@lumag
Copy link
Contributor Author

lumag commented Apr 5, 2023

Ugh. @tarcieri Another request, but now for pkcs1, to get RsaPssParams::SALT_LEN_DEFAULT

lumag added 2 commits April 5, 2023 04:48
@lumag
feat: implement associated AlgorithmIdentifier traits for PKCS1v15 an…
bd0c090 
…d PSS keys

Allow getting the algorithm identifiers for signing keys

Signed-off-by: Dmitry Baryshkov <[email protected]>
@lumag
pkcs1v15: implement SignatureAlgorithmIdentifier for PKCS1v15 structs
4cf8531 
Signed-off-by: Dmitry Baryshkov <[email protected]>
@tarcieri tarcieri merged commit cf90255 into RustCrypto:master Apr 5, 2023
@lumag lumag deleted the kai branch April 5, 2023 06:59
@lumag
Copy link
Contributor Author

lumag commented Apr 5, 2023

@tarcieri thank you!

@tarcieri tarcieri mentioned this pull request Apr 27, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tarcieri tarcieri tarcieri approved these changes

+1 more reviewer

@baloo baloo baloo left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lumag @rozbb @baloo @tarcieri