From df52a0ef95400d933f5ef5dcd43d9b60f2b0cdb5 Mon Sep 17 00:00:00 2001 From: Esteban Ordano Date: Mon, 3 Nov 2014 23:29:09 -0300 Subject: [PATCH 1/3] fix: Passphrase getting generated correctly --- js/plugins/InsightStorage.js | 82 ++++++++++++++++++++-- test/plugin.insight.js | 131 +++++++++++++++++++++++++++++++++++ test/util.crypto.js | 3 + util/build.js | 3 + 4 files changed, 214 insertions(+), 5 deletions(-) create mode 100644 test/plugin.insight.js diff --git a/js/plugins/InsightStorage.js b/js/plugins/InsightStorage.js index 799163e9c..5d5d2f88c 100644 --- a/js/plugins/InsightStorage.js +++ b/js/plugins/InsightStorage.js @@ -1,5 +1,6 @@ var request = require('request'); var cryptoUtil = require('../util/crypto'); +var buffers = require('buffer'); var querystring = require('querystring'); var Identity = require('../models/Identity'); @@ -18,6 +19,7 @@ InsightStorage.prototype.setCredentials = function(email, password, opts) { InsightStorage.prototype.createItem = function(name, value, callback) { var self = this; + this.getItem(name, function(err, retrieved) { if (err || !retrieved) { return self.setItem(name, value, callback); @@ -27,12 +29,35 @@ InsightStorage.prototype.createItem = function(name, value, callback) { }); }; +function mayBeOldPassword(password) { + // Test for base64 + return /^[a-zA-Z0-9\/=\+]+$/.test(password); +} + InsightStorage.prototype.getItem = function(name, callback) { var key = cryptoUtil.kdf(this.password + this.email); - var secret = cryptoUtil.kdf(key, this.password); - var encodedEmail = encodeURIComponent(this.email); - var retrieveUrl = this.storeUrl + '/retrieve/' + encodedEmail; - this.request.get(retrieveUrl + '?' + querystring.encode({secret: secret, key: name}), + var secret = this.makeSecret(key); + var self = this; + + this._makeGetRequest(secret, name, function(err, body) { + if (err && err.indexOf('PNOTFOUND') !== -1 && mayBeOldPassword(self.password)) { + return self._brokenGetItem(key, name, callback); + } + return callback(err, body); + }); +}; + +InsightStorage.prototype.makeSecret = function(key) { + return cryptoUtil.kdf(key + this.password); +}; + +InsightStorage.prototype._makeGetRequest = function(secret, key, callback) { + var authHeader = new Buffer(this.email + ':' + secret).toString('base64'); + var retrieveUrl = this.storeUrl + '/retrieve'; + this.request.get({ + url: retrieveUrl + '?' + querystring.encode({key: key}), + headers: {'Authorization': authHeader} + }, function(err, response, body) { if (err) { return callback('Connection error'); @@ -48,9 +73,56 @@ InsightStorage.prototype.getItem = function(name, callback) { ); }; +InsightStorage.prototype._brokenGetItem = function(key, name, callback) { + var secret = this._makeBrokenSecret(key); + var self = this; + this._makeGetRequest(secret, name, function(err, body) { + if (!err) { + return self._changePassword(function(err) { + if (err) { + return callback(err); + } + return callback(null, body); + }); + } + return callback(err); + }); +}; + +InsightStorage.prototype._makeBrokenSecret = function(key) { + return cryptoUtil.kdf(key, this.password); +}; + +InsightStorage.prototype._changePassword = function(callback) { + var key = cryptoUtil.kdf(this.password + this.email); + var secret = this._makeBrokenSecret(key); + var newSecret = this.makeSecret(key); + + var url = this.storeUrl + '/change_passphrase'; + this.request.post({ + url: url, + body: querystring.encode({ + email: this.email, + secret: secret, + newSecret: newSecret + }) + }, function(err, response, body) { + if (err) { + return callback('Connection error'); + } + if (response.statusCode === 409) { + return callback('BADCREDENTIALS: Invalid username or password'); + } + if (response.statusCode !== 200) { + return callback('Unable to store data on insight'); + } + return callback(); + }); +}; + InsightStorage.prototype.setItem = function(name, value, callback) { var key = cryptoUtil.kdf(this.password + this.email); - var secret = cryptoUtil.kdf(key, this.password); + var secret = this.makeSecret(key); var registerUrl = this.storeUrl + '/register'; this.request.post({ url: registerUrl, diff --git a/test/plugin.insight.js b/test/plugin.insight.js new file mode 100644 index 000000000..2a202c849 --- /dev/null +++ b/test/plugin.insight.js @@ -0,0 +1,131 @@ +var InsightStorage = require('../js/plugins/InsightStorage'); +var assert = require('assert'); +var querystring = require('querystring'); + +describe('insight storage plugin', function() { + + var requestMock = sinon.stub(); + var storage = new InsightStorage({request: requestMock}); + var email = 'john@doe.com'; + var password = '1234'; + + var data = '{"random": true}'; + var namespace = 'profile::0000000000000000000000000000000000000000'; + + var oldSecret = 'rFA+F/N+ZvKXp717zBdfCKYQ5v9Fjry0W6tautj5etIH' + + 'KLQliZBEYXA7AXjTJ9K3DglzGWJKost3QJUCMbhM/A==' + var newSecret = '2YTcmtkmC/WSVrfmjAEyOmDnmntVQ8A9wS/q1DbD6rkc' + + 'si8LrIvS2Ru85Feb5Bvk2ziQbEN6PzlL1mIyQ3a+Vw==' + + var setupStorageCredentials = function() { + storage.setCredentials(email, password); + }; + + beforeEach(function() { + requestMock.reset(); + requestMock.get = sinon.stub(); + requestMock.post = sinon.stub(); + setupStorageCredentials(); + }); + + var setupForCreation = function() { + requestMock.get.onFirstCall().callsArgWith(1, 'Not found'); + requestMock.post.onFirstCall().callsArgWith(1, null, {statusCode: 200}); + }; + + it('should be able to create a namespace for storage', function(done) { + + setupForCreation(); + + storage.createItem(namespace, data, function(err) { + assert(!err); + return done(); + }); + }); + + var setupForRetrieval = function() { + requestMock.get.onFirstCall().callsArgWith(1, null, {statusCode: 200}, data); + }; + + it('should be able to retrieve data in a namespace', function(done) { + + setupForRetrieval(); + + storage.getItem(namespace, function(err, retrieved) { + assert(!err); + assert(retrieved === data); + return done(); + }); + }); + + var setupForSave = function () { + requestMock.post.onFirstCall().callsArgWith(1, null, {statusCode: 200}); + }; + + it('should be able to overwrite data when using same password', function(done) { + setupForSave(); + + storage.setItem(namespace, data, function(err) { + assert(!err); + assert(requestMock.post.firstCall.args[0].url.indexOf('register') !== -1); + return done(); + }); + }); + + it('won\'t make an unnecessary request if old password can\'t work', function(done) { + storage.setCredentials(email, '!'); + setupForRetrieval(); + + storage.getItem(namespace, function(err, retrieved) { + assert(requestMock.get.firstCall); + assert(!requestMock.get.secondCall); + return done(); + }); + }); + + it('shouldn\'t be able to create a namespace twice', function(done) { + setupForRetrieval(); + + storage.createItem(namespace, data, function(err) { + assert(err); + assert(requestMock.get.firstCall.args[0].url.indexOf('retrieve') !== -1); + assert(!requestMock.post.firstCall); + return done(); + }); + }); + + var setupForOldData = function() { + requestMock.get = sinon.stub(); + requestMock.get.onFirstCall().callsArgWith(1, null, {statusCode: 403}); + requestMock.get.onSecondCall().callsArgWith(1, null, {statusCode: 200}, data); + requestMock.post = sinon.stub(); + requestMock.post.onFirstCall().callsArgWith(1, null, {statusCode: 200}); + } + + it('should be able to restore 0.7.2 data', function(done) { + + setupForOldData(); + + storage.getItem(namespace, function(error, dataReturned) { + assert(!error); + done(); + }); + }); + + it('should change the remote passphrase if retrieved with 0.7.2 passphrase', + function(done) { + + setupForOldData(); + + storage.getItem(namespace, function(error, dataReturned) { + var receivedArgs = requestMock.post.firstCall.args[0].body; + var url = requestMock.post.firstCall.args[0].url; + var args = querystring.decode(receivedArgs); + assert(url.indexOf('change_passphrase') !== -1); + assert(args.secret === oldSecret); + assert(args.newSecret === newSecret); + done(); + }); + } + ); +}); diff --git a/test/util.crypto.js b/test/util.crypto.js index f220f2401..cc991b947 100644 --- a/test/util.crypto.js +++ b/test/util.crypto.js @@ -48,6 +48,9 @@ describe('crypto utils', function() { var phrase = cryptoUtils.kdf(t.word, t.salt, t.iterations); phrase.should.equal(t.phrase); }); + it('should generate a passphrase from weird chars', function() { + var phrase = cryptoUtils.kdf('Pwd123!@#$%^&*(){}[]\|/?.>,<=+-_`~åéþ䲤þçæ¶'); + }); }); diff --git a/util/build.js b/util/build.js index d3ec91ebd..908729562 100644 --- a/util/build.js +++ b/util/build.js @@ -88,6 +88,9 @@ var createBundle = function(opts) { b.require('./js/plugins/InsightStorage', { expose: '../plugins/InsightStorage' }); + b.require('./js/plugins/InsightStorage', { + expose: '../js/plugins/InsightStorage' + }); b.require('./js/plugins/LocalStorage', { expose: '../plugins/LocalStorage' }); From 49fbf2c849c0ce51da6b5a7970277a5c50de56f8 Mon Sep 17 00:00:00 2001 From: Esteban Ordano Date: Tue, 4 Nov 2014 11:47:38 -0300 Subject: [PATCH 2/3] Use hmac instead of a second pbkdf --- js/plugins/InsightStorage.js | 56 ++++++++++++++++++++---------------- js/util/crypto.js | 11 +++++++ test/plugin.insight.js | 9 +++--- test/util.crypto.js | 3 ++ 4 files changed, 51 insertions(+), 28 deletions(-) diff --git a/js/plugins/InsightStorage.js b/js/plugins/InsightStorage.js index 5d5d2f88c..f360ce439 100644 --- a/js/plugins/InsightStorage.js +++ b/js/plugins/InsightStorage.js @@ -4,6 +4,8 @@ var buffers = require('buffer'); var querystring = require('querystring'); var Identity = require('../models/Identity'); +var SEPARATOR = '|'; + function InsightStorage(config) { this.type = 'DB'; this.storeUrl = config.url || 'https://test-insight.bitpay.com:443/api/email'; @@ -15,6 +17,7 @@ InsightStorage.prototype.init = function () {}; InsightStorage.prototype.setCredentials = function(email, password, opts) { this.email = email; this.password = password; + this._cachedKey = null; }; InsightStorage.prototype.createItem = function(name, value, callback) { @@ -35,24 +38,23 @@ function mayBeOldPassword(password) { } InsightStorage.prototype.getItem = function(name, callback) { - var key = cryptoUtil.kdf(this.password + this.email); - var secret = this.makeSecret(key); + var passphrase = this.getPassphrase(); var self = this; - this._makeGetRequest(secret, name, function(err, body) { + this._makeGetRequest(passphrase, name, function(err, body) { if (err && err.indexOf('PNOTFOUND') !== -1 && mayBeOldPassword(self.password)) { - return self._brokenGetItem(key, name, callback); + return self._brokenGetItem(name, callback); } return callback(err, body); }); }; -InsightStorage.prototype.makeSecret = function(key) { - return cryptoUtil.kdf(key + this.password); +InsightStorage.prototype.getPassphrase = function() { + return cryptoUtil.hmac(this.getKey(), this.password); }; -InsightStorage.prototype._makeGetRequest = function(secret, key, callback) { - var authHeader = new Buffer(this.email + ':' + secret).toString('base64'); +InsightStorage.prototype._makeGetRequest = function(passphrase, key, callback) { + var authHeader = new buffers.Buffer(this.email + ':' + passphrase).toString('base64'); var retrieveUrl = this.storeUrl + '/retrieve'; this.request.get({ url: retrieveUrl + '?' + querystring.encode({key: key}), @@ -73,12 +75,12 @@ InsightStorage.prototype._makeGetRequest = function(secret, key, callback) { ); }; -InsightStorage.prototype._brokenGetItem = function(key, name, callback) { - var secret = this._makeBrokenSecret(key); +InsightStorage.prototype._brokenGetItem = function(name, callback) { + var passphrase = this._makeBrokenSecret(); var self = this; - this._makeGetRequest(secret, name, function(err, body) { + this._makeGetRequest(passphrase, name, function(err, body) { if (!err) { - return self._changePassword(function(err) { + return self._changePassphrase(function(err) { if (err) { return callback(err); } @@ -89,22 +91,29 @@ InsightStorage.prototype._brokenGetItem = function(key, name, callback) { }); }; -InsightStorage.prototype._makeBrokenSecret = function(key) { +InsightStorage.prototype.getKey = function() { + if (!this._cachedKey) { + this._cachedKey = cryptoUtil.kdf(this.password + SEPARATOR + this.email); + } + return this._cachedKey; +}; + +InsightStorage.prototype._makeBrokenSecret = function() { + var key = cryptoUtil.kdf(this.password + this.email); return cryptoUtil.kdf(key, this.password); }; -InsightStorage.prototype._changePassword = function(callback) { - var key = cryptoUtil.kdf(this.password + this.email); - var secret = this._makeBrokenSecret(key); - var newSecret = this.makeSecret(key); +InsightStorage.prototype._changePassphrase = function(callback) { + var passphrase = this._makeBrokenSecret(); + var newPassphrase = this.getPassphrase(); + var authHeader = new buffers.Buffer(this.email + ':' + passphrase).toString('base64'); var url = this.storeUrl + '/change_passphrase'; this.request.post({ url: url, + headers: {'Authorization': authHeader}, body: querystring.encode({ - email: this.email, - secret: secret, - newSecret: newSecret + newPassphrase: newPassphrase }) }, function(err, response, body) { if (err) { @@ -121,15 +130,14 @@ InsightStorage.prototype._changePassword = function(callback) { }; InsightStorage.prototype.setItem = function(name, value, callback) { - var key = cryptoUtil.kdf(this.password + this.email); - var secret = this.makeSecret(key); + var passphrase = this.getPassphrase(); + var authHeader = new buffers.Buffer(this.email + ':' + passphrase).toString('base64'); var registerUrl = this.storeUrl + '/register'; this.request.post({ url: registerUrl, + headers: {'Authorization': authHeader}, body: querystring.encode({ key: name, - email: this.email, - secret: secret, record: value }) }, function(err, response, body) { diff --git a/js/util/crypto.js b/js/util/crypto.js index 46da407df..b1a521c0c 100644 --- a/js/util/crypto.js +++ b/js/util/crypto.js @@ -23,6 +23,17 @@ module.exports = { ); }, + /** + * @param {string} key + * @param {string} data + * @return {string} base64 encoded hmac + */ + hmac: function(key, data) { + return sjcl.codec.base64.fromBits( + new sjcl.misc.hmac(key, sjcl.hash.sha256).encrypt(data) + ); + }, + /** * @param {string} password * @param {string} salt - base64 encoded, defaults to 'mjuBtGybi/4=' diff --git a/test/plugin.insight.js b/test/plugin.insight.js index 2a202c849..77d323c96 100644 --- a/test/plugin.insight.js +++ b/test/plugin.insight.js @@ -14,8 +14,7 @@ describe('insight storage plugin', function() { var oldSecret = 'rFA+F/N+ZvKXp717zBdfCKYQ5v9Fjry0W6tautj5etIH' + 'KLQliZBEYXA7AXjTJ9K3DglzGWJKost3QJUCMbhM/A==' - var newSecret = '2YTcmtkmC/WSVrfmjAEyOmDnmntVQ8A9wS/q1DbD6rkc' - + 'si8LrIvS2Ru85Feb5Bvk2ziQbEN6PzlL1mIyQ3a+Vw==' + var newSecret = '+72pwnQ/ukrXVXZ/L4vFeiykwn522uVz0J6p81TGXvI='; var setupStorageCredentials = function() { storage.setCredentials(email, password); @@ -122,8 +121,10 @@ describe('insight storage plugin', function() { var url = requestMock.post.firstCall.args[0].url; var args = querystring.decode(receivedArgs); assert(url.indexOf('change_passphrase') !== -1); - assert(args.secret === oldSecret); - assert(args.newSecret === newSecret); + assert(requestMock.post.firstCall.args[0].headers.Authorization + === + new Buffer(email + ':' + oldSecret).toString('base64')); + assert(args.newPassphrase === newSecret); done(); }); } diff --git a/test/util.crypto.js b/test/util.crypto.js index cc991b947..2cb42211c 100644 --- a/test/util.crypto.js +++ b/test/util.crypto.js @@ -50,6 +50,9 @@ describe('crypto utils', function() { }); it('should generate a passphrase from weird chars', function() { var phrase = cryptoUtils.kdf('Pwd123!@#$%^&*(){}[]\|/?.>,<=+-_`~åéþ䲤þçæ¶'); + var expected = 'CZwb5KdikvZHVsEoZUdJckAy+yyzGnd++XhyqxJXbc30' + + 'pEoO+WqHgqBbdf0gn2wiyWZv3zymB+7L75Xnz3uSlg=='; + phrase.should.equal(expected); }); }); From b0afbe9fba0e0180bdb42d05252513b1ef121376 Mon Sep 17 00:00:00 2001 From: Esteban Ordano Date: Mon, 10 Nov 2014 13:09:43 -0300 Subject: [PATCH 3/3] Change endpoint --- js/plugins/InsightStorage.js | 2 +- test/plugin.insight.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/js/plugins/InsightStorage.js b/js/plugins/InsightStorage.js index f360ce439..030775711 100644 --- a/js/plugins/InsightStorage.js +++ b/js/plugins/InsightStorage.js @@ -132,7 +132,7 @@ InsightStorage.prototype._changePassphrase = function(callback) { InsightStorage.prototype.setItem = function(name, value, callback) { var passphrase = this.getPassphrase(); var authHeader = new buffers.Buffer(this.email + ':' + passphrase).toString('base64'); - var registerUrl = this.storeUrl + '/register'; + var registerUrl = this.storeUrl + '/save'; this.request.post({ url: registerUrl, headers: {'Authorization': authHeader}, diff --git a/test/plugin.insight.js b/test/plugin.insight.js index 77d323c96..5f65942ea 100644 --- a/test/plugin.insight.js +++ b/test/plugin.insight.js @@ -66,7 +66,7 @@ describe('insight storage plugin', function() { storage.setItem(namespace, data, function(err) { assert(!err); - assert(requestMock.post.firstCall.args[0].url.indexOf('register') !== -1); + assert(requestMock.post.firstCall.args[0].url.indexOf('save') !== -1); return done(); }); });