diff --git a/CHANGELOG.md b/CHANGELOG.md index 37ace8a79ae..db76c0be3a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Enterprise Fixes: - [data-manager] Fixed editing an event whose key contains `&` creating undeletable duplicate rows in the events table Security Fixes: +- [core] Standardised the default security headers: dropped `X-XSS-Protection`, which is deprecated and removed from current browsers, dropped `preload` from the HSTS default so it stays an operator's choice, and added `Referrer-Policy`, `Permissions-Policy`, `Cross-Origin-Opener-Policy`. Routes that are embedded by SDKs (the ratings popup and its assets) drop the framing and cross-origin isolation headers as they already did for `X-Frame-Options`. An upgrade script brings existing installs in line while preserving any headers an operator added - [hooks] Internal event hooks are now scoped to the apps the hook belongs to: app creation is a global-admin-only event, and remote-config, cohort, alert and hook-chaining events are only delivered when the event's app is one the hook is scoped to - [compliance-hub] The consents table now returns a fixed set of fields; a projection supplied on the request is no longer used to widen the response beyond the consent columns - [dashboards] Widgets are no longer copied when the copying user has no access to the apps they reference, and widget app ids are validated on widget create and update diff --git a/api/api.js b/api/api.js index 2b250e1e94b..4b704e5c8b9 100644 --- a/api/api.js +++ b/api/api.js @@ -121,8 +121,13 @@ plugins.connectToAllDatabases().then(function() { password_rotation: 3, password_autocomplete: true, robotstxt: "User-agent: *\nDisallow: /", - dashboard_additional_headers: "X-Frame-Options:deny\nX-XSS-Protection:1; mode=block\nStrict-Transport-Security:max-age=31536000; includeSubDomains; preload\nX-Content-Type-Options: nosniff", - api_additional_headers: "X-Frame-Options:deny\nX-XSS-Protection:1; mode=block\nStrict-Transport-Security:max-age=31536000; includeSubDomains; preload\nAccess-Control-Allow-Origin:*", + dashboard_additional_headers: "X-Frame-Options:deny\nStrict-Transport-Security:max-age=31536000; includeSubDomains\nX-Content-Type-Options: nosniff\nReferrer-Policy: strict-origin-when-cross-origin\nPermissions-Policy: camera=(), microphone=(), geolocation=(), payment=()\nCross-Origin-Opener-Policy: same-origin-allow-popups", + api_additional_headers: "X-Frame-Options:deny\nStrict-Transport-Security:max-age=31536000; includeSubDomains\nX-Content-Type-Options: nosniff\nReferrer-Policy: strict-origin-when-cross-origin\nPermissions-Policy: camera=(), microphone=(), geolocation=(), payment=()\nAccess-Control-Allow-Origin:*", + //Off by default: allow_access_control_origin is labelled as an + //Access-Control-Origin list only, so enforcing it as a framing rule could + //block an embed that works today. Turn on once the reported violations + //from a real deployment come back clean. + widget_frame_ancestors_enforce: false, dashboard_rate_limit_window: 60, dashboard_rate_limit_requests: 500, api_rate_limit_window: 0, diff --git a/api/utils/common.js b/api/utils/common.js index 05119c59521..1efb0d29dbf 100644 --- a/api/utils/common.js +++ b/api/utils/common.js @@ -1435,6 +1435,120 @@ common.returnRaw = function(params, returnCode, body, heads) { } }; +/** +* Origin shapes accepted into a CSP source list: scheme, host and an optional +* port, with nothing after it. Deliberately strict, because a browser discards +* an entire directive that contains one source it cannot parse -- so a single +* malformed entry in an app's list must not be able to void the whole policy. +*/ +var FRAME_ANCESTOR_ORIGIN_REGEX = /^[a-zA-Z][a-zA-Z0-9+.-]*:\/\/(?:\[[0-9A-Fa-f:.]+\]|[a-zA-Z0-9._-]+)(?::\d{1,5})?$/; + +/** +* Build the value of a CSP frame-ancestors directive out of an app's newline +* separated allowed origin list, which is the same +* app.plugins.allow_access_control_origin list that already drives +* Access-Control-Allow-Origin. Entries that are not plain origins are skipped +* rather than emitted. +* +* 'self' is always included, because the dashboard previews these same widget +* routes in an iframe of its own and nobody lists their own Countly origin in +* an allowed-origins list meant for their customer facing sites. +* @param {String} allowedOrigins - newline separated list of exact origins +* @returns {String|null} the directive value, or null when nothing usable is +* configured, in which case the caller must omit the +* header entirely instead of falling back to 'none' +**/ +common.buildFrameAncestorsPolicy = function(allowedOrigins) { + if (typeof allowedOrigins !== "string") { + return null; + } + var entries = allowedOrigins.replace(/\r\n|\r|\n/g, "\n").split("\n"); + var origins = []; + for (let i = 0; i < entries.length; i++) { + //a trailing slash is the one typo worth normalizing, since origins get + //copied out of a browser address bar; anything else has to already be + //an exact origin, the same way the CORS side requires + var origin = entries[i].trim().replace(/\/+$/, ""); + if (FRAME_ANCESTOR_ORIGIN_REGEX.test(origin) && origins.indexOf(origin) === -1) { + origins.push(origin); + } + } + if (!origins.length) { + return null; + } + origins.unshift("'self'"); + return "frame-ancestors " + origins.join(" "); +}; + +/** +* Scope an SDK-embeddable widget response to the origins configured for its +* app. X-Frame-Options is always removed: it cannot express a list of allowed +* domains (ALLOW-FROM was never really implemented by browsers) and a browser +* seeing both headers applies the stricter one, which would block the embed +* regardless of the CSP. +* +* When the app has no usable origin configured the CSP header is left off, so +* the many apps that never filled in the list keep embedding as before. +* +* Reported, not enforced, unless an operator opts in with +* security.widget_frame_ancestors_enforce. allow_access_control_origin is +* labelled purely as an Access-Control-Origin list and says nothing about +* framing, so an app that listed only the origin its SDK makes XHR from would +* have a working embed on another host blocked the moment this is enforced. +* Report-only blocks nothing and surfaces those violations first. +* @param {Object} res - express response object +* @param {Object} app - app document, may be null when it could not be resolved +**/ +common.setWidgetFrameHeaders = function(res, app) { + if (!res || typeof res.removeHeader !== "function") { + return; + } + res.removeHeader("X-Frame-Options"); + var policy = common.buildFrameAncestorsPolicy(app && app.plugins && app.plugins.allow_access_control_origin); + if (!policy || typeof res.setHeader !== "function") { + return; + } + var enforce = false; + try { + enforce = !!(plugins.getConfig("security") || {}).widget_frame_ancestors_enforce; + } + catch (e) { + //no config loaded in this process: stay with the harmless default + enforce = false; + } + var headerName = enforce ? "Content-Security-Policy" : "Content-Security-Policy-Report-Only"; + //an operator can already put a Content-Security-Policy into + //security.dashboard_additional_headers, so merge instead of overwriting and + //losing their directives; if they scoped framing themselves, leave it be + var existing = typeof res.getHeader === "function" ? res.getHeader(headerName) : null; + if (typeof existing === "string" && existing.trim().length) { + if (existing.indexOf("frame-ancestors") !== -1) { + return; + } + res.setHeader(headerName, existing.trim().replace(/;$/, "") + "; " + policy); + return; + } + res.setHeader(headerName, policy); +}; + +/** +* Look up the app behind an embeddable widget request so its response can be +* scoped to that app's configured origins. Best effort by design: any missing +* or unexpected value resolves to null and the caller then omits the header. +* @param {Object} db - countly database object +* @param {String} appId - app id taken from the widget document +* @param {Function} callback - called with the app document, or null +* @returns {void} nothing, the result is handed to the callback +**/ +common.getAppForWidget = function(db, appId, callback) { + if (!db || !appId || !/^[a-f0-9]{24}$/i.test(appId + "")) { + return callback(null); + } + db.collection("apps").findOne({_id: db.ObjectID(appId + "")}, {projection: {"plugins.allow_access_control_origin": 1}}, function(err, app) { + callback(err ? null : app || null); + }); +}; + common.returnMessage = function(params, returnCode, message, heads, noResult = false) { params.response = { code: returnCode, diff --git a/bin/upgrade/DEV/scripts/standardize_security_headers.js b/bin/upgrade/DEV/scripts/standardize_security_headers.js new file mode 100644 index 00000000000..b2f444284a8 --- /dev/null +++ b/bin/upgrade/DEV/scripts/standardize_security_headers.js @@ -0,0 +1,113 @@ +/** + * Bring an existing install's configured security headers in line with the current + * defaults. + * + * Config defaults only seed keys that are absent: pluginManager.checkConfigs uses + * getObjectDiff, which copies a default across only when the stored value is undefined. + * So an install that already has security.dashboard_additional_headers keeps whatever it + * was first seeded with, forever, and a change to the shipped default reaches new + * installs only. That is why this script exists. + * + * It edits rather than overwrites, so anything an operator added by hand survives: + * + * - drops X-XSS-Protection. Deprecated, removed from every current browser, and usable + * as an attack primitive: it could be steered into disabling a page's own scripts, + * and with mode=block the aborted load is observable cross-origin, which makes it an + * oracle for reading page contents. + * - drops the `preload` token from Strict-Transport-Security. Preload is a one-way + * commitment that needs the domain submitted to the browser preload list and is + * painful to undo, so it should be an operator's choice rather than a default. The + * max-age and includeSubDomains parts are left alone. + * - appends Referrer-Policy, Permissions-Policy and X-Content-Type-Options if absent. + * + * Deliberately does not add Cross-Origin-Opener-Policy or Cross-Origin-Resource-Policy: + * these headers are applied by global middleware that also covers the embeddable widget + * routes (/feedback/rating and the widget asset routes), which customers embed from + * their own origins, so setting them here would break those. + */ + +const pluginManager = require('../../../../plugins/pluginManager.js'); + +const KEYS = ['dashboard_additional_headers', 'api_additional_headers']; + +const REQUIRED = [ + {name: 'X-Content-Type-Options', line: 'X-Content-Type-Options: nosniff'}, + {name: 'Referrer-Policy', line: 'Referrer-Policy: strict-origin-when-cross-origin'}, + {name: 'Permissions-Policy', line: 'Permissions-Policy: camera=(), microphone=(), geolocation=(), payment=()'} +]; + +/** + * Rewrite one configured header block. + * @param {string} value - the stored newline separated header string + * @returns {string|null} the new value, or null when nothing needed changing + */ +function rewrite(value) { + if (typeof value !== 'string') { + return null; + } + const lines = value.replace(/\r\n|\r/g, '\n').split('\n'); + const kept = []; + + for (const line of lines) { + const name = line.split(':')[0].trim().toLowerCase(); + if (name === 'x-xss-protection') { + continue; + } + if (name === 'strict-transport-security') { + // remove only the preload token, keep max-age and includeSubDomains + kept.push(line.replace(/;\s*preload\b/i, '')); + continue; + } + kept.push(line); + } + + const present = kept.map((l) => l.split(':')[0].trim().toLowerCase()); + for (const req of REQUIRED) { + if (present.indexOf(req.name.toLowerCase()) === -1) { + kept.push(req.line); + } + } + + const next = kept.filter((l) => l.trim().length).join('\n'); + return next === value ? null : next; +} + +pluginManager.dbConnection().then(async(db) => { + try { + const doc = await db.collection('plugins').findOne({_id: 'plugins'}); + const security = (doc && doc.security) || {}; + const update = {}; + + for (const key of KEYS) { + if (typeof security[key] === 'undefined') { + // never configured, so the shipped default applies already + console.log('security.' + key + ': not set, leaving to the default'); + continue; + } + const next = rewrite(security[key]); + if (next === null) { + console.log('security.' + key + ': already current'); + continue; + } + update['security.' + key] = next; + console.log('security.' + key + ': updating'); + console.log(' from: ' + JSON.stringify(security[key])); + console.log(' to: ' + JSON.stringify(next)); + } + + if (Object.keys(update).length) { + await db.collection('plugins').updateOne({_id: 'plugins'}, {$set: update}); + console.log('Security headers updated'); + } + else { + console.log('Nothing to update'); + } + } + catch (err) { + console.error('Error while standardizing security headers', err); + } + + db.close(); +}); + +module.exports = {rewrite}; diff --git a/frontend/express/app.js b/frontend/express/app.js index 2389ebb7f66..6e615592b05 100644 --- a/frontend/express/app.js +++ b/frontend/express/app.js @@ -159,8 +159,13 @@ plugins.setConfigs("security", { password_rotation: 3, password_autocomplete: true, robotstxt: "User-agent: *\nDisallow: /", - dashboard_additional_headers: "X-Frame-Options:deny\nX-XSS-Protection:1; mode=block\nStrict-Transport-Security:max-age=31536000; includeSubDomains; preload\nX-Content-Type-Options: nosniff", - api_additional_headers: "X-Frame-Options:deny\nX-XSS-Protection:1; mode=block\nStrict-Transport-Security:max-age=31536000; includeSubDomains; preload\nAccess-Control-Allow-Origin:*", + dashboard_additional_headers: "X-Frame-Options:deny\nStrict-Transport-Security:max-age=31536000; includeSubDomains\nX-Content-Type-Options: nosniff\nReferrer-Policy: strict-origin-when-cross-origin\nPermissions-Policy: camera=(), microphone=(), geolocation=(), payment=()\nCross-Origin-Opener-Policy: same-origin-allow-popups", + api_additional_headers: "X-Frame-Options:deny\nStrict-Transport-Security:max-age=31536000; includeSubDomains\nX-Content-Type-Options: nosniff\nReferrer-Policy: strict-origin-when-cross-origin\nPermissions-Policy: camera=(), microphone=(), geolocation=(), payment=()\nAccess-Control-Allow-Origin:*", + //Off by default: allow_access_control_origin is labelled as an + //Access-Control-Origin list only, so enforcing it as a framing rule could + //block an embed that works today. Turn on once the reported violations + //from a real deployment come back clean. + widget_frame_ancestors_enforce: false, dashboard_rate_limit_window: 60, dashboard_rate_limit_requests: 500 }); diff --git a/plugins/star-rating/frontend/app.js b/plugins/star-rating/frontend/app.js index 17cd80d2a30..bd3244a291d 100644 --- a/plugins/star-rating/frontend/app.js +++ b/plugins/star-rating/frontend/app.js @@ -22,7 +22,7 @@ var STAR_RATING_EXT_TO_MIME = { }; (function(plugin) { - plugin.init = function(app) { + plugin.init = function(app, countlyDb) { var SAFE_PROVIDED_PATH_RE = /^[a-zA-Z0-9\-_./]*$/; /** @@ -45,6 +45,27 @@ var STAR_RATING_EXT_TO_MIME = { return rawValue; } + /** + * Resolve the app that owns the requested widget, so the response can be + * scoped to the origins that app is allowed to be embedded on. Never + * fails the request: an unknown widget just yields no app, and the + * frame-ancestors header is then omitted. + * @param {*} widgetId - widget_id query parameter + * @param {Function} callback - called with the app document, or null + * @returns {void} nothing, the result is handed to the callback + */ + function getWidgetApp(widgetId, callback) { + if (!countlyDb || typeof widgetId !== 'string' || !/^[a-f0-9]{24}$/i.test(widgetId)) { + return callback(null); + } + countlyDb.collection('feedback_widgets').findOne({_id: countlyDb.ObjectID(widgetId)}, {projection: {app_id: 1}}, function(err, widget) { + if (err || !widget) { + return callback(null); + } + common.getAppForWidget(countlyDb, widget.app_id, callback); + }); + } + /** * Method that render ratings popup template * @param {*} req - Express request object @@ -64,8 +85,21 @@ var STAR_RATING_EXT_TO_MIME = { countlyPath = `/${countlyPath}`; } + //This popup is meant to be embedded by an SDK in a page on the customer's own + //origin, so the framing and cross-origin isolation headers the dashboard sets + //globally have to come back off for this response. + // + //Cross-Origin-Opener-Policy matters when the SDK opens the popup as a window + //rather than an iframe: it would otherwise sever window.opener and with it any + //callback the SDK expects. res.removeHeader('X-Frame-Options'); - res.render('../../../plugins/star-rating/frontend/public/templates/feedback-popup', { countlyPath }); + res.removeHeader('Cross-Origin-Opener-Policy'); + //X-Frame-Options cannot name the origins allowed to frame this popup, so + //scope it with CSP frame-ancestors built from the app's own configuration + getWidgetApp(req.query.widget_id, function(widgetApp) { + common.setWidgetFrameHeaders(res, widgetApp); + res.render('../../../plugins/star-rating/frontend/public/templates/feedback-popup', { countlyPath }); + }); } app.get(countlyConfig.path + '/feedback/rating', renderPopup); diff --git a/plugins/star-rating/tests.js b/plugins/star-rating/tests.js index 5910b044545..a08bf9fb110 100644 --- a/plugins/star-rating/tests.js +++ b/plugins/star-rating/tests.js @@ -57,6 +57,26 @@ describe('Testing Rating plugin', function() { }); }); + it('should not send X-Frame-Options and should not scope framing for an unresolvable widget', function(done) { + request.get('/feedback/rating') + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + //the widget must stay embeddable, so X-Frame-Options is gone + should.not.exist(res.headers['x-frame-options']); + //and with no app resolved there is nothing to scope framing to, + //so no frame-ancestors may be emitted at all, enforced or reported + ['content-security-policy', 'content-security-policy-report-only'].forEach(function(h) { + if (res.headers[h]) { + res.headers[h].should.not.containEql('frame-ancestors'); + } + }); + done(); + }); + }); + it('should prefix every asset path with provided_url when it has a leading slash', function(done) { var providedUrl = '/reverse-proxy/countly'; request.get('/feedback/rating?provided_url=' + encodeURIComponent(providedUrl)) diff --git a/test/unit-tests/api.utils.common.frame-ancestors.js b/test/unit-tests/api.utils.common.frame-ancestors.js new file mode 100644 index 00000000000..76e1d86a401 --- /dev/null +++ b/test/unit-tests/api.utils.common.frame-ancestors.js @@ -0,0 +1,350 @@ +var should = require("should"); +var common = require("../../api/utils/common"); +var plugins = require("../../plugins/pluginManager.js"); + +// The enforce switch is read through plugins.getConfig("security"); drive it +// directly rather than standing up a config store. +var originalGetConfig = plugins.getConfig; + +/** + * Force security.widget_frame_ancestors_enforce for the duration of a test. + * @param {Boolean} enforce - value the code under test should observe + * @returns {void} nothing + */ +function setEnforce(enforce) { + plugins.getConfig = function(name) { + if (name === "security") { + return {widget_frame_ancestors_enforce: enforce}; + } + return originalGetConfig.apply(plugins, arguments); + }; +} + +/** + * Put plugins.getConfig back the way it was. + * @returns {void} nothing + */ +function restoreConfig() { + plugins.getConfig = originalGetConfig; +} + +/** + * Minimal stand-in for the parts of an express response the widget routes + * touch, recording what ends up on the wire. + * @param {Object} initialHeaders - headers already set by earlier middleware + * @returns {Object} fake response object + */ +function fakeRes(initialHeaders) { + var headers = {}; + Object.keys(initialHeaders || {}).forEach(function(name) { + headers[name.toLowerCase()] = initialHeaders[name]; + }); + return { + headers: headers, + setHeader: function(name, value) { + headers[name.toLowerCase()] = value; + }, + getHeader: function(name) { + return headers[name.toLowerCase()]; + }, + removeHeader: function(name) { + delete headers[name.toLowerCase()]; + }, + get: function(name) { + return headers[name.toLowerCase()]; + } + }; +} + +/** + * Build an app document carrying the given allowed origin list. + * @param {String} allowedOrigins - newline separated origin list + * @returns {Object} app document + */ +function appWithOrigins(allowedOrigins) { + return {plugins: {allow_access_control_origin: allowedOrigins}}; +} + +describe("Widget frame-ancestors headers", function() { + describe("buildFrameAncestorsPolicy", function() { + it("should return null when nothing is configured", function() { + should.equal(common.buildFrameAncestorsPolicy(undefined), null); + should.equal(common.buildFrameAncestorsPolicy(null), null); + should.equal(common.buildFrameAncestorsPolicy(""), null); + should.equal(common.buildFrameAncestorsPolicy(" "), null); + should.equal(common.buildFrameAncestorsPolicy("\n\n"), null); + }); + + it("should not accept a non string list", function() { + should.equal(common.buildFrameAncestorsPolicy(["https://example.com"]), null); + should.equal(common.buildFrameAncestorsPolicy({}), null); + should.equal(common.buildFrameAncestorsPolicy(42), null); + }); + + it("should build the directive for a single origin", function() { + common.buildFrameAncestorsPolicy("https://example.com") + .should.eql("frame-ancestors 'self' https://example.com"); + }); + + it("should build the directive for several origins", function() { + common.buildFrameAncestorsPolicy("https://example.com\nhttps://shop.example.com\nhttp://localhost:8080") + .should.eql("frame-ancestors 'self' https://example.com https://shop.example.com http://localhost:8080"); + }); + + it("should accept the CRLF and CR line endings the config field can contain", function() { + common.buildFrameAncestorsPolicy("https://a.example.com\r\nhttps://b.example.com\rhttps://c.example.com") + .should.eql("frame-ancestors 'self' https://a.example.com https://b.example.com https://c.example.com"); + }); + + it("should tolerate surrounding whitespace, blank lines and a trailing slash", function() { + common.buildFrameAncestorsPolicy(" https://example.com/ \n\n\thttps://other.example.com\n") + .should.eql("frame-ancestors 'self' https://example.com https://other.example.com"); + }); + + it("should keep non http schemes used by hybrid app webviews", function() { + common.buildFrameAncestorsPolicy("capacitor://localhost\nionic://localhost") + .should.eql("frame-ancestors 'self' capacitor://localhost ionic://localhost"); + }); + + it("should keep a bracketed IPv6 host with a port", function() { + common.buildFrameAncestorsPolicy("http://[::1]:6001") + .should.eql("frame-ancestors 'self' http://[::1]:6001"); + }); + + it("should skip malformed entries and keep the good ones", function() { + common.buildFrameAncestorsPolicy([ + "https://good.example.com", + "not-an-origin", + "https://bad.example.com/some/path", + "https://query.example.com?a=b", + "https://fragment.example.com#x", + "https://space.example.com other.example.com", + "https://semicolon.example.com; script-src *", + "example.com", + "//example.com", + "https://", + "*", + "https://*.example.com", + "'self'", + "javascript:alert(1)", + "https://still-good.example.com:8443" + ].join("\n")).should.eql("frame-ancestors 'self' https://good.example.com https://still-good.example.com:8443"); + }); + + it("should omit the directive when every entry is malformed", function() { + //a list of wildcards or typos must not collapse into a policy that + //blocks everything, so it has to behave exactly like an empty list + should.equal(common.buildFrameAncestorsPolicy("*"), null); + should.equal(common.buildFrameAncestorsPolicy("https://*.example.com\n*\nexample.com"), null); + should.equal(common.buildFrameAncestorsPolicy("'none'"), null); + }); + + it("should not repeat a duplicated origin", function() { + common.buildFrameAncestorsPolicy("https://example.com\nhttps://example.com/\n https://example.com ") + .should.eql("frame-ancestors 'self' https://example.com"); + }); + + it("should always allow the dashboard's own origin to frame the widget", function() { + //the dashboard previews these same routes in an iframe, and an + //allowed-origins list describes customer sites, not the Countly host + common.buildFrameAncestorsPolicy("https://example.com").should.containEql("'self'"); + common.buildFrameAncestorsPolicy("https://a.example.com\nhttps://b.example.com").should.containEql("'self'"); + }); + }); + + describe("setWidgetFrameHeaders (default: report-only)", function() { + var REPORT = "Content-Security-Policy-Report-Only"; + + beforeEach(function() { + setEnforce(false); + }); + + afterEach(restoreConfig); + + it("should report and never enforce while the operator has not opted in", function() { + //enforcing repurposes a list configured for CORS; a widget embedded on a + //host that is not in it must keep working until an operator opts in + var res = fakeRes({"X-Frame-Options": "deny"}); + common.setWidgetFrameHeaders(res, appWithOrigins("https://example.com")); + res.getHeader(REPORT).should.eql("frame-ancestors 'self' https://example.com"); + should.not.exist(res.getHeader("Content-Security-Policy")); + }); + + it("should omit the header when the app has no list configured", function() { + var res = fakeRes({"X-Frame-Options": "deny"}); + common.setWidgetFrameHeaders(res, appWithOrigins("")); + should.not.exist(res.getHeader(REPORT)); + should.not.exist(res.getHeader("Content-Security-Policy")); + }); + + it("should omit the header when the app has no plugins object at all", function() { + var res = fakeRes({"X-Frame-Options": "deny"}); + common.setWidgetFrameHeaders(res, {}); + should.not.exist(res.getHeader(REPORT)); + }); + + it("should omit the header when the app could not be resolved", function() { + var res = fakeRes({"X-Frame-Options": "deny"}); + common.setWidgetFrameHeaders(res, null); + should.not.exist(res.getHeader(REPORT)); + }); + + it("should report several configured origins", function() { + var res = fakeRes({"X-Frame-Options": "deny"}); + common.setWidgetFrameHeaders(res, appWithOrigins("https://example.com\nhttps://shop.example.com")); + res.getHeader(REPORT) + .should.eql("frame-ancestors 'self' https://example.com https://shop.example.com"); + }); + + it("should skip malformed entries when setting the header", function() { + var res = fakeRes({"X-Frame-Options": "deny"}); + common.setWidgetFrameHeaders(res, appWithOrigins("https://example.com\nnot-an-origin\nhttps://ok.example.com")); + res.getHeader(REPORT) + .should.eql("frame-ancestors 'self' https://example.com https://ok.example.com"); + }); + + it("should remove X-Frame-Options whether or not a policy is emitted", function() { + var withList = fakeRes({"X-Frame-Options": "deny"}); + common.setWidgetFrameHeaders(withList, appWithOrigins("https://example.com")); + should.not.exist(withList.getHeader("X-Frame-Options")); + + var withoutList = fakeRes({"X-Frame-Options": "deny"}); + common.setWidgetFrameHeaders(withoutList, appWithOrigins("")); + should.not.exist(withoutList.getHeader("X-Frame-Options")); + + var unresolved = fakeRes({"X-Frame-Options": "sameorigin"}); + common.setWidgetFrameHeaders(unresolved, null); + should.not.exist(unresolved.getHeader("X-Frame-Options")); + }); + + it("should merge into a report-only policy an operator already configured", function() { + var res = fakeRes({"Content-Security-Policy-Report-Only": "default-src 'self'"}); + common.setWidgetFrameHeaders(res, appWithOrigins("https://example.com")); + res.getHeader(REPORT) + .should.eql("default-src 'self'; frame-ancestors 'self' https://example.com"); + }); + + it("should leave an enforced policy untouched while in report-only mode", function() { + //the operator's own enforced CSP is not ours to edit from here + var res = fakeRes({"Content-Security-Policy": "default-src 'self'"}); + common.setWidgetFrameHeaders(res, appWithOrigins("https://example.com")); + res.getHeader("Content-Security-Policy").should.eql("default-src 'self'"); + res.getHeader(REPORT).should.eql("frame-ancestors 'self' https://example.com"); + }); + + it("should not throw on a response object that cannot carry headers", function() { + should.doesNotThrow(function() { + common.setWidgetFrameHeaders(null, appWithOrigins("https://example.com")); + common.setWidgetFrameHeaders({}, appWithOrigins("https://example.com")); + }); + }); + }); + + describe("setWidgetFrameHeaders (operator opted in to enforcing)", function() { + beforeEach(function() { + setEnforce(true); + }); + + afterEach(restoreConfig); + + it("should enforce the policy and emit no report-only header", function() { + var res = fakeRes({"X-Frame-Options": "deny"}); + common.setWidgetFrameHeaders(res, appWithOrigins("https://example.com")); + res.getHeader("Content-Security-Policy").should.eql("frame-ancestors 'self' https://example.com"); + should.not.exist(res.getHeader("Content-Security-Policy-Report-Only")); + }); + + it("should still omit the header entirely when nothing usable is configured", function() { + var res = fakeRes({"X-Frame-Options": "deny"}); + common.setWidgetFrameHeaders(res, appWithOrigins("*\nhttps://*.example.com")); + should.not.exist(res.getHeader("Content-Security-Policy")); + should.not.exist(res.getHeader("Content-Security-Policy-Report-Only")); + }); + + it("should still remove X-Frame-Options", function() { + var res = fakeRes({"X-Frame-Options": "deny"}); + common.setWidgetFrameHeaders(res, appWithOrigins("https://example.com")); + should.not.exist(res.getHeader("X-Frame-Options")); + }); + + it("should merge into an enforced policy an operator already configured", function() { + var res = fakeRes({"Content-Security-Policy": "default-src 'self'"}); + common.setWidgetFrameHeaders(res, appWithOrigins("https://example.com")); + res.getHeader("Content-Security-Policy") + .should.eql("default-src 'self'; frame-ancestors 'self' https://example.com"); + }); + + it("should not duplicate frame-ancestors when the existing policy already scopes framing", function() { + var res = fakeRes({"Content-Security-Policy": "frame-ancestors https://operator.example.com"}); + common.setWidgetFrameHeaders(res, appWithOrigins("https://example.com")); + res.getHeader("Content-Security-Policy") + .should.eql("frame-ancestors https://operator.example.com"); + }); + }); + + describe("getAppForWidget", function() { + /** + * Stub of the countly db object exposing a single apps collection. + * @param {Object} appDoc - document the apps collection returns + * @param {Object} findErr - error the apps collection returns + * @returns {Object} fake db object plus a record of the queries made + */ + function fakeDb(appDoc, findErr) { + var queries = []; + return { + queries: queries, + ObjectID: function(id) { + return {oid: id}; + }, + collection: function(name) { + return { + findOne: function(query, options, callback) { + queries.push({name: name, query: query, options: options}); + callback(findErr || null, appDoc); + } + }; + } + }; + } + + it("should resolve the app and only read the origin list", function(done) { + var db = fakeDb(appWithOrigins("https://example.com")); + common.getAppForWidget(db, "5f8e1a2b3c4d5e6f7a8b9c0d", function(app) { + app.plugins.allow_access_control_origin.should.eql("https://example.com"); + db.queries.length.should.eql(1); + db.queries[0].name.should.eql("apps"); + db.queries[0].options.projection.should.eql({"plugins.allow_access_control_origin": 1}); + done(); + }); + }); + + it("should not query the database for an app id that is not an object id", function(done) { + var db = fakeDb(appWithOrigins("https://example.com")); + common.getAppForWidget(db, "../../etc/passwd", function(app) { + should.equal(app, null); + db.queries.length.should.eql(0); + done(); + }); + }); + + it("should resolve to null for a missing app id or db", function(done) { + common.getAppForWidget(fakeDb(null), undefined, function(app) { + should.equal(app, null); + common.getAppForWidget(null, "5f8e1a2b3c4d5e6f7a8b9c0d", function(app2) { + should.equal(app2, null); + done(); + }); + }); + }); + + it("should resolve to null when the lookup errors or finds nothing", function(done) { + common.getAppForWidget(fakeDb(null, new Error("boom")), "5f8e1a2b3c4d5e6f7a8b9c0d", function(app) { + should.equal(app, null); + common.getAppForWidget(fakeDb(null), "5f8e1a2b3c4d5e6f7a8b9c0d", function(app2) { + should.equal(app2, null); + done(); + }); + }); + }); + }); +}); diff --git a/test/unit-tests/upgrade.standardize-security-headers.js b/test/unit-tests/upgrade.standardize-security-headers.js new file mode 100644 index 00000000000..5c231169cd2 --- /dev/null +++ b/test/unit-tests/upgrade.standardize-security-headers.js @@ -0,0 +1,111 @@ +require("should"); +var path = require("path"); +var fs = require("fs"); +var vm = require("vm"); + +// The upgrade script edits configuration an operator may have customised, so the two +// things that matter are that it removes exactly what it should and preserves everything +// else. Loading it normally would open a database connection, so only the pure rewrite +// function is lifted out. +describe("upgrade: standardize security headers", function() { + var rewrite; + + before(function() { + var file = path.join(__dirname, "../../bin/upgrade/DEV/scripts/standardize_security_headers.js"); + var src = fs.readFileSync(file, "utf8"); + // drop the db-connecting tail, keep the declarations and rewrite() + var cut = src.indexOf("pluginManager.dbConnection()"); + (cut > -1).should.equal(true, "script shape changed; update this test"); + var sandbox = { + module: {exports: {}}, + console: {log: function() {}, error: function() {}}, + require: function() { + return {}; + } + }; + sandbox.exports = sandbox.module.exports; + vm.createContext(sandbox); + vm.runInContext(src.slice(0, cut) + "\nmodule.exports = {rewrite: rewrite};", sandbox); + rewrite = sandbox.module.exports.rewrite; + }); + + describe("what it removes", function() { + it("drops X-XSS-Protection", function() { + var out = rewrite("X-Frame-Options:deny\nX-XSS-Protection:1; mode=block\nX-Content-Type-Options: nosniff"); + out.should.not.match(/X-XSS-Protection/i); + out.should.match(/X-Frame-Options:deny/); + }); + + it("drops only the preload token from HSTS, keeping the rest", function() { + var out = rewrite("Strict-Transport-Security:max-age=31536000; includeSubDomains; preload"); + out.should.match(/max-age=31536000/); + out.should.match(/includeSubDomains/); + out.should.not.match(/preload/); + }); + + it("leaves an HSTS line that never had preload untouched", function() { + var out = rewrite("Strict-Transport-Security:max-age=600; includeSubDomains\nX-Content-Type-Options: nosniff\nReferrer-Policy: no-referrer\nPermissions-Policy: camera=()"); + (out === null).should.equal(true); + }); + }); + + describe("what it adds", function() { + it("appends the three missing headers", function() { + var out = rewrite("X-Frame-Options:deny"); + out.should.match(/X-Content-Type-Options: nosniff/); + out.should.match(/Referrer-Policy: strict-origin-when-cross-origin/); + out.should.match(/Permissions-Policy: camera=\(\)/); + }); + + it("does not duplicate a header the operator already set, whatever its value", function() { + // their choice of value wins; we only ensure the header is present + var out = rewrite("Referrer-Policy: no-referrer\nX-Content-Type-Options: nosniff\nPermissions-Policy: geolocation=()"); + (out === null).should.equal(true); + }); + + it("matches header names case-insensitively when deciding what is missing", function() { + var out = rewrite("referrer-policy: no-referrer\nx-content-type-options: nosniff\npermissions-policy: camera=()"); + (out === null).should.equal(true); + }); + }); + + describe("what it preserves", function() { + it("keeps headers it knows nothing about", function() { + var out = rewrite("X-XSS-Protection:1; mode=block\nX-Custom-Operator-Header: keep-me\nAccess-Control-Allow-Origin:*"); + out.should.match(/X-Custom-Operator-Header: keep-me/); + out.should.match(/Access-Control-Allow-Origin:\*/); + out.should.not.match(/X-XSS-Protection/i); + }); + + it("keeps the order of what was already there", function() { + var out = rewrite("A-One: 1\nX-XSS-Protection:1\nB-Two: 2"); + out.indexOf("A-One").should.be.below(out.indexOf("B-Two")); + }); + + it("is idempotent", function() { + var once = rewrite("X-Frame-Options:deny\nX-XSS-Protection:1; mode=block\nStrict-Transport-Security:max-age=31536000; includeSubDomains; preload"); + (rewrite(once) === null).should.equal(true); + }); + + it("handles CRLF input without leaving stray carriage returns", function() { + var out = rewrite("X-Frame-Options:deny\r\nX-XSS-Protection:1\r\n"); + out.indexOf("\r").should.equal(-1); + out.should.not.match(/X-XSS-Protection/i); + }); + + it("drops blank lines rather than emitting empty header entries", function() { + var out = rewrite("X-Frame-Options:deny\n\n\nX-XSS-Protection:1"); + out.split("\n").every(function(l) { + return l.trim().length > 0; + }).should.equal(true); + }); + }); + + describe("inputs that are not strings", function() { + it("returns null rather than throwing", function() { + (rewrite(undefined) === null).should.equal(true); + (rewrite(null) === null).should.equal(true); + (rewrite(42) === null).should.equal(true); + }); + }); +});