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

Suggestion: add length check for the context of crypto_kdf_derive_from_key #322

Open
nikgraf opened this issue Jul 31, 2023 · 2 comments
Open

Comments

@nikgraf
Copy link

nikgraf commented Jul 31, 2023

We noticed when using a smaller context string than 8 characters sometimes (couldn't reproduce it in tests, but in two different applications) the kdf function returns a different key. When using context strings with exactly 8 characters there are no issues.

This matches the expected documentation stating: They must be crypto_kdf_CONTEXTBYTES bytes long.
https://libsodium.gitbook.io/doc/key_derivation

It would be great if there was a check that throws an error in case the context is not exactly 8 characters long. Similar to invalid key length checks.

@jedisct1
Copy link
Owner

Mmmm it should already be the case:

"length": "libsodium._crypto_kdf_contextbytes()"

if ({var_name}_length != undefined && {var_name}.length - 1 !== {var_name}_length) {
_free_and_throw_type_error(address_pool, "invalid {var_name} length");
}

Maybe not in the version published on npm, but in the current code, it is.

@nikgraf
Copy link
Author

nikgraf commented Jul 31, 2023

@jedisct1 hmmm if I do git blame both code parts have been introduced more than 3 years ago, but the latest release is 5 months ago.

If you run this code you can test it yourself:

index.js

const sodium = require('libsodium-wrappers');

console.log(`Hello Node.js v${process.versions.node}!`);

async function main() {
  await sodium.ready;

  const masterKey = new Uint8Array([
    22, 83, 213, 220, 116, 80, 161, 0, 43, 65, 106, 114, 33, 69, 205, 24, 25,
    246, 101, 60, 140, 226, 94, 246, 161, 28, 253, 212, 149, 52, 225, 93,
  ]);

  const subkey1 = sodium.crypto_kdf_derive_from_key(
    sodium.crypto_secretbox_KEYBYTES,
    10,
    'locker',
    masterKey
  );
  const subkey2 = sodium.crypto_kdf_derive_from_key(
    sodium.crypto_secretbox_KEYBYTES,
    10,
    '1',
    masterKey
  );
  const subkey3 = sodium.crypto_kdf_derive_from_key(
    sodium.crypto_secretbox_KEYBYTES,
    10,
    'locker123456',
    masterKey
  );

  console.log(subkey1);
  console.log(subkey2);
  console.log(subkey3);
}

main();

package.json

{
  "name": "node-starter",
  "version": "0.0.0",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "dependencies": {
    "libsodium-wrappers": "^0.7.11"
  }
}

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

2 participants