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

Fail better in extra::crypto::sha1::add_input() #2346

Closed
catamorphism opened this issue May 3, 2012 · 10 comments
Closed

Fail better in extra::crypto::sha1::add_input() #2346

catamorphism opened this issue May 3, 2012 · 10 comments
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup.

Comments

@catamorphism
Copy link
Contributor

There's a bare fail and a FIXME saying "need better failure mode".

@nikomatsakis
Copy link
Contributor

This should... probably use a condition? I don't really understand the conditions that would lead to this failure. Is this an assertion? @graydon?

@sonwow
Copy link
Contributor

sonwow commented Apr 29, 2013

Even if it is fixed to use a condition, there is a problem about using conditions from another crate. (#5446)

@bblum
Copy link
Contributor

bblum commented Jul 3, 2013

nominating production-ready

@graydon
Copy link
Contributor

graydon commented Jul 11, 2013

just a bug, removing milestone/nomination.

@omasanori
Copy link
Contributor

@nikomatsakis This is essentially an overflow check. len_low and len_high emulates u64 and are stored the length of a message in bits. The result will be broken when overflow occurs. FYI.

@huonw
Copy link
Member

huonw commented Jul 27, 2013

It seems that this is a fundamental limitation of the SHA1 algorithm: it cannot hash more than 2^64 - 1 bits. Personally, I think just changing it to something like fail!("SHA1 does not support hashing more than 2^64 - 1 bits") is a decent solution.

(cc @DaGenix.)

@DaGenix
Copy link

DaGenix commented Jul 28, 2013

I agree.

The only part of Sha1 that can't handle messages that size is the bit count field which is a 64 bit field. MD5 is defined to allow that bit count to wrap around. Sha1, on the other hand, is simply not defined for inputs bigger than 64 bits. So, one option is to simply ignore that part of the spec and just silently allow that bit count to wrap around. That seems like a bad idea, though, since its non-compliant with the spec. I don't think there really is anything that the caller can do to correct the issue - if the caller needs a Sha1 digest for an input that's too big, that seems like an application / protocol issue and task failure seems appropriate to me.

@emberian
Copy link
Member

emberian commented Nov 4, 2013

Closing; said crypto stuff has been removed.

@huonw
Copy link
Member

huonw commented Dec 23, 2013

(Carrying out the long-delayed closure @cmr threatened...)

@huonw huonw closed this as completed Dec 23, 2013
@emberian
Copy link
Member

Heh, oops.

bors added a commit to rust-lang-ci/rust that referenced this issue Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup.
Projects
None yet
Development

No branches or pull requests

9 participants