Fix for the issue 25366.

In particular:
  - DH groups of size < 1024 are disabled by default
    (there is only one such group: modp1)
  - a new cmdline switch --enable-small-dh-groups and
    SMALL_DH_GROUPS_ENABLE env. variable are introduced;
    they override the default setting and therefore enable
    modp1 group
  - the docs & tests are updated

doc/api/crypto.markdown

@@ -502,21 +502,29 @@ expected.
 ## crypto.getDiffieHellman(group_name)
 
 Creates a predefined Diffie-Hellman key exchange object.  The
-supported groups are: `'modp1'`, `'modp2'`, `'modp5'` (defined in [RFC
-2412][]) and `'modp14'`, `'modp15'`, `'modp16'`, `'modp17'`,
-`'modp18'` (defined in [RFC 3526][]).  The returned object mimics the
-interface of objects created by [crypto.createDiffieHellman()][]
-above, but will not allow to change the keys (with
-[diffieHellman.setPublicKey()][] for example).  The advantage of using
-this routine is that the parties don't have to generate nor exchange
-group modulus beforehand, saving both processor and communication
-time.
+supported groups are: `'modp1'`, `'modp2'`, `'modp5'` (defined in
+[RFC 2412][]) and `'modp14'`, `'modp15'`, `'modp16'`, `'modp17'`,
+`'modp18'` (defined in [RFC 3526][]).
+
+The returned object mimics the interface of objects created by
+[crypto.createDiffieHellman()][] above, but will not allow to change
+the keys (with [diffieHellman.setPublicKey()][] for example).  The
+advantage of using this routine is that the parties do not have to
+generate nor exchange group modulus beforehand, saving both processor
+and communication time.
+
+The groups `'modp1'`, `'modp2'` and `'modp5'` (i.e., the groups with
+size smaller than 2048 bits) are considered **deprecated** and should
+not be used in new code. Moreover, the use of the `'modp1'` group must
+be explicitly enabled: either via `'--enable-small-dh-groups'` switch to
+node, or by setting the `'ENABLE_SMALL_DH_GROUPS'` environment variable
+to any value.
 
 Example (obtaining a shared secret):
 
     var crypto = require('crypto');
-    var alice = crypto.getDiffieHellman('modp5');
-    var bob = crypto.getDiffieHellman('modp5');
+    var alice = crypto.getDiffieHellman('modp14');
+    var bob = crypto.getDiffieHellman('modp14');
 
     alice.generateKeys();
     bob.generateKeys();

src/node.cc

@@ -2936,6 +2936,7 @@ static void PrintHelp() {
 #endif
          "  --enable-ssl2        enable ssl2\n"
          "  --enable-ssl3        enable ssl3\n"
+         "  --enable-small-dh-groups  enable small dh groups (not recommended)\n"
          "  --cipher-list=val    specify the default TLS cipher list\n"
          "  --enable-legacy-cipher-list=val \n"
          "                       val = v0.10.38, v0.10.39, v0.12.2 or v0.12.3\n"
@@ -2956,6 +2957,7 @@ static void PrintHelp() {
          "                       (will extend linked-in data)\n"
 #endif
 #endif
+         "ENABLE_SMALL_DH_GROUPS enable small dh groups (not recommended)\n"
          "NODE_CIPHER_LIST       Override the default TLS cipher list\n"
          "NODE_LEGACY_CIPHER_LIST=val\n"
          "                       val = v0.10.38, v0.10.39, v0.12.2 or v0.12.3\n"
@@ -3072,6 +3074,8 @@ static void ParseArgs(int* argc,
     } else if (strncmp(arg, "--icu-data-dir=", 15) == 0) {
       icu_data_dir = arg + 15;
 #endif
+    } else if (strcmp(arg, "--enable-small-dh-groups") == 0) {
+      SMALL_DH_GROUPS_ENABLE = true;
     } else {
       // V8 option.  Pass through as-is.
       new_v8_argv[new_v8_argc] = arg;
@@ -3467,6 +3471,11 @@ void Init(int* argc,
                      "(check NODE_ICU_DATA or --icu-data-dir parameters)");
   }
 #endif
+
+  const char *enableSmallDHGroups  = getenv("ENABLE_SMALL_DH_GROUPS");
+  if (enableSmallDHGroups != NULL) {
+    SMALL_DH_GROUPS_ENABLE = true;
+  }
   // The const_cast doesn't violate conceptual const-ness.  V8 doesn't modify
   // the argv array or the elements it points to.
   V8::SetFlagsFromCommandLine(&v8_argc, const_cast<char**>(v8_argv), true);

src/node_crypto.cc

@@ -77,6 +77,7 @@ namespace node {
 
 bool SSL2_ENABLE = false;
 bool SSL3_ENABLE = false;
+bool SMALL_DH_GROUPS_ENABLE = false;
 const char * DEFAULT_CIPHER_LIST = DEFAULT_CIPHER_LIST_HEAD;
 
 namespace crypto {
@@ -3839,7 +3840,6 @@ bool DiffieHellman::Init(const char* p, int p_len, const char* g, int g_len) {
   return true;
 }
 
-
 void DiffieHellman::DiffieHellmanGroup(
     const FunctionCallbackInfo<Value>& args) {
   HandleScope scope(args.GetIsolate());
@@ -3860,6 +3860,9 @@ void DiffieHellman::DiffieHellmanGroup(
     if (strcasecmp(*group_name, it->name) != 0)
       continue;
 
+    if (it->bits < 1024 && !SMALL_DH_GROUPS_ENABLE)
+      return env->ThrowError("Small DH groups disabled (see documentation)");
+
     initialized = diffieHellman->Init(it->prime,
                                       it->prime_size,
                                       it->gen,
@@ -5192,6 +5195,7 @@ void InitCrypto(Handle<Object> target,
 
   NODE_DEFINE_CONSTANT(target, SSL3_ENABLE);
   NODE_DEFINE_CONSTANT(target, SSL2_ENABLE);
+  NODE_DEFINE_CONSTANT(target, SMALL_DH_GROUPS_ENABLE);
 
   NODE_DEFINE_STRING_CONSTANT(env->isolate(), target, DEFAULT_CIPHER_LIST);
   NODE_SET_METHOD(target, "getLegacyCiphers", DefaultCiphers);

src/node_crypto.h

@@ -104,6 +104,7 @@ namespace node {
 
 extern bool SSL2_ENABLE;
 extern bool SSL3_ENABLE;
+extern bool SMALL_DH_GROUPS_ENABLE;
 extern const char * DEFAULT_CIPHER_LIST;
 
 namespace crypto {

src/node_crypto_groups.h

@@ -24,8 +24,16 @@
 
 /*
     These modular groups were literally taken from:
-       * RFC 2412 (groups 1 and 2)
-       * RFC 3526 (groups 5, 14, 15, 16, 17 and 18)
+       * RFC 2412:
+          - group 1 (768 bits)
+          - group 2 (1024 bits)
+          - group 5 (1536 bits)
+       * RFC 3526:
+          - group 14 (2048 bits)
+          - group 15 (3072 bits)
+          - group 16 (4096 bits)
+          - group 17 (6144 bits)
+          - group 18 (8192 bits)
     They all use 2 as a generator.
 */
 
@@ -390,6 +398,7 @@ static const unsigned char group_modp18[] = {
 
 typedef struct {
   const char* name;
+  unsigned int bits;
   const char* prime;
   unsigned int prime_size;
   const char* gen;
@@ -398,14 +407,14 @@ typedef struct {
 
 static const modp_group modp_groups[] = {
 #define V(var) reinterpret_cast<const char*>(var)
-  { "modp1", V(group_modp1), sizeof(group_modp1), V(two_generator), 1 },
-  { "modp2", V(group_modp2), sizeof(group_modp2), V(two_generator), 1 },
-  { "modp5", V(group_modp5), sizeof(group_modp5), V(two_generator), 1 },
-  { "modp14", V(group_modp14), sizeof(group_modp14), V(two_generator), 1 },
-  { "modp15", V(group_modp15), sizeof(group_modp15), V(two_generator), 1 },
-  { "modp16", V(group_modp16), sizeof(group_modp16), V(two_generator), 1 },
-  { "modp17", V(group_modp17), sizeof(group_modp17), V(two_generator), 1 },
-  { "modp18", V(group_modp18), sizeof(group_modp18), V(two_generator), 1 }
+  { "modp1", 768,  V(group_modp1), sizeof(group_modp1), V(two_generator), 1 },
+  { "modp2", 1024, V(group_modp2), sizeof(group_modp2), V(two_generator), 1 },
+  { "modp5", 1536, V(group_modp5), sizeof(group_modp5), V(two_generator), 1 },
+  { "modp14", 2048, V(group_modp14), sizeof(group_modp14), V(two_generator), 1 },
+  { "modp15", 3072, V(group_modp15), sizeof(group_modp15), V(two_generator), 1 },
+  { "modp16", 4096, V(group_modp16), sizeof(group_modp16), V(two_generator), 1 },
+  { "modp17", 6144, V(group_modp17), sizeof(group_modp17), V(two_generator), 1 },
+  { "modp18", 8192, V(group_modp18), sizeof(group_modp18), V(two_generator), 1 }
 #undef V
 };
 

test/simple/test-crypto.js

@@ -25,6 +25,7 @@
 var common = require('../common');
 var assert = require('assert');
 var util = require('util');
+var spawn = require('child_process').spawn;
 
 try {
   var crypto = require('crypto');
@@ -779,8 +780,8 @@ assert.equal(alice.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
 assert.equal(bob.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
 
 // Ensure specific generator (buffer) works as expected.
-var modp1 = crypto.createDiffieHellmanGroup('modp1');
-var modp1buf = new Buffer([
+var modp14 = crypto.createDiffieHellmanGroup('modp14');
+var modp14buf = new Buffer([
   0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc9, 0x0f,
   0xda, 0xa2, 0x21, 0x68, 0xc2, 0x34, 0xc4, 0xc6, 0x62, 0x8b,
   0x80, 0xdc, 0x1c, 0xd1, 0x29, 0x02, 0x4e, 0x08, 0x8a, 0x67,
@@ -789,47 +790,63 @@ var modp1buf = new Buffer([
   0x19, 0xb3, 0xcd, 0x3a, 0x43, 0x1b, 0x30, 0x2b, 0x0a, 0x6d,
   0xf2, 0x5f, 0x14, 0x37, 0x4f, 0xe1, 0x35, 0x6d, 0x6d, 0x51,
   0xc2, 0x45, 0xe4, 0x85, 0xb5, 0x76, 0x62, 0x5e, 0x7e, 0xc6,
-  0xf4, 0x4c, 0x42, 0xe9, 0xa6, 0x3a, 0x36, 0x20, 0xff, 0xff,
+  0xf4, 0x4c, 0x42, 0xe9, 0xa6, 0x37, 0xed, 0x6b, 0x0b, 0xff,
+  0x5c, 0xb6, 0xf4, 0x06, 0xb7, 0xed, 0xee, 0x38, 0x6b, 0xfb,
+  0x5a, 0x89, 0x9f, 0xa5, 0xae, 0x9f, 0x24, 0x11, 0x7c, 0x4b,
+  0x1f, 0xe6, 0x49, 0x28, 0x66, 0x51, 0xec, 0xe4, 0x5b, 0x3d,
+  0xc2, 0x00, 0x7c, 0xb8, 0xa1, 0x63, 0xbf, 0x05, 0x98, 0xda,
+  0x48, 0x36, 0x1c, 0x55, 0xd3, 0x9a, 0x69, 0x16, 0x3f, 0xa8,
+  0xfd, 0x24, 0xcf, 0x5f, 0x83, 0x65, 0x5d, 0x23, 0xdc, 0xa3,
+  0xad, 0x96, 0x1c, 0x62, 0xf3, 0x56, 0x20, 0x85, 0x52, 0xbb,
+  0x9e, 0xd5, 0x29, 0x07, 0x70, 0x96, 0x96, 0x6d, 0x67, 0x0c,
+  0x35, 0x4e, 0x4a, 0xbc, 0x98, 0x04, 0xf1, 0x74, 0x6c, 0x08,
+  0xca, 0x18, 0x21, 0x7c, 0x32, 0x90, 0x5e, 0x46, 0x2e, 0x36,
+  0xce, 0x3b, 0xe3, 0x9e, 0x77, 0x2c, 0x18, 0x0e, 0x86, 0x03,
+  0x9b, 0x27, 0x83, 0xa2, 0xec, 0x07, 0xa2, 0x8f, 0xb5, 0xc5,
+  0x5d, 0xf0, 0x6f, 0x4c, 0x52, 0xc9, 0xde, 0x2b, 0xcb, 0xf6,
+  0x95, 0x58, 0x17, 0x18, 0x39, 0x95, 0x49, 0x7c, 0xea, 0x95,
+  0x6a, 0xe5, 0x15, 0xd2, 0x26, 0x18, 0x98, 0xfa, 0x05, 0x10,
+  0x15, 0x72, 0x8e, 0x5a, 0x8a, 0xac, 0xaa, 0x68, 0xff, 0xff,
   0xff, 0xff, 0xff, 0xff, 0xff, 0xff
 ]);
-var exmodp1 = crypto.createDiffieHellman(modp1buf, new Buffer([2]));
-modp1.generateKeys();
-exmodp1.generateKeys();
-var modp1Secret = modp1.computeSecret(exmodp1.getPublicKey()).toString('hex');
-var exmodp1Secret = exmodp1.computeSecret(modp1.getPublicKey()).toString('hex');
-assert.equal(modp1Secret, exmodp1Secret);
-assert.equal(modp1.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
-assert.equal(exmodp1.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
+var exmodp14 = crypto.createDiffieHellman(modp14buf, new Buffer([2]));
+modp14.generateKeys();
+exmodp14.generateKeys();
+var modp14Secret = modp14.computeSecret(exmodp14.getPublicKey()).toString('hex');
+var exmodp14Secret = exmodp14.computeSecret(modp14.getPublicKey()).toString('hex');
+assert.equal(modp14Secret, exmodp14Secret);
+assert.equal(modp14.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
+assert.equal(exmodp14.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
 
 
 // Ensure specific generator (string with encoding) works as expected.
-var exmodp1_2 = crypto.createDiffieHellman(modp1buf, '02', 'hex');
-exmodp1_2.generateKeys();
-modp1Secret = modp1.computeSecret(exmodp1_2.getPublicKey()).toString('hex');
-var exmodp1_2Secret = exmodp1_2.computeSecret(modp1.getPublicKey())
+var exmodp14_2 = crypto.createDiffieHellman(modp14buf, '02', 'hex');
+exmodp14_2.generateKeys();
+modp14Secret = modp14.computeSecret(exmodp14_2.getPublicKey()).toString('hex');
+var exmodp14_2Secret = exmodp14_2.computeSecret(modp14.getPublicKey())
                                .toString('hex');
-assert.equal(modp1Secret, exmodp1_2Secret);
-assert.equal(exmodp1_2.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
+assert.equal(modp14Secret, exmodp14_2Secret);
+assert.equal(exmodp14_2.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
 
 
 // Ensure specific generator (string without encoding) works as expected.
-var exmodp1_3 = crypto.createDiffieHellman(modp1buf, '\x02');
-exmodp1_3.generateKeys();
-modp1Secret = modp1.computeSecret(exmodp1_3.getPublicKey()).toString('hex');
-var exmodp1_3Secret = exmodp1_3.computeSecret(modp1.getPublicKey())
+var exmodp14_3 = crypto.createDiffieHellman(modp14buf, '\x02');
+exmodp14_3.generateKeys();
+modp14Secret = modp14.computeSecret(exmodp14_3.getPublicKey()).toString('hex');
+var exmodp14_3Secret = exmodp14_3.computeSecret(modp14.getPublicKey())
                                .toString('hex');
-assert.equal(modp1Secret, exmodp1_3Secret);
-assert.equal(exmodp1_3.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
+assert.equal(modp14Secret, exmodp14_3Secret);
+assert.equal(exmodp14_3.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
 
 
 // Ensure specific generator (numeric) works as expected.
-var exmodp1_4 = crypto.createDiffieHellman(modp1buf, 2);
-exmodp1_4.generateKeys();
-modp1Secret = modp1.computeSecret(exmodp1_4.getPublicKey()).toString('hex');
-var exmodp1_4Secret = exmodp1_4.computeSecret(modp1.getPublicKey())
+var exmodp14_4 = crypto.createDiffieHellman(modp14buf, 2);
+exmodp14_4.generateKeys();
+modp14Secret = modp14.computeSecret(exmodp14_4.getPublicKey()).toString('hex');
+var exmodp14_4Secret = exmodp14_4.computeSecret(modp14.getPublicKey())
                                .toString('hex');
-assert.equal(modp1Secret, exmodp1_4Secret);
-assert.equal(exmodp1_4.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
+assert.equal(modp14Secret, exmodp14_4Secret);
+assert.equal(exmodp14_4.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
 
 
 var p = 'FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74' +
@@ -839,6 +856,40 @@ var p = 'FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74' +
 var bad_dh = crypto.createDiffieHellman(p, 'hex');
 assert.equal(bad_dh.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
 
+function node_output(code, args, env, cb) {
+  var out = '', err = '';
+  var p = spawn(process.execPath, [ '-e', code ].concat(args), { env: env });
+  p.stdout.on('data', function(data) { out += data; });
+  p.stderr.on('data', function(data) { err += data; });
+  p.on('close', function(code, signal) { cb(out, err, code); });
+}
+
+function no_output(out, err, code) {
+  assert.equal(out + err, '');
+  assert.equal(code, 0);
+}
+
+// test if fails on deprecated group
+node_output("require('crypto').getDiffieHellman('modp1')",
+            [], {}, function(out, err, code) {
+              assert.equal(out, '');
+              assert.ok(err.indexOf('Small DH groups disabled') > -1);
+              assert.equal(code, 1);
+            });
+
+// test if the environment variable makes it work
+node_output("require('crypto').getDiffieHellman('modp1')",
+            [], { 'ENABLE_SMALL_DH_GROUPS': '' }, no_output);
+
+// test if the cmdline switch makes it work
+node_output("require('crypto').getDiffieHellman('modp1')",
+            [ '--enable-small-dh-groups' ], {}, no_output);
+
+// test if does not fail on the next group
+node_output("require('crypto').getDiffieHellman('modp2')",
+            [], {}, no_output);
+
+
 // Test RSA encryption/decryption
 (function() {
   var input = 'I AM THE WALRUS';