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

sha2 version update #1162

Merged
merged 1 commit into from
Jun 24, 2017
Merged

sha2 version update #1162

merged 1 commit into from
Jun 24, 2017

Conversation

newpavlov
Copy link
Contributor

Version 0.6 of sha2 crate is independent from rust-crypto and should be a bit faster compared to v0.1.2.

@brson brson merged commit 5ca0691 into rust-lang:master Jun 24, 2017
@brson
Copy link
Contributor

brson commented Jun 24, 2017

Thanks @newpavlov !

@lilianmoraru
Copy link

lilianmoraru commented Jun 28, 2017

@brson What do you think about switching the archives(.sha256 -> .sha512) and this to SHA-512 for collision resistance and speed improvements?
This comment(and the entire issue actually) might be of interest: mozilla/sccache#108 (comment)

Note that ring still uses a lot of assembly and C code.

@Diggsey
Copy link
Contributor

Diggsey commented Jun 28, 2017

@lilianmoraru The hash is calculated concurrently with the download so it's much more likely to be bottle-necked on network speed, and in terms of collision resistance there's not enough of a difference between sha256 and sha512 to make it worth-while IMO:

sha256 will require significant breakthroughs in order to achieve even a practical collision attack, and if such an attack does become practical, it is more likely to be caused by a weakness in the algorithm (which would then affect sha512 too) than by the digest length being too short.

@lilianmoraru
Copy link

lilianmoraru commented Jun 28, 2017

@Diggsey It just seems like a win-win(considering that we are in the 64-bit world for a good while - most users would be positively affected by such a simple change).

@Diggsey
Copy link
Contributor

Diggsey commented Jun 28, 2017

But neither of them are wins for our users. This change will neither speed up rustup, nor improve security in a measurable way.

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.

4 participants