crypto: allow custom generator for DiffieHellman

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.
+Creates a Diffie-Hellman key exchange object using the supplied `prime` and an
-The generator used is `2`. Encoding can be `'binary'`, `'hex'`, or
+optional specific `generator`.
-`'base64'`.  If no encoding is specified, then a buffer is expected.
+`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

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,36 @@ 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);
+    return new DiffieHellman(sizeOrKey, keyEncoding, generator, genEncoding);
-
+
-  if (!sizeOrKey)
+  if (keyEncoding) {
-    this._binding = new binding.DiffieHellman();
+    if (typeof keyEncoding !== 'string' ||
-  else {
+        (!Buffer.isEncoding(keyEncoding) && keyEncoding !== 'buffer')) {
-    encoding = encoding || exports.DEFAULT_ENCODING;
+      genEncoding = generator;
-    sizeOrKey = toBuf(sizeOrKey, encoding);
+      generator = keyEncoding;
-    this._binding = new binding.DiffieHellman(sizeOrKey);
+      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 +499,11 @@ 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
+  });
 }
 
 

src/env.h

@@ -167,6 +167,7 @@ namespace node {
   V(used_heap_size_string, "used_heap_size")                                  \
   V(valid_from_string, "valid_from")                                          \
   V(valid_to_string, "valid_to")                                              \
+  V(verify_error_string, "verifyError")                                       \
   V(version_major_string, "versionMajor")                                     \
   V(version_minor_string, "versionMinor")                                     \
   V(version_string, "version")                                                \

src/node_constants.cc

@@ -930,6 +930,22 @@ void DefineOpenSSLConstants(Handle<Object> 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);

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<Value>& args) {
 void DiffieHellman::Initialize(Environment* env, Handle<Object> target) {
   Local<FunctionTemplate> t = FunctionTemplate::New(New);
 
+  static enum PropertyAttribute attributes =
+      static_cast<PropertyAttribute>(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<Object> 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<Value>(),
+                                     v8::DEFAULT,
+                                     attributes);
+
   target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "DiffieHellman"),
               t->GetFunction());
 
@@ -3182,14 +3194,21 @@ void DiffieHellman::Initialize(Environment* env, Handle<Object> 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<Value>(),
+                                      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<const unsigned char*>(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<const unsigned char*>(p), p_len, 0);
   dh->g = BN_bin2bn(reinterpret_cast<const unsigned char*>(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,
+    initialized = diffieHellman->Init(it->prime,
-                        it->prime_size,
+                                      it->prime_size,
-                        it->gen,
+                                      it->gen,
-                        it->gen_size);
+                                      it->gen_size);
+    if (!initialized)
+      ThrowError("Initialization failed");
     return;
   }
 
@@ -3258,12 +3284,23 @@ void DiffieHellman::New(const FunctionCallbackInfo<Value>& 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]),
+      if (args[1]->IsInt32()) {
-                                        Buffer::Length(args[0]));
+        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<Value>& args) {
 }
 
 
+void DiffieHellman::VerifyErrorGetter(Local<String> property,
+                                      const PropertyCallbackInfo<Value>& args) {
+  HandleScope scope(args.GetIsolate());
+
+  DiffieHellman* diffieHellman = Unwrap<DiffieHellman>(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)
+  verifyError_ = codes;
-    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;
   return true;
 }
 

src/node_crypto.h

@@ -541,8 +541,8 @@ class DiffieHellman : public BaseObject {
 
   static void Initialize(Environment* env, v8::Handle<v8::Object> target);
 
-  bool Init(int primeLength);
+  bool Init(int primeLength, int g);
-  bool Init(const char* p, int p_len);
+  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,14 @@ class DiffieHellman : public BaseObject {
   static void ComputeSecret(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void SetPublicKey(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void SetPrivateKey(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void VerifyErrorGetter(
+      v8::Local<v8::String> property,
+      const v8::PropertyCallbackInfo<v8::Value>& args);
 
   DiffieHellman(Environment* env, v8::Local<v8::Object> wrap)
       : BaseObject(env, wrap),
         initialised_(false),
+        verifyError_(0),
         dh(NULL) {
     MakeWeak<DiffieHellman>(this);
   }
@@ -569,6 +573,7 @@ class DiffieHellman : public BaseObject {
   bool VerifyContext();
 
   bool initialised_;
+  int verifyError_;
   DH* dh;
 };
 

test/simple/test-crypto-binary-default.js

@@ -25,6 +25,7 @@
 
 var common = require('../common');
 var assert = require('assert');
+var constants = require('constants');
 
 try {
   var crypto = require('crypto');
@@ -569,13 +570,12 @@ var secret3 = dh3.computeSecret(key2, 'hex', 'base64');
 assert.equal(secret1, secret3);
 
 // https://github.com/joyent/node/issues/2338
-assert.throws(function() {
+var p = 'FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74' +
-  var p = 'FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74' +
+        '020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F1437' +
-          '020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F1437' +
+        '4FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7ED' +
-          '4FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7ED' +
+        'EE386BFB5A899FA5AE9F24117C4B1FE649286651ECE65381FFFFFFFFFFFFFFFF';
-          'EE386BFB5A899FA5AE9F24117C4B1FE649286651ECE65381FFFFFFFFFFFFFFFF';
+var d = crypto.createDiffieHellman(p, 'hex');
-  crypto.createDiffieHellman(p, 'hex');
+assert.equal(d.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
-});
 
 // Test RSA key signing/verification
 var rsaSign = crypto.createSign('RSA-SHA1');

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);
-
+assert.equal(alice.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
-
+assert.equal(bob.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
-// https://github.com/joyent/node/issues/2338
+
-assert.throws(function() {
+// Ensure specific generator (buffer) works as expected.
-  var p = 'FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74' +
+var modp1 = crypto.createDiffieHellmanGroup('modp1');
-          '020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F1437' +
+var modp1buf = new Buffer([
-          '4FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7ED' +
+  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc9, 0x0f,
-          'EE386BFB5A899FA5AE9F24117C4B1FE649286651ECE65381FFFFFFFFFFFFFFFF';
+  0xda, 0xa2, 0x21, 0x68, 0xc2, 0x34, 0xc4, 0xc6, 0x62, 0x8b,
-  crypto.createDiffieHellman(p, 'hex');
+  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');