From f89941412aeb26d46fcbdcec75b9f2ff9066de94 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Sat, 11 Jan 2014 20:36:01 -0800 Subject: [PATCH 1/3] Smooth over some cross-browser CSP differences. * Adding "foo.com" to your CSP via browser-policy now adds both "http://foo.com" and "https://foo.com". This smooths over the fact that some browsers interpret "foo.com" as "http://foo.com" and some interpret it as http AND https. * Trim trailing slashes from origins. Firefox does not allow content from foo.com if you add "foo.com/" to your CSP. --- docs/client/packages/browser-policy.html | 7 +- .../browser-policy-content.js | 69 ++++++++++++++----- .../browser-policy/browser-policy-test.js | 24 +++++++ 3 files changed, 81 insertions(+), 19 deletions(-) diff --git a/docs/client/packages/browser-policy.html b/docs/client/packages/browser-policy.html index cc46e01f3a..19d531af40 100644 --- a/docs/client/packages/browser-policy.html +++ b/docs/client/packages/browser-policy.html @@ -119,8 +119,11 @@ Allows this type of content to be loaded from the given origin. `origin` is a string and can include an optional scheme (such as `http` or `https`), an optional wildcard at the beginning, and an optional port which can be a wildcard. Examples include `example.com`, `https://*.example.com`, and -`example.com:*`. You can call these functions multiple times with different -origins to specify a whitelist of allowed origins. +`example.com:*`. You can call these functions multiple times with +different origins to specify a whitelist of allowed origins. Origins +that don't specify a protocol will allow content over both HTTP and +HTTPS: passing `example.com` will allow content from both +`http://example.com` and `https://example.com`. {{/dtdd}} {{#dtdd "BrowserPolicy.content.allow<ContentType>DataUrl()"}} diff --git a/packages/browser-policy-content/browser-policy-content.js b/packages/browser-policy-content/browser-policy-content.js index b9ef18c27b..52d9c898e2 100644 --- a/packages/browser-policy-content/browser-policy-content.js +++ b/packages/browser-policy-content/browser-policy-content.js @@ -37,10 +37,12 @@ var cspSrcs; var cachedCsp; // Avoid constructing the header out of cspSrcs when possible. // CSP keywords have to be single-quoted. -var unsafeInline = "'unsafe-inline'"; -var unsafeEval = "'unsafe-eval'"; -var selfKeyword = "'self'"; -var noneKeyword = "'none'"; +var keywords = { + unsafeInline: "'unsafe-inline'", + unsafeEval: "'unsafe-eval'", + self: "'self'", + none: "'none'" +}; BrowserPolicy.content = {}; @@ -52,7 +54,7 @@ var parseCsp = function (csp) { policy = policy.substring(0, policy.length - 1); var srcs = policy.split(" "); var directive = srcs[0]; - if (_.indexOf(srcs, noneKeyword) !== -1) + if (_.indexOf(srcs, keywords.none) !== -1) cspSrcs[directive] = null; else cspSrcs[directive] = srcs.slice(1); @@ -81,6 +83,39 @@ var prepareForCspDirective = function (directive) { cspSrcs[directive] = _.clone(cspSrcs["default-src"]); }; +// Add `src` to the list of allowed sources for `directive`, with the +// following modifications if `src` is an origin: +// - If `src` does not have a protocol specified, then add both +// http:// and https://. This is to mask differing +// cross-browser behavior; some browsers interpret an origin without a +// protocol as http:// and some interpret it as both http:// +// and https:// +// - Trim trailing slashes from `src`. +var addSourceForDirective = function (directive, src) { + if (_.contains(_.values(keywords), src)) { + cspSrcs[directive].push(src); + } else { + src = src.toLowerCase(); + + // Trim trailing slashes. + while (src.charAt(src.length - 1) === "/") { + src = src.substring(0, src.length - 1); + } + + var toAdd = []; + // If there is no protocol, add both http:// and https://. + if (! /^([a-z0-9.+-]+:)/.test(src)) { + toAdd.push("http://" + src); + toAdd.push("https://" + src); + } else { + toAdd.push(src); + } + _.each(toAdd, function (s) { + cspSrcs[directive].push(s); + }); + } +}; + var setDefaultPolicy = function () { // By default, unsafe inline scripts and styles are allowed, since we expect // many apps will use them for analytics, etc. Unsafe eval is disallowed, and @@ -111,7 +146,7 @@ _.extend(BrowserPolicy.content, { var header = _.map(cspSrcs, function (srcs, directive) { srcs = srcs || []; if (_.isEmpty(srcs)) - srcs = [noneKeyword]; + srcs = [keywords.none]; var directiveCsp = _.uniq(srcs).join(" "); return directive + " " + directiveCsp + ";"; }); @@ -129,7 +164,7 @@ _.extend(BrowserPolicy.content, { cachedCsp = null; parseCsp(csp); setWebAppInlineScripts( - BrowserPolicy.content._keywordAllowed("script-src", unsafeInline) + BrowserPolicy.content._keywordAllowed("script-src", keywords.unsafeInline) ); }, @@ -142,34 +177,34 @@ _.extend(BrowserPolicy.content, { allowInlineScripts: function () { prepareForCspDirective("script-src"); - cspSrcs["script-src"].push(unsafeInline); + cspSrcs["script-src"].push(keywords.unsafeInline); setWebAppInlineScripts(true); }, disallowInlineScripts: function () { prepareForCspDirective("script-src"); - removeCspSrc("script-src", unsafeInline); + removeCspSrc("script-src", keywords.unsafeInline); setWebAppInlineScripts(false); }, allowEval: function () { prepareForCspDirective("script-src"); - cspSrcs["script-src"].push(unsafeEval); + cspSrcs["script-src"].push(keywords.unsafeEval); }, disallowEval: function () { prepareForCspDirective("script-src"); - removeCspSrc("script-src", unsafeEval); + removeCspSrc("script-src", keywords.unsafeEval); }, allowInlineStyles: function () { prepareForCspDirective("style-src"); - cspSrcs["style-src"].push(unsafeInline); + cspSrcs["style-src"].push(keywords.unsafeInline); }, disallowInlineStyles: function () { prepareForCspDirective("style-src"); - removeCspSrc("style-src", unsafeInline); + removeCspSrc("style-src", keywords.unsafeInline); }, // Functions for setting defaults allowSameOriginForAll: function () { - BrowserPolicy.content.allowOriginForAll(selfKeyword); + BrowserPolicy.content.allowOriginForAll(keywords.self); }, allowDataUrlForAll: function () { BrowserPolicy.content.allowOriginForAll("data:"); @@ -177,7 +212,7 @@ _.extend(BrowserPolicy.content, { allowOriginForAll: function (origin) { prepareForCspDirective("default-src"); _.each(_.keys(cspSrcs), function (directive) { - cspSrcs[directive].push(origin); + addSourceForDirective(directive, origin); }); }, disallowAll: function () { @@ -214,7 +249,7 @@ _.each(["script", "object", "img", "media", BrowserPolicy.content[allowMethodName] = function (src) { prepareForCspDirective(directive); - cspSrcs[directive].push(src); + addSourceForDirective(directive, src); }; if (resource === "script") { BrowserPolicy.content[disallowMethodName] = function () { @@ -230,7 +265,7 @@ _.each(["script", "object", "img", "media", }; BrowserPolicy.content[allowSelfMethodName] = function () { prepareForCspDirective(directive); - cspSrcs[directive].push(selfKeyword); + cspSrcs[directive].push(keywords.self); }; }); diff --git a/packages/browser-policy/browser-policy-test.js b/packages/browser-policy/browser-policy-test.js index e8b00b20a1..b51320eed5 100644 --- a/packages/browser-policy/browser-policy-test.js +++ b/packages/browser-policy/browser-policy-test.js @@ -112,6 +112,30 @@ Tinytest.add("browser-policy - csp", function (test) { BrowserPolicy.content.disallowObject(); test.isTrue(cspsEqual(BrowserPolicy.content._constructCsp(), "default-src 'self'; object-src 'none';")); + + // Allow foo.com; it should allow both http://foo.com and + // https://foo.com. + BrowserPolicy.content.allowImageOrigin("foo.com"); + test.isTrue(cspsEqual(BrowserPolicy.content._constructCsp(), + "default-src 'self'; object-src 'none'; " + + "img-src 'self' http://foo.com https://foo.com;")); + // "Disallow all " followed by "allow foo.com for all" results + // in srcs from foo.com. + BrowserPolicy.content.allowOriginForAll("foo.com"); + test.isTrue(cspsEqual(BrowserPolicy.content._constructCsp(), + "default-src 'self' http://foo.com https://foo.com; " + + "object-src http://foo.com https://foo.com; " + + "img-src 'self' http://foo.com https://foo.com;")); + + // Check that trailing slashes are trimmed from origins. + BrowserPolicy.content.disallowAll(); + BrowserPolicy.content.allowScriptOrigin("https://foo.com/"); + test.isTrue(cspsEqual(BrowserPolicy.content._constructCsp(), + "default-src 'none'; script-src https://foo.com;")); + BrowserPolicy.content.allowObjectOrigin("foo.com//"); + test.isTrue(cspsEqual(BrowserPolicy.content._constructCsp(), + "default-src 'none'; script-src https://foo.com; " + + "object-src http://foo.com https://foo.com;")); }); Tinytest.add("browser-policy - x-frame-options", function (test) { From 189845f1fba51d291484efeff9a99944380adba1 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Sat, 11 Jan 2014 20:43:36 -0800 Subject: [PATCH 2/3] Add frame-src to browser-policy-content. --- docs/client/packages/browser-policy.html | 8 +++++++- packages/browser-policy-content/browser-policy-content.js | 2 +- packages/browser-policy/browser-policy-test.js | 6 +++--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/docs/client/packages/browser-policy.html b/docs/client/packages/browser-policy.html index 19d531af40..5bede3d5b2 100644 --- a/docs/client/packages/browser-policy.html +++ b/docs/client/packages/browser-policy.html @@ -111,7 +111,7 @@ Disallows inline CSS. Finally, you can configure a whitelist of allowed requests that various types of content can make. The following functions are defined for the content types -script, object, image, media, font, and connect. +script, object, image, media, font, frame, and connect.
{{#dtdd "BrowserPolicy.content.allow<ContentType>Origin(origin)"}} @@ -162,6 +162,12 @@ allows images to have their `src` attributes point to images served from `https://example.com`. * `BrowserPolicy.content.allowConnectOrigin("https://example.com")` allows XMLHttpRequest and WebSocket connections to `https://example.com`. +* `BrowserPolicy.content.allowFrameOrigin("https://example.com")` allows +your site to load the origin `https://example.com` in a frame or +iframe. The `BrowserPolicy.framing` API allows you to control which +sites can frame your site, while +`BrowserPolicy.content.allowFrameOrigin` allows you to control which +sites can be loaded inside frames on your site. {{/better_markdown}} diff --git a/packages/browser-policy-content/browser-policy-content.js b/packages/browser-policy-content/browser-policy-content.js index 52d9c898e2..328a7b5833 100644 --- a/packages/browser-policy-content/browser-policy-content.js +++ b/packages/browser-policy-content/browser-policy-content.js @@ -227,7 +227,7 @@ _.extend(BrowserPolicy.content, { // allowOrigin, allowData, allowself, and // disallow methods for each type of resource. _.each(["script", "object", "img", "media", - "font", "connect", "style"], + "font", "connect", "style", "frame"], function (resource) { var directive = resource + "-src"; var methodResource; diff --git a/packages/browser-policy/browser-policy-test.js b/packages/browser-policy/browser-policy-test.js index b51320eed5..48f999c9ba 100644 --- a/packages/browser-policy/browser-policy-test.js +++ b/packages/browser-policy/browser-policy-test.js @@ -129,12 +129,12 @@ Tinytest.add("browser-policy - csp", function (test) { // Check that trailing slashes are trimmed from origins. BrowserPolicy.content.disallowAll(); - BrowserPolicy.content.allowScriptOrigin("https://foo.com/"); + BrowserPolicy.content.allowFrameOrigin("https://foo.com/"); test.isTrue(cspsEqual(BrowserPolicy.content._constructCsp(), - "default-src 'none'; script-src https://foo.com;")); + "default-src 'none'; frame-src https://foo.com;")); BrowserPolicy.content.allowObjectOrigin("foo.com//"); test.isTrue(cspsEqual(BrowserPolicy.content._constructCsp(), - "default-src 'none'; script-src https://foo.com; " + + "default-src 'none'; frame-src https://foo.com; " + "object-src http://foo.com https://foo.com;")); }); From bdf6fd248574997f2d5da462d031a57139d135bd Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 13 Jan 2014 18:27:58 -0800 Subject: [PATCH 3/3] Use regex to replace trailing slashes from browser-policy URLs. Thanks glasser! --- packages/browser-policy-content/browser-policy-content.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/browser-policy-content/browser-policy-content.js b/packages/browser-policy-content/browser-policy-content.js index 328a7b5833..569bf9c4d0 100644 --- a/packages/browser-policy-content/browser-policy-content.js +++ b/packages/browser-policy-content/browser-policy-content.js @@ -90,7 +90,8 @@ var prepareForCspDirective = function (directive) { // cross-browser behavior; some browsers interpret an origin without a // protocol as http:// and some interpret it as both http:// // and https:// -// - Trim trailing slashes from `src`. +// - Trim trailing slashes from `src`, since some browsers interpret +// "foo.com/" as "foo.com" and some don't. var addSourceForDirective = function (directive, src) { if (_.contains(_.values(keywords), src)) { cspSrcs[directive].push(src); @@ -98,9 +99,7 @@ var addSourceForDirective = function (directive, src) { src = src.toLowerCase(); // Trim trailing slashes. - while (src.charAt(src.length - 1) === "/") { - src = src.substring(0, src.length - 1); - } + src = src.replace(/\/+$/, ''); var toAdd = []; // If there is no protocol, add both http:// and https://.