From abb070234d69f66eb1feffa41f6b86fb1d07add8 Mon Sep 17 00:00:00 2001 From: Brian White Date: Mon, 17 Feb 2014 20:57:08 -0500 Subject: [PATCH] crypto: allow custom generator for DiffieHellman --- doc/api/crypto.markdown | 28 ++++++++++--- lib/crypto.js | 50 ++++++++++++++++++----- src/env.h | 1 + src/node_constants.cc | 16 ++++++++ src/node_crypto.cc | 83 +++++++++++++++++++++++++++++--------- src/node_crypto.h | 8 +++- test/simple/test-crypto.js | 79 +++++++++++++++++++++++++++++++----- 7 files changed, 217 insertions(+), 48 deletions(-) diff --git a/doc/api/crypto.markdown b/doc/api/crypto.markdown index 269419c2d48..a029d6bcfab 100644 --- a/doc/api/crypto.markdown +++ b/doc/api/crypto.markdown @@ -388,16 +388,21 @@ the data and public key. Note: `verifier` object can not be used after `verify()` method has been called. -## crypto.createDiffieHellman(prime_length) +## crypto.createDiffieHellman(prime_length, [generator]) Creates a Diffie-Hellman key exchange object and generates a prime of -the given bit length. The generator used is `2`. +`prime_length` bits and using an optional specific numeric `generator`. +If no `generator` is specified, then `2` is used. -## crypto.createDiffieHellman(prime, [encoding]) +## crypto.createDiffieHellman(prime, [prime_encoding], [generator], [generator_encoding]) -Creates a Diffie-Hellman key exchange object using the supplied prime. -The generator used is `2`. Encoding can be `'binary'`, `'hex'`, or -`'base64'`. If no encoding is specified, then a buffer is expected. +Creates a Diffie-Hellman key exchange object using the supplied `prime` and an +optional specific `generator`. +`generator` can be a number, string, or Buffer. +If no `generator` is specified, then `2` is used. +`prime_encoding` and `generator_encoding` can be `'binary'`, `'hex'`, or `'base64'`. +If no `prime_encoding` is specified, then a Buffer is expected for `prime`. +If no `generator_encoding` is specified, then a Buffer is expected for `generator`. ## Class: DiffieHellman @@ -405,6 +410,17 @@ The class for creating Diffie-Hellman key exchanges. Returned by `crypto.createDiffieHellman`. +### diffieHellman.verifyError + +A bit field containing any warnings and/or errors as a result of a check performed +during initialization. The following values are valid for this property +(defined in `constants` module): + +* `DH_CHECK_P_NOT_SAFE_PRIME` +* `DH_CHECK_P_NOT_PRIME` +* `DH_UNABLE_TO_CHECK_GENERATOR` +* `DH_NOT_SUITABLE_GENERATOR` + ### diffieHellman.generateKeys([encoding]) Generates private and public Diffie-Hellman key values, and returns diff --git a/lib/crypto.js b/lib/crypto.js index 82262414906..4d7c33994ed 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -39,6 +39,8 @@ var constants = require('constants'); var stream = require('stream'); var util = require('util'); +var DH_GENERATOR = 2; + // This is here because many functions accepted binary strings without // any explicit encoding in older versions of node, and we don't want // to break them unnecessarily. @@ -456,17 +458,39 @@ Verify.prototype.verify = function(object, signature, sigEncoding) { exports.createDiffieHellman = exports.DiffieHellman = DiffieHellman; -function DiffieHellman(sizeOrKey, encoding) { +function DiffieHellman(sizeOrKey, keyEncoding, generator, genEncoding) { if (!(this instanceof DiffieHellman)) - return new DiffieHellman(sizeOrKey, encoding); - - if (!sizeOrKey) - this._binding = new binding.DiffieHellman(); - else { - encoding = encoding || exports.DEFAULT_ENCODING; - sizeOrKey = toBuf(sizeOrKey, encoding); - this._binding = new binding.DiffieHellman(sizeOrKey); + return new DiffieHellman(sizeOrKey, keyEncoding, generator, genEncoding); + + if (keyEncoding) { + if (typeof keyEncoding !== 'string' || + (!Buffer.isEncoding(keyEncoding) && keyEncoding !== 'buffer')) { + genEncoding = generator; + generator = keyEncoding; + keyEncoding = false; + } } + + keyEncoding = keyEncoding || exports.DEFAULT_ENCODING; + genEncoding = genEncoding || exports.DEFAULT_ENCODING; + + if (typeof sizeOrKey !== 'number') + sizeOrKey = toBuf(sizeOrKey, keyEncoding); + + if (!generator) + generator = DH_GENERATOR; + else if (typeof generator !== 'number') + generator = toBuf(generator, genEncoding); + + this._binding = new binding.DiffieHellman(sizeOrKey, generator); + Object.defineProperty(this, + 'verifyError', + { + enumerable: true, + value: this._binding.verifyError, + writable: false + } + ); } @@ -478,6 +502,14 @@ function DiffieHellmanGroup(name) { if (!(this instanceof DiffieHellmanGroup)) return new DiffieHellmanGroup(name); this._binding = new binding.DiffieHellmanGroup(name); + Object.defineProperty(this, + 'verifyError', + { + enumerable: true, + value: this._binding.verifyError, + writable: false + } + ); } diff --git a/src/env.h b/src/env.h index 99f0e2092bf..c990d3da9f0 100644 --- a/src/env.h +++ b/src/env.h @@ -173,6 +173,7 @@ namespace node { V(windows_verbatim_arguments_string, "windowsVerbatimArguments") \ V(writable_string, "writable") \ V(write_queue_size_string, "writeQueueSize") \ + V(verify_error_string, "verifyError") \ #define ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V) \ V(async_listener_run_function, v8::Function) \ diff --git a/src/node_constants.cc b/src/node_constants.cc index 81d4124c9fc..c903dd90ea3 100644 --- a/src/node_constants.cc +++ b/src/node_constants.cc @@ -930,6 +930,22 @@ void DefineOpenSSLConstants(Handle target) { # endif // !OPENSSL_NO_ENGINE +#ifdef DH_CHECK_P_NOT_SAFE_PRIME + NODE_DEFINE_CONSTANT(target, DH_CHECK_P_NOT_SAFE_PRIME); +#endif + +#ifdef DH_CHECK_P_NOT_PRIME + NODE_DEFINE_CONSTANT(target, DH_CHECK_P_NOT_PRIME); +#endif + +#ifdef DH_UNABLE_TO_CHECK_GENERATOR + NODE_DEFINE_CONSTANT(target, DH_UNABLE_TO_CHECK_GENERATOR); +#endif + +#ifdef DH_NOT_SUITABLE_GENERATOR + NODE_DEFINE_CONSTANT(target, DH_NOT_SUITABLE_GENERATOR); +#endif + #ifdef OPENSSL_NPN_NEGOTIATED #define NPN_ENABLED 1 NODE_DEFINE_CONSTANT(target, NPN_ENABLED); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index d0a26a605c9..fe7e0b3b28b 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -88,6 +88,8 @@ using v8::Local; using v8::Null; using v8::Object; using v8::Persistent; +using v8::PropertyAttribute; +using v8::PropertyCallbackInfo; using v8::String; using v8::ThrowException; using v8::V8; @@ -3158,6 +3160,9 @@ void Verify::VerifyFinal(const FunctionCallbackInfo& args) { void DiffieHellman::Initialize(Environment* env, Handle target) { Local t = FunctionTemplate::New(New); + static enum PropertyAttribute attributes = + static_cast(v8::ReadOnly | v8::DontDelete); + t->InstanceTemplate()->SetInternalFieldCount(1); NODE_SET_PROTOTYPE_METHOD(t, "generateKeys", GenerateKeys); @@ -3169,6 +3174,13 @@ void DiffieHellman::Initialize(Environment* env, Handle target) { NODE_SET_PROTOTYPE_METHOD(t, "setPublicKey", SetPublicKey); NODE_SET_PROTOTYPE_METHOD(t, "setPrivateKey", SetPrivateKey); + t->InstanceTemplate()->SetAccessor(env->verify_error_string(), + DiffieHellman::VerifyErrorGetter, + NULL, + Handle(), + v8::DEFAULT, + attributes); + target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "DiffieHellman"), t->GetFunction()); @@ -3182,14 +3194,21 @@ void DiffieHellman::Initialize(Environment* env, Handle target) { NODE_SET_PROTOTYPE_METHOD(t2, "getPublicKey", GetPublicKey); NODE_SET_PROTOTYPE_METHOD(t2, "getPrivateKey", GetPrivateKey); + t2->InstanceTemplate()->SetAccessor(env->verify_error_string(), + DiffieHellman::VerifyErrorGetter, + NULL, + Handle(), + v8::DEFAULT, + attributes); + target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "DiffieHellmanGroup"), t2->GetFunction()); } -bool DiffieHellman::Init(int primeLength) { +bool DiffieHellman::Init(int primeLength, int g) { dh = DH_new(); - DH_generate_parameters_ex(dh, primeLength, DH_GENERATOR_2, 0); + DH_generate_parameters_ex(dh, primeLength, g, 0); bool result = VerifyContext(); if (!result) return false; @@ -3198,11 +3217,11 @@ bool DiffieHellman::Init(int primeLength) { } -bool DiffieHellman::Init(const char* p, int p_len) { +bool DiffieHellman::Init(const char* p, int p_len, int g) { dh = DH_new(); dh->p = BN_bin2bn(reinterpret_cast(p), p_len, 0); dh->g = BN_new(); - if (!BN_set_word(dh->g, 2)) + if (!BN_set_word(dh->g, g)) return false; bool result = VerifyContext(); if (!result) @@ -3216,6 +3235,9 @@ bool DiffieHellman::Init(const char* p, int p_len, const char* g, int g_len) { dh = DH_new(); dh->p = BN_bin2bn(reinterpret_cast(p), p_len, 0); dh->g = BN_bin2bn(reinterpret_cast(g), g_len, 0); + bool result = VerifyContext(); + if (!result) + return false; initialised_ = true; return true; } @@ -3232,6 +3254,8 @@ void DiffieHellman::DiffieHellmanGroup( return ThrowError("No group name given"); } + bool initialized = false; + const String::Utf8Value group_name(args[0]); for (unsigned int i = 0; i < ARRAY_SIZE(modp_groups); ++i) { const modp_group* it = modp_groups + i; @@ -3239,10 +3263,12 @@ void DiffieHellman::DiffieHellmanGroup( if (strcasecmp(*group_name, it->name) != 0) continue; - diffieHellman->Init(it->prime, - it->prime_size, - it->gen, - it->gen_size); + initialized = diffieHellman->Init(it->prime, + it->prime_size, + it->gen, + it->gen_size); + if (!initialized) + ThrowError("Initialization failed"); return; } @@ -3258,12 +3284,23 @@ void DiffieHellman::New(const FunctionCallbackInfo& args) { new DiffieHellman(env, args.This()); bool initialized = false; - if (args.Length() > 0) { + if (args.Length() == 2) { if (args[0]->IsInt32()) { - initialized = diffieHellman->Init(args[0]->Int32Value()); + if (args[1]->IsInt32()) { + initialized = diffieHellman->Init(args[0]->Int32Value(), + args[1]->Int32Value()); + } } else { - initialized = diffieHellman->Init(Buffer::Data(args[0]), - Buffer::Length(args[0])); + if (args[1]->IsInt32()) { + initialized = diffieHellman->Init(Buffer::Data(args[0]), + Buffer::Length(args[0]), + args[1]->Int32Value()); + } else { + initialized = diffieHellman->Init(Buffer::Data(args[0]), + Buffer::Length(args[0]), + Buffer::Data(args[1]), + Buffer::Length(args[1])); + } } } @@ -3490,18 +3527,24 @@ void DiffieHellman::SetPrivateKey(const FunctionCallbackInfo& args) { } +void DiffieHellman::VerifyErrorGetter(Local property, + const PropertyCallbackInfo& args) { + HandleScope scope(args.GetIsolate()); + + DiffieHellman* diffieHellman = Unwrap(args.This()); + + if (!diffieHellman->initialised_) + return ThrowError("Not initialized"); + + args.GetReturnValue().Set(diffieHellman->verifyError_); +} + + bool DiffieHellman::VerifyContext() { int codes; if (!DH_check(dh, &codes)) return false; - if (codes & DH_CHECK_P_NOT_SAFE_PRIME) - return false; - if (codes & DH_CHECK_P_NOT_PRIME) - return false; - if (codes & DH_UNABLE_TO_CHECK_GENERATOR) - return false; - if (codes & DH_NOT_SUITABLE_GENERATOR) - return false; + verifyError_ = codes; return true; } diff --git a/src/node_crypto.h b/src/node_crypto.h index 6efd00deeb7..8855c2f8cdf 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -541,8 +541,8 @@ class DiffieHellman : public BaseObject { static void Initialize(Environment* env, v8::Handle target); - bool Init(int primeLength); - bool Init(const char* p, int p_len); + bool Init(int primeLength, int g); + bool Init(const char* p, int p_len, int g); bool Init(const char* p, int p_len, const char* g, int g_len); protected: @@ -557,10 +557,13 @@ class DiffieHellman : public BaseObject { static void ComputeSecret(const v8::FunctionCallbackInfo& args); static void SetPublicKey(const v8::FunctionCallbackInfo& args); static void SetPrivateKey(const v8::FunctionCallbackInfo& args); + static void VerifyErrorGetter(v8::Local property, + const v8::PropertyCallbackInfo& args); DiffieHellman(Environment* env, v8::Local wrap) : BaseObject(env, wrap), initialised_(false), + verifyError_(0), dh(NULL) { MakeWeak(this); } @@ -569,6 +572,7 @@ class DiffieHellman : public BaseObject { bool VerifyContext(); bool initialised_; + int verifyError_; DH* dh; }; diff --git a/test/simple/test-crypto.js b/test/simple/test-crypto.js index d5ae099b17d..39fc1216aae 100644 --- a/test/simple/test-crypto.js +++ b/test/simple/test-crypto.js @@ -37,6 +37,7 @@ crypto.DEFAULT_ENCODING = 'buffer'; var fs = require('fs'); var path = require('path'); +var constants = require('constants'); // Test Certificates var caPem = fs.readFileSync(common.fixturesDir + '/test_ca.pem', 'ascii'); @@ -695,13 +696,15 @@ assert.throws(function() { // using various encodings as we go along var dh1 = crypto.createDiffieHellman(256); var p1 = dh1.getPrime('buffer'); -var dh2 = crypto.createDiffieHellman(p1, 'base64'); +var dh2 = crypto.createDiffieHellman(p1, 'buffer'); var key1 = dh1.generateKeys(); var key2 = dh2.generateKeys('hex'); var secret1 = dh1.computeSecret(key2, 'hex', 'base64'); var secret2 = dh2.computeSecret(key1, 'binary', 'buffer'); assert.equal(secret1, secret2.toString('base64')); +assert.equal(dh1.verifyError, 0); +assert.equal(dh2.verifyError, 0); // Create "another dh1" using generated keys from dh1, // and compute secret again @@ -714,6 +717,7 @@ assert.deepEqual(dh1.getPrime(), dh3.getPrime()); assert.deepEqual(dh1.getGenerator(), dh3.getGenerator()); assert.deepEqual(dh1.getPublicKey(), dh3.getPublicKey()); assert.deepEqual(dh1.getPrivateKey(), dh3.getPrivateKey()); +assert.equal(dh3.verifyError, 0); var secret3 = dh3.computeSecret(key2, 'hex', 'base64'); @@ -742,16 +746,69 @@ bob.generateKeys(); var aSecret = alice.computeSecret(bob.getPublicKey()).toString('hex'); var bSecret = bob.computeSecret(alice.getPublicKey()).toString('hex'); assert.equal(aSecret, bSecret); - - -// https://github.com/joyent/node/issues/2338 -assert.throws(function() { - var p = 'FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74' + - '020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F1437' + - '4FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7ED' + - 'EE386BFB5A899FA5AE9F24117C4B1FE649286651ECE65381FFFFFFFFFFFFFFFF'; - crypto.createDiffieHellman(p, 'hex'); -}); +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([ + 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, + 0xcc, 0x74, 0x02, 0x0b, 0xbe, 0xa6, 0x3b, 0x13, 0x9b, 0x22, + 0x51, 0x4a, 0x08, 0x79, 0x8e, 0x34, 0x04, 0xdd, 0xef, 0x95, + 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, + 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); + + +// 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()) + .toString('hex'); +assert.equal(modp1Secret, exmodp1_2Secret); +assert.equal(exmodp1_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()) + .toString('hex'); +assert.equal(modp1Secret, exmodp1_3Secret); +assert.equal(exmodp1_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()) + .toString('hex'); +assert.equal(modp1Secret, exmodp1_4Secret); +assert.equal(exmodp1_4.verifyError, constants.DH_NOT_SUITABLE_GENERATOR); + + +var p = 'FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74' + + '020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F1437' + + '4FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7ED' + + 'EE386BFB5A899FA5AE9F24117C4B1FE649286651ECE65381FFFFFFFFFFFFFFFF'; +var bad_dh = crypto.createDiffieHellman(p, 'hex'); +assert.equal(bad_dh.verifyError, constants.DH_NOT_SUITABLE_GENERATOR); // Test RSA key signing/verification var rsaSign = crypto.createSign('RSA-SHA1');