TechFindings

When you build a service backend you should keep your users’ passwords safe. That is not so easy anymore. You should

1. hash and salt (md5)
2. but rather use strong hash (sha)
3. but rather use a very expensive hash (pbkdf2, bcrypt)
4. but rather use a hash that is very expensive on GPUs and cryptominers (argon2)

Argon2 seems to be the best choice (read elsewhere about it)!

node-argon2

Argon2 is very easy to use on Node.js. You basically just:

$ npm install argon2

Then your code is:

/* To hash a password */
hash = await argon2.hash('password');

/* To test a password */
if ( await argon2.verify(hash,'password') )
  console.log('OK');
else
  console.log('Not OK');

Great! What is not to like about that?

$ du -sh node_modules/*
 20K  node_modules/abbrev
 20K  node_modules/ansi-regex
 20K  node_modules/aproba
 44K  node_modules/are-we-there-yet
348K  node_modules/argon2
 24K  node_modules/balanced-match
 24K  node_modules/brace-expansion
 24K  node_modules/chownr
 20K  node_modules/code-point-at
 40K  node_modules/concat-map
 32K  node_modules/console-control-strings
 44K  node_modules/core-util-is
120K  node_modules/debug
 36K  node_modules/deep-extend
 40K  node_modules/delegates
 44K  node_modules/detect-libc
 28K  node_modules/fs-minipass
 32K  node_modules/fs.realpath
104K  node_modules/gauge
 72K  node_modules/glob
 20K  node_modules/has-unicode
412K  node_modules/iconv-lite
 24K  node_modules/ignore-walk
 20K  node_modules/inflight
 24K  node_modules/inherits
 24K  node_modules/ini
 36K  node_modules/isarray
 20K  node_modules/is-fullwidth-code-point
 48K  node_modules/minimatch
108K  node_modules/minimist
 52K  node_modules/minipass
 32K  node_modules/minizlib
 32K  node_modules/mkdirp
 20K  node_modules/ms
332K  node_modules/needle
956K  node_modules/node-addon-api
240K  node_modules/node-pre-gyp
 48K  node_modules/nopt
 24K  node_modules/npm-bundled
 36K  node_modules/npmlog
172K  node_modules/npm-normalize-package-bin
 28K  node_modules/npm-packlist
 20K  node_modules/number-is-nan
 20K  node_modules/object-assign
 20K  node_modules/once
 20K  node_modules/osenv
 20K  node_modules/os-homedir
 20K  node_modules/os-tmpdir
 20K  node_modules/path-is-absolute
 32K  node_modules/@phc
 20K  node_modules/process-nextick-args
 64K  node_modules/rc
224K  node_modules/readable-stream
 32K  node_modules/rimraf
 48K  node_modules/safe-buffer
 64K  node_modules/safer-buffer
 72K  node_modules/sax
 88K  node_modules/semver
 24K  node_modules/set-blocking
 32K  node_modules/signal-exit
 88K  node_modules/string_decoder
 20K  node_modules/string-width
 20K  node_modules/strip-ansi
 20K  node_modules/strip-json-comments
196K  node_modules/tar
 28K  node_modules/util-deprecate
 20K  node_modules/wide-align
 20K  node_modules/wrappy
 36K  node_modules/yallist

That is 69 node modules of 5.1MB. If you think that is cool for your backend password encyption code (in order to provide two functions: encrypt and verify) you can stop reading here.

I am NOT fine with it, because:

• it will cause me trouble, one day, when I run npm install, and something is not exactly as I expected, perhaps in production
• how safe is this? it is the password encryption code we are talking about – what if any of these libraries are compromised?
• it is outright ugly and wasteful

Well, argon2 has a reference implementation written in C (link). If you download it you can compile, run test and try it like:

$ make
$ make test
$ ./argon2 -h
Usage: ./argon2-linux-x64 [-h] salt [-i|-d|-id] [-t iterations] [-m log2(memory in KiB) | -k memory in KiB] [-p parallelism] [-l hash length] [-e|-r] [-v (10|13)]
Password is read from stdin
Parameters:
 salt The salt to use, at least 8 characters
 -i   Use Argon2i (this is the default)
 -d   Use Argon2d instead of Argon2i
 -id  Use Argon2id instead of Argon2i
 -t N Sets the number of iterations to N (default = 3)
 -m N Sets the memory usage of 2^N KiB (default 12)
 -k N Sets the memory usage of N KiB (default 4096)
 -p N Sets parallelism to N threads (default 1)
 -l N Sets hash output length to N bytes (default 32)
 -e   Output only encoded hash
- r   Output only the raw bytes of the hash
 -v (10|13) Argon2 version (defaults to the most recent version, currently 13)
 -h   Print ./argon2-linux-x64 usage

It builds to a single binary (mine is 280kb on linux-x64). It does most everything you need. How many lines of code do you think you need to write for node.js to use that binary instead of the 69 npm packages? The answer is less than 69. Here comes some notes and all the code (implementing argon2.hash and argon2.verify as used above):

1. you can make binaries for different platforms and name them accordingly (argon2-linux-x64, argon2-darwin-x64 and so on), so you can move your code (and binaries) between different computers with no hazzle (as JavaScript should be)
2. if you want to change argon2-parameters you can do it here, and if you want to pass an option-object to the hash function that is an easy fix
3. options are parsed from hash (just as the node-argon2 package) when verifying, so you don’t need to “remember” what parameters you used when hashing to be able to verify

/* argon2-wrapper.js */

const nodeCrypto = require('crypto');
const nodeOs    = require('os');
const nodeSpawn = require('child_process').spawn;
/* NOTE 1 */
const binary    = './argon2';
// st binary    = './argon2-' + nodeOs.platform() + '-' + nodeOs.arch();

const run = (args,pass,callback) => {
  const proc = nodeSpawn(binary,args);
  let hash = '';
  let err = '';
  let inerr = false;
  proc.stdout.on('data', (data) => { hash += data; });
  proc.stderr.on('data', (data) => { err += data; });
  proc.stdin.on('error', () => { inerr = true; });
  proc.on('exit', (code) => {
    if ( err ) callback(err);
    else if ( inerr ) callback('I/O error');
    else if ( 0 !== code ) callback('Nonzero exit code ' + code);
    else if ( !hash ) callback('No hash');
    else callback(null,hash.trim());
  });
  proc.stdin.end(pass);
};

exports.hash = (pass) => {
  return new Promise((resolve,reject) => {
    nodeCrypto.randomBytes(12,(e,b) => {
      if ( e ) return reject(e);
      const salt = b.toString('base64');
      const args = [salt,'-id','-e'];
/* NOTE 2 */
//    const args = [salt,'-d','-v','13','-m','12','-t','3','-p','1','-e'];
      run(args,pass,(e,h) => {
        if ( e ) reject(e);
        else resolve(h);
      });
    });
  });
};

exports.verify = (hash,pass) => {
  return new Promise((resolve,reject) => {
    const hashfields = hash.split('$');
    const perffields = hashfields[3].split(',');
/* NOTE 3 */
    const args = [
        Buffer.from(hashfields[4],'base64').toString()
      , '-' + hashfields[1].substring(6) // -i, -d, -id
      , '-v', (+hashfields[2].split('=')[1]).toString(16)
      , '-k', perffields[0].split('=')[1]
      , '-t', perffields[1].split('=')[1]
      , '-p', perffields[2].split('=')[1]
      , '-e'
    ];
    run(args,pass,(e,h) => {
      if ( e ) reject(e);
      else resolve(h===hash);
    });
  });
};

And for those of you who want to test it, here is a little test program that you can run. It requires

• npm install argon2
• argon2 reference implementation binary

const argon2package = require('argon2');
const argon2wrapper = require('./argon2-wrapper.js');

const bench = async (n,argon2) => {
  const passwords = [];
  const hashes = [];
  const start = Date.now();
  let errors = 0;

  for ( let i=0 ; i<n ; i++ ) {
    let pw = 'password-' + i;
    passwords.push(pw);
    hashes.push(await argon2.hash(pw));
  }
  const half = Date.now();
  console.log('Hashed ' + n + ' passwords in ' + (half-start) + ' ms');

  for ( let i=0 ; i<n ; i++ ) {
    // first try wrong password
    if ( await argon2.verify(hashes[i],'password-ill-typed') ) {
      console.log('ERROR: wrong password was verified as correct');
      errors++;
    }
    if ( !(await argon2.verify(hashes[i],passwords[i]) ) ) {
      console.log('ERROR: correct password failed to verify');
      errors++;
    }
  }
  const end = Date.now();
  console.log('Verified 2x' + n + ' passwords in ' + (end-half) + ' ms');
  console.log('Error count: ' + errors);
  console.log('Hash example:\n' + hashes[0]);
};

const main = async (n) => {
  console.log('Testing with package');
  await bench(n,argon2package);
  console.log('\n\n');

  console.log('Testing with binary wrapper');
  await bench(n,argon2wrapper);
  console.log('\n\n');
}
main(100);

Give it a try!

Performance

I find that in Linux x64, wrapping the binary is slightly faster than using the node-package. That is weird. But perhaps those 69 dependencies don’t come for free after all.

Problems?

I see one problem. The node-argon2 package generates binary hashes random salts and sends to the hash algorithm. Those binary salts come out base64-encoded in the hash. However, a binary value (a byte array using 0-255) is not very easy to pass on the command line to the reference implementation (as first parameter). My wrapper-implementation also generate a random salt, but it base64-encodes it before it passes it to argon2 as salt (and argon2 then base64-encodes it again in the hash string).

So if you already use the node-package the reference c-implementation is not immediately compatible with the hashes you already have produced. The other way around is fine: “my” hashes are easily consumed by the node package.

If this is a real problem for you that you want to solve I see two solutions:

1. make a minor modification to the C-program so it expects a salt in hex format (it will be twice as long on the command line)
2. start supplying your own compatible hashes using the option-object now, and don’t switch to the wrapper+c until the passwords have been updated

Conclusion

There are bindings between languages and node-packages for stuff. But unix already comes with an API for programs written in different languages to use: process forking and pipes.

In Linux it is extremely cheap. It is quite easy to use and test, since you easily have access to the command line. And the spawn in node is easy to use.

Argon2 is nice and easy to use! Use it! Forget about bcrypt.

The best thing you can do without any dependencies is pbkdf2 which comes with node.js and is accessible in its crypto module. It is standardized/certified, that is why it is included.

For someone like me working in a (very) small development team SonarQube is good. It reviews my code and it teaches me about both old and new things that I did not know about.

There are some things I don’t agree about though (all JavaScript below).

“switch” statements should have at least 3 “case” clauses

I understand the point. But I have my reasons. Let us say I have orders with different statuses (DRAFT,OPEN,CLOSED). Then often I have switch statements like:

switch (order.status) {
case 'DRAFT':
  ...
  break;
case 'OPEN':
  ...
  break;
case 'CLOSED':
  ...
  break;
default:
  throw new Error(...);
}

This switch over order.status happens in many places. Sometimes it is just:

switch (order.status) {
case 'CLOSED':
  ...
  break;
}

I don’t want to refactor the (rewrite if with switch) code if/when I need to do something for other states. And I like a consistent form.

Another case can be that I allow the user to upload files in different formats, but for now I just allow PDF.

switch ( upload.format ) {
case 'pdf':
  process_pdf();
  break;
default:
  throw new Error('Unsupported upload.format: ' + upload.format);
}

A switch is much more clear than an if-statement. However, if the above code was in a function called uploadPdf() then the code would have been:

if ( 'pdf' !== upload.format ) throw new Error(...);
...
...

But in conclusion, I think switch statements with few (even 1) case clause are often fine.

This branch’s code block is the same as the block for the branch on line …

This happens to me often. A good rule is to never trust user input. So I may have code that looks like this (simplified):

if ( !(user = validateToken(request.params.token)) ) {
  error = 401;
} else if ( !authorizeAction(user,'upload') ) {
  error = 401;
} else if ( !reqest.params.orderid ) {
  error = 400;
  errormsg = 'orderid required';
} else if ( user !== (order = getOrder(request.params.orderid)).owner ) {
  error = 401;
} else if ( !validateOrder(order) ) {
  error = 400;
  errormsg = 'order failed to validate';
} else ...

The point is that important part of the code is not the code blocks, but rather the checks. The content of the code blocks may, or may not, be identical. That hardly counts as duplication of code. The important thing is that the conditions are executed one-by-one, in order and that they all pass.

I could obviously write the first two ones as the same (with a more complex condition), since both gives 401. But code is debugged, extended and refactored and for those purposes my code is much better than:

if ( !(user = validateToken(request.params.token) ) ||
     !authorizeAction(user.upload) ) {
  error = 401;
} else ...

My code is easier to read, reorganize, debug (add temporary breakpoints or printouts) or improve (perhaps all 401 are not equal one day).

Remove this …… label

The reason I get this warning is mostly that I use what i learnt in my own post Faking a good goto in JavaScript.

SonarCube also complained when I wanted to break out of a nested loop this way:

break_block {
  for ( i=0 ; i<10 ; i++ ) {
    for ( j=0 ; j<10 ; j++ ) {
      if ( (obj=lookForObjInMatrix(matrix,i,j) ) ) break break_block;
    }
  }
  // obj not found, not set
}

It is a stupid simplified example, but it happens.