3

I wonder how secure is this JavaScript password generator:

var Password = {

  _pattern : /[a-zA-Z0-9_\-\+\.\!\@\#\$\%\^\&\*\(\)\_\+\{\}\:\"\<\>\?\|\[\]\;\'\,\.\/\`\~\']/,

  _getRandomByte : function()
  {

    if(window.crypto && window.crypto.getRandomValues) 
    {
      var result = new Uint8Array(1);
      window.crypto.getRandomValues(result);
      return result[0];
    }
    else if(window.msCrypto && window.msCrypto.getRandomValues) 
    {
      var result = new Uint8Array(1);
      window.msCrypto.getRandomValues(result);
      return result[0];
    }
    else
    {
      return Math.floor(Math.random() * 256);
    }
  },

  generate : function(length)
  {
    return Array.apply(null, {'length': length})
      .map(function()
      {
        var result;
        while(true) 
        {
          result = String.fromCharCode(this._getRandomByte());
          if(this._pattern.test(result))
          {
            return result;
          }
        }        
      }, this)
      .join('');  
  }    

};

Also if someone has suggestion to make it better or more secure, you are welcome.

One more thing, the lenght of genereated password is fixed value="16", perhaps that can be bad idea and wouldn't be better to generate password with random lenght, for ex. betwen 12 - 16, or something else.

2

Your problem is Math.random(). That is not cryptographically secure. I think it is securely seeded on most implementations, though, so if you are just generating one password you are fine. But if you use this to generate loads of passwords they will become predictable. That is bad. But it is sort of hard to work around in JavaScript - if you don't have a CSPRNG, you don't have a CSPRNG...

There might be other issues as well that I have missed, but that is what stands out to me.

As for password length, there are way fewer passwords that are 1 to 15 characters long than there are passwords that are 16 characters long, so you aren't really missing out on a lot. If you still do want a random length anyway, you need to think about what distribution you draw the length from. Using a uniform distribution (e.g. your _getRandomByte) will give you less entropy and not more, since you are to likely to generate short passwords.

  • 1
    There is w3c.github.io/webcrypto/Overview.html, but it's an evolving specification, so probably shouldn't be relied on. – Matthew Jan 24 '17 at 17:22
  • 2
    window.crypto.getRandomValues() will get you cryptographically secure values. Math.random is actually secure in Opera, but not other browsers. – J.A.K. Jan 24 '17 at 19:19
  • Cool, didn't know about opera. window.crypto is not available in all situations, though. – Anders Jan 24 '17 at 19:35
  • For bitstrings the number having length upto-N-1 is almost equal to exactly-N. For strings over an alphabet of about 100 chars like here, exactly-N is almost 100 times upto-N-1. – dave_thompson_085 Jan 25 '17 at 5:44
  • 1
    @J.A.K. Since Opera is dead and the Opera skin for Chrome supports window.crypto.getRandomValues, there is little reason to allow Math.random at all. – CodesInChaos Jan 25 '17 at 9:12
1

The overall principle is valid: generate a password by selecting length characters at random. The details are where the problems lie:

  1. You must use a cryptographically secure random number generator. Math.random() isn't so, and must never be used for this.
  2. The way you're filtering the random choices is correct (all characters are selected with equal probability) but inefficient; you have a loop where you draw a random byte (256 possible values) and throw it away if it isn't one of your allowable characters, repeating until you pick a suitable one. The set of allowable characters is I guess about 95, which means most of the time you get a "miss." If you first take the remainder of dividing the random byte between 128 you also get correct results and you improve the "hit rate" noticeably. (This a is standard random number generation trick—you should use a library that does this for you already!)
  3. You need to think clearly about the security target and thus the minimum allowable password length. If your passwords draw from 95 distinct characters, then each character contributes about log2(95) = 6.6 bits of entropy. You need 10 characters to get to 64 bits of entropy, which I think is the bare minimum anybody should ever use (website logins for sites where it wouldn't break the bank if somebody cracked the password, and you might want to memorize the passwords). 12 characters gets you to 79 bits, which I think is a good balance between length and security.
0

You should validate the length parameter. If you call Password.generate(), you may expect that a password is generated, but actually it returns an empty string.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.