Skip to content

Commit f679055

Browse files
KhafraDevmetcoder95
authored andcommitted
feat(fetch): implement spec compliant integrity checks (nodejs#1613)
1 parent 13c142f commit f679055

File tree

3 files changed

+200
-10
lines changed

3 files changed

+200
-10
lines changed

lib/fetch/index.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ const { Headers } = require('./headers')
1313
const { Request, makeRequest } = require('./request')
1414
const zlib = require('zlib')
1515
const {
16-
matchRequestIntegrity,
16+
bytesMatch,
1717
makePolicyContainer,
1818
clonePolicyContainer,
1919
requestBadPort,
@@ -725,7 +725,7 @@ async function mainFetch (fetchParams, recursive = false) {
725725
const processBody = (bytes) => {
726726
// 1. If bytes do not match request’s integrity metadata,
727727
// then run processBodyError and abort these steps. [SRI]
728-
if (!matchRequestIntegrity(request, bytes)) {
728+
if (!bytesMatch(bytes, request.integrity)) {
729729
processBodyError('integrity mismatch')
730730
return
731731
}

lib/fetch/util.js

+120-6
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,19 @@ const { performance } = require('perf_hooks')
55
const { isBlobLike, toUSVString, ReadableStreamFrom } = require('../core/util')
66
const assert = require('assert')
77
const { isUint8Array } = require('util/types')
8-
const { createHash } = require('crypto')
98

109
let File
1110

11+
// https://nodejs.org/api/crypto.html#determining-if-crypto-support-is-unavailable
12+
/** @type {import('crypto')|undefined} */
13+
let crypto
14+
15+
try {
16+
crypto = require('crypto')
17+
} catch {
18+
19+
}
20+
1221
// https://fetch.spec.whatwg.org/#block-bad-port
1322
const badPorts = [
1423
'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) {
341350
return 'no-referrer'
342351
}
343352

344-
function matchRequestIntegrity (request, bytes) {
345-
const [algo, expectedHashValue] = request.integrity.split('-', 2)
346-
return createHash(algo).update(bytes).digest('hex') === expectedHashValue
353+
/**
354+
* @see https://w3c.github.io/webappsec-subresource-integrity/#does-response-match-metadatalist
355+
* @param {Uint8Array} bytes
356+
* @param {string} metadataList
357+
*/
358+
function bytesMatch (bytes, metadataList) {
359+
// If node is not built with OpenSSL support, we cannot check
360+
// a request's integrity, so allow it by default (the spec will
361+
// allow requests if an invalid hash is given, as precedence).
362+
/* istanbul ignore if: only if node is built with --without-ssl */
363+
if (crypto === undefined) {
364+
return true
365+
}
366+
367+
// 1. Let parsedMetadata be the result of parsing metadataList.
368+
const parsedMetadata = parseMetadata(metadataList)
369+
370+
// 2. If parsedMetadata is no metadata, return true.
371+
if (parsedMetadata === 'no metadata') {
372+
return true
373+
}
374+
375+
// 3. If parsedMetadata is the empty set, return true.
376+
if (parsedMetadata.length === 0) {
377+
return true
378+
}
379+
380+
// 4. Let metadata be the result of getting the strongest
381+
// metadata from parsedMetadata.
382+
// Note: this will only work for SHA- algorithms and it's lazy *at best*.
383+
const metadata = parsedMetadata.sort((c, d) => d.algo.localeCompare(c.algo))
384+
385+
// 5. For each item in metadata:
386+
for (const item of metadata) {
387+
// 1. Let algorithm be the alg component of item.
388+
const algorithm = item.algo
389+
390+
// 2. Let expectedValue be the val component of item.
391+
const expectedValue = item.hash
392+
393+
// 3. Let actualValue be the result of applying algorithm to bytes.
394+
// Note: "applying algorithm to bytes" converts the result to base64
395+
const actualValue = crypto.createHash(algorithm).update(bytes).digest('base64')
396+
397+
// 4. If actualValue is a case-sensitive match for expectedValue,
398+
// return true.
399+
if (actualValue === expectedValue) {
400+
return true
401+
}
402+
}
403+
404+
// 6. Return false.
405+
return false
406+
}
407+
408+
// https://w3c.github.io/webappsec-subresource-integrity/#grammardef-hash-with-options
409+
// hash-algo is defined in Content Security Policy 2 Section 4.2
410+
// base64-value is similary defined there
411+
// VCHAR is defined https://www.rfc-editor.org/rfc/rfc5234#appendix-B.1
412+
const parseHashWithOptions = /((?<algo>sha256|sha384|sha512)-(?<hash>[A-z0-9+/]{1}.*={1,2}))( +[\x21-\x7e]?)?/i
413+
414+
/**
415+
* @see https://w3c.github.io/webappsec-subresource-integrity/#parse-metadata
416+
* @param {string} metadata
417+
*/
418+
function parseMetadata (metadata) {
419+
// 1. Let result be the empty set.
420+
/** @type {{ algo: string, hash: string }[]} */
421+
const result = []
422+
423+
// 2. Let empty be equal to true.
424+
let empty = true
425+
426+
const supportedHashes = crypto.getHashes()
427+
428+
// 3. For each token returned by splitting metadata on spaces:
429+
for (const token of metadata.split(' ')) {
430+
// 1. Set empty to false.
431+
empty = false
432+
433+
// 2. Parse token as a hash-with-options.
434+
const parsedToken = parseHashWithOptions.exec(token)
435+
436+
// 3. If token does not parse, continue to the next token.
437+
if (parsedToken === null || parsedToken.groups === undefined) {
438+
// Note: Chromium blocks the request at this point, but Firefox
439+
// gives a warning that an invalid integrity was given. The
440+
// correct behavior is to ignore these, and subsequently not
441+
// check the integrity of the resource.
442+
continue
443+
}
444+
445+
// 4. Let algorithm be the hash-algo component of token.
446+
const algorithm = parsedToken.groups.algo
447+
448+
// 5. If algorithm is a hash function recognized by the user
449+
// agent, add the parsed token to result.
450+
if (supportedHashes.includes(algorithm.toLowerCase())) {
451+
result.push(parsedToken.groups)
452+
}
453+
}
454+
455+
// 4. Return no metadata if empty is true, otherwise return result.
456+
if (empty === true) {
457+
return 'no metadata'
458+
}
459+
460+
return result
347461
}
348462

349463
// https://w3c.github.io/webappsec-upgrade-insecure-requests/#upgrade-request
@@ -501,7 +615,6 @@ module.exports = {
501615
toUSVString,
502616
tryUpgradeRequestToAPotentiallyTrustworthyURL,
503617
coarsenedSharedCurrentTime,
504-
matchRequestIntegrity,
505618
determineRequestsReferrer,
506619
makePolicyContainer,
507620
clonePolicyContainer,
@@ -528,5 +641,6 @@ module.exports = {
528641
isValidHeaderValue,
529642
hasOwn,
530643
isErrorLike,
531-
fullyReadBody
644+
fullyReadBody,
645+
bytesMatch
532646
}

test/fetch/integrity.js

+78-2
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@
22

33
const { test } = require('tap')
44
const { createServer } = require('http')
5-
const { createHash } = require('crypto')
5+
const { createHash, getHashes } = require('crypto')
66
const { gzipSync } = require('zlib')
77
const { fetch, setGlobalDispatcher, Agent } = require('../..')
8+
const { once } = require('events')
9+
10+
const supportedHashes = getHashes()
811

912
setGlobalDispatcher(new Agent({
1013
keepAliveTimeout: 1,
@@ -44,7 +47,7 @@ test('request with wrong integrity checksum', (t) => {
4447
fetch(`http://localhost:${server.address().port}`, {
4548
integrity: `sha256-${hash}`
4649
}).then(response => {
47-
t.fail('fetch did not fail')
50+
t.pass('request did not fail')
4851
}).catch((err) => {
4952
t.equal(err.cause.message, 'integrity mismatch')
5053
}).finally(() => {
@@ -72,3 +75,76 @@ test('request with integrity checksum on encoded body', (t) => {
7275
t.end()
7376
})
7477
})
78+
79+
test('request with a totally incorrect integrity', async (t) => {
80+
const server = createServer((req, res) => {
81+
res.end()
82+
}).listen(0)
83+
84+
t.teardown(server.close.bind(server))
85+
await once(server, 'listening')
86+
87+
await t.resolves(fetch(`http://localhost:${server.address().port}`, {
88+
integrity: 'what-integrityisthis'
89+
}))
90+
})
91+
92+
test('request with mixed in/valid integrities', async (t) => {
93+
const body = 'Hello world!'
94+
const hash = createHash('sha256').update(body).digest('base64')
95+
96+
const server = createServer((req, res) => {
97+
res.end(body)
98+
}).listen(0)
99+
100+
t.teardown(server.close.bind(server))
101+
await once(server, 'listening')
102+
103+
await t.resolves(fetch(`http://localhost:${server.address().port}`, {
104+
integrity: `invalid-integrity sha256-${hash}`
105+
}))
106+
})
107+
108+
test('request with sha384 hash', { skip: !supportedHashes.includes('sha384') }, async (t) => {
109+
const body = 'Hello world!'
110+
const hash = createHash('sha384').update(body).digest('base64')
111+
112+
const server = createServer((req, res) => {
113+
res.end(body)
114+
}).listen(0)
115+
116+
t.teardown(server.close.bind(server))
117+
await once(server, 'listening')
118+
119+
// request should succeed
120+
await t.resolves(fetch(`http://localhost:${server.address().port}`, {
121+
integrity: `sha384-${hash}`
122+
}))
123+
124+
// request should fail
125+
await t.rejects(fetch(`http://localhost:${server.address().port}`, {
126+
integrity: 'sha384-ypeBEsobvcr6wjGzmiPcTaeG7/gUfE5yuYB3ha/uSLs='
127+
}))
128+
})
129+
130+
test('request with sha512 hash', { skip: !supportedHashes.includes('sha512') }, async (t) => {
131+
const body = 'Hello world!'
132+
const hash = createHash('sha512').update(body).digest('base64')
133+
134+
const server = createServer((req, res) => {
135+
res.end(body)
136+
}).listen(0)
137+
138+
t.teardown(server.close.bind(server))
139+
await once(server, 'listening')
140+
141+
// request should succeed
142+
await t.resolves(fetch(`http://localhost:${server.address().port}`, {
143+
integrity: `sha512-${hash}`
144+
}))
145+
146+
// request should fail
147+
await t.rejects(fetch(`http://localhost:${server.address().port}`, {
148+
integrity: 'sha512-ypeBEsobvcr6wjGzmiPcTaeG7/gUfE5yuYB3ha/uSLs='
149+
}))
150+
})

0 commit comments

Comments
 (0)