From 1d64b7f0308769c76ba00ca84d908cebb65e4dbc Mon Sep 17 00:00:00 2001 From: John Mellor Date: Mon, 4 Sep 2017 08:14:21 -0700 Subject: [PATCH] [Background Fetch] Add security checks copied from Fetch Adds some security checks to the Background Fetch API, based on https://fetch.spec.whatwg.org/#main-fetch. 1. Blocks invalid URLs. 2. Blocks CSP violations. 3. Blocks blacklisted ports. 4. Blocks credentials embedded in the url. 5. Blocks protocols other than http:// and https:// (https://github.com/WICG/background-fetch/issues/44). 6. Blocks Mixed Content (with an additional restriction that insecure http: cannot be requested from http://127.0.0.1). 7. Blocks URLs with dangling markup. 8. Temporarily blocks requests that require a CORS preflight, as BackgroundFetchCrossOriginFilter cannot yet handle them safely. This restriction will be lifted eventually. Bug: 711354,757441 Change-Id: I3d93c861ce4cbc9f460f61f3ed38a65131c2a620 Reviewed-on: https://chromium-review.googlesource.com/582007 Commit-Queue: John Mellor Reviewed-by: Peter Beverloo Cr-Commit-Position: refs/heads/master@{#499500} --- .../content-security-policy.https.window.js | 20 +++++ .../credentials-in-url.https.window.js | 32 ++++++++ .../dangling-markup.https.window.js | 17 ++++ ...ontent-and-allowed-schemes.https.window.js | 80 +++++++++++++++++++ .../port-blocking.https.window.js | 35 ++++++++ background-fetch/resources/sw.js | 1 + background-fetch/resources/utils.js | 26 ++++++ 7 files changed, 211 insertions(+) create mode 100644 background-fetch/content-security-policy.https.window.js create mode 100644 background-fetch/credentials-in-url.https.window.js create mode 100644 background-fetch/dangling-markup.https.window.js create mode 100644 background-fetch/mixed-content-and-allowed-schemes.https.window.js create mode 100644 background-fetch/port-blocking.https.window.js create mode 100644 background-fetch/resources/sw.js create mode 100644 background-fetch/resources/utils.js diff --git a/background-fetch/content-security-policy.https.window.js b/background-fetch/content-security-policy.https.window.js new file mode 100644 index 00000000000000..a6dc7c302c01dd --- /dev/null +++ b/background-fetch/content-security-policy.https.window.js @@ -0,0 +1,20 @@ +// META: script=/service-workers/service-worker/resources/test-helpers.sub.js +// META: script=resources/utils.js +'use strict'; + +// Tests that requests blocked by Content Security Policy are rejected. +// https://w3c.github.io/webappsec-csp/#should-block-request + +// This is not a comprehensive test of Content Security Policy - it is just +// intended to check that CSP checks are enabled. + +var meta = document.createElement('meta'); +meta.setAttribute('http-equiv', 'Content-Security-Policy'); +meta.setAttribute('content', "connect-src 'none'"); +document.head.appendChild(meta); + +backgroundFetchTest((t, bgFetch) => { + return promise_rejects( + t, new TypeError(), + bgFetch.fetch(uniqueTag(), 'https://example.com')); +}, 'fetch blocked by CSP should reject'); diff --git a/background-fetch/credentials-in-url.https.window.js b/background-fetch/credentials-in-url.https.window.js new file mode 100644 index 00000000000000..28b37b340b9691 --- /dev/null +++ b/background-fetch/credentials-in-url.https.window.js @@ -0,0 +1,32 @@ +// META: script=/service-workers/service-worker/resources/test-helpers.sub.js +// META: script=resources/utils.js +'use strict'; + +// "If parsedURL includes credentials, then throw a TypeError." +// https://fetch.spec.whatwg.org/#dom-request +// (Added by https://github.com/whatwg/fetch/issues/26). +// "A URL includes credentials if its username or password is not the empty +// string." +// https://url.spec.whatwg.org/#include-credentials + +backgroundFetchTest((t, bgFetch) => { + return bgFetch.fetch(uniqueTag(), 'https://example.com'); +}, 'fetch without credentials in URL should register ok'); + +backgroundFetchTest((t, bgFetch) => { + return promise_rejects( + t, new TypeError(), + bgFetch.fetch(uniqueTag(), 'https://username:password@example.com')); +}, 'fetch with username and password in URL should reject'); + +backgroundFetchTest((t, bgFetch) => { + return promise_rejects( + t, new TypeError(), + bgFetch.fetch(uniqueTag(), 'https://username:@example.com')); +}, 'fetch with username and empty password in URL should reject'); + +backgroundFetchTest((t, bgFetch) => { + return promise_rejects( + t, new TypeError(), + bgFetch.fetch(uniqueTag(), 'https://:password@example.com')); +}, 'fetch with empty username and password in URL should reject'); diff --git a/background-fetch/dangling-markup.https.window.js b/background-fetch/dangling-markup.https.window.js new file mode 100644 index 00000000000000..af7c395d751040 --- /dev/null +++ b/background-fetch/dangling-markup.https.window.js @@ -0,0 +1,17 @@ +// META: script=/service-workers/service-worker/resources/test-helpers.sub.js +// META: script=resources/utils.js +'use strict'; + +// "If request's url's potentially-dangling-markup flag is set, and request's +// url's scheme is an HTTP(S) scheme, then set response to a network error." +// https://github.com/whatwg/fetch/pull/519 +// https://github.com/whatwg/fetch/issues/546 + +// This is not a comprehensive test of dangling markup detection - it is just +// intended to check that detection is enabled. + +backgroundFetchTest((t, bgFetch) => { + return promise_rejects( + t, new TypeError(), + bgFetch.fetch(uniqueTag(), 'https://example.com/?\n<')); +}, 'fetch to URL containing \\n and < should reject'); diff --git a/background-fetch/mixed-content-and-allowed-schemes.https.window.js b/background-fetch/mixed-content-and-allowed-schemes.https.window.js new file mode 100644 index 00000000000000..50614a3565b0d1 --- /dev/null +++ b/background-fetch/mixed-content-and-allowed-schemes.https.window.js @@ -0,0 +1,80 @@ +// META: script=/service-workers/service-worker/resources/test-helpers.sub.js +// META: script=resources/utils.js +'use strict'; + +// Tests that Mixed Content requests are blocked. +// https://w3c.github.io/webappsec-mixed-content/#should-block-fetch +// https://w3c.github.io/webappsec-mixed-content/#a-priori-authenticated-url +// https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy + +// With an additional restriction that only https:// and loopback http:// +// requests are allowed. Hence the wss:, file:, data:, etc schemes are blocked. +// https://github.com/WICG/background-fetch/issues/44 + +// This is not a comprehensive test of mixed content blocking - it is just +// intended to check that blocking is enabled. + +backgroundFetchTest((t, bgFetch) => { + return bgFetch.fetch(uniqueTag(), 'https://example.com'); +}, 'https: fetch should register ok'); + +backgroundFetchTest((t, bgFetch) => { + return bgFetch.fetch(uniqueTag(), 'http://127.0.0.1'); +}, 'loopback IPv4 http: fetch should register ok'); + +backgroundFetchTest((t, bgFetch) => { + return bgFetch.fetch(uniqueTag(), 'http://[::1]'); +}, 'loopback IPv6 http: fetch should register ok'); + +// http://localhost is not tested here since the correct behavior from +// https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy +// depends on whether the UA conforms to the name resolution rules in +// https://tools.ietf.org/html/draft-west-let-localhost-be-localhost + +backgroundFetchTest((t, bgFetch) => { + return promise_rejects(t, new TypeError(), + bgFetch.fetch(uniqueTag(), 'http://example.com')); +}, 'non-loopback http: fetch should reject'); + +backgroundFetchTest((t, bgFetch) => { + return promise_rejects(t, new TypeError(), + bgFetch.fetch(uniqueTag(), 'http://192.0.2.0')); +}, 'non-loopback IPv4 http: fetch should reject'); + +backgroundFetchTest((t, bgFetch) => { + return promise_rejects(t, new TypeError(), + bgFetch.fetch(uniqueTag(), 'http://[2001:db8::1]')); +}, 'non-loopback IPv6 http: fetch should reject'); + +backgroundFetchTest((t, bgFetch) => { + return promise_rejects(t, new TypeError(), + bgFetch.fetch(uniqueTag(), ['https://example.com', + 'http://example.com'])); +}, 'https: and non-loopback http: fetch should reject'); + +backgroundFetchTest((t, bgFetch) => { + return promise_rejects(t, new TypeError(), + bgFetch.fetch(uniqueTag(), ['http://example.com', + 'https://example.com'])); +}, 'non-loopback http: and https: fetch should reject'); + + +backgroundFetchTest((t, bgFetch) => { + return promise_rejects(t, new TypeError(), + bgFetch.fetch(uniqueTag(), 'wss:127.0.0.1')); +}, 'wss: fetch should reject'); + +backgroundFetchTest((t, bgFetch) => { + return promise_rejects(t, new TypeError(), + bgFetch.fetch(uniqueTag(), 'file:///')); +}, 'file: fetch should reject'); + +backgroundFetchTest((t, bgFetch) => { + return promise_rejects(t, new TypeError(), + bgFetch.fetch(uniqueTag(), 'data:text/plain,foo')); +}, 'data: fetch should reject'); + +backgroundFetchTest((t, bgFetch) => { + return promise_rejects(t, new TypeError(), + bgFetch.fetch(uniqueTag(), 'foobar:bazqux')); +}, 'unknown scheme fetch should reject'); diff --git a/background-fetch/port-blocking.https.window.js b/background-fetch/port-blocking.https.window.js new file mode 100644 index 00000000000000..dbf8a1a4d9f2c3 --- /dev/null +++ b/background-fetch/port-blocking.https.window.js @@ -0,0 +1,35 @@ +// META: script=/service-workers/service-worker/resources/test-helpers.sub.js +// META: script=resources/utils.js +'use strict'; + +// Tests that requests to bad ports are blocked. +// https://fetch.spec.whatwg.org/#port-blocking + +// This is not a comprehensive test of blocked ports - it is just intended to +// check that blocking is enabled. + +backgroundFetchTest((t, bgFetch) => { + return bgFetch.fetch(uniqueTag(), 'https://example.com'); +}, 'fetch to default https port should register ok'); + +backgroundFetchTest((t, bgFetch) => { + return bgFetch.fetch(uniqueTag(), 'http://127.0.0.1'); +}, 'fetch to default http port should register ok'); + +backgroundFetchTest((t, bgFetch) => { + return bgFetch.fetch(uniqueTag(), 'https://example.com:443'); +}, 'fetch to port 443 should register ok'); + +backgroundFetchTest((t, bgFetch) => { + return bgFetch.fetch(uniqueTag(), 'https://example.com:80'); +}, 'fetch to port 80 should register ok, even over https'); + +backgroundFetchTest((t, bgFetch) => { + return bgFetch.fetch(uniqueTag(), 'https://example.com:8080'); +}, 'fetch to non-default non-bad port (8080) should register ok'); + +backgroundFetchTest((t, bgFetch) => { + return promise_rejects( + t, new TypeError(), + bgFetch.fetch(uniqueTag(), 'https://example.com:587')); +}, 'fetch to bad port (SMTP) should reject'); diff --git a/background-fetch/resources/sw.js b/background-fetch/resources/sw.js new file mode 100644 index 00000000000000..d4dc941796a15b --- /dev/null +++ b/background-fetch/resources/sw.js @@ -0,0 +1 @@ +// Deliberately left empty for now. \ No newline at end of file diff --git a/background-fetch/resources/utils.js b/background-fetch/resources/utils.js new file mode 100644 index 00000000000000..f630f1085d4c61 --- /dev/null +++ b/background-fetch/resources/utils.js @@ -0,0 +1,26 @@ +'use strict'; + +// Depends on /service-workers/service-worker/resources/test-helpers.sub.js +async function registerAndActivateServiceWorker(test) { + const script = 'resources/sw.js'; + const scope = 'resources/scope' + location.pathname; + let serviceWorkerRegistration = + await service_worker_unregister_and_register(test, script, scope); + add_completion_callback(() => { + serviceWorkerRegistration.unregister(); + }); + await wait_for_state(test, serviceWorkerRegistration.installing, 'activated'); + return serviceWorkerRegistration; +} + +function backgroundFetchTest(func, description) { + promise_test(async t => { + const serviceWorkerRegistration = await registerAndActivateServiceWorker(t); + return func(t, serviceWorkerRegistration.backgroundFetch); + }, description); +} + +let _nextBackgroundFetchTag = 0; +function uniqueTag() { + return 'tag' + _nextBackgroundFetchTag++; +} \ No newline at end of file