crypto: DH prime for bits <= 15 is always 35963

[bnoordhuis]

With today's master:

const assert = require('assert');
const crypto = require('crypto');
for (let bits = 3; bits <= 15; bits++) {
  const p = crypto.createDiffieHellman(bits).getPrime();
  assert.strictEqual(p.length, 2);
  assert.strictEqual(p.readUInt16BE(0), 35963);
}

I'd expect the test case to fail (it should find different primes) but it returns the same prime for sizes 3 to 15. It's only at bits >= 16 that it starts to find different primes.

I'm not sure if this is a Node.js or an openssl bug. log2(35963) > 15 though so it definitely seems wrong to return that prime (and only that prime) for bits <= 15.

[sendilkumarn]

This seems like a bug(?) in openssl. openssl prime works exactly the same way till bits <= 15

[bnoordhuis]

This appears to be a bug that was fixed in openssl 1.0.2 (openssl/openssl#4656) and indeed when I run out/Release/openssl-cli I get random primes:

$ for bits in {3..15}; do ./out/Release/openssl-cli prime -bits $bits -generate; done
7
13
31
59
127
211
409
1009
1901
4079
7901
15541
30971

(openssl-cli is built when you build Node.js)

Node.js produces the same prime every time so it must be our bug:

$ for bits in {3..15}; do ./out/Release/node -p "crypto.createDiffieHellman($bits).getPrime().readUInt16BE(0)"; done
35963
35963
35963
35963
35963
35963
35963
35963
35963
35963
35963
35963
35963

Aside: openssl-cli -bits 4 -generate -safe busy-loops, I guess because there's no safe prime that small.

[sendilkumarn]

@bnoordhuis Yeah the random primes are generated when the bits is more than 3.

Does it mean we have to update the openssl version in Node?

[bnoordhuis]

openssl-cli produces random numbers and that's built against the same version of openssl that node builds with so the issue must be on our side somewhere.

[bnoordhuis]

Jotting down some notes so I don't forget:

• crypto.createDiffieHellman() calls DH_generate_parameters_ex()

• openssl prime and DH_generate_parameters_ex() both call BN_generate_prime_ex() but the latter passes in add and rem arguments.

• openssl prime does not and that results in different branches being taken here:
  

  node/deps/openssl/openssl/crypto/bn/bn_prime.c

  Lines 84 to 96 in 54fcb14

  	/* make a random number and set the top and bottom bits */
  	if (add == NULL) {
  	if (!probable_prime(ret, bits, mods))
  	goto err;
  	} else {
  	if (safe) {
  	if (!probable_prime_dh_safe(ret, bits, add, rem, ctx))
  	goto err;
  	} else {
  	if (!bn_probable_prime_dh(ret, bits, add, rem, ctx))
  	goto err;
  	}
  	}

So it looks like this is an openssl issue after all.

Aside: BN_generate_prime_ex(bn, 4, /* safe */ true, 0, 0, 0) seems like a denial-of-service waiting to happen because it never terminates. Needs more investigation.

[sam-github]

@bnoordhuis will you report upstream to OpenSSL?

[jasnell]

@bnoordhuis ... in current master this appears to be fixed in 12.x, 14.x and master with a couple of exceptions:

1. When bits is <= 8, getPrime() can return a Buffer with 1 or 2 bytes (causing your tests to fail because they assume at least two bytes)
2. bits 3 and 4 always produce the same result (0x0b)
3. bits 5 and 6 always produce the same result (0x3b)
4. getPrime() does not start producing reliably variable results until bits 7 at least.

It is still an issue in 10.x

[bnoordhuis]

When bits is <= 8, getPrime() can return a Buffer with 1 or 2 bytes (causing your tests to fail because they assume at least two bytes)

Right, that's good, that means it's not repeatedly finding the same prime.

bits 3 and 4 always produce the same result (0x0b)

I think that's okay. At first glance it looks like a bug for the bits=3 case because 2**3 < 0x0b but I believe openssl searches for primes up to 2 * 2**bits - 1, i.e., in the range 0..15 inclusive for bits=3.

[jasnell]

Given that this appears to be fixed in v12 and upwards, and given that there won't be any more 10.x releases before it hits end of life, closing this.