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

incorrect hash in android 4.3.1 and lower #8

Closed
rubensayshi opened this issue Oct 2, 2015 · 15 comments
Closed

incorrect hash in android 4.3.1 and lower #8

rubensayshi opened this issue Oct 2, 2015 · 15 comments
Assignees
Labels

Comments

@rubensayshi
Copy link

when using createHash('sha256') I'm getting to hash 'synchronous write' I'm getting
1e0a06bb7cb5399c5389c14ff182b700834f9a00bcfd5b0067e12bdde24a5300 instead of 1e0a06bb7cb5399c5389c14ff182b748834f9aedbcfd5bb467e12bdde24a5370.

using crypto-js I'm getting the correct hash.

I'm using this repo to test it with inside an Android 4.3.1 emulator (Nexus 4 Intel Atom x64):
https://github.com/rubensayshi/browserify-cryptojs-android

using node v0.10 or v0.12 to browserify and run cordova doesn't make a difference

@rubensayshi
Copy link
Author

idk if it's me or not ...
if anyone has any ideas of things to test without wanting to spend time actually coding/running it then just tell me and I'll do it ;-)

@calvinmetcalf
Copy link
Contributor

what the actual code your using to hash the value, when I do

crypto.createHash('sha256').update('synchronous write').digest('hex');

I get

1e0a06bb7cb5399c5389c14ff182b748834f9aedbcfd5bb467e12bdde24a5370

@rubensayshi
Copy link
Author

ooops sorry, yea, was trying difference message, updated original post now.

the odd (or maybe not) thing is that the result is always very similar (when I manually compare I can barely see the difference).

using crypto.createHash('sha256').update('synchronous write').digest('hex') also results in 1e0a06bb7cb5399c5389c14ff182b700834f9a00bcfd5b0067e12bdde24a5300 for me btw

@calvinmetcalf
Copy link
Contributor

yeah looks to be an issue with the sufix of the hash

@calvinmetcalf
Copy link
Contributor

(in the browser createHash and crypto.createHash are actually the same)

@calvinmetcalf calvinmetcalf self-assigned this Oct 2, 2015
@calvinmetcalf
Copy link
Contributor

I hopefully will be able to get to this this weekend

@dcousens dcousens added the bug label Oct 2, 2015
@dcousens
Copy link
Member

dcousens commented Oct 2, 2015

1e0a06bb7cb5399c5389c14ff182b7 00 834f9a 00 bcfd5b 00 67e12bdde24a53 00 
1e0a06bb7cb5399c5389c14ff182b7 48 834f9a ed bcfd5b b4 67e12bdde24a53 70
1e0a06 bb 7cb539 9c 5389c1 4f f182b7 00 834f9a 00 bcfd5b 00 67e12b dd e24a53 00 
1e0a06 bb 7cb539 9c 5389c1 4f f182b7 48 834f9a ed bcfd5b b4 67e12b dd e24a53 70

It isn't just the suffix, seems to be on the 4th byte (though, not consistently).
Most likely an issue with writeUInt32BE or similar.

@calvinmetcalf
Copy link
Contributor

ok able to confirm that the issue is not present on android 4.4 (which as it turns out is the oldest devise I own) installing the emulator now

@calvinmetcalf
Copy link
Contributor

ok so testing on an emulator running 4.3.3 (modifing the test page to print to the dom) I am unable to reproduce the problem
screen shot 2015-10-03 at 11 25 30 am

@calvinmetcalf
Copy link
Contributor

so that being said I am giving another go at setting up browser tests for the whole module family (crypto-browserify/crypto-browserify#139) which should catch this.

@dcousens
Copy link
Member

dcousens commented Oct 4, 2015

@rubensayshi do you have a more in depth reproduction environment? Can't reproduce either.

@calvinmetcalf
Copy link
Contributor

@rubensayshi
Copy link
Author

I was testing on 4.3.1, maybe 4.3.3 is already version where it's fixed ...
You could also try 4.2.2, same failure

@rubensayshi rubensayshi changed the title incorrect hash in android4.3 and lower incorrect hash in android 4.3.1 and lower Oct 5, 2015
@calvinmetcalf
Copy link
Contributor

it seems to be a issue with the browserify buffer implementation as I've been able to write a failing test there.

@dcousens
Copy link
Member

dcousens commented Oct 6, 2015

See feross/buffer#81, closing in favour of that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants