feat(server): Remove unsafe-eval and reportOnly CSP rules! (#4595) r=vladikoff,jrgm

`unsafe-eval` was added to allow Selenium tests to run. With geckodriver, unsafe-eval
is no longer needed! This removes support for unsafe-eval, and only hooks up the reportOnly
CSP middleware if there are any declarations.

fixes #4594

Author: Shane Tomlinson

server/bin/fxa-content-server.js

@@ -87,7 +87,11 @@ function makeApp() {
     app.use(csp({ rules: cspRulesBlocking }));
   }
   if (config.get('csp.reportOnlyEnabled')) {
-    app.use(csp({ rules: cspRulesReportOnly }));
+    // There has to be more than a `reportUri`
+    // to enable reportOnly CSP.
+    if (Object.keys(cspRulesReportOnly.directives).length > 1) {
+      app.use(csp({ rules: cspRulesReportOnly }));
+    }
   }
   if (config.get('hpkp.enabled')) {
     app.use(hpkp(config));

server/lib/csp/blocking.js

@@ -37,10 +37,8 @@ module.exports = function (config) {
   // The sha of the embedded <style> tag in default-profile.svg.
   const EMBEDDED_STYLE_SHA = "'sha256-9n6ek6ecEYlqel7uDyKLy6fdGNo3vw/uScXSq9ooQlk='";
   // keyword sources - https://www.w3.org/TR/CSP2/#keyword_source
-  // Note: "'unsafe-inline'" is not yet used in this module.
+  // Note: "'unsafe-inline'" and "'unsafe-eval'" are not used in this module.
   const SELF = "'self'";
-  const UNSAFE_EVAL = "'unsafe-eval'";
-
 
   function addCdnRuleIfRequired(target) {
     if (CDN_URL !== PUBLIC_URL) {
@@ -75,11 +73,7 @@ module.exports = function (config) {
       objectSrc: [NONE],
       reportUri: config.get('csp.reportUri'),
       scriptSrc: addCdnRuleIfRequired([
-        SELF,
-        // allow unsafe-eval for functional tests. A report-only middleware
-        // is also added that does not allow 'unsafe-eval' so that we can see
-        // if other scripts are being added.
-        UNSAFE_EVAL
+        SELF
       ]),
       styleSrc: addCdnRuleIfRequired([
         SELF,
@@ -101,8 +95,7 @@ module.exports = function (config) {
       PROFILE_IMAGES_SERVER,
       PROFILE_SERVER,
       PUBLIC_URL,
-      SELF,
-      UNSAFE_EVAL
+      SELF
     }
   };
 

server/lib/csp/report-only.js

@@ -5,28 +5,16 @@
 
 /**
  * reportOnlyCspMiddleware is where to declare experimental rules that
- * will not cause a resource to be blocked if it runs afowl of a rule, but will
- * cause the resource to be reported.
+ * will not cause a resource to be blocked if it runs afowl of a rule, but
+ * will cause the resource to be reported.
+ *
+ * If no directives other than `reportUri` are declared, the CSP reportOnly
+ * middleware will not be added.
  */
 module.exports = function (config) {
-  var CDN_URL = config.get('static_resource_url');
-  var PUBLIC_URL = config.get('public_url');
-  var SELF = "'self'";
-
-  function addCdnRuleIfRequired(target) {
-    if (CDN_URL !== PUBLIC_URL) {
-      target.push(CDN_URL);
-    }
-
-    return target;
-  }
-
   return {
     directives: {
-      reportUri: config.get('csp.reportOnlyUri'),
-      scriptSrc: addCdnRuleIfRequired([
-        SELF
-      ])
+      reportUri: config.get('csp.reportOnlyUri')
     },
     reportOnly: true
   };

tests/server/csp.js

@@ -77,9 +77,8 @@ define([
     assert.include(objectSrc, Sources.NONE);
 
     const scriptSrc = directives.scriptSrc;
-    assert.lengthOf(scriptSrc, 3);
+    assert.lengthOf(scriptSrc, 2);
     assert.include(scriptSrc, Sources.SELF);
-    assert.include(scriptSrc, Sources.UNSAFE_EVAL);
     assert.include(scriptSrc, CDN_SERVER);
 
     const styleSrc = directives.styleSrc;
@@ -91,4 +90,3 @@ define([
 
   registerSuite(suite);
 });
-