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

Commit 4000f3b

Browse files
committed
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
1 parent 6f8400a commit 4000f3b

File tree

6 files changed

+91
-25
lines changed

6 files changed

+91
-25
lines changed

doc/api/crypto.markdown

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -425,21 +425,29 @@ expected.
425425
## crypto.getDiffieHellman(group_name)
426426

427427
Creates a predefined Diffie-Hellman key exchange object. The
428-
supported groups are: `'modp1'`, `'modp2'`, `'modp5'` (defined in [RFC
429-
2412][]) and `'modp14'`, `'modp15'`, `'modp16'`, `'modp17'`,
430-
`'modp18'` (defined in [RFC 3526][]). The returned object mimics the
431-
interface of objects created by [crypto.createDiffieHellman()][]
432-
above, but will not allow to change the keys (with
433-
[diffieHellman.setPublicKey()][] for example). The advantage of using
434-
this routine is that the parties don't have to generate nor exchange
435-
group modulus beforehand, saving both processor and communication
436-
time.
428+
supported groups are: `'modp1'`, `'modp2'`, `'modp5'` (defined in
429+
[RFC 2412][]) and `'modp14'`, `'modp15'`, `'modp16'`, `'modp17'`,
430+
`'modp18'` (defined in [RFC 3526][]).
431+
432+
The returned object mimics the interface of objects created by
433+
[crypto.createDiffieHellman()][] above, but will not allow to change
434+
the keys (with [diffieHellman.setPublicKey()][] for example). The
435+
advantage of using this routine is that the parties do not have to
436+
generate nor exchange group modulus beforehand, saving both processor
437+
and communication time.
438+
439+
The groups `'modp1'`, `'modp2'` and `'modp5'` (i.e., the groups with
440+
size smaller than 2048 bits) are considered **deprecated** and should
441+
not be used in new code. Moreover, the use of the `'modp1'` group must
442+
be explicitly enabled: either via `'--enable-small-dh-groups'` switch to
443+
node, or by setting the `'ENABLE_SMALL_DH_GROUPS'` environment variable
444+
to any value.
437445

438446
Example (obtaining a shared secret):
439447

440448
var crypto = require('crypto');
441-
var alice = crypto.getDiffieHellman('modp5');
442-
var bob = crypto.getDiffieHellman('modp5');
449+
var alice = crypto.getDiffieHellman('modp14');
450+
var bob = crypto.getDiffieHellman('modp14');
443451

444452
alice.generateKeys();
445453
bob.generateKeys();

src/node.cc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2566,6 +2566,7 @@ static void PrintHelp() {
25662566
" --max-stack-size=val set max v8 stack size (bytes)\n"
25672567
" --enable-ssl2 enable ssl2\n"
25682568
" --enable-ssl3 enable ssl3\n"
2569+
" --enable-small-dh-groups enable small dh groups (not recommended)\n"
25692570
"\n"
25702571
"Environment variables:\n"
25712572
#ifdef _WIN32
@@ -2577,6 +2578,7 @@ static void PrintHelp() {
25772578
"NODE_MODULE_CONTEXTS Set to 1 to load modules in their own\n"
25782579
" global contexts.\n"
25792580
"NODE_DISABLE_COLORS Set to 1 to disable colors in the REPL\n"
2581+
"ENABLE_SMALL_DH_GROUPS enable small dh groups (not recommended)\n"
25802582
"\n"
25812583
"Documentation can be found at http://nodejs.org/\n");
25822584
}
@@ -2605,6 +2607,9 @@ static void ParseArgs(int argc, char **argv) {
26052607
} else if (strcmp(arg, "--enable-ssl3") == 0) {
26062608
SSL3_ENABLE = true;
26072609
argv[i] = const_cast<char*>("");
2610+
} else if (strcmp(arg, "--enable-small-dh-groups") == 0) {
2611+
SMALL_DH_GROUPS_ENABLE = true;
2612+
argv[i] = const_cast<char*>("");
26082613
} else if (strcmp(arg, "--help") == 0 || strcmp(arg, "-h") == 0) {
26092614
PrintHelp();
26102615
exit(0);
@@ -2930,6 +2935,12 @@ char** Init(int argc, char *argv[]) {
29302935

29312936
// Parse a few arguments which are specific to Node.
29322937
node::ParseArgs(argc, argv);
2938+
2939+
const char *enableSmallDHGroups = getenv("ENABLE_SMALL_DH_GROUPS");
2940+
if (enableSmallDHGroups != NULL) {
2941+
SMALL_DH_GROUPS_ENABLE = true;
2942+
}
2943+
29332944
// Parse the rest of the args (up to the 'option_end_index' (where '--' was
29342945
// in the command line))
29352946
int v8argc = option_end_index;

src/node_crypto.cc

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ const char* root_certs[] = {
7171

7272
bool SSL2_ENABLE = false;
7373
bool SSL3_ENABLE = false;
74+
bool SMALL_DH_GROUPS_ENABLE = false;
7475

7576
namespace crypto {
7677

@@ -802,7 +803,7 @@ size_t ClientHelloParser::Write(const uint8_t* data, size_t len) {
802803
HandleScope scope;
803804

804805
assert(state_ != kEnded);
805-
806+
806807
// Just accumulate data, everything will be pushed to BIO later
807808
if (state_ == kPaused) return 0;
808809

@@ -3538,6 +3539,9 @@ class DiffieHellman : public ObjectWrap {
35383539
}
35393540

35403541
if (it->name != NULL) {
3542+
if (it->bits < 1024 && !SMALL_DH_GROUPS_ENABLE)
3543+
return ThrowException(Exception::Error(
3544+
String::New("Small DH groups disabled (see documentation)")));
35413545
diffieHellman->Init(it->prime, it->prime_size,
35423546
it->gen, it->gen_size);
35433547
} else {
@@ -4264,6 +4268,7 @@ void InitCrypto(Handle<Object> target) {
42644268

42654269
NODE_DEFINE_CONSTANT(target, SSL3_ENABLE);
42664270
NODE_DEFINE_CONSTANT(target, SSL2_ENABLE);
4271+
NODE_DEFINE_CONSTANT(target, SMALL_DH_GROUPS_ENABLE);
42674272
}
42684273

42694274
} // namespace crypto

src/node_crypto.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ namespace node {
4747

4848
extern bool SSL2_ENABLE;
4949
extern bool SSL3_ENABLE;
50+
extern bool SMALL_DH_GROUPS_ENABLE;
5051

5152
namespace crypto {
5253

src/node_crypto_groups.h

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
1-
21
/*
32
These modular groups were literally taken from:
4-
* RFC 2412 (groups 1 and 2)
5-
* RFC 3526 (groups 5, 14, 15, 16, 17 and 18)
3+
* RFC 2412:
4+
- group 1 (768 bits)
5+
- group 2 (1024 bits)
6+
- group 5 (1536 bits)
7+
* RFC 3526:
8+
- group 14 (2048 bits)
9+
- group 15 (3072 bits)
10+
- group 16 (4096 bits)
11+
- group 17 (6144 bits)
12+
- group 18 (8192 bits)
613
They all use 2 as a generator.
714
*/
815

@@ -367,20 +374,21 @@ unsigned char group_modp18[] = {
367374

368375
typedef struct {
369376
const char* name;
377+
unsigned int bits;
370378
unsigned char* prime;
371379
int prime_size;
372380
unsigned char* gen;
373381
int gen_size;
374382
} modp_group;
375383

376384
modp_group modp_groups[] = {
377-
{ "modp1", group_modp1, sizeof(group_modp1) / sizeof(group_modp1[0]), two_generator, 1 },
378-
{ "modp2", group_modp2, sizeof(group_modp2) / sizeof(group_modp2[0]), two_generator, 1 },
379-
{ "modp5", group_modp5, sizeof(group_modp5) / sizeof(group_modp5[0]), two_generator, 1 },
380-
{ "modp14", group_modp14, sizeof(group_modp14) / sizeof(group_modp14[0]), two_generator, 1 },
381-
{ "modp15", group_modp15, sizeof(group_modp15) / sizeof(group_modp15[0]), two_generator, 1 },
382-
{ "modp16", group_modp16, sizeof(group_modp16) / sizeof(group_modp16[0]), two_generator, 1 },
383-
{ "modp17", group_modp17, sizeof(group_modp17) / sizeof(group_modp17[0]), two_generator, 1 },
384-
{ "modp18", group_modp18, sizeof(group_modp18) / sizeof(group_modp18[0]), two_generator, 1 },
385-
{ NULL, NULL, 0, NULL, 0 }
386-
};
385+
{ "modp1", 768, group_modp1, sizeof(group_modp1) / sizeof(group_modp1[0]), two_generator, 1 },
386+
{ "modp2", 1024, group_modp2, sizeof(group_modp2) / sizeof(group_modp2[0]), two_generator, 1 },
387+
{ "modp5", 1536, group_modp5, sizeof(group_modp5) / sizeof(group_modp5[0]), two_generator, 1 },
388+
{ "modp14", 2048, group_modp14, sizeof(group_modp14) / sizeof(group_modp14[0]), two_generator, 1 },
389+
{ "modp15", 3072, group_modp15, sizeof(group_modp15) / sizeof(group_modp15[0]), two_generator, 1 },
390+
{ "modp16", 4096, group_modp16, sizeof(group_modp16) / sizeof(group_modp16[0]), two_generator, 1 },
391+
{ "modp17", 6144, group_modp17, sizeof(group_modp17) / sizeof(group_modp17[0]), two_generator, 1 },
392+
{ "modp18", 8192, group_modp18, sizeof(group_modp18) / sizeof(group_modp18[0]), two_generator, 1 },
393+
{ NULL, 0, NULL, 0, NULL, 0 }
394+
};

test/simple/test-crypto.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
var common = require('../common');
2626
var assert = require('assert');
27+
var spawn = require('child_process').spawn;
2728

2829
try {
2930
var crypto = require('crypto');
@@ -722,6 +723,38 @@ var aSecret = alice.computeSecret(bob.getPublicKey()).toString('hex');
722723
var bSecret = bob.computeSecret(alice.getPublicKey()).toString('hex');
723724
assert.equal(aSecret, bSecret);
724725

726+
function node_output(code, args, env, cb) {
727+
var out = '', err = '';
728+
var p = spawn(process.execPath, [ '-e', code ].concat(args), { env: env });
729+
p.stdout.on('data', function(data) { out += data; });
730+
p.stderr.on('data', function(data) { err += data; });
731+
p.on('close', function(code, signal) { cb(out, err, code); });
732+
}
733+
734+
function no_output(out, err, code) {
735+
assert.equal(out + err, '');
736+
assert.equal(code, 0);
737+
}
738+
739+
// test if fails on deprecated group
740+
node_output("require('crypto').getDiffieHellman('modp1')",
741+
[], {}, function(out, err, code) {
742+
assert.equal(out, '');
743+
assert.ok(err.indexOf('Small DH groups disabled') > -1);
744+
assert.notEqual(code, 1);
745+
});
746+
747+
// test if the environment variable makes it work
748+
node_output("require('crypto').getDiffieHellman('modp1')",
749+
[], { 'ENABLE_SMALL_DH_GROUPS': '' }, no_output);
750+
751+
// test if the cmdline switch makes it work
752+
node_output("require('crypto').getDiffieHellman('modp1')",
753+
[ '--enable-small-dh-groups' ], {}, no_output);
754+
755+
// test if does not fail on the next group
756+
node_output("require('crypto').getDiffieHellman('modp2')",
757+
[], {}, no_output);
725758

726759
// https://github.com/joyent/node/issues/2338
727760
assert.throws(function() {

0 commit comments

Comments
 (0)