browserify / sha.js

Streamable SHA hashes in pure javascript
Other
288 stars 60 forks source link

Add SHA #20

Closed knjcode closed 9 years ago

knjcode commented 9 years ago

This pull request adds an implementation for SHA (aka SHA-0) into sha.js.

SHA source code is much the same as SHA-1 of the same repository. The difference between SHA and SHA-1 is just a bitwise rotate left operation was added.

I want to use SHA algorithm into crypto-browserify.

dominictarr commented 9 years ago

Why would you want to use sha-0? it has known flaws, which was why it was rapidly replaced with sha1... and theoretical flaws have been discovered in sha1, and it's not recommended to use sha1 in new software.

Is it to be compatible with a legacy system?

knjcode commented 9 years ago

Yes. Compatibility with legacy systems is one of the purpose to use sha-0. In addition, I want to use npm packages for the cryptographic research purpose in our browser.

dcousens commented 9 years ago

I understand the use case, but I feel like its so special cased it could almost just be its own repository. I'll leave this to your good judgement @dominictarr.

knjcode commented 9 years ago

Isn't it the main purpose of crypto-browserify/sha.js to improve the compatibility with Node.js? In Node.js, we can still use sha-0 hash. (Of course, need OpenSSL)

It is true that sha-0 has known flows. However I simply want to use crypto-browserify in the same way as Node.js crypto.

dcousens commented 9 years ago

In that case, then I'm for it. Didn't realize node had support for it. On 4 Apr 2015 02:54, "Kenji Doi" notifications@github.com wrote:

Isn't it the main purpose of crypto-browserify/sha.js to improve the compatibility with Node.js? In Node.js, we can still use sha-0 hash. (Of course, need OpenSSL)

It is true that sha-0 has known flows. However I simply want to use crypto-browserify in the same way as Node.js crypto.

— Reply to this email directly or view it on GitHub https://github.com/crypto-browserify/sha.js/pull/20#issuecomment-89336070 .

knjcode commented 9 years ago

quote from Node.js v0.12.2 Manual & Documentation

var hashes = crypto.getHashes(); console.log(hashes); // ['sha', 'sha1', 'sha1WithRSAEncryption', ...]

To use sha (sha-0) in Node.js, specify sha into createHash method.

In Node.js

// get sha0 hash of 'hello'
crypto.createHash('sha').update('hello').digest('hex')
// work fine -> 'ac62a630ca850b4ea07eda664eaecf9480843152'

In browserify (with sha.js)

// get sha0 hash of 'hello'
crypto.createHash('sha').update('hello').digest('hex')
// raise an error -> 'Error: sha is not supported (we accept pull requests)'

So I thought pull request is welcome.

dominictarr commented 9 years ago

Although I am not happy to merge this, because I don't want anyone using insecure crypto, the stated aim of crypto-browserify is that it should be compatible with node's crypto, so the correct thing is to merge this.

dcousens commented 9 years ago

Exactly my opinion. Maybe a warning? On 5 Apr 2015 15:40, "Dominic Tarr" notifications@github.com wrote:

Although I am not happy to merge this, because I don't want anyone using insecure crypto, the stated aim of crypto-browserify is that it should be compatible with node's crypto, so the correct thing is to merge this.

— Reply to this email directly or view it on GitHub https://github.com/crypto-browserify/sha.js/pull/20#issuecomment-89720415 .

dominictarr commented 9 years ago

@dcousens I think we should just document it.

dominictarr commented 9 years ago

merged into 2.4.0

dominictarr commented 9 years ago

added documentation to discourage people from using sha0/1 in new systems.

knjcode commented 9 years ago

I agree with adding documentation. Thanks.