keeweb/kdbxweb

Insecure PRNG

dchest opened this issue · 11 comments

Mixing a bunch of Math.random with Salsa20 doesn't make PRNG secure. Please use crypto.getRandomValues in browser and crypto.randomBytes in Node, for example:

  var getRandomBytes = (
    (typeof self !== 'undefined' && (self.crypto || self.msCrypto))
      ? function() { // Browsers
          var crypto = (self.crypto || self.msCrypto), QUOTA = 65536;
          return function(n) {
            var a = new Uint8Array(n);
            for (var i = 0; i < n; i += QUOTA) {
              crypto.getRandomValues(a.subarray(i, i + Math.min(n - i, QUOTA)));
            }
            return a;
          };
        }
      : function() { // Node
          return require("crypto").randomBytes;
        }
  )();

This will give you random bytes:

aesKey = getRandomBytes(18)

Thank you, I'll replace it of course.

I've added system generator usage. But I'll leave XOR'ing it with Salsa20, to prevent backdoors which might be inserted into such 'recommended' functions. Anyway, XORing will never make a sequence less secure.

cxxr commented

Why are you XORing the PRNG with the CSRNG?

This doesn't make any sense. If you're worried that browser's random source has a backdoor, XORing with Salsa20 stream keyed by the same backdoored random source will get you nothing.

Salsa20 is not some magic thing that will add entropy: it's a stream cipher, which generates deterministic sequences of bytes from the key and nonce that you give it.

Please simply use crypto.getRandomValues by itself. It is illogical to trust one API provided by your browser more than another API provided by the same browser.

If your browser is compromised, then your browser is compromised. There is no workaround for a compromised browser, so please do not attempt to do anything about it.

Why are you XORing the PRNG with the CSRNG?

Why not? This will never make it weaker.

If you're worried that browser's random source has a backdoor, XORing with Salsa20 stream keyed by the same backdoored random source will get you nothing.

I'm afraid, you are wrong here. If we use backdoored random source, combining it with some less or more random sequence will make the backdoor useless.

Salsa20 is not some magic thing that will add entropy

Of course but data is taken from random position of it, so it will alter the random sequence.

Please simply use crypto.getRandomValues by itself.

Why? There's no disadvantage of XORing. Are there any?

It is illogical to trust one API provided by your browser more than another API

Which API is trusted more? Another algorithms, like AES, cannot be modified in any way, unlike random. Browser might use system built-in random, which might use recommended generator, which we would not like to use.

There is no workaround for a compromised browser

There is a workaround for compromised algorithm. This way of defense (mix with another cipher) is written in some papers, and even in Wikipedia: https://en.wikipedia.org/wiki/Random_number_generator_attack

Browser might use system built-in random, which might use recommended generator, which we would not like to use.

Why are you making this decision for the user? Let me trust my own OS. I don't want you prescribing your own value of trust to my OS.

Of course but data is taken from random position of it, so it will alter the random sequence.

What is the benefit of that to you? Entropy has not increased and an attacker knows exactly how you used Salsa20 so it literally provides zero security benefit.

There is a workaround for compromised algorithm. This way of defense (mix with another cipher) is written in some papers, and even in Wikipedia: https://en.wikipedia.org/wiki/Random_number_generator_attack

Yes, that is a real mitigation but it only helps with closed source server-side software. This app is open source and everyone knows exactly what steps your algorithm takes. Mixing PRNG with CSRNG does not benefit the security of open source software.

What is the benefit of that to you? Entropy has not increased.

Entropy is increased because we use salsa20 with:

  1. random nonce
  2. with a value depending on usage count

Well, let's even suppose that 'it literally provides zero security benefit': it does not result in any drawback, do you agree with that?

Just to clarify.
The way RNG backdoors work: we have a certain sequence by which we can guess and try to match, so the result effort will be O(N) where N is the length of the sequence. Pretty easy.
If we mix (xor) the result with some another sequence generated by other algorithm, the attack effort becomes O(N*M) where M is the length of another sequence (for Salsa20 it's 2^64). As the attacker doesn't know the nonce, it will be even more hard.
Am I mistaken here?

This is old, but perhaps provide an option? Default to whichever you like, but allow the tech-savvy user to select which he wishes to use.

This must be implemented as a plugin, once we support them...