Fix for the issue 25366.

doc/api/crypto.markdown

@@ -425,21 +425,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'` are not recommended, due
+to their small size. Moreover, the group `'modp1'` is **deprecated**
+due to extremely low security, and is not enabled by default. If you
+want to use it, please enable it explicitly: 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

@@ -2566,6 +2566,7 @@ static void PrintHelp() {
          "  --max-stack-size=val set max v8 stack size (bytes)\n"
          "  --enable-ssl2        enable ssl2\n"
          "  --enable-ssl3        enable ssl3\n"
+         "  --enable-small-dh-groups  enable small dh groups\n"
          "\n"
          "Environment variables:\n"
 #ifdef _WIN32
@@ -2577,6 +2578,7 @@ static void PrintHelp() {
          "NODE_MODULE_CONTEXTS   Set to 1 to load modules in their own\n"
          "                       global contexts.\n"
          "NODE_DISABLE_COLORS    Set to 1 to disable colors in the REPL\n"
+         "ENABLE_SMALL_DH_GROUPS enable small dh groups\n"
          "\n"
          "Documentation can be found at http://nodejs.org/\n");
 }
@@ -2605,6 +2607,9 @@ static void ParseArgs(int argc, char **argv) {
     } else if (strcmp(arg, "--enable-ssl3") == 0) {
       SSL3_ENABLE = true;
       argv[i] = const_cast<char*>("");
+    } else if (strcmp(arg, "--enable-small-dh-groups") == 0) {
+      SMALL_DH_GROUPS_ENABLE = true;
+      argv[i] = const_cast<char*>("");
     } else if (strcmp(arg, "--help") == 0 || strcmp(arg, "-h") == 0) {
       PrintHelp();
       exit(0);
@@ -2930,6 +2935,12 @@ char** Init(int argc, char *argv[]) {
 
   // Parse a few arguments which are specific to Node.
   node::ParseArgs(argc, argv);
+
+  const char *enableSmallDHGroups  = getenv("ENABLE_SMALL_DH_GROUPS");
+  if (enableSmallDHGroups != NULL) {
+      SMALL_DH_GROUPS_ENABLE = true;
+  }
+
   // Parse the rest of the args (up to the 'option_end_index' (where '--' was
   // in the command line))
   int v8argc = option_end_index;

src/node_crypto.cc

@@ -71,6 +71,7 @@ const char* root_certs[] = {
 
 bool SSL2_ENABLE = false;
 bool SSL3_ENABLE = false;
+bool SMALL_DH_GROUPS_ENABLE = false;
 
 namespace crypto {
 
@@ -3538,6 +3539,9 @@ class DiffieHellman : public ObjectWrap {
     }
 
     if (it->name != NULL) {
+      if (it->bits < 1024 && !SMALL_DH_GROUPS_ENABLE)
+        return ThrowException(Exception::Error(
+          String::New("Small DH groups disabled (see documentation)")));
       diffieHellman->Init(it->prime, it->prime_size,
               it->gen, it->gen_size);
     } else {
@@ -4264,6 +4268,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);
 }
 
 }  // namespace crypto

src/node_crypto.h

@@ -47,6 +47,7 @@ namespace node {
 
 extern bool SSL2_ENABLE;
 extern bool SSL3_ENABLE;
+extern bool SMALL_DH_GROUPS_ENABLE;
 
 namespace crypto {
 

src/node_crypto_groups.h

@@ -1,8 +1,15 @@
-
 /*
     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.
 */
 
@@ -367,20 +374,21 @@ unsigned char group_modp18[] = {
 
 typedef struct {
   const char* name;
+  unsigned int bits;
   unsigned char* prime;
   int prime_size;
   unsigned char* gen;
   int gen_size;
 } modp_group;
 
 modp_group modp_groups[] = {
-  { "modp1", group_modp1, sizeof(group_modp1) / sizeof(group_modp1[0]), two_generator, 1 },
-  { "modp2", group_modp2, sizeof(group_modp2) / sizeof(group_modp2[0]), two_generator, 1 },
-  { "modp5", group_modp5, sizeof(group_modp5) / sizeof(group_modp5[0]), two_generator, 1 },
-  { "modp14", group_modp14, sizeof(group_modp14) / sizeof(group_modp14[0]), two_generator, 1 },
-  { "modp15", group_modp15, sizeof(group_modp15) / sizeof(group_modp15[0]), two_generator, 1 },
-  { "modp16", group_modp16, sizeof(group_modp16) / sizeof(group_modp16[0]), two_generator, 1 },
-  { "modp17", group_modp17, sizeof(group_modp17) / sizeof(group_modp17[0]), two_generator, 1 },
-  { "modp18", group_modp18, sizeof(group_modp18) / sizeof(group_modp18[0]), two_generator, 1 },
-  { NULL, NULL, 0, NULL, 0 }
-};
+  { "modp1", 768, group_modp1, ARRAY_SIZE(group_modp1), two_generator, 1 },
+  { "modp2", 1024, group_modp2, ARRAY_SIZE(group_modp2), two_generator, 1 },
+  { "modp5", 1536, group_modp5, ARRAY_SIZE(group_modp5), two_generator, 1 },
+  { "modp14", 2048, group_modp14, ARRAY_SIZE(group_modp14), two_generator, 1 },
+  { "modp15", 3072, group_modp15, ARRAY_SIZE(group_modp15), two_generator, 1 },
+  { "modp16", 4096, group_modp16, ARRAY_SIZE(group_modp16), two_generator, 1 },
+  { "modp17", 6144, group_modp17, ARRAY_SIZE(group_modp17), two_generator, 1 },
+  { "modp18", 8192, group_modp18, ARRAY_SIZE(group_modp18), two_generator, 1 },
+  { NULL, 0, NULL, 0, NULL, 0 }
+};

test/simple/test-crypto.js

@@ -24,6 +24,7 @@
 
 var common = require('../common');
 var assert = require('assert');
+var spawn = require('child_process').spawn;
 
 try {
   var crypto = require('crypto');
@@ -722,6 +723,45 @@ var aSecret = alice.computeSecret(bob.getPublicKey()).toString('hex');
 var bSecret = bob.computeSecret(alice.getPublicKey()).toString('hex');
 assert.equal(aSecret, bSecret);
 
+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 assert_output(contents) {
+  return function(out, err, code) {
+    assert.equal(out.trim(), contents);
+    assert.equal(err, '');
+    assert.equal(code, 0);
+  };
+}
+var assert_no_output = assert_output('');
+
+// 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.notEqual(code, 1);
+            });
+
+// test if the environment variable makes it work
+node_output("require('crypto').getDiffieHellman('modp1');" +
+            "console.log(process.binding('crypto').SMALL_DH_GROUPS_ENABLE)",
+            [], { 'ENABLE_SMALL_DH_GROUPS': '' }, assert_output('1'));
+
+// test if the cmdline switch makes it work
+node_output("require('crypto').getDiffieHellman('modp1');" +
+            "console.log(process.binding('crypto').SMALL_DH_GROUPS_ENABLE)",
+            [ '--enable-small-dh-groups' ], {}, assert_output('1'));
+
+// test if does not fail on the next group
+node_output("require('crypto').getDiffieHellman('modp2');" +
+            "console.log(process.binding('crypto').SMALL_DH_GROUPS_ENABLE)",
+            [], {}, assert_output('0'));
 
 // https://github.com/joyent/node/issues/2338
 assert.throws(function() {