From 2d45be13b5edf1e47957841b51902a7ee0fc4e8d Mon Sep 17 00:00:00 2001 From: Amitosh Swain Mahapatra Date: Wed, 4 Apr 2018 19:32:43 +0530 Subject: [PATCH] Allow to choose bcrypt minor version --- CHANGELOG.md | 4 ++++ README.md | 8 ++++--- bcrypt.js | 28 ++++++++++++++++++++----- src/bcrypt.cc | 8 +++---- src/bcrypt_node.cc | 52 +++++++++++++++++++++++++++++++--------------- src/node_blf.h | 6 +++--- test/async.js | 22 +++++++++++++++++++- test/sync.js | 16 ++++++++++++++ 8 files changed, 111 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a77a8a9b..1703b38a 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index 41160a93..2d22ba75 100644 --- a/README.md +++ b/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. diff --git a/bcrypt.js b/bcrypt.js index e0eaedbb..f50a0d37 100644 --- a/bcrypt.js +++ b/bcrypt.js @@ -12,7 +12,7 @@ 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; @@ -20,13 +20,20 @@ module.exports.genSaltSync = function genSaltSync(rounds) { 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 @@ -34,14 +41,16 @@ module.exports.genSalt = function genSalt(rounds, ignore, cb) { // 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); }); }; diff --git a/src/bcrypt.cc b/src/bcrypt.cc index 28c0974e..be5c0975 100644 --- a/src/bcrypt.cc +++ b/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$ diff --git a/src/bcrypt_node.cc b/src/bcrypt_node.cc index 24a1d6ea..385270bd 100644 --- a/src/bcrypt_node.cc +++ b/src/bcrypt_node.cc @@ -62,20 +62,25 @@ bool ValidateSalt(const char* salt) { return true; } +char ToCharVersion(Local 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,27 +97,34 @@ 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()) != 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(info[0]).FromMaybe(0); - Local seed = info[1].As(); - Local callback = Local::Cast(info[2]); + if (!Buffer::HasInstance(info[2]) || Buffer::Length(info[2].As()) != 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(info[1]).FromMaybe(0); + Local seed = info[2].As(); + Local callback = Local::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); } @@ -120,21 +132,27 @@ NAN_METHOD(GenerateSalt) { 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()) != 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()) != 16) { + Nan::ThrowTypeError("Third argument must be a 16 byte Buffer"); return; } - const int32_t rounds = Nan::To(info[0]).FromMaybe(0); - u_int8_t* seed = (u_int8_t*)Buffer::Data(info[1].As()); + const char minor_ver = ToCharVersion(info[0]->ToString()); + const int32_t rounds = Nan::To(info[1]).FromMaybe(0); + u_int8_t* seed = (u_int8_t*)Buffer::Data(info[2].As()); 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)); } diff --git a/src/node_blf.h b/src/node_blf.h index ad674f0d..9d0840d1 100644 --- a/src/node_blf.h +++ b/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 diff --git a/test/async.js b/test/async.js index 79805873..1e10321e 100644 --- a/test/async.js +++ b/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) { diff --git a/test/sync.js b/test/sync.js index 08328567..04998237 100644 --- a/test/sync.js +++ b/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();