From 5fa55c11023db53abe2fe830cb25ce9cab441e56 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Wed, 19 Oct 2022 10:47:52 +0200 Subject: [PATCH 1/4] =?UTF-8?q?feat:=20implement=208.2=20Set=20request?= =?UTF-8?q?=E2=80=99s=20referrer=20policy=20on=20redirect?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/fetch/util.js | 10 ++++++++-- test/fetch/util.js | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index c71ab16eec1..7bd10bbe5b3 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -200,8 +200,14 @@ function setRequestReferrerPolicyOnRedirect (request, actualResponse) { // 1. Let policy be the result of executing § 8.1 Parse a referrer policy // from a Referrer-Policy header on actualResponse. - // TODO: https://w3c.github.io/webappsec-referrer-policy/#parse-referrer-policy-from-header - const policy = '' + + // 8.1 Parse a referrer policy from a Referrer-Policy header + // 1. Let policy-tokens be the result of extracting header list values given `Referrer-Policy` and response’s header list. + const { headersList } = actualResponse + // 2. Let policy be the empty string. + // 3. For each token in policy-tokens, if token is a referrer policy and token is not the empty string, then set policy to token. + // 4. Return policy. + const policy = headersList.get('referrer-policy') ?? '' // 2. If policy is not the empty string, then set request’s referrer policy to policy. if (policy !== '') { diff --git a/test/fetch/util.js b/test/fetch/util.js index fcd9d94cbbe..4191cd0e18c 100644 --- a/test/fetch/util.js +++ b/test/fetch/util.js @@ -134,6 +134,45 @@ test('isURLPotentiallyTrustworthy', (t) => { } }) +test('setRequestReferrerPolicyOnRedirect', nested => { + nested.plan(2) + + nested.test('should set referrer policy from response headers on redirect', t => { + const request = { + referrerPolicy: 'no-referrer, strict-origin-when-cross-origin' + } + + const actualResponse = { + headersList: new HeadersList() + } + + actualResponse.headersList.append('Connection', 'close') + actualResponse.headersList.append('Location', 'https://some-location.com/redirect') + actualResponse.headersList.append('Referrer-Policy', 'origin') + util.setRequestReferrerPolicyOnRedirect(request, actualResponse) + + t.equal(request.referrerPolicy, 'origin') + t.end() + }) + + nested.test('should set not change request referrer policy if no Referrer-Policy from initial redirect response', t => { + const request = { + referrerPolicy: 'no-referrer, strict-origin-when-cross-origin' + } + + const actualResponse = { + headersList: new HeadersList() + } + + actualResponse.headersList.append('Connection', 'close') + actualResponse.headersList.append('Location', 'https://some-location.com/redirect') + util.setRequestReferrerPolicyOnRedirect(request, actualResponse) + + t.equal(request.referrerPolicy, 'no-referrer, strict-origin-when-cross-origin') + t.end() + }) +}) + test('determineRequestsReferrer', (t) => { t.plan(7) From 6c88ca6dc0bfae85ec3be462d92531172cc1d2d5 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Fri, 21 Oct 2022 11:02:37 +0200 Subject: [PATCH 2/4] feat: extend to double-check if a valid referrer policy --- lib/fetch/util.js | 24 +++++++++++++++++++++--- test/fetch/util.js | 27 ++++++++++++++++++++++++--- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index 7bd10bbe5b3..75266d6eefc 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -26,6 +26,18 @@ const badPorts = [ '10080' ] +// https://w3c.github.io/webappsec-referrer-policy/#referrer-policies +const referrerPolicyTokens = [ + 'no-referrer', + 'no-referrer-when-downgrade', + 'same-origin', + 'origin', + 'strict-origin', + 'origin-when-cross-origin', + 'strict-origin-when-cross-origin', + 'unsafe-url' +] + function responseURL (response) { // https://fetch.spec.whatwg.org/#responses // A response has an associated URL. It is a pointer to the last URL @@ -207,11 +219,17 @@ function setRequestReferrerPolicyOnRedirect (request, actualResponse) { // 2. Let policy be the empty string. // 3. For each token in policy-tokens, if token is a referrer policy and token is not the empty string, then set policy to token. // 4. Return policy. - const policy = headersList.get('referrer-policy') ?? '' + const token = headersList.get('referrer-policy') ?? '' // 2. If policy is not the empty string, then set request’s referrer policy to policy. - if (policy !== '') { - request.referrerPolicy = policy + if (token !== '') { + for (const policyToken of referrerPolicyTokens) { + // if token is a referrer policy and token is not an empty string, then set policy to token. + if (token === policyToken) { + request.referrerPolicy = token + break + } + } } } diff --git a/test/fetch/util.js b/test/fetch/util.js index 4191cd0e18c..446ba82702b 100644 --- a/test/fetch/util.js +++ b/test/fetch/util.js @@ -135,7 +135,7 @@ test('isURLPotentiallyTrustworthy', (t) => { }) test('setRequestReferrerPolicyOnRedirect', nested => { - nested.plan(2) + nested.plan(3) nested.test('should set referrer policy from response headers on redirect', t => { const request = { @@ -146,13 +146,14 @@ test('setRequestReferrerPolicyOnRedirect', nested => { headersList: new HeadersList() } + t.plan(1) + actualResponse.headersList.append('Connection', 'close') actualResponse.headersList.append('Location', 'https://some-location.com/redirect') actualResponse.headersList.append('Referrer-Policy', 'origin') util.setRequestReferrerPolicyOnRedirect(request, actualResponse) t.equal(request.referrerPolicy, 'origin') - t.end() }) nested.test('should set not change request referrer policy if no Referrer-Policy from initial redirect response', t => { @@ -164,12 +165,32 @@ test('setRequestReferrerPolicyOnRedirect', nested => { headersList: new HeadersList() } + t.plan(1) + actualResponse.headersList.append('Connection', 'close') actualResponse.headersList.append('Location', 'https://some-location.com/redirect') util.setRequestReferrerPolicyOnRedirect(request, actualResponse) t.equal(request.referrerPolicy, 'no-referrer, strict-origin-when-cross-origin') - t.end() + }) + + nested.test('should set not change request referrer policy if the policy is a non-valid Referrer Policy', t => { + const initial = 'no-referrer, strict-origin-when-cross-origin' + const request = { + referrerPolicy: initial + } + const actualResponse = { + headersList: new HeadersList() + } + + t.plan(1) + + actualResponse.headersList.append('Connection', 'close') + actualResponse.headersList.append('Location', 'https://some-location.com/redirect') + actualResponse.headersList.append('Referrer-Policy', 'asdasd') + util.setRequestReferrerPolicyOnRedirect(request, actualResponse) + + t.equal(request.referrerPolicy, initial) }) }) From be9cfd1c4b88a360c95654da060e4940c64c9a1d Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Wed, 26 Oct 2022 11:04:14 +0200 Subject: [PATCH 3/4] refactor: apply review suggestions --- lib/fetch/constants.js | 14 +++++++++++++- lib/fetch/util.js | 32 +++++--------------------------- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/lib/fetch/constants.js b/lib/fetch/constants.js index fe5028304b0..87eea34e0a9 100644 --- a/lib/fetch/constants.js +++ b/lib/fetch/constants.js @@ -8,6 +8,17 @@ const nullBodyStatus = [101, 204, 205, 304] const redirectStatus = [301, 302, 303, 307, 308] +// 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', + '87', '95', '101', '102', '103', '104', '109', '110', '111', '113', '115', '117', '119', '123', '135', '137', + '139', '143', '161', '179', '389', '427', '465', '512', '513', '514', '515', '526', '530', '531', '532', + '540', '548', '554', '556', '563', '587', '601', '636', '989', '990', '993', '995', '1719', '1720', '1723', + '2049', '3659', '4045', '5060', '5061', '6000', '6566', '6665', '6666', '6667', '6668', '6669', '6697', + '10080' +] + +// https://w3c.github.io/webappsec-referrer-policy/#referrer-policies const referrerPolicy = [ '', 'no-referrer', @@ -108,5 +119,6 @@ module.exports = { redirectStatus, corsSafeListedMethods, nullBodyStatus, - safeMethods + safeMethods, + badPorts } diff --git a/lib/fetch/util.js b/lib/fetch/util.js index 75266d6eefc..c738a378178 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -1,6 +1,6 @@ 'use strict' -const { redirectStatus } = require('./constants') +const { redirectStatus, badPorts, referrerPolicy: referrerPolicyTokens } = require('./constants') const { performance } = require('perf_hooks') const { isBlobLike, toUSVString, ReadableStreamFrom } = require('../core/util') const assert = require('assert') @@ -16,28 +16,6 @@ try { } -// 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', - '87', '95', '101', '102', '103', '104', '109', '110', '111', '113', '115', '117', '119', '123', '135', '137', - '139', '143', '161', '179', '389', '427', '465', '512', '513', '514', '515', '526', '530', '531', '532', - '540', '548', '554', '556', '563', '587', '601', '636', '989', '990', '993', '995', '1719', '1720', '1723', - '2049', '3659', '4045', '5060', '5061', '6000', '6566', '6665', '6666', '6667', '6668', '6669', '6697', - '10080' -] - -// https://w3c.github.io/webappsec-referrer-policy/#referrer-policies -const referrerPolicyTokens = [ - 'no-referrer', - 'no-referrer-when-downgrade', - 'same-origin', - 'origin', - 'strict-origin', - 'origin-when-cross-origin', - 'strict-origin-when-cross-origin', - 'unsafe-url' -] - function responseURL (response) { // https://fetch.spec.whatwg.org/#responses // A response has an associated URL. It is a pointer to the last URL @@ -219,14 +197,14 @@ function setRequestReferrerPolicyOnRedirect (request, actualResponse) { // 2. Let policy be the empty string. // 3. For each token in policy-tokens, if token is a referrer policy and token is not the empty string, then set policy to token. // 4. Return policy. - const token = headersList.get('referrer-policy') ?? '' + const policy = headersList.get('referrer-policy') ?? '' // 2. If policy is not the empty string, then set request’s referrer policy to policy. - if (token !== '') { + if (policy !== '') { for (const policyToken of referrerPolicyTokens) { // if token is a referrer policy and token is not an empty string, then set policy to token. - if (token === policyToken) { - request.referrerPolicy = token + if (policy === policyToken) { + request.referrerPolicy = policy break } } From 683ef3aaa32577d43d82a6e8f27c9afa58c94d1e Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Fri, 28 Oct 2022 10:45:54 +0200 Subject: [PATCH 4/4] feat: add support for fallback policies --- lib/fetch/util.js | 27 +++++++++++----- test/fetch/util.js | 78 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 96 insertions(+), 9 deletions(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index c738a378178..a738c077e02 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -197,18 +197,29 @@ function setRequestReferrerPolicyOnRedirect (request, actualResponse) { // 2. Let policy be the empty string. // 3. For each token in policy-tokens, if token is a referrer policy and token is not the empty string, then set policy to token. // 4. Return policy. - const policy = headersList.get('referrer-policy') ?? '' - - // 2. If policy is not the empty string, then set request’s referrer policy to policy. - if (policy !== '') { - for (const policyToken of referrerPolicyTokens) { - // if token is a referrer policy and token is not an empty string, then set policy to token. - if (policy === policyToken) { - request.referrerPolicy = policy + const policyHeader = (headersList.get('referrer-policy') ?? '').split(',') + + // Note: As the referrer-policy can contain multiple policies + // separated by comma, we need to loop through all of them + // and pick the first valid one. + // Ref: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy#specify_a_fallback_policy + let policy = '' + if (policyHeader.length > 0) { + // The right-most policy takes precedence. + // The left-most policy is the fallback. + for (let i = policyHeader.length; i !== 0; i--) { + const token = policyHeader[i - 1].trim() + if (referrerPolicyTokens.includes(token)) { + policy = token break } } } + + // 2. If policy is not the empty string, then set request’s referrer policy to policy. + if (policy !== '') { + request.referrerPolicy = policy + } } // https://fetch.spec.whatwg.org/#cross-origin-resource-policy-check diff --git a/test/fetch/util.js b/test/fetch/util.js index 446ba82702b..5d6af5ce62b 100644 --- a/test/fetch/util.js +++ b/test/fetch/util.js @@ -135,7 +135,7 @@ test('isURLPotentiallyTrustworthy', (t) => { }) test('setRequestReferrerPolicyOnRedirect', nested => { - nested.plan(3) + nested.plan(7) nested.test('should set referrer policy from response headers on redirect', t => { const request = { @@ -156,6 +156,63 @@ test('setRequestReferrerPolicyOnRedirect', nested => { t.equal(request.referrerPolicy, 'origin') }) + nested.test('should select the first valid policy from a response', t => { + const request = { + referrerPolicy: 'no-referrer, strict-origin-when-cross-origin' + } + + const actualResponse = { + headersList: new HeadersList() + } + + t.plan(1) + + actualResponse.headersList.append('Connection', 'close') + actualResponse.headersList.append('Location', 'https://some-location.com/redirect') + actualResponse.headersList.append('Referrer-Policy', 'asdas, origin') + util.setRequestReferrerPolicyOnRedirect(request, actualResponse) + + t.equal(request.referrerPolicy, 'origin') + }) + + nested.test('should select the first valid policy from a response#2', t => { + const request = { + referrerPolicy: 'no-referrer, strict-origin-when-cross-origin' + } + + const actualResponse = { + headersList: new HeadersList() + } + + t.plan(1) + + actualResponse.headersList.append('Connection', 'close') + actualResponse.headersList.append('Location', 'https://some-location.com/redirect') + actualResponse.headersList.append('Referrer-Policy', 'no-referrer, asdas, origin, 0943sd') + util.setRequestReferrerPolicyOnRedirect(request, actualResponse) + + t.equal(request.referrerPolicy, 'origin') + }) + + nested.test('should pick the last fallback over invalid policy tokens', t => { + const request = { + referrerPolicy: 'no-referrer, strict-origin-when-cross-origin' + } + + const actualResponse = { + headersList: new HeadersList() + } + + t.plan(1) + + actualResponse.headersList.append('Connection', 'close') + actualResponse.headersList.append('Location', 'https://some-location.com/redirect') + actualResponse.headersList.append('Referrer-Policy', 'origin, asdas, asdaw34') + util.setRequestReferrerPolicyOnRedirect(request, actualResponse) + + t.equal(request.referrerPolicy, 'origin') + }) + nested.test('should set not change request referrer policy if no Referrer-Policy from initial redirect response', t => { const request = { referrerPolicy: 'no-referrer, strict-origin-when-cross-origin' @@ -192,6 +249,25 @@ test('setRequestReferrerPolicyOnRedirect', nested => { t.equal(request.referrerPolicy, initial) }) + + nested.test('should set not change request referrer policy if the policy is a non-valid Referrer Policy', t => { + const initial = 'no-referrer, strict-origin-when-cross-origin' + const request = { + referrerPolicy: initial + } + const actualResponse = { + headersList: new HeadersList() + } + + t.plan(1) + + actualResponse.headersList.append('Connection', 'close') + actualResponse.headersList.append('Location', 'https://some-location.com/redirect') + actualResponse.headersList.append('Referrer-Policy', 'asdasd, asdasa, 12daw,') + util.setRequestReferrerPolicyOnRedirect(request, actualResponse) + + t.equal(request.referrerPolicy, initial) + }) }) test('determineRequestsReferrer', (t) => {