Skip to content

dsa: capture error #941

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

Merged
merged 1 commit into from
Apr 23, 2025
Merged

Conversation

baloo
Copy link
Member

@baloo baloo commented Apr 19, 2025

No description provided.

@baloo baloo marked this pull request as draft April 19, 2025 16:10
@baloo baloo force-pushed the baloo/dsa/capture-error branch 2 times, most recently from 149d490 to 895e437 Compare April 23, 2025 17:32
@baloo baloo marked this pull request as ready for review April 23, 2025 17:32
@baloo baloo marked this pull request as draft April 23, 2025 17:53
@baloo baloo force-pushed the baloo/dsa/capture-error branch from 895e437 to 40003e0 Compare April 23, 2025 20:25
@baloo baloo marked this pull request as ready for review April 23, 2025 20:25
@baloo baloo force-pushed the baloo/dsa/capture-error branch from 40003e0 to 5f7c6ad Compare April 23, 2025 20:41
ecdsa/src/der.rs Outdated
assert!(
Signature::from_der(&[
der::Tag::Sequence.into(),
der::Tag::Sequence.number().value().try_into().unwrap(),
Copy link
Member Author

Choose a reason for hiding this comment

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

Not exactly a fan.

Maybe we should have a impl TryFrom<der::Tag> for u8 or something.

Copy link
Member Author

@baloo baloo Apr 23, 2025

Choose a reason for hiding this comment

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

and we lose the constructed bit.

Copy link
Member

Choose a reason for hiding this comment

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

There used to be a From impl, but I guess it was removed when tags were changed to support longer than 1-byte lengths

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, and we lost the Constructed bit at this moment so you have to rebuild it manually. I don't know.
I think I'm fine with just hardcoding the value here with a comment.

Copy link
Member

@tarcieri tarcieri Apr 23, 2025

Choose a reason for hiding this comment

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

The old From impl and a hypothetical TryFrom impl should preserve the constructed bit, but hardcoding it seems fine for now

@baloo baloo force-pushed the baloo/dsa/capture-error branch from 5f7c6ad to c5854a9 Compare April 23, 2025 21:18
@tarcieri tarcieri merged commit afe73a9 into RustCrypto:master Apr 23, 2025
75 checks passed
@baloo baloo deleted the baloo/dsa/capture-error branch April 23, 2025 21:37
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