firstfloorsoftware / flutter_sodium

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

Inconsistencies with cryptoPwhashStr across libraries #46

Closed vishnukvmd closed 3 years ago

vishnukvmd commented 3 years ago

When I call cryptoPwhashStr, the resulting output seems to always be of length 128 and is returned padded with \u000s.

I don't see this behavior documented. All I can find is:

out must be large enough to hold crypto_pwhash_STRBYTES bytes, but the actual output string may be shorter.

Which makes me wonder if this check is necessary.

This extra padding seems to be breaking counterparts like libsodium.js which rejects calls to crypto_pwhash_str_verify for hashes that are generated from the flutter_sodium library. Everything works fine if I trim out all but the last \u000.

Apologies if I've missed something here. If I haven't, it would be great if we could have consistency across the multiple implementations of libsodium.

Thanks again for everything!

kozw commented 3 years ago

You are right, those padded \u000s shouldn't be there. And the verify function string length check is invalid.

The use of null-terminated strings in Dart does raise some issues, so here's a backwards incompatible design proposal.

The high-level PasswordHash class will not change and continue to use Dart Strings. Internally it will convert Dart strings to a null terminated Uint8List.

If you want to use the flutter_sodium hashes with other libraries, you should use the core Sodium functions. If you stick with Dart only you can use PasswordHash.

Thoughts?

vishnukvmd commented 3 years ago

Since I'm fairly new to libsodium, I'm not sure if other consumers would have an expectation that the data types of function arguments and the return value should resemble the ones documented.

If not, please go ahead. As someone who prefers unassuming bytes over strings, your proposal sounds great.

kozw commented 3 years ago

Thanks for the feedback. I'm fairly happy with replacing the string return value with bytes, this is fine for the low level Sodium functions.

flutter_sodium 0.1.8 is now available with the changes

vishnukvmd commented 3 years ago

Works flawlessly, thank you!