Merge pull request #587 from agathver/hash-version-support

Allow to choose bcrypt minor version

Author: recrsn

CHANGELOG.md

@@ -1,3 +1,7 @@
+# UNRELEASED
+
+  * Make `2b` the default bcrypt version
+
 # 1.0.2 (2016-12-31)
 
   * Fix `compare` promise rejection with invalid arguments

README.md

@@ -181,10 +181,12 @@ If you are using bcrypt on a simple script, using the sync mode is perfectly fin
 
 `BCrypt.`
 
-  * `genSaltSync(rounds)`
+  * `genSaltSync(rounds, minor)`
     * `rounds` - [OPTIONAL] - the cost of processing the data. (default - 10)
-  * `genSalt(rounds, cb)`
+    * `minor` - [OPTIONAL] - minor version of bcrypt to use. (default - b)
+  * `genSalt(rounds, minor, cb)`
     * `rounds` - [OPTIONAL] - the cost of processing the data. (default - 10)
+    * `minor` - [OPTIONAL] - minor version of bcrypt to use. (default - b)
     * `cb` - [OPTIONAL] - a callback to be fired once the salt has been generated. uses eio making it asynchronous. If `cb` is not specified, a `Promise` is returned if Promise support is available.
       * `err` - First parameter to the callback detailing any errors.
       * `salt` - Second parameter to the callback providing the generated salt.
@@ -267,7 +269,7 @@ The code for this comes from a few sources:
 * [Nate Rajlich][tootallnate] - Bindings and build process.
 * [Sean McArthur][seanmonstar] - Windows Support
 * [Fanie Oosthuysen][weareu] - Windows Support
-* [Amitosh Swain Mahapatra][agathver] - ES6 Promise Support
+* [Amitosh Swain Mahapatra][agathver] - $2b$ hash support, ES6 Promise support
 
 ## License
 Unless stated elsewhere, file headers or otherwise, the license as stated in the LICENSE file.

bcrypt.js

@@ -12,36 +12,45 @@ var promises = require('./lib/promises');
 /// generate a salt (sync)
 /// @param {Number} [rounds] number of rounds (default 10)
 /// @return {String} salt
-module.exports.genSaltSync = function genSaltSync(rounds) {
+module.exports.genSaltSync = function genSaltSync(rounds, minor) {
     // default 10 rounds
     if (!rounds) {
         rounds = 10;
     } else if (typeof rounds !== 'number') {
         throw new Error('rounds must be a number');
     }
 
-    return bindings.gen_salt_sync(rounds, crypto.randomBytes(16));
+    if(!minor) {
+        minor = 'b';
+    } else if(minor !== 'b' && minor !== 'a') {
+        console.log(minor, typeof minor);
+        throw new Error('minor must be either "a" or "b"');
+    }
+
+    return bindings.gen_salt_sync(minor, rounds, crypto.randomBytes(16));
 };
 
 /// generate a salt
 /// @param {Number} [rounds] number of rounds (default 10)
 /// @param {Function} cb callback(err, salt)
-module.exports.genSalt = function genSalt(rounds, ignore, cb) {
+module.exports.genSalt = function genSalt(rounds, minor, cb) {
     var error;
 
     // if callback is first argument, then use defaults for others
     if (typeof arguments[0] === 'function') {
         // have to set callback first otherwise arguments are overriden
         cb = arguments[0];
         rounds = 10;
+        minor = 'b';
     // callback is second argument
     } else if (typeof arguments[1] === 'function') {
         // have to set callback first otherwise arguments are overriden
         cb = arguments[1];
+        minor = 'b';
     }
 
     if (!cb) {
-        return promises.promise(genSalt, this, [rounds, ignore]);
+        return promises.promise(genSalt, this, [rounds, minor]);
     }
 
     // default 10 rounds
@@ -55,13 +64,22 @@ module.exports.genSalt = function genSalt(rounds, ignore, cb) {
         });
     }
 
+    if(!minor) {
+        minor = 'b'
+    } else if(minor !== 'b' && minor !== 'a') {
+        error = new Error('minor must be either "a" or "b"');
+        return process.nextTick(function() {
+            cb(error);
+        });
+    }
+
     crypto.randomBytes(16, function(error, randomBytes) {
         if (error) {
             cb(error);
             return;
         }
 
-        bindings.gen_salt(rounds, randomBytes, cb);
+        bindings.gen_salt(minor, rounds, randomBytes, cb);
     });
 };
 

src/bcrypt.cc

@@ -111,11 +111,11 @@ decode_base64(u_int8_t *buffer, u_int16_t len, u_int8_t *data)
 }
 
 void
-encode_salt(char *salt, u_int8_t *csalt, u_int16_t clen, u_int8_t logr)
+encode_salt(char *salt, u_int8_t *csalt, char minor, u_int16_t clen, u_int8_t logr)
 {
 	salt[0] = '$';
 	salt[1] = BCRYPT_VERSION;
-	salt[2] = 'b';
+	salt[2] = minor;
 	salt[3] = '$';
 
 	snprintf(salt + 4, 4, "%2.2u$", logr);
@@ -130,14 +130,14 @@ encode_salt(char *salt, u_int8_t *csalt, u_int16_t clen, u_int8_t logr)
    from: http://mail-index.netbsd.org/tech-crypto/2002/05/24/msg000204.html
 */
 void
-bcrypt_gensalt(u_int8_t log_rounds, u_int8_t *seed, char *gsalt)
+bcrypt_gensalt(char minor, u_int8_t log_rounds, u_int8_t *seed, char *gsalt)
 {
 	if (log_rounds < 4)
 		log_rounds = 4;
 	else if (log_rounds > 31)
 		log_rounds = 31;
 
-	encode_salt(gsalt, seed, BCRYPT_MAXSALT, log_rounds);
+	encode_salt(gsalt, seed, minor, BCRYPT_MAXSALT, log_rounds);
 }
 
 /* We handle $Vers$log2(NumRounds)$salt+passwd$

src/bcrypt_node.cc

@@ -62,20 +62,25 @@ bool ValidateSalt(const char* salt) {
     return true;
 }
 
+char ToCharVersion(Local<String> str) {
+  String::Utf8Value value(str);
+  return **value;
+}
+
 /* SALT GENERATION */
 
 class SaltAsyncWorker : public Nan::AsyncWorker {
 public:
-    SaltAsyncWorker(Nan::Callback *callback, std::string seed, ssize_t rounds)
+    SaltAsyncWorker(Nan::Callback *callback, std::string seed, ssize_t rounds, char minor_ver)
         : Nan::AsyncWorker(callback, "bcrypt:SaltAsyncWorker"), seed(seed),
-          rounds(rounds) {
+          rounds(rounds), minor_ver(minor_ver) {
     }
 
     ~SaltAsyncWorker() {}
 
     void Execute() {
         char salt[_SALT_LEN];
-        bcrypt_gensalt(rounds, (u_int8_t *)&seed[0], salt);
+        bcrypt_gensalt(minor_ver, rounds, (u_int8_t *)&seed[0], salt);
         this->salt = std::string(salt);
     }
 
@@ -92,49 +97,62 @@ class SaltAsyncWorker : public Nan::AsyncWorker {
     std::string seed;
     std::string salt;
     ssize_t rounds;
+    char minor_ver;
 };
 
 NAN_METHOD(GenerateSalt) {
     Nan::HandleScope scope;
 
-    if (info.Length() < 3) {
-        Nan::ThrowTypeError("3 arguments expected");
+    if (info.Length() < 4) {
+        Nan::ThrowTypeError("4 arguments expected");
         return;
     }
 
-    if (!Buffer::HasInstance(info[1]) || Buffer::Length(info[1].As<Object>()) != 16) {
-        Nan::ThrowTypeError("Second argument must be a 16 byte Buffer");
+    if(!info[0]->IsString()) {
+        Nan::ThrowTypeError("First argument must be a string");
         return;
     }
 
-    const int32_t rounds = Nan::To<int32_t>(info[0]).FromMaybe(0);
-    Local<Object> seed = info[1].As<Object>();
-    Local<Function> callback = Local<Function>::Cast(info[2]);
+    if (!Buffer::HasInstance(info[2]) || Buffer::Length(info[2].As<Object>()) != 16) {
+        Nan::ThrowTypeError("Third argument must be a 16 byte Buffer");
+        return;
+    }
+
+    const char minor_ver = ToCharVersion(info[0]->ToString());
+    const int32_t rounds = Nan::To<int32_t>(info[1]).FromMaybe(0);
+    Local<Object> seed = info[2].As<Object>();
+    Local<Function> callback = Local<Function>::Cast(info[3]);
 
     SaltAsyncWorker* saltWorker = new SaltAsyncWorker(new Nan::Callback(callback),
-        std::string(Buffer::Data(seed), 16), rounds);
+        std::string(Buffer::Data(seed), 16), rounds, minor_ver);
 
     Nan::AsyncQueueWorker(saltWorker);
 }
 
 NAN_METHOD(GenerateSaltSync) {
     Nan::HandleScope scope;
 
-    if (info.Length() < 2) {
+    if (info.Length() < 3) {
         Nan::ThrowTypeError("2 arguments expected");
         return;
     }
 
-    if (!Buffer::HasInstance(info[1]) || Buffer::Length(info[1].As<Object>()) != 16) {
-        Nan::ThrowTypeError("Second argument must be a 16 byte Buffer");
+    if(!info[0]->IsString()) {
+        Nan::ThrowTypeError("First argument must be a string");
+        return;
+    }
+
+    if (!Buffer::HasInstance(info[2]) || Buffer::Length(info[2].As<Object>()) != 16) {
+        Nan::ThrowTypeError("Third argument must be a 16 byte Buffer");
         return;
     }
 
-    const int32_t rounds = Nan::To<int32_t>(info[0]).FromMaybe(0);
-    u_int8_t* seed = (u_int8_t*)Buffer::Data(info[1].As<Object>());
+    const char minor_ver = ToCharVersion(info[0]->ToString());
+    const int32_t rounds = Nan::To<int32_t>(info[1]).FromMaybe(0);
+    u_int8_t* seed = (u_int8_t*)Buffer::Data(info[2].As<Object>());
 
     char salt[_SALT_LEN];
-    bcrypt_gensalt(rounds, seed, salt);
+    bcrypt_gensalt(minor_ver, rounds, seed, salt);
 
     info.GetReturnValue().Set(Nan::Encode(salt, strlen(salt), Nan::BINARY));
 }

src/node_blf.h

@@ -42,7 +42,7 @@
 #define u_int64_t uint64_t
 #endif
 
-#ifdef _WIN32 
+#ifdef _WIN32
 #define u_int8_t unsigned __int8
 #define u_int16_t unsigned __int16
 #define u_int32_t unsigned __int32
@@ -103,9 +103,9 @@ void blf_cbc_decrypt(blf_ctx *, u_int8_t *, u_int8_t *, u_int32_t);
 u_int32_t Blowfish_stream2word(const u_int8_t *, u_int16_t , u_int16_t *);
 
 /* bcrypt functions*/
-void bcrypt_gensalt(u_int8_t, u_int8_t*, char *);
+void bcrypt_gensalt(char, u_int8_t, u_int8_t*, char *);
 void bcrypt(const char *, const char *, char *);
-void encode_salt(char *, u_int8_t *, u_int16_t, u_int8_t);
+void encode_salt(char *, u_int8_t *, char, u_int16_t, u_int8_t);
 u_int32_t bcrypt_get_rounds(const char *);
 
 #endif

test/async.js

@@ -19,11 +19,31 @@ module.exports = {
         });
     },
     test_salt_rounds_is_string_non_number: function(assert) {
-        bcrypt.genSalt('b', function (err, salt) {
+        bcrypt.genSalt('z', function (err, salt) {
             assert.ok((err instanceof Error), "Should throw an Error. genSalt requires rounds to of type number.");
             assert.done();
         });
     },
+    test_salt_minor: function(assert) {
+        assert.expect(3);
+        bcrypt.genSalt(10, 'a', function(err, salt) {
+            assert.strictEqual(29, salt.length, "Salt isn't the correct length.");
+            var split_salt = salt.split('$');
+            assert.strictEqual(split_salt[1], '2a');
+            assert.strictEqual(split_salt[2], '10');
+            assert.done();
+        });
+    },
+    test_salt_minor_b: function(assert) {
+        assert.expect(3);
+        bcrypt.genSalt(10, 'b', function(err, salt) {
+            assert.strictEqual(29, salt.length, "Salt isn't the correct length.");
+            var split_salt = salt.split('$');
+            assert.strictEqual(split_salt[1], '2b');
+            assert.strictEqual(split_salt[2], '10');
+            assert.done();
+        });
+    },
     test_hash: function(assert) {
         assert.expect(1);
         bcrypt.genSalt(10, function(err, salt) {

test/sync.js

@@ -25,6 +25,22 @@ module.exports = {
         assert.throws(function() {bcrypt.genSaltSync('b');}, "Should throw an Error. gen_salt requires rounds to be a number.");
         assert.done();
     },
+    test_salt_minor_a: function(assert) {
+        var salt = bcrypt.genSaltSync(10, 'a');
+        assert.strictEqual(29, salt.length, "Salt isn't the correct length.");
+        var split_salt = salt.split('$');
+        assert.strictEqual(split_salt[1], '2a');
+        assert.strictEqual(split_salt[2], '10');
+        assert.done();
+    },
+    test_salt_minor_b: function(assert) {
+        var salt = bcrypt.genSaltSync(10, 'b');
+        assert.strictEqual(29, salt.length, "Salt isn't the correct length.");
+        var split_salt = salt.split('$');
+        assert.strictEqual(split_salt[1], '2b');
+        assert.strictEqual(split_salt[2], '10');
+        assert.done();
+    },
     test_hash: function(assert) {
         assert.ok(bcrypt.hashSync('password', bcrypt.genSaltSync(10)), "Shouldn't throw an Error.");
         assert.done();