search

stefanpenner / hash-for-dep

7 stars 11 forks source link

Replace MD5 with SHA1 #24

Closed baderbuddy closed 8 years ago

baderbuddy commented 8 years ago

Could you replace the project's hashing algorithm from MD5 to something like SHA? Several organizations have trouble with MD5 and there's a very limited cost to upgrading to SHA.

stefanpenner commented 8 years ago

Several organizations have trouble with MD5

I am aware that MD5 is considered broken for use in any sort of sensitive scenario. As malicious attacks have been demonstrated (in some constrained scenarios). But unintentional collisions are quite unlikely. It's like 1 in 2^128 http://www.miketaylor.org.uk/tech/law.html

As it stands this library is not designed for use in sensitive scenarios, rather to generate a fast and reasonable cache-key representing a module and its dependencies.

MD5 was chosen, as it is more performant then SHA1 and the original use-case was not for any sensitive scenario.

If this library is now being used for other use-cases, I would love to hear about them and facilitate if possible.

baderbuddy commented 8 years ago

It's not being used for a sensitive scenario, so I agree it's kind of silly :smile: with Node it actually looks like sha1 might perform slightly better than MD5, nodejs-hash-performances

I figured it might be easier to get you to use SHA1 instead of fighting the holy war with my security people about the use of MD5 :)

If you decide not to, then holy war it is.

Thanks for the quick response.

stefanpenner commented 8 years ago

@baderbuddy if the performance is comparable, I would absolutely accept a PR using sha1 instead.

baderbuddy commented 8 years ago

PR has been created.