From d5820667433a3af3212a0ca6570af3f925ce2cc6 Mon Sep 17 00:00:00 2001 From: Matt Hamann Date: Wed, 15 May 2019 23:27:38 -0400 Subject: [PATCH] Upgraded encryption using CipherIV (#322) * Remove package-lock.json from gitignore * Update dependencies and fix repo url * Fix test * Update to cipheriv * Bump version * Sync package-lock * Revert extraneous package changes * Revert minor doc change --- .gitignore | 4 +-- README.md | 10 +++++--- lib/nconf/stores/file.js | 45 +++++++++++++++++++++++++--------- package-lock.json | 11 +++------ package.json | 1 - test/fixtures/secure-iv.json | 22 +++++++++++++++++ test/hierarchy.test.js | 4 +-- test/stores/file-store.test.js | 30 +++++++++++++++++++---- 8 files changed, 94 insertions(+), 33 deletions(-) create mode 100644 test/fixtures/secure-iv.json diff --git a/.gitignore b/.gitignore index 4c0e4c7..e25ffa6 100644 --- a/.gitignore +++ b/.gitignore @@ -6,8 +6,8 @@ test/fixtures/*.json !test/fixtures/bom.json !test/fixtures/no-bom.json !test/fixtures/secure.json +!test/fixtures/secure-iv.json node_modules/ node_modules/* npm-debug.log -coverage -package-lock.json +coverage \ No newline at end of file diff --git a/README.md b/README.md index eabc7f3..e5c5c7c 100644 --- a/README.md +++ b/README.md @@ -230,7 +230,7 @@ The input `obj` contains two properties passed in the following format: The transformation function may alter both the key and the value. -The function may return either an object in the asme format as the input or a value that evaluates to false. +The function may return either an object in the same format as the input or a value that evaluates to false. If the return value is falsey, the entry will be dropped from the store, otherwise it will replace the original key/value. *Note: If the return value doesn't adhere to the above rules, an exception will be thrown.* @@ -397,17 +397,19 @@ nconf.file('secure-file', { }) ``` -This will encrypt each key using [`crypto.createCipher`](https://nodejs.org/api/crypto.html#crypto_crypto_createcipher_algorithm_password), defaulting to `aes-256-ctr`. The encrypted file contents will look like this: +This will encrypt each key using [`crypto.createCipheriv`](https://nodejs.org/api/crypto.html#crypto_crypto_createcipheriv_algorithm_key_iv_options), defaulting to `aes-256-ctr`. The encrypted file contents will look like this: ``` { "config-key-name": { "alg": "aes-256-ctr", // cipher used - "value": "af07fbcf" // encrypted contents + "value": "af07fbcf", // encrypted contents + "iv": "49e7803a2a5ef98c7a51a8902b76dd10" // initialization vector }, "another-config-key": { "alg": "aes-256-ctr", // cipher used - "value": "e310f6d94f13" // encrypted contents + "value": "e310f6d94f13", // encrypted contents + "iv": "b654e01aed262f37d0acf200be193985" // initialization vector }, } ``` diff --git a/lib/nconf/stores/file.js b/lib/nconf/stores/file.js index 8d1e1e1..0f0d5e3 100644 --- a/lib/nconf/stores/file.js +++ b/lib/nconf/stores/file.js @@ -8,7 +8,7 @@ var fs = require('fs'), path = require('path'), util = require('util'), - Secure = require('secure-keys'), + crypto = require('crypto'), formats = require('../formats'), Memory = require('./memory').Memory; @@ -50,12 +50,6 @@ var File = exports.File = function (options) { if (!this.secure.secret) { throw new Error('secure.secret option is required'); } - - this.keys = new Secure({ - secret: this.secure.secret, - alg: this.secure.alg, - format: this.format - }); } if (options.search) { @@ -185,7 +179,16 @@ File.prototype.stringify = function (format) { } if (this.secure) { - data = this.keys.encrypt(data); + var self = this; + data = Object.keys(data).reduce(function (acc, key) { + var value = format.stringify(data[key]); + var iv = crypto.randomBytes(16); + var cipher = crypto.createCipheriv(self.secure.alg, self.secure.secret, iv); + var ciphertext = cipher.update(value, 'utf8', 'hex'); + ciphertext += cipher.final('hex'); + acc[key] = { alg: self.secure.alg, value: ciphertext, iv: iv.toString('hex') }; + return acc; + }, {}); } return format.stringify(data, null, this.spacing); @@ -199,11 +202,31 @@ File.prototype.stringify = function (format) { File.prototype.parse = function (contents) { var parsed = this.format.parse(contents); - if (!this.secure) { - return parsed; + if (this.secure) { + var self = this; + var outdated = false; + parsed = Object.keys(parsed).reduce(function (acc, key) { + var value = parsed[key]; + var decipher = crypto.createDecipher(value.alg, self.secure.secret); + if (value.iv) { + // For backward compatibility, use createDecipheriv only if there is iv stored in file + decipher = crypto.createDecipheriv(value.alg, self.secure.secret, Buffer.from(value.iv, 'hex')); + } else { + outdated = true; + } + var plaintext = decipher.update(value.value, 'hex', 'utf8'); + plaintext += decipher.final('utf8'); + acc[key] = self.format.parse(plaintext); + return acc; + }, {}); + + if (outdated) { + // warn user if the file is encrypted without iv + console.warn('Your encrypted file is outdated (encrypted without iv). Please re-encrypt your file.'); + } } - return this.keys.decrypt(parsed); + return parsed; }; diff --git a/package-lock.json b/package-lock.json index f53197b..7666fec 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3606,9 +3606,9 @@ }, "dependencies": { "resolve": { - "version": "1.10.1", - "resolved": "https://registry.npmjs.org/resolve/-/resolve-1.10.1.tgz", - "integrity": "sha512-KuIe4mf++td/eFb6wkaPbMDnP6kObCaEtIDuHOUED6MNUo4K670KZUHuuvYPZDxNF0WVLw49n06M2m2dXphEzA==", + "version": "1.11.0", + "resolved": "https://registry.npmjs.org/resolve/-/resolve-1.11.0.tgz", + "integrity": "sha512-WL2pBDjqT6pGUNSUzMw00o4T7If+z4H2x3Gz893WoUQ5KW8Vr9txp00ykiP16VBaZF5+j/OcXJHZ9+PCvdiDKw==", "requires": { "path-parse": "^1.0.6" } @@ -4506,11 +4506,6 @@ "resolved": "https://registry.npmjs.org/sax/-/sax-1.2.4.tgz", "integrity": "sha512-NqVDv9TpANUjFm0N8uM5GxL36UgKi9/atZw+x7YFnQ8ckwFGKrl4xX4yWtrey3UJm5nP1kUbnYgLopqWNSRhWw==" }, - "secure-keys": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/secure-keys/-/secure-keys-1.0.0.tgz", - "integrity": "sha1-8MgtmKOxOah3aogIBQuCRDEIf8o=" - }, "semver": { "version": "5.7.0", "resolved": "https://registry.npmjs.org/semver/-/semver-5.7.0.tgz", diff --git a/package.json b/package.json index 069646d..437c1c5 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,6 @@ "dependencies": { "ini": "^1.3.0", "jest": "^21.2.1", - "secure-keys": "^1.0.0", "yargs": "^10.0.3" }, "devDependencies": { diff --git a/test/fixtures/secure-iv.json b/test/fixtures/secure-iv.json new file mode 100644 index 0000000..995b02c --- /dev/null +++ b/test/fixtures/secure-iv.json @@ -0,0 +1,22 @@ +{ + "isNull": { + "alg": "aes-256-ctr", + "value": "16f39325", + "iv": "49e7803a2a5ef98c7a51a8902b76dd10" + }, + "literal": { + "alg": "aes-256-ctr", + "value": "647965c22605", + "iv": "b654e01aed262f37d0acf200be193985" + }, + "arr": { + "alg": "aes-256-ctr", + "value": "70c6d3b2ec3820e4d4fa3d7834fcf7fd51190a9e580944583ca7b30f42d230b7f271e72287f6c2ab4624685c779936aa2ed19b90", + "iv": "b5263665331158c2165fe623a895870f" + }, + "obj": { + "alg": "aes-256-ctr", + "value": "521f1995698896d2fe7d76d16bec9e86b9b5b25bcfae3a4f3c52ad6f0425c3fd059793b9dec7927c616f4c4c57f2b9ee833f8c865d9aa42bbb37203763703755a0c79afd13f8c0e96e9ac47f490a12105583e31ff628104e6b216265b849cdf6642eb8a4b5bd6f532339a5f5737bd6a7ae6e7bb22148cbcfa549802336cb1322fb701151b25b5863c5c6d5047875aed9806350ae0e292c259e82d5a561eb672402d20230a0c3107ff2b1e6259ccb1bbbe7a25ce965ce56cb32f679", + "iv": "355f307363d69ed58345ae395744f5f0" + } +} \ No newline at end of file diff --git a/test/hierarchy.test.js b/test/hierarchy.test.js index 12010ba..9e6fc94 100644 --- a/test/hierarchy.test.js +++ b/test/hierarchy.test.js @@ -106,14 +106,14 @@ describe('nconf/hierarchy, When using nconf', () => { var argv = ['--candy--something', 'foo', '--candy--something5--second', 'bar']; var data = ''; process.env.candy__bonbon = 'sweet'; - var child = spawn('node', [script].concat(argv)); + var child = spawn('node', [script].concat(argv), {env: process.env}); delete process.env.candy__bonbon; child.stdout.on('data', function (d) { data += d; }); child.on('close', function () { - console.log(data) + console.log(data); expect(JSON.parse(data)).toEqual({ apples: true, candy: { diff --git a/test/stores/file-store.test.js b/test/stores/file-store.test.js index 88329b8..9f66654 100644 --- a/test/stores/file-store.test.js +++ b/test/stores/file-store.test.js @@ -183,7 +183,7 @@ describe('nconf/stores/file', () => { describe("When using the nconf file store", () => { it("the search() method when the target file exists higher in the directory tree should update the file appropriately", () => { - var searchBase = process.env.HOME; + var searchBase = require('os').homedir(); var filePath = path.join(searchBase, '.nconf'); fs.writeFileSync(filePath, JSON.stringify(data, null, 2)); var store = new nconf.File({ @@ -206,8 +206,8 @@ describe('nconf/stores/file', () => { }) describe("When using the nconf file store", () => { var secureStore = new nconf.File({ - file: path.join(__dirname, '..', 'fixtures', 'secure.json'), - secure: 'super-secretzzz' + file: path.join(__dirname, '..', 'fixtures', 'secure-iv.json'), + secure: 'super-secret-key-32-characterszz' }); secureStore.store = data; @@ -218,6 +218,7 @@ describe('nconf/stores/file', () => { expect(typeof contents[key]).toBe('object'); expect(typeof contents[key].value).toBe('string'); expect(contents[key].alg).toEqual('aes-256-ctr'); + expect(typeof contents[key].iv).toBe('string'); }); }); it("the parse() method should decrypt properly", () => { @@ -232,8 +233,27 @@ describe('nconf/stores/file', () => { }); }); it("the loadSync() method should decrypt properly", () => { - var loaded = secureStore.loadSync() + var loaded = secureStore.loadSync(); expect(loaded).toEqual(data); }); }) -}); + + describe("When using the nconf file store", () => { + var secureStore = new nconf.File({ + file: path.join(__dirname, '..', 'fixtures', 'secure.json'), + secure: 'super-secretzzz' + }); + + it("the load() method should decrypt legacy file properly", () => { + secureStore.load(function (err, loaded) { + expect(err).toBe(null); + expect(loaded).toEqual(data); + }); + }); + it("the loadSync() method should decrypt legacy file properly", () => { + var loaded = secureStore.loadSync(); + expect(loaded).toEqual(data); + }); + }) + +}); \ No newline at end of file