Skip to content
This repository has been archived by the owner on Dec 30, 2020. It is now read-only.

Not expected hash #6

Closed
homakov opened this issue May 13, 2017 · 12 comments
Closed

Not expected hash #6

homakov opened this issue May 13, 2017 · 12 comments

Comments

@homakov
Copy link

homakov commented May 13, 2017

hex returned is not equal one from node's scrypt or scrypt-async-js libs, for default or any other opts https://gist.github.com/homakov/a3d9ae0a2d83b45e25cc297030dd5522

Is there any example that would return same value for the above libraries and for this plugin? I simply pass password/salt as bare strings UTF8 encoded

@ggozad
Copy link
Contributor

ggozad commented May 13, 2017

On certain android devices (have had this with an s8 for example) for unknown reasons libscrypt gives us wrong results. @demetris-manikas had tried to figure this out with the authors of libscrypt unsuccessfully iirc.
For Crypho we ended up testing first with fast values and then using the sjcl implementation if it failed.
I would definitely appreciate more eyes on this and somebody figuring out, but cannot be of help at the moment.

@homakov
Copy link
Author

homakov commented May 13, 2017

Can confirm that emulator is working fine... My suggestion would be to put a warning on README for the time being - s8 is quite a popular phone.

I will try to figure out.

@demetris-manikas
Copy link
Contributor

The truth is that the problem was solved by using code from another source. Details here. technion/libscrypt#39
@ggozad should we revisit this?

@ggozad
Copy link
Contributor

ggozad commented May 15, 2017

Feel free to fix, I have no time to dig into this at the moment but can test and will accept a PR.

@demetris-manikas
Copy link
Contributor

@ggozad There is a branch in this project called FEAT-tarsnap which I believe you already have tested that solves the problem exactly as discussed in technion/libscrypt#39. Feel free to retest!
@homakov Could you test it as well?

@homakov
Copy link
Author

homakov commented May 16, 2017

Have you tried to run it with phonegap? Because this never runs second alert:


phonegap create itest "itest" "itest"
cd itest
phonegap platform add ios
phonegap plugin add cordova-plugin-scrypt

insert this in index.js

setTimeout(function(){
    alert(window.plugins.scrypt)
    window.plugins.scrypt(
        function (res) { alert(key); },
        function (err) { alert(err) },
        'password', 'salt', {N: 16384}
    )

},3000)

phonegap run ios

It simply does not call success nor error callback in emulator. Will try plain cordova later.

@demetris-manikas
Copy link
Contributor

Your code has a mistake. In the success callback "key" is not defined so change it to "res" and report back. Thanks

@homakov
Copy link
Author

homakov commented May 16, 2017

Sorry, my original code had no mistake and test was wrong. Anyway it still not working:


setTimeout(function(){
    alert(window.plugins.scrypt)
    window.plugins.scrypt(
        function (res) { alert(1); },
        function (err) { alert(2) },
        'password', 'salt', {N: 16384}
    )
},3000)

@homakov
Copy link
Author

homakov commented May 16, 2017

And just tested same sequence with "cordova" instead of phonegap and it worked.

I can easily switch to cordova but i wonder why phonegap is failing. I have no idea how to debug this one.

@homakov
Copy link
Author

homakov commented May 17, 2017

I've merged FEAT-tarsnap and added plugin from my repo cordova plugin add https://github.com/homakov/cordova-plugin-scrypt.git and I get 82e08... on both emulator and my s8. Good results! I will do more tests for my app (efficient scrypt is critical part of it) and thank you for your work and saving me ton of time!

@ggozad
Copy link
Contributor

ggozad commented May 17, 2017

Hey, I've been sick the last few days :( Tomorrow I should be good to go to the office again and will look further into this. I need to review the changes on tarsnap but it seems the issue was the void * casts which should be resolved now.

@ggozad
Copy link
Contributor

ggozad commented May 19, 2017

Fixed in 2.1.1

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

No branches or pull requests

3 participants