Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Commit

Permalink
crypto: discourage and deprecate small DH groups
Browse files Browse the repository at this point in the history
This fixes issue 25366.

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

The change has been triggered by the security report "Imperfect Forward
Secrecy: How Diffie-Hellman Fails in Practice" which proved that small,
predefined DH primes groups should not be used.
  • Loading branch information
thinred committed Jun 26, 2015
1 parent 6f8400a commit a45cb6c
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 24 deletions.
30 changes: 19 additions & 11 deletions doc/api/crypto.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
11 changes: 11 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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");
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ const char* root_certs[] = {

bool SSL2_ENABLE = false;
bool SSL3_ENABLE = false;
bool SMALL_DH_GROUPS_ENABLE = false;

namespace crypto {

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/node_crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ namespace node {

extern bool SSL2_ENABLE;
extern bool SSL3_ENABLE;
extern bool SMALL_DH_GROUPS_ENABLE;

namespace crypto {

Expand Down
34 changes: 21 additions & 13 deletions src/node_crypto_groups.h
Original file line number Diff line number Diff line change
@@ -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.
*/

Expand Down Expand Up @@ -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 }
};
40 changes: 40 additions & 0 deletions test/simple/test-crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

var common = require('../common');
var assert = require('assert');
var spawn = require('child_process').spawn;

try {
var crypto = require('crypto');
Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit a45cb6c

Please sign in to comment.