search

firstfloorsoftware / flutter_sodium

Flutter bindings for libsodium
BSD 3-Clause "New" or "Revised" License
102 stars 47 forks source link

KeyPair functions should return an object rather than a map #8

Closed rmtmckenzie closed 6 years ago

rmtmckenzie commented 6 years ago

Any functions returning a map would be better off returning an object with members. As it stands, the developer needs to figure out that the public key and secret key are returned under the keys 'pk' and 'sk', and perform their own check that those parameters are actually returned.

At the least the map should be documented and there should be constants for the various keys.

If this were to be changed it is a breaking change, but one that could be mitigated either completely or partially by making the class being returned either implement Map<String,Uint8List> or at least override operator [](String key).

The same goes for the functions that reutrn tx/rx keys and nonce+encrypted key or key+nonce.

rmtmckenzie commented 6 years ago

By the way - while I found this annoying, the library is otherwise excellent.

kozw commented 6 years ago

Fair point on documentation and constants for the various keys, this should be added.

The Sodium class contains a 1:1 mapping of Dart to native libsodium functions. My thinking here is that the API should closely match the libsodium signatures using core Dart classes and types. I wanted to leave out any opinionated, custom type from this low-level Sodium API.

To address your concern, a high-level opinionated API has been created, which does contain KeyPair, SessionKeys, and other return types for using libsodium in a Dart-friendly manner. See CryptoBox.generateKeyPair for example, it does return a typed KeyPair instance.

As far as breaking changes is concerned, this plugin is pre-v1, and I'm still toying with the API. I'm not too concerned about introducing breaking changes.

rmtmckenzie commented 6 years ago

Ah, didn't see that, I was just looking at the Sodium class for the most part and made the invalid assumption that the dart API and sodium API would be mostly incompatible (as the libsodium-jni library tends to be - I've used it in the past and have been less than impressed). I'll close this as the wrapper classes seem to resolve any concerns I may have had.

Documentation might be advisable but as an open-source project I 100% understand (I maintain a couple of small libraries myself and realize it's a lot of work - if I get a chance I will try to add a PR). Hopefully this issue will help someone else at least =).

And just out of curiosity have you looked at the LazySodium library as an alternative to libsodium-jni? Their java/android api is a whole lot better than libsodium-jni, although I don't think they compile to quite as many platforms right now and it seems as though you mostly use the native api anyways.

kozw commented 6 years ago

I did a few tests with LazySodium. I like what they're doing, but I got some incorrect results from various crypto functions. Decided to stick with libsodium_jni for now.

gurpreet- commented 5 years ago

Hi @kozw,

Maintainer of Lazysodium here. I have fixed a few bugs in the library related to password hashing. Perhaps that solves your incorrect results problem?

Please let me know if it works, you can find my email in my bio. Happy to discuss any problems with lazysodium you're having.

Sorry for resurrecting an old thread :)

kozw commented 5 years ago

Cool, I'll give it another try. Will keep you in the loop.