revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-26 07:20:57 +01:00
Code Issues Releases Wiki Activity

Emit a "Content-Security-Policy" HTTP header

Browse source 
Summary:
See PHI399. Ref T4340. This header provides an additional layer of protection against various attacks, including XSS attacks which embed inline `<script ...>` or `onhover="..."` content into the document.

**style-src**: The "unsafe-inline" directive affects both `style="..."` and `<style>`. We use a lot of `style="..."`, some very legitimately, so we can't realistically get away from this any time soon. We only use one `<style>` (for monospaced font preferences) but can't disable `<style>` without disabling `style="..."`.

**img-src**: We use "data:" URIs to inline small images into CSS, and there's a significant performance benefit from doing this. There doesn't seem to be a way to allow "data" URIs in CSS without allowing them in the document itself.

**script-src** and **frame-src**: For a small number of flows (Recaptcha, Stripe) we embed external javascript, some of which embeds child elements (or additional resources) into the document. We now whitelist these narrowly on the respective pages.

This won't work with Quicksand, so I've blacklisted it for now.

**connect-src**: We need to include `'self'` for AJAX to work, and any websocket URIs.

**Clickjacking**: We now have three layers of protection:

  - X-Frame-Options: works in older browsers.
  - `frame-ancestors 'none'`: does the same thing.
  - Explicit framebust in JX.Stratcom after initialization: works in ancient IE.

We could probably drop the explicit framebust but it wasn't difficult to retain.

**script tags**: We previously used an inline `<script>` tag to start Javelin. I've moved this to `<data data-javelin-init ...>` tags, which seems to work properly.

**`__DEV__`**: We previously used an inline `<script>` tag to set the `__DEV__` mode flag. I tried using the "initialization" tags for this, but they fire too late. I moved it to `<html data-developer-mode="1">`, which seems OK everywhere.

**CSP Scope**: Only the CSP header on the original request appears to matter -- you can't refine the scope by emitting headers on CSS/JS. To reduce confusion, I disabled the headers on those response types. More headers could be disabled, although we're likely already deep in the land of diminishing returns.

**Initialization**: The initialization sequence has changed slightly. Previously, we waited for the <script> in bottom of the document to evaluate. Now, we go fishing for tags when domcontentready fires.

Test Plan:
  - Browsed around in Firefox, Safari and Chrome looking for console warnings. Interacted with various Javascript behaviors. Enabled Quicksand.
  - Disabled all the framebusting, launched a clickjacking attack, verified that each layer of protection is individually effective.
  - Verified that the XHProf iframe in Darkconsole and the PHPAST frame layout work properly.
  - Enabled notifications, verified no complaints about connecting to Aphlict.
  - Hit `__DEV__` mode warnings based on the new data attribute.
  - Tried to do sketchy stuff with `data:` URIs and SVGs. This works but doesn't seem to be able to do anything dangerous.
  - Went through the Stripe and Recaptcha workflows.
  - Dumped and examined the CSP headers with `curl`, etc.
  - Added a raw <script> tag to a page (as though I'd found an XSS attack), verified it was no longer executed.

Maniphest Tasks: T4340

Differential Revision: https://secure.phabricator.com/D19143
This commit is contained in:
epriestley 2018-02-27 06:56:15 -08:00
parent f450c6c55b
commit dba4c4bdf6
11 changed files with 322 additions and 70 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
resources/celerity/map.php
View file

@ -10,7 +10,7 @@ return array(
'conpherence.pkg.css' => 'e68cf1fa',
'conpherence.pkg.js' => '15191c65',
'core.pkg.css' => '2fa91e14',
'core.pkg.js' => 'dc13d4b7',
'core.pkg.js' => '7aa5bd92',
'darkconsole.pkg.js' => '1f9a31bc',
'differential.pkg.css' => '113e692c',
'differential.pkg.js' => 'f6d809c0',
@ -211,12 +211,12 @@ return array(
'rsrc/externals/font/lato/lato-regular.woff' => '13d39fe2',
'rsrc/externals/font/lato/lato-regular.woff2' => '57a9f742',
'rsrc/externals/javelin/core/Event.js' => '2ee659ce',
'rsrc/externals/javelin/core/Stratcom.js' => '6ad39b6f',
'rsrc/externals/javelin/core/Stratcom.js' => '327f418a',
'rsrc/externals/javelin/core/__tests__/event-stop-and-kill.js' => '717554e4',
'rsrc/externals/javelin/core/__tests__/install.js' => 'c432ee85',
'rsrc/externals/javelin/core/__tests__/stratcom.js' => '88bf7313',
'rsrc/externals/javelin/core/__tests__/util.js' => 'e251703d',
'rsrc/externals/javelin/core/init.js' => '3010e992',
'rsrc/externals/javelin/core/init.js' => '638a4e2b',
'rsrc/externals/javelin/core/init_node.js' => 'c234aded',
'rsrc/externals/javelin/core/install.js' => '05270951',
'rsrc/externals/javelin/core/util.js' => '93cc50d6',
@ -722,7 +722,7 @@ return array(
'javelin-install' => '05270951',
'javelin-json' => '69adf288',
'javelin-leader' => '7f243deb',
'javelin-magical-init' => '3010e992',
'javelin-magical-init' => '638a4e2b',
'javelin-mask' => '8a41885b',
'javelin-quicksand' => '6b8ef10b',
'javelin-reactor' => '2b8de964',
@ -735,7 +735,7 @@ return array(
'javelin-router' => '29274e2b',
'javelin-scrollbar' => '9065f639',
'javelin-sound' => '949c0fe5',
'javelin-stratcom' => '6ad39b6f',
'javelin-stratcom' => '327f418a',
'javelin-tokenizer' => '8d3bc1b2',
'javelin-typeahead' => '70baed2f',
'javelin-typeahead-composite-source' => '503e17fd',
@ -1131,6 +1131,12 @@ return array(
'javelin-dom',
'javelin-workflow',
),
'327f418a' => array(
'javelin-install',
'javelin-event',
'javelin-util',
'javelin-magical-init',
),
'358b8c04' => array(
'javelin-install',
'javelin-util',
@ -1446,12 +1452,6 @@ return array(
'69adf288' => array(
'javelin-install',
),
'6ad39b6f' => array(
'javelin-install',
'javelin-event',
'javelin-util',
'javelin-magical-init',
),
'6b8ef10b' => array(
'javelin-install',
),

127
src/aphront/response/AphrontResponse.php
View file

@ -7,9 +7,11 @@ abstract class AphrontResponse extends Phobject {
private $canCDN;
private $responseCode = 200;
private $lastModified = null;
private $contentSecurityPolicyURIs;
private $disableContentSecurityPolicy;
protected $frameable;
public function setRequest($request) {
$this->request = $request;
return $this;
@ -19,6 +21,32 @@ abstract class AphrontResponse extends Phobject {
return $this->request;
}
final public function addContentSecurityPolicyURI($kind, $uri) {
if ($this->contentSecurityPolicyURIs === null) {
$this->contentSecurityPolicyURIs = array(
'script' => array(),
'connect' => array(),
'frame' => array(),
);
}
if (!isset($this->contentSecurityPolicyURIs[$kind])) {
throw new Exception(
pht(
'Unknown Content-Security-Policy URI kind "%s".',
$kind));
}
$this->contentSecurityPolicyURIs[$kind][] = (string)$uri;
return $this;
}
final public function setDisableContentSecurityPolicy($disable) {
$this->disableContentSecurityPolicy = $disable;
return $this;
}
/* -( Content )------------------------------------------------------------ */
@ -59,9 +87,106 @@ abstract class AphrontResponse extends Phobject {
);
}
$csp = $this->newContentSecurityPolicyHeader();
if ($csp !== null) {
$headers[] = array('Content-Security-Policy', $csp);
}
return $headers;
}
private function newContentSecurityPolicyHeader() {
if ($this->disableContentSecurityPolicy) {
return null;
}
$csp = array();
$cdn = PhabricatorEnv::getEnvConfig('security.alternate-file-domain');
if ($cdn) {
$default = $this->newContentSecurityPolicySource($cdn);
} else {
$default = "'self'";
}
$csp[] = "default-src {$default}";
// We use "data:" URIs to inline small images into CSS. This policy allows
// "data:" URIs to be used anywhere, but there doesn't appear to be a way
// to say that "data:" URIs are okay in CSS files but not in the document.
$csp[] = "img-src {$default} data:";
// We use inline style="..." attributes in various places, many of which
// are legitimate. We also currently use a <style> tag to implement the
// "Monospaced Font Preference" setting.
$csp[] = "style-src {$default} 'unsafe-inline'";
// On a small number of pages, including the Stripe workflow and the
// ReCAPTCHA challenge, we embed external Javascript directly.
$csp[] = $this->newContentSecurityPolicy('script', $default);
// We need to specify that we can connect to ourself in order for AJAX
// requests to work.
$csp[] = $this->newContentSecurityPolicy('connect', "'self'");
// DarkConsole and PHPAST both use frames to render some content.
$csp[] = $this->newContentSecurityPolicy('frame', "'self'");
// This is a more modern flavor of of "X-Frame-Options" and prevents
// clickjacking attacks where the page is included in a tiny iframe and
// the user is convinced to click a element on the page, which really
// clicks a dangerous button hidden under a picture of a cat.
if ($this->frameable) {
$csp[] = "frame-ancestors 'self'";
} else {
$csp[] = "frame-ancestors 'none'";
}
$csp = implode('; ', $csp);
return $csp;
}
private function newContentSecurityPolicy($type, $defaults) {
if ($defaults === null) {
$sources = array();
} else {
$sources = (array)$defaults;
}
$uris = $this->contentSecurityPolicyURIs;
if (isset($uris[$type])) {
foreach ($uris[$type] as $uri) {
$sources[] = $this->newContentSecurityPolicySource($uri);
}
}
$sources = array_unique($sources);
return "{$type}-src ".implode(' ', $sources);
}
private function newContentSecurityPolicySource($uri) {
// Some CSP URIs are ultimately user controlled (like notification server
// URIs and CDN URIs) so attempt to stop an attacker from injecting an
// unsafe source (like 'unsafe-eval') into the CSP header.
$uri = id(new PhutilURI($uri))
->setPath(null)
->setFragment(null)
->setQueryParams(array());
$uri = (string)$uri;
if (preg_match('/[ ;\']/', $uri)) {
throw new Exception(
pht(
'Attempting to emit a response with an unsafe source ("%s") in the '.
'Content-Security-Policy header.',
$uri));
}
return $uri;
}
public function setCacheDurationInSeconds($duration) {
$this->cacheable = $duration;
return $this;

83
src/applications/celerity/CelerityStaticResourceResponse.php
View file

@ -17,6 +17,7 @@ final class CelerityStaticResourceResponse extends Phobject {
private $behaviors = array();
private $hasRendered = array();
private $postprocessorKey;
private $contentSecurityPolicyURIs = array();
public function __construct() {
if (isset($_REQUEST['__metablock__'])) {
@ -37,6 +38,15 @@ final class CelerityStaticResourceResponse extends Phobject {
return $this->metadataBlock.'_'.$id;
}
public function addContentSecurityPolicyURI($kind, $uri) {
$this->contentSecurityPolicyURIs[$kind][] = $uri;
return $this;
}
public function getContentSecurityPolicyURIMap() {
return $this->contentSecurityPolicyURIs;
}
public function getMetadataBlock() {
return $this->metadataBlock;
}
@ -196,23 +206,16 @@ final class CelerityStaticResourceResponse extends Phobject {
$type));
}
public function renderHTMLFooter() {
public function renderHTMLFooter($is_frameable) {
$this->metadataLocked = true;
$data = array();
if ($this->metadata) {
$json_metadata = AphrontResponse::encodeJSONForHTTPResponse(
$this->metadata);
$this->metadata = array();
} else {
$json_metadata = '{}';
}
// Even if there is no metadata on the page, Javelin uses the mergeData()
// call to start dispatching the event queue.
$data[] = 'JX.Stratcom.mergeData('.$this->metadataBlock.', '.
$json_metadata.');';
$merge_data = array(
'block' => $this->metadataBlock,
'data' => $this->metadata,
);
$this->metadata = array();
$onload = array();
$behavior_lists = array();
if ($this->behaviors) {
$behaviors = $this->behaviors;
$this->behaviors = array();
@ -241,24 +244,52 @@ final class CelerityStaticResourceResponse extends Phobject {
if (!$group) {
continue;
}
$group_json = AphrontResponse::encodeJSONForHTTPResponse(
$group);
$onload[] = 'JX.initBehaviors('.$group_json.')';
$behavior_lists[] = $group;
}
}
if ($onload) {
foreach ($onload as $func) {
$data[] = 'JX.onload(function(){'.$func.'});';
}
$initializers = array();
// Even if there is no metadata on the page, Javelin uses the mergeData()
// call to start dispatching the event queue, so we always want to include
// this initializer.
$initializers[] = array(
'kind' => 'merge',
'data' => $merge_data,
);
foreach ($behavior_lists as $behavior_list) {
$initializers[] = array(
'kind' => 'behaviors',
'data' => $behavior_list,
);
}
if ($data) {
$data = implode("\n", $data);
return self::renderInlineScript($data);
} else {
return '';
if ($is_frameable) {
$initializers[] = array(
'data' => 'frameable',
'kind' => (bool)$is_frameable,
);
}
$tags = array();
foreach ($initializers as $initializer) {
$data = $initializer['data'];
if (is_array($data)) {
$json_data = AphrontResponse::encodeJSONForHTTPResponse($data);
} else {
$json_data = json_encode($data);
}
$tags[] = phutil_tag(
'data',
array(
'data-javelin-init-kind' => $initializer['kind'],
'data-javelin-init-data' => $json_data,
));
}
return $tags;
}
public static function renderInlineScript($data) {

5
src/applications/celerity/controller/CelerityResourceController.php
View file

@ -106,6 +106,11 @@ abstract class CelerityResourceController extends PhabricatorController {
$response = id(new AphrontFileResponse())
->setMimeType($type_map[$type]);
// The "Content-Security-Policy" header has no effect on the actual
// resources, only on the main request. Disable it on the resource
// responses to limit confusion.
$response->setDisableContentSecurityPolicy(true);
$range = AphrontRequest::getHTTPHeader('Range');
if (strlen($range)) {

8
src/applications/phortune/provider/PhortuneStripePaymentProvider.php
View file

@ -275,12 +275,18 @@ final class PhortuneStripePaymentProvider extends PhortunePaymentProvider {
AphrontRequest $request,
array $errors) {
$src = 'https://js.stripe.com/v2/';
$ccform = id(new PhortuneCreditCardForm())
->setSecurityAssurance(
pht('Payments are processed securely by Stripe.'))
->setUser($request->getUser())
->setErrors($errors)
->addScript('https://js.stripe.com/v2/');
->addScript($src);
CelerityAPI::getStaticResourceResponse()
->addContentSecurityPolicyURI('script', $src)
->addContentSecurityPolicyURI('frame', $src);
Javelin::initBehavior(
'stripe-payment-form',

26
src/view/form/control/AphrontFormRecaptchaControl.php
View file

@ -42,16 +42,24 @@ final class AphrontFormRecaptchaControl extends AphrontFormControl {
$js = 'https://www.google.com/recaptcha/api.js';
$pubkey = PhabricatorEnv::getEnvConfig('recaptcha.public-key');
return array(
phutil_tag('div', array(
'class' => 'g-recaptcha',
'data-sitekey' => $pubkey,
)),
CelerityAPI::getStaticResourceResponse()
->addContentSecurityPolicyURI('script', $js)
->addContentSecurityPolicyURI('script', 'https://www.gstatic.com/')
->addContentSecurityPolicyURI('frame', 'https://www.google.com/');
phutil_tag('script', array(
'type' => 'text/javascript',
'src' => $js,
)),
return array(
phutil_tag(
'div',
array(
'class' => 'g-recaptcha',
'data-sitekey' => $pubkey,
)),
phutil_tag(
'script',
array(
'type' => 'text/javascript',
'src' => $js,
)),
);
}
}

9
src/view/page/AphrontPageView.php
View file

@ -59,9 +59,15 @@ abstract class AphrontPageView extends AphrontView {
),
array($body, $tail));
if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) {
$data_fragment = phutil_safe_html(' data-developer-mode="1"');
} else {
$data_fragment = null;
}
$response = hsprintf(
'<!DOCTYPE html>'.
'<html>'.
'<html%s>'.
'<head>'.
'<meta charset="UTF-8" />'.
'<title>%s</title>'.
@ -69,6 +75,7 @@ abstract class AphrontPageView extends AphrontView {
'</head>'.
'%s'.
'</html>',
$data_fragment,
$title,
$head,
$body);

10
src/view/page/PhabricatorBarePageView.php
View file

@ -59,11 +59,6 @@ class PhabricatorBarePageView extends AphrontPageView {
}
protected function getHead() {
$framebust = null;
if (!$this->getFrameable()) {
$framebust = '(top == self) || top.location.replace(self.location.href);';
}
$viewport_tag = null;
if ($this->getDeviceReady()) {
$viewport_tag = phutil_tag(
@ -140,9 +135,8 @@ class PhabricatorBarePageView extends AphrontPageView {
}
}
$developer = PhabricatorEnv::getEnvConfig('phabricator.developer-mode');
return hsprintf(
'%s%s%s%s%s%s%s%s%s',
'%s%s%s%s%s%s%s%s',
$viewport_tag,
$mask_icon,
$icon_tag_76,
@ -150,8 +144,6 @@ class PhabricatorBarePageView extends AphrontPageView {
$icon_tag_152,
$favicon_tag,
$referrer_tag,
CelerityStaticResourceResponse::renderInlineScript(
$framebust.jsprintf('window.__DEV__=%d;', ($developer ? 1 : 0))),
$response->renderResourcesOfType('css'));
}

28
src/view/page/PhabricatorStandardPageView.php
View file

@ -608,12 +608,15 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView
Javelin::initBehavior(
'aphlict-listen',
array(
'websocketURI' => (string)$client_uri,
'websocketURI' => (string)$client_uri,
) + $this->buildAphlictListenConfigData());
CelerityAPI::getStaticResourceResponse()
->addContentSecurityPolicyURI('connect', $client_uri);
}
}
$tail[] = $response->renderHTMLFooter();
$tail[] = $response->renderHTMLFooter($this->getFrameable());
return $tail;
}
@ -860,6 +863,19 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView
$blacklist[] = $application->getQuicksandURIPatternBlacklist();
}
// See T4340. Currently, Phortune and Auth both require pulling in external
// Javascript (for Stripe card management and Recaptcha, respectively).
// This can put us in a position where the user loads a page with a
// restrictive Content-Security-Policy, then uses Quicksand to navigate to
// a page which needs to load external scripts. For now, just blacklist
// these entire applications since we aren't giving up anything
// significant by doing so.
$blacklist[] = array(
'/phortune/.*',
'/auth/.*',
);
return array_mergev($blacklist);
}
@ -903,9 +919,17 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView
->setContent($content);
} else {
$content = $this->render();
$response = id(new AphrontWebpageResponse())
->setContent($content)
->setFrameable($this->getFrameable());
$static = CelerityAPI::getStaticResourceResponse();
foreach ($static->getContentSecurityPolicyURIMap() as $kind => $uris) {
foreach ($uris as $uri) {
$response->addContentSecurityPolicyURI($kind, $uri);
}
}
}
return $response;

31
webroot/rsrc/externals/javelin/core/Stratcom.js vendored
View file

@ -517,6 +517,36 @@ JX.install('Stratcom', {
return len ? this._execContext[len - 1].event : null;
},
initialize: function(initializers) {
var frameable = false;
for (var ii = 0; ii < initializers.length; ii++) {
var kind = initializers[ii].kind;
var data = initializers[ii].data;
switch (kind) {
case 'behaviors':
JX.initBehaviors(data);
break;
case 'merge':
JX.Stratcom.mergeData(data.block, data.data);
JX.Stratcom.ready = true;
break;
case 'frameable':
frameable = !!data;
break;
}
}
// If the initializer tags did not explicitly allow framing, framebust.
// This protects us from clickjacking attacks on older versions of IE.
// The "X-Frame-Options" and "Content-Security-Policy" headers provide
// more modern variations of this protection.
if (!frameable) {
if (window.top != window.self) {
window.top.location.replace(window.self.location.href);
}
}
},
/**
* Merge metadata. You must call this (even if you have no metadata) to
@ -542,7 +572,6 @@ JX.install('Stratcom', {
} else {
this._data[block] = data;
if (block === 0) {
JX.Stratcom.ready = true;
JX.flushHoldingQueue('install-init', function(fn) {
fn();
});

43
webroot/rsrc/externals/javelin/core/init.js vendored
View file

@ -46,25 +46,51 @@
makeHoldingQueue('behavior');
makeHoldingQueue('install-init');
window.__DEV__ = window.__DEV__ || 0;
var loaded = false;
var onload = [];
var master_event_queue = [];
var root = document.documentElement;
var has_add_event_listener = !!root.addEventListener;
window.__DEV__ = !!root.getAttribute('data-developer-mode');
JX.__rawEventQueue = function(what) {
master_event_queue.push(what);
// Evade static analysis - JX.Stratcom
var ii;
var Stratcom = JX['Stratcom'];
if (Stratcom && Stratcom.ready) {
// Empty the queue now so that exceptions don't cause us to repeatedly
// try to handle events.
if (!loaded && what.type == 'domready') {
var initializers = [];
var tags = JX.DOM.scry(document.body, 'data');
for (ii = 0; ii < tags.length; ii++) {
// Ignore tags which are not immediate children of the document
// body. If an attacker somehow injects arbitrary tags into the
// content of the document, that should not give them access to
// modify initialization behaviors.
if (tags[ii].parentNode !== document.body) {
continue;
}
var tag_kind = tags[ii].getAttribute('data-javelin-init-kind');
var tag_data = tags[ii].getAttribute('data-javelin-init-data');
tag_data = JX.JSON.parse(tag_data);
initializers.push({kind: tag_kind, data: tag_data});
}
Stratcom.initialize(initializers);
loaded = true;
}
if (loaded) {
// Empty the queue now so that exceptions don't cause us to repeatedly
// try to handle events.
var local_queue = master_event_queue;
master_event_queue = [];
for (var ii = 0; ii < local_queue.length; ++ii) {
for (ii = 0; ii < local_queue.length; ++ii) {
var evt = local_queue[ii];
// Sometimes IE gives us events which throw when ".type" is accessed;
@ -72,11 +98,10 @@
// figure out where these are coming from.
try { var test = evt.type; } catch (x) { continue; }
if (!loaded && evt.type == 'domready') {
if (evt.type == 'domready') {
// NOTE: Firefox interprets "document.body.id = null" as the string
// literal "null".
document.body && (document.body.id = '');
loaded = true;
for (var jj = 0; jj < onload.length; jj++) {
onload[jj]();
}