search

poowf / react-native-argon2

MIT License
5 stars 8 forks source link

add iterations and memoryCost as parameters #2

Closed tbrent closed 4 years ago

tbrent commented 4 years ago

Hi @zanechua!

Love that you put this wrapper together! You may already be aware of this, but this is the only truly native argon2 implementation out there! All the other options require picking up node dependencies. Using the signal java library makes a lot of sense (and so does argon2id over argon2).

I'm interested in doing relatively fast weak hashes as well as slower key derivations. I'm not a java developer myself, but I've gone ahead and opened this PR with an example of the kind of thing that would work for my use case. Of course, these should probably be optional parameters (do you use a map for that over the RN bridge?). Or, there could be multiple functions each with sensible presets.

Anyway, just wanted to start the conversation. My path of least resistance would normally be to fork this and take it fully in-house, but before doing that wanted to see if we should join efforts and build something more generic.

zanechua commented 4 years ago

Thanks for opening a PR! I was digging around and there wasn't an available solution. Stumbled upon the Argon2 libraries by SignalApp and realised they were native implementations so I took the plunge to see if I could create a wrapper package and that's why we have this!

This is definitely one of the things that I would like to change for this package, the configurable values of hashing are currently hardcoded and I agree that they should be optional with a predefined default. I need to go back and refer to the RN docs as this is still new to me and I'm not particularly familiar with the RN bridge either. I am hoping to add a third variable config and that would be where you specify the configurable values.

const config = {
    version: 13,
    iterations: 2,
    memoryCost: 2,
    parallelism: 1,
    hashLength: 32,
    type: 'Argon2id'
}
const result = await argon2(password, salt, config);
const { rawHash, encodedHash } = result;

If you had time to look at the source, you would notice that even though both libraries are made by SignalApp, their actual usage in iOS and Android codebases differ slightly, so there would be a need to do a mapper from the keys we use in the javascript side and then the native side.

On a slightly unrelated note, this library itself is under the MIT license but I believe that both the native modules are GPL 2.0.

tbrent commented 4 years ago

That is a shame regarding the SignalApp licenses. I've bumped the outstanding issue in their repo with the hope that it will be added, and be less restrictive than GPL 2.0.

The config approach seems right. Are you busy with other projects right now? I could take a stab at the workflow for developing native modules and open a real PR in a few weeks potentially.

tbrent commented 4 years ago

Looks like they've uploaded a GPL v3 license. That's too bad.

zanechua commented 4 years ago

I'm a little busy but I probably could spend time to check it out. I'm gonna have to update the package so that we can have full auto linking now that they've published their Pod.

I'll do some DevOps fixes like switch to semantic versioning and include some scripts to help with versioning and release management along with changelogs

tbrent commented 4 years ago

With the SignalApp license the way it is, I won't be able to use this for my usecase unfortunately. I'm using scrypt for the time being. It's not what I wanted, but it'll do.

If you determine the upstream licenses are an issue for the repo, this looks like a MIT license Java argon2 implementation. Not sure about the ios side of things though.

Best of luck with things! I'll certainly keep subscribed to this repo. (By the way, my team is specifically looking for mobile and web engineers right now, feel free to reach out if that's interesting.)

zanechua commented 4 years ago

Ah yeah. It seems that I have to make a licensing change to this package too if I want to continue distributing it and abide by the GPL license. I would prefer keeping this as MIT though. Might have to find another library to utilise. I was looking at the libsodium library as that has cross-platform support with Argon2 and someone has already made a react-native package for that.

https://www.npmjs.com/package/react-native-sodium

zanechua commented 4 years ago

Hey @tbrent.

As of https://github.com/poowf/react-native-argon2/pull/5 and 2.0.0 of the package, this PR has been implemented.

The package was also reworked with other implementations of argon2 that have a more permissible license. So I think this would fit your eligibility requirements.

Cheers.

tbrent commented 4 years ago

Awesome changes! I've updated to the 2.0.0 release and I'm using it on android successfully already. Thanks for making that effort. I expect this package is the currently the best method for key derivation in react-native, which is pretty awesome.