From f679055dd0c282bc35ffb43b3cc2f9bf1641ffa6 Mon Sep 17 00:00:00 2001 From: Khafra <42794878+KhafraDev@users.noreply.github.com> Date: Mon, 22 Aug 2022 02:31:05 -0400 Subject: [PATCH] feat(fetch): implement spec compliant integrity checks (#1613) --- lib/fetch/index.js | 4 +- lib/fetch/util.js | 126 ++++++++++++++++++++++++++++++++++++++-- test/fetch/integrity.js | 80 ++++++++++++++++++++++++- 3 files changed, 200 insertions(+), 10 deletions(-) diff --git a/lib/fetch/index.js b/lib/fetch/index.js index a95d11b93fd..c7c88ec40b3 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -13,7 +13,7 @@ const { Headers } = require('./headers') const { Request, makeRequest } = require('./request') const zlib = require('zlib') const { - matchRequestIntegrity, + bytesMatch, makePolicyContainer, clonePolicyContainer, requestBadPort, @@ -725,7 +725,7 @@ async function mainFetch (fetchParams, recursive = false) { const processBody = (bytes) => { // 1. If bytes do not match request’s integrity metadata, // then run processBodyError and abort these steps. [SRI] - if (!matchRequestIntegrity(request, bytes)) { + if (!bytesMatch(bytes, request.integrity)) { processBodyError('integrity mismatch') return } diff --git a/lib/fetch/util.js b/lib/fetch/util.js index ae20a05d815..01bf254d53f 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -5,10 +5,19 @@ const { performance } = require('perf_hooks') const { isBlobLike, toUSVString, ReadableStreamFrom } = require('../core/util') const assert = require('assert') const { isUint8Array } = require('util/types') -const { createHash } = require('crypto') let File +// https://nodejs.org/api/crypto.html#determining-if-crypto-support-is-unavailable +/** @type {import('crypto')|undefined} */ +let crypto + +try { + crypto = require('crypto') +} catch { + +} + // https://fetch.spec.whatwg.org/#block-bad-port const badPorts = [ '1', '7', '9', '11', '13', '15', '17', '19', '20', '21', '22', '23', '25', '37', '42', '43', '53', '69', '77', '79', @@ -341,9 +350,114 @@ function determineRequestsReferrer (request) { return 'no-referrer' } -function matchRequestIntegrity (request, bytes) { - const [algo, expectedHashValue] = request.integrity.split('-', 2) - return createHash(algo).update(bytes).digest('hex') === expectedHashValue +/** + * @see https://w3c.github.io/webappsec-subresource-integrity/#does-response-match-metadatalist + * @param {Uint8Array} bytes + * @param {string} metadataList + */ +function bytesMatch (bytes, metadataList) { + // If node is not built with OpenSSL support, we cannot check + // a request's integrity, so allow it by default (the spec will + // allow requests if an invalid hash is given, as precedence). + /* istanbul ignore if: only if node is built with --without-ssl */ + if (crypto === undefined) { + return true + } + + // 1. Let parsedMetadata be the result of parsing metadataList. + const parsedMetadata = parseMetadata(metadataList) + + // 2. If parsedMetadata is no metadata, return true. + if (parsedMetadata === 'no metadata') { + return true + } + + // 3. If parsedMetadata is the empty set, return true. + if (parsedMetadata.length === 0) { + return true + } + + // 4. Let metadata be the result of getting the strongest + // metadata from parsedMetadata. + // Note: this will only work for SHA- algorithms and it's lazy *at best*. + const metadata = parsedMetadata.sort((c, d) => d.algo.localeCompare(c.algo)) + + // 5. For each item in metadata: + for (const item of metadata) { + // 1. Let algorithm be the alg component of item. + const algorithm = item.algo + + // 2. Let expectedValue be the val component of item. + const expectedValue = item.hash + + // 3. Let actualValue be the result of applying algorithm to bytes. + // Note: "applying algorithm to bytes" converts the result to base64 + const actualValue = crypto.createHash(algorithm).update(bytes).digest('base64') + + // 4. If actualValue is a case-sensitive match for expectedValue, + // return true. + if (actualValue === expectedValue) { + return true + } + } + + // 6. Return false. + return false +} + +// https://w3c.github.io/webappsec-subresource-integrity/#grammardef-hash-with-options +// hash-algo is defined in Content Security Policy 2 Section 4.2 +// base64-value is similary defined there +// VCHAR is defined https://www.rfc-editor.org/rfc/rfc5234#appendix-B.1 +const parseHashWithOptions = /((?sha256|sha384|sha512)-(?[A-z0-9+/]{1}.*={1,2}))( +[\x21-\x7e]?)?/i + +/** + * @see https://w3c.github.io/webappsec-subresource-integrity/#parse-metadata + * @param {string} metadata + */ +function parseMetadata (metadata) { + // 1. Let result be the empty set. + /** @type {{ algo: string, hash: string }[]} */ + const result = [] + + // 2. Let empty be equal to true. + let empty = true + + const supportedHashes = crypto.getHashes() + + // 3. For each token returned by splitting metadata on spaces: + for (const token of metadata.split(' ')) { + // 1. Set empty to false. + empty = false + + // 2. Parse token as a hash-with-options. + const parsedToken = parseHashWithOptions.exec(token) + + // 3. If token does not parse, continue to the next token. + if (parsedToken === null || parsedToken.groups === undefined) { + // Note: Chromium blocks the request at this point, but Firefox + // gives a warning that an invalid integrity was given. The + // correct behavior is to ignore these, and subsequently not + // check the integrity of the resource. + continue + } + + // 4. Let algorithm be the hash-algo component of token. + const algorithm = parsedToken.groups.algo + + // 5. If algorithm is a hash function recognized by the user + // agent, add the parsed token to result. + if (supportedHashes.includes(algorithm.toLowerCase())) { + result.push(parsedToken.groups) + } + } + + // 4. Return no metadata if empty is true, otherwise return result. + if (empty === true) { + return 'no metadata' + } + + return result } // https://w3c.github.io/webappsec-upgrade-insecure-requests/#upgrade-request @@ -501,7 +615,6 @@ module.exports = { toUSVString, tryUpgradeRequestToAPotentiallyTrustworthyURL, coarsenedSharedCurrentTime, - matchRequestIntegrity, determineRequestsReferrer, makePolicyContainer, clonePolicyContainer, @@ -528,5 +641,6 @@ module.exports = { isValidHeaderValue, hasOwn, isErrorLike, - fullyReadBody + fullyReadBody, + bytesMatch } diff --git a/test/fetch/integrity.js b/test/fetch/integrity.js index bd464218d55..70802a6aecf 100644 --- a/test/fetch/integrity.js +++ b/test/fetch/integrity.js @@ -2,9 +2,12 @@ const { test } = require('tap') const { createServer } = require('http') -const { createHash } = require('crypto') +const { createHash, getHashes } = require('crypto') const { gzipSync } = require('zlib') const { fetch, setGlobalDispatcher, Agent } = require('../..') +const { once } = require('events') + +const supportedHashes = getHashes() setGlobalDispatcher(new Agent({ keepAliveTimeout: 1, @@ -44,7 +47,7 @@ test('request with wrong integrity checksum', (t) => { fetch(`http://localhost:${server.address().port}`, { integrity: `sha256-${hash}` }).then(response => { - t.fail('fetch did not fail') + t.pass('request did not fail') }).catch((err) => { t.equal(err.cause.message, 'integrity mismatch') }).finally(() => { @@ -72,3 +75,76 @@ test('request with integrity checksum on encoded body', (t) => { t.end() }) }) + +test('request with a totally incorrect integrity', async (t) => { + const server = createServer((req, res) => { + res.end() + }).listen(0) + + t.teardown(server.close.bind(server)) + await once(server, 'listening') + + await t.resolves(fetch(`http://localhost:${server.address().port}`, { + integrity: 'what-integrityisthis' + })) +}) + +test('request with mixed in/valid integrities', async (t) => { + const body = 'Hello world!' + const hash = createHash('sha256').update(body).digest('base64') + + const server = createServer((req, res) => { + res.end(body) + }).listen(0) + + t.teardown(server.close.bind(server)) + await once(server, 'listening') + + await t.resolves(fetch(`http://localhost:${server.address().port}`, { + integrity: `invalid-integrity sha256-${hash}` + })) +}) + +test('request with sha384 hash', { skip: !supportedHashes.includes('sha384') }, async (t) => { + const body = 'Hello world!' + const hash = createHash('sha384').update(body).digest('base64') + + const server = createServer((req, res) => { + res.end(body) + }).listen(0) + + t.teardown(server.close.bind(server)) + await once(server, 'listening') + + // request should succeed + await t.resolves(fetch(`http://localhost:${server.address().port}`, { + integrity: `sha384-${hash}` + })) + + // request should fail + await t.rejects(fetch(`http://localhost:${server.address().port}`, { + integrity: 'sha384-ypeBEsobvcr6wjGzmiPcTaeG7/gUfE5yuYB3ha/uSLs=' + })) +}) + +test('request with sha512 hash', { skip: !supportedHashes.includes('sha512') }, async (t) => { + const body = 'Hello world!' + const hash = createHash('sha512').update(body).digest('base64') + + const server = createServer((req, res) => { + res.end(body) + }).listen(0) + + t.teardown(server.close.bind(server)) + await once(server, 'listening') + + // request should succeed + await t.resolves(fetch(`http://localhost:${server.address().port}`, { + integrity: `sha512-${hash}` + })) + + // request should fail + await t.rejects(fetch(`http://localhost:${server.address().port}`, { + integrity: 'sha512-ypeBEsobvcr6wjGzmiPcTaeG7/gUfE5yuYB3ha/uSLs=' + })) +})