From 29727889dde1e1e42242537ef5ff116021bacf96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Louren=C3=A7o?= Date: Sat, 1 Apr 2023 15:58:22 -0300 Subject: [PATCH 1/4] fix: faster stream verification --- lib/index.js | 54 +++++++++++++++++++++++++++++++++++++++++++-------- test/check.js | 3 +++ test/match.js | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 test/match.js diff --git a/lib/index.js b/lib/index.js index e142431..1742f04 100644 --- a/lib/index.js +++ b/lib/index.js @@ -29,10 +29,18 @@ class IntegrityStream extends MiniPass { this.#getOptions() // options used for calculating stream. can't be changed. - const algorithms = opts?.algorithms || DEFAULT_ALGORITHMS - this.algorithms = Array.from( - new Set(algorithms.concat(this.algorithm ? [this.algorithm] : [])) - ) + if (opts?.algorithms) { + this.algorithms = Array.from( + new Set(opts.algorithms.concat(this.algorithm ? [this.algorithm] : [])) + ) + } else { + this.algorithms = DEFAULT_ALGORITHMS + + if (this.algorithm !== null && this.algorithm !== DEFAULT_ALGORITHMS[0]) { + this.algorithms.push(this.algorithm) + } + } + this.hashes = this.algorithms.map(crypto.createHash) } @@ -40,8 +48,20 @@ class IntegrityStream extends MiniPass { // For verification this.sri = this.opts?.integrity ? parse(this.opts?.integrity, this.opts) : null this.expectedSize = this.opts?.size - this.goodSri = this.sri ? !!Object.keys(this.sri).length : false - this.algorithm = this.goodSri ? this.sri.pickAlgorithm(this.opts) : null + + if (!this.sri) { + this.algorithm = null + } else if (this.sri.isIntegrity) { + this.goodSri = !this.sri.isEmpty() + + if (this.goodSri) { + this.algorithm = this.sri.pickAlgorithm(this.opts) + } + } else if (this.sri.isHash) { + this.goodSri = true + this.algorithm = this.sri.algorithm + } + this.digests = this.goodSri ? this.sri[this.algorithm] : null this.optString = getOptString(this.opts?.options) } @@ -159,6 +179,24 @@ class Hash { return this.toString() } + match (integrity, opts) { + const other = parse(integrity, opts) + if (!other) { + return false + } + if (other instanceof Integrity) { + const algo = other.pickAlgorithm(opts) + const foundHash = other[algo].find(hash => hash.digest === this.digest) + + if (foundHash) { + return foundHash + } + + return false + } + return other.digest === this.digest ? other : false + } + toString (opts) { if (opts?.strict) { // Strict mode enforces the standard as close to the foot of the @@ -399,7 +437,7 @@ function fromStream (stream, opts) { sri = s }) istream.on('end', () => resolve(sri)) - istream.on('data', () => {}) + istream.resume() }) } @@ -466,7 +504,7 @@ function checkStream (stream, sri, opts) { verified = s }) checker.on('end', () => resolve(verified)) - checker.on('data', () => {}) + checker.resume() }) } diff --git a/test/check.js b/test/check.js index f8cdc6e..6ca6cca 100644 --- a/test/check.js +++ b/test/check.js @@ -160,6 +160,9 @@ test('checkStream', t => { }) }).then(res => { t.same(res, meta, 'Accepts Hash-like SRI') + return ssri.checkStream(fileStream(), `sha512-${hash(TEST_DATA, 'sha512')}`, { single: true }) + }).then(res => { + t.same(res, meta, 'Process successfully with single option') return ssri.checkStream( fileStream(), `sha512-nope sha512-${hash(TEST_DATA, 'sha512')}` diff --git a/test/match.js b/test/match.js new file mode 100644 index 0000000..79d990d --- /dev/null +++ b/test/match.js @@ -0,0 +1,51 @@ +'use strict' + +const crypto = require('crypto') +const fs = require('fs') +const test = require('tap').test + +const ssri = require('..') + +const TEST_DATA = fs.readFileSync(__filename) + +function hash (data, algorithm) { + return crypto.createHash(algorithm).update(data).digest('base64') +} + +test('hashes should match when valid', t => { + const sha = hash(TEST_DATA, 'sha512') + const integrity = `sha512-${sha}` + const parsed = ssri.parse(integrity, { single: true }) + t.same( + parsed.match(integrity, { single: true }), + parsed, + 'should return the same algo when digest is equal (single option)' + ) + t.same( + parsed.match('sha-233', { single: true }), + false, + 'invalid integrity should not match (single option)' + ) + t.same( + parsed.match(null, { single: true }), + false, + 'null integrity just returns false (single option)' + ) + + t.same( + parsed.match(integrity), + parsed, + 'should return the same algo when digest is equal' + ) + t.same( + parsed.match('sha-233'), + false, + 'invalid integrity should not match' + ) + t.same( + parsed.match(null), + false, + 'null integrity just returns false' + ) + t.end() +}) From 8adaf9bbcedc7e314dbd0f56f45dbec870951d65 Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 11 Apr 2023 10:05:29 -0700 Subject: [PATCH 2/4] fix: Integrity#match prioritizes overlapping hashes --- lib/index.js | 43 ++++++++++++++++++++++++++++--------------- test/match.js | 9 +++++++-- 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/lib/index.js b/lib/index.js index 1742f04..412376c 100644 --- a/lib/index.js +++ b/lib/index.js @@ -51,15 +51,12 @@ class IntegrityStream extends MiniPass { if (!this.sri) { this.algorithm = null - } else if (this.sri.isIntegrity) { - this.goodSri = !this.sri.isEmpty() - - if (this.goodSri) { - this.algorithm = this.sri.pickAlgorithm(this.opts) - } } else if (this.sri.isHash) { this.goodSri = true this.algorithm = this.sri.algorithm + } else { + this.goodSri = !this.sri.isEmpty() + this.algorithm = this.sri.pickAlgorithm(this.opts) } this.digests = this.goodSri ? this.sri[this.algorithm] : null @@ -184,8 +181,13 @@ class Hash { if (!other) { return false } - if (other instanceof Integrity) { - const algo = other.pickAlgorithm(opts) + if (other.isIntegrity) { + const algo = other.pickAlgorithm(opts, [this.algorithm]) + + if (!algo) { + return false + } + const foundHash = other[algo].find(hash => hash.digest === this.digest) if (foundHash) { @@ -323,8 +325,9 @@ class Integrity { if (!other) { return false } - const algo = other.pickAlgorithm(opts) + const algo = other.pickAlgorithm(opts, Object.keys(this)) return ( + !!algo && this[algo] && other[algo] && this[algo].find(hash => @@ -335,12 +338,22 @@ class Integrity { ) || false } - pickAlgorithm (opts) { + // Pick the highest priority algorithm present, optionally also limited to a + // set of hashes found in another integrity. When limiting it may return + // nothing. + pickAlgorithm (opts, hashes) { const pickAlgorithm = opts?.pickAlgorithm || getPrioritizedHash - const keys = Object.keys(this) - return keys.reduce((acc, algo) => { - return pickAlgorithm(acc, algo) || acc + const keys = Object.keys(this).filter(k => { + if (hashes?.length) { + return hashes.includes(k) + } + return true }) + if (keys.length) { + return keys.reduce((acc, algo) => pickAlgorithm(acc, algo) || acc) + } + // no intersection between this and hashes, + return null } } @@ -550,7 +563,7 @@ function createIntegrity (opts) { } } -const NODE_HASHES = new Set(crypto.getHashes()) +const NODE_HASHES = crypto.getHashes() // This is a Best Effort™ at a reasonable priority for hash algos const DEFAULT_PRIORITY = [ @@ -560,7 +573,7 @@ const DEFAULT_PRIORITY = [ 'sha3', 'sha3-256', 'sha3-384', 'sha3-512', 'sha3_256', 'sha3_384', 'sha3_512', -].filter(algo => NODE_HASHES.has(algo)) +].filter(algo => NODE_HASHES.includes(algo)) function getPrioritizedHash (algo1, algo2) { /* eslint-disable-next-line max-len */ diff --git a/test/match.js b/test/match.js index 79d990d..5e313a4 100644 --- a/test/match.js +++ b/test/match.js @@ -13,8 +13,8 @@ function hash (data, algorithm) { } test('hashes should match when valid', t => { - const sha = hash(TEST_DATA, 'sha512') - const integrity = `sha512-${sha}` + const integrity = `sha512-${hash(TEST_DATA, 'sha512')}` + const otherIntegrity = `sha512-${hash('mismatch', 'sha512')}` const parsed = ssri.parse(integrity, { single: true }) t.same( parsed.match(integrity, { single: true }), @@ -47,5 +47,10 @@ test('hashes should match when valid', t => { false, 'null integrity just returns false' ) + t.same( + parsed.match(otherIntegrity), + false, + 'should not match with a totally different integrity' + ) t.end() }) From 5951e1eb0545a2b9bd8f291ee991e8c18fb9c9f6 Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 11 Apr 2023 11:22:37 -0700 Subject: [PATCH 3/4] fix: prevent DEFAULT_ALGORITHM mutation --- lib/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/index.js b/lib/index.js index 412376c..f14cd6e 100644 --- a/lib/index.js +++ b/lib/index.js @@ -34,7 +34,7 @@ class IntegrityStream extends MiniPass { new Set(opts.algorithms.concat(this.algorithm ? [this.algorithm] : [])) ) } else { - this.algorithms = DEFAULT_ALGORITHMS + this.algorithms = [...DEFAULT_ALGORITHMS] if (this.algorithm !== null && this.algorithm !== DEFAULT_ALGORITHMS[0]) { this.algorithms.push(this.algorithm) @@ -416,7 +416,7 @@ function fromHex (hexDigest, algorithm, opts) { module.exports.fromData = fromData function fromData (data, opts) { - const algorithms = opts?.algorithms || DEFAULT_ALGORITHMS + const algorithms = opts?.algorithms || [...DEFAULT_ALGORITHMS] const optString = getOptString(opts?.options) return algorithms.reduce((acc, algo) => { const digest = crypto.createHash(algo).update(data).digest('base64') @@ -528,7 +528,7 @@ function integrityStream (opts = Object.create(null)) { module.exports.create = createIntegrity function createIntegrity (opts) { - const algorithms = opts?.algorithms || DEFAULT_ALGORITHMS + const algorithms = opts?.algorithms || [...DEFAULT_ALGORITHMS] const optString = getOptString(opts?.options) const hashes = algorithms.map(crypto.createHash) From 69ebd722906cc42781e86b395bd3ebb7684738ec Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 11 Apr 2023 11:28:58 -0700 Subject: [PATCH 4/4] fix: optimize adding this.algorithm to algorithms list --- lib/index.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/index.js b/lib/index.js index f14cd6e..7db8dcf 100644 --- a/lib/index.js +++ b/lib/index.js @@ -30,15 +30,12 @@ class IntegrityStream extends MiniPass { // options used for calculating stream. can't be changed. if (opts?.algorithms) { - this.algorithms = Array.from( - new Set(opts.algorithms.concat(this.algorithm ? [this.algorithm] : [])) - ) + this.algorithms = [...opts.algorithms] } else { this.algorithms = [...DEFAULT_ALGORITHMS] - - if (this.algorithm !== null && this.algorithm !== DEFAULT_ALGORITHMS[0]) { - this.algorithms.push(this.algorithm) - } + } + if (this.algorithm !== null && !this.algorithms.includes(this.algorithm)) { + this.algorithms.push(this.algorithm) } this.hashes = this.algorithms.map(crypto.createHash)