bug 1332829: Improve some security related headers on public app (#228)…

…. r=jlorenzo,april

auslib/test/web/test_client.py

@@ -906,31 +906,90 @@ def testCacheControlIsNotSetFor404(self):
         ret = self.client.get('/whizzybang')
         self.assertEqual(ret.headers.get("Cache-Control"), None)
 
+    def testContentSecurityPolicyIsSet(self):
+        ret = self.client.get('/update/3/c/15.0/1/p/l/a/a/default/a/update.xml')
+        self.assertEqual(ret.headers.get("Content-Security-Policy"), "default-src 'none'; frame-ancestors 'none'")
+
+    def testContentSecurityPolicyIsSetFor404(self):
+        ret = self.client.get('/whizzybang')
+        self.assertEqual(ret.headers.get("Content-Security-Policy"), "default-src 'none'; frame-ancestors 'none'")
+
+    def testContentSecurityPolicyIsSetFor400(self):
+        with mock.patch('auslib.web.views.client.ClientRequestView.get') as m:
+            m.side_effect = BadDataError('I break!')
+            ret = self.client.get('/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml')
+            self.assertEqual(ret.headers.get("Content-Security-Policy"), "default-src 'none'; frame-ancestors 'none'")
+
+    def testContentSecurityPolicyIsSetFor500(self):
+        with mock.patch('auslib.web.views.client.ClientRequestView.get') as m:
+            m.side_effect = Exception('I break!')
+            ret = self.client.get('/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml')
+            self.assertEqual(ret.headers.get("Content-Security-Policy"), "default-src 'none'; frame-ancestors 'none'")
+
+    def testXContentTypeOptionsIsSet(self):
+        ret = self.client.get('/update/3/c/15.0/1/p/l/a/a/default/a/update.xml')
+        self.assertEqual(ret.headers.get("X-Content-Type-Options"), "nosniff")
+
+    def testXContentTypeOptionsIsSetFor404(self):
+        ret = self.client.get('/whizzybang')
+        self.assertEqual(ret.headers.get("X-Content-Type-Options"), "nosniff")
+
+    def testXContentTypeOptionsIsSetFor400(self):
+        with mock.patch('auslib.web.views.client.ClientRequestView.get') as m:
+            m.side_effect = BadDataError('I break!')
+            ret = self.client.get('/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml')
+            self.assertEqual(ret.headers.get("X-Content-Type-Options"), "nosniff")
+
+    def testXContentTypeOptionsIsSetFor500(self):
+        with mock.patch('auslib.web.views.client.ClientRequestView.get') as m:
+            m.side_effect = Exception('I break!')
+            ret = self.client.get('/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml')
+            self.assertEqual(ret.headers.get("X-Content-Type-Options"), "nosniff")
+
     def testEmptySnippetOn404(self):
         ret = self.client.get('/whizzybang')
         self.assertUpdatesAreEmpty(ret)
 
-    def testEmptySnippetOn500(self):
+    def testErrorMessageOn500(self):
         with mock.patch('auslib.web.views.client.ClientRequestView.get') as m:
             m.side_effect = Exception('I break!')
-            with self.assertRaises(Exception) as exc:
-                self.client.get('/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml')
-            self.assertEqual('I break!', str(exc.exception.message))
+            ret = self.client.get('/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml')
+            self.assertEqual(ret.status_code, 500)
+            self.assertEqual(ret.mimetype, "text/plain")
+            self.assertEqual('I break!', ret.data)
+
+    def testEscapedOutputOn500(self):
+        with mock.patch('auslib.web.views.client.ClientRequestView.get') as m:
+            m.side_effect = Exception('50.1.0zibj5<img src%3da onerror%3dalert(document.domain)>')
+            ret = self.client.get('/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml')
+            self.assertEqual(ret.status_code, 500)
+            self.assertEqual(ret.mimetype, "text/plain")
+            self.assertEqual('50.1.0zibj5&lt;img src%3da onerror%3dalert(document.domain)&gt;', ret.data)
+
+    def testEscapedOutputOn400(self):
+        with mock.patch("auslib.web.views.client.ClientRequestView.get") as m:
+            m.side_effect = BadDataError('Version number 50.1.0zibj5<img src%3da onerror%3dalert(document.domain)> is invalid.')
+            ret = self.client.get("/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml")
+            self.assertEqual(ret.status_code, 400, ret.data)
+            self.assertEqual(ret.mimetype, "text/plain")
+            self.assertEqual("Version number 50.1.0zibj5&lt;img src%3da onerror%3dalert(document.domain)&gt; is invalid.", ret.data)
 
     def testSentryBadDataError(self):
         with mock.patch("auslib.web.views.client.ClientRequestView.get") as m, mock.patch("auslib.web.base.sentry") as sentry:
             m.side_effect = BadDataError("exterminate!")
             ret = self.client.get("/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml")
             self.assertFalse(sentry.captureException.called)
             self.assertEqual(ret.status_code, 400, ret.data)
+            self.assertEqual(ret.mimetype, "text/plain")
 
     def testSentryRealError(self):
-        with mock.patch("auslib.web.views.client.ClientRequestView.get") as m:
+        with mock.patch("auslib.web.views.client.ClientRequestView.get") as m, mock.patch("auslib.web.base.sentry") as sentry:
             m.side_effect = Exception("exterminate!")
-            with mock.patch("auslib.web.base.sentry") as sentry, self.assertRaises(Exception) as exc:
-                self.client.get("/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml")
+            ret = self.client.get("/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml")
+            self.assertEqual(ret.status_code, 500)
+            self.assertEqual(ret.mimetype, "text/plain")
             self.assertTrue(sentry.captureException.called)
-            self.assertEqual('exterminate!', str(exc.exception.message))
+            self.assertEqual('exterminate!', ret.data)
 
     def testNonSubstitutedUrlVariablesReturnEmptyUpdate(self):
         request1 = '/update/1/%PRODUCT%/%VERSION%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/update.xml'

auslib/web/base.py

@@ -1,6 +1,4 @@
-from auslib.log import configure_logging
-configure_logging()
-
+import cgi
 import logging
 log = logging.getLogger(__name__)
 
@@ -30,6 +28,17 @@ def heartbeat_database_function(dbo):
 create_dockerflow_endpoints(app, heartbeat_database_function)
 
 
+@app.after_request
+def apply_security_headers(response):
+    # There's no use cases for content served by Balrog to load additional content
+    # nor be embedded elsewhere, so we apply a strict Content Security Policy.
+    # We also need to set X-Content-Type-Options to nosniff for Firefox to obey this.
+    # See https://bugzilla.mozilla.org/show_bug.cgi?id=1332829#c4 for background.
+    response.headers["Content-Security-Policy"] = app.config.get("CONTENT_SECURITY_POLICY", "default-src 'none'; frame-ancestors 'none'")
+    response.headers["X-Content-Type-Options"] = app.config.get("CONTENT_TYPE_OPTIONS", "nosniff")
+    return response
+
+
 @app.errorhandler(404)
 def fourohfour(error):
     """We don't return 404s in AUS. Instead, we return empty XML files"""
@@ -45,12 +54,17 @@ def generic(error):
     because BadDataErrors are considered to be the client's fault.
     Otherwise, the error is just re-raised (which causes a 500)."""
 
+    # Escape exception messages before replying with them, because they may
+    # contain user input.
+    # See https://bugzilla.mozilla.org/show_bug.cgi?id=1332829 for background.
+    error.message = cgi.escape(error.message)
     if isinstance(error, BadDataError):
-        return Response(status=400, response=error.message)
+        return Response(status=400, mimetype="text/plain", response=error.message)
 
     if sentry.client:
         sentry.captureException()
-    raise error
+
+    return Response(status=500, mimetype="text/plain", response=error.message)
 
 
 @app.route('/robots.txt')