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

Commit

Permalink
Fix for the issue 25366.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
thinred committed Jun 23, 2015
1 parent 6f8400a commit 4000f3b
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 25 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'` (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();
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 (not recommended)\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 (not recommended)\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
7 changes: 6 additions & 1 deletion 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 @@ -802,7 +803,7 @@ size_t ClientHelloParser::Write(const uint8_t* data, size_t len) {
HandleScope scope;

assert(state_ != kEnded);

// Just accumulate data, everything will be pushed to BIO later
if (state_ == kPaused) return 0;

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, sizeof(group_modp1) / sizeof(group_modp1[0]), two_generator, 1 },
{ "modp2", 1024, group_modp2, sizeof(group_modp2) / sizeof(group_modp2[0]), two_generator, 1 },
{ "modp5", 1536, group_modp5, sizeof(group_modp5) / sizeof(group_modp5[0]), two_generator, 1 },
{ "modp14", 2048, group_modp14, sizeof(group_modp14) / sizeof(group_modp14[0]), two_generator, 1 },
{ "modp15", 3072, group_modp15, sizeof(group_modp15) / sizeof(group_modp15[0]), two_generator, 1 },
{ "modp16", 4096, group_modp16, sizeof(group_modp16) / sizeof(group_modp16[0]), two_generator, 1 },
{ "modp17", 6144, group_modp17, sizeof(group_modp17) / sizeof(group_modp17[0]), two_generator, 1 },
{ "modp18", 8192, group_modp18, sizeof(group_modp18) / sizeof(group_modp18[0]), two_generator, 1 },
{ NULL, 0, NULL, 0, NULL, 0 }
};
33 changes: 33 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,38 @@ 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 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.notEqual(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);

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

0 comments on commit 4000f3b

Please sign in to comment.