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

Null characters in return buffer #101

Closed
rflaperuta opened this issue Nov 30, 2016 · 5 comments
Closed

Null characters in return buffer #101

rflaperuta opened this issue Nov 30, 2016 · 5 comments

Comments

@rflaperuta
Copy link

Hello, I've started using the module, but when calling:

var sodium_api = require('sodium').api;

var password_out = sodium_api.crypto_pwhash_argon2i_str(
    new Buffer(password_in),
    sodium_api.crypto_pwhash_OPSLIMIT_INTERACTIVE,
    sodium_api.crypto_pwhash_MEMLIMIT_INTERACTIVE);

password_out is a buffer (as it is supposed to) and contains null characters.

These are not stripped even after calling password_out.toString('ascii').

Although, printing on console using console.log strips them.

Is this by design? How to get rid of the trailing nulls?

The goal is to send the response inside a JSON object like:

var response = {password: password_out.toString('ascii')}

Thanks in advance.

jackschmidt added a commit to jackschmidt/node-sodium that referenced this issue Jan 16, 2017
… _str_verify is willing to accept such a string now. Adjust all 3 variants and add a test.
@richchurcher
Copy link
Contributor

Isn't it by design? In the libsodium docs it says,

The generated key has the size defined by the application, no matter what the password length is.

Assuming that the output buffer is always the same size but is null-padded, it would follow that to store the key in a database as a string, you'd need the string to be padded too. Otherwise when you regenerate the key in crypto_pwhash_argon2i_str_verify you'd not end up with the same result.

@jedisct1
Copy link
Contributor

crypto_pwhash_argon2i_STRBYTES represents the maximum length of the returned string.

The string itself can be shorter, and should be trimmed after the first \0, even though the verification function will tolerate padding after it.

@richchurcher
Copy link
Contributor

I see! Thanks, that's helpful.

@rflaperuta
Copy link
Author

Do I need to close the Issue Before or After the PR is merged?

Thanks for the help.

@sarahscheffler
Copy link

Is it documented somewhere that you have to strip off the \0 bytes? Or is there any interest in removing them automatically? We just had the same issue encoding/decoding from base64 and it took us a little while to track down the error.

@paixaop paixaop closed this as completed Nov 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants