crypto: allow custom generator for DiffieHellman

doc/api/crypto.markdown

@@ -388,23 +388,39 @@ 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
 
 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);
-
-  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 +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,17 +3254,21 @@ 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;
 
     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<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]),
-                                        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<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)
-    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;
 }
 

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(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,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' +
-          '020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F1437' +
-          '4FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7ED' +
-          'EE386BFB5A899FA5AE9F24117C4B1FE649286651ECE65381FFFFFFFFFFFFFFFF';
-  crypto.createDiffieHellman(p, 'hex');
-});
+var p = 'FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74' +
+        '020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F1437' +
+        '4FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7ED' +
+        'EE386BFB5A899FA5AE9F24117C4B1FE649286651ECE65381FFFFFFFFFFFFFFFF';
+var d = crypto.createDiffieHellman(p, 'hex');
+assert.equal(d.verifyError, constants.DH_NOT_SUITABLE_GENERATOR);
 
 // Test RSA key signing/verification
 var rsaSign = crypto.createSign('RSA-SHA1');