Fix: Content Security Policy (CSP) Not Implemented (DataBiosphere/azu…

…l-private#6)

lambdas/service/app.py

@@ -12,6 +12,7 @@
 )
 import json
 import logging.config
+import secrets
 from typing import (
     Any,
     Callable,
@@ -30,6 +31,7 @@
     Response,
     UnauthorizedError,
 )
+import chevron
 from furl import (
     furl,
 )
@@ -488,10 +490,18 @@ def static_resource(file):
     cache_control='no-store'
 )
 def oauth2_redirect():
-    oauth2_redirect_html = app.load_static_resource('swagger', 'oauth2-redirect.html')
+    file_name = 'oauth2-redirect.html.template.mustache'
+    template = app.load_static_resource('swagger', file_name)
+    nonce = secrets.token_urlsafe(32)
+    html = chevron.render(template, {
+        'CSP_NONCE': json.dumps(nonce)
+    })
     return Response(status_code=200,
-                    headers={'Content-Type': 'text/html'},
-                    body=oauth2_redirect_html)
+                    headers={
+                        'Content-Type': 'text/html',
+                        'Content-Security-Policy': app.content_security_policy(nonce)
+                    },
+                    body=html)
 
 
 common_specs = CommonEndpointSpecs(app_name='service')

src/azul/chalice.py

@@ -15,6 +15,7 @@
 import mimetypes
 import os
 import pathlib
+import secrets
 from typing import (
     Any,
     Iterator,
@@ -193,33 +194,48 @@ def _api_gateway_context_middleware(self, event, get_response):
         finally:
             config.lambda_is_handling_api_gateway_request = False
 
-    hsts_max_age = 60 * 60 * 24 * 365 * 2
-
-    # Headers added to every response from the app, as well as canned 4XX and
-    # 5XX responses from API Gateway. Use of these headers addresses known
-    # security vulnerabilities.
-    #
-    security_headers = {
-        'Content-Security-Policy': jw('default-src', sq('self')),
-        'Referrer-Policy': 'strict-origin-when-cross-origin',
-        'Strict-Transport-Security': jw(f'max-age={hsts_max_age};',
-                                        'includeSubDomains;',
-                                        'preload'),
-        'X-Content-Type-Options': 'nosniff',
-        'X-Frame-Options': 'DENY',
-        'X-XSS-Protection': '1; mode=block'
-    }
+    @classmethod
+    def content_security_policy(cls, nonce: str | None = None) -> str:
+        self_ = sq('self')
+        none = sq('none')
+        nonce = [] if nonce is None else [sq('nonce-' + nonce)]
+
+        return ';'.join([
+            jw('default-src', self_),
+            jw('img-src', self_, 'data:'),
+            jw('script-src', self_, *nonce),
+            jw('style-src', self_, *nonce),
+            jw('frame-ancestors', none),
+        ])
+
+    @classmethod
+    def security_headers(cls) -> dict[str, str]:
+        """
+        Default values for headers added to every response from the app, as well
+        as canned 4XX and 5XX responses from API Gateway. Use of these headers
+        addresses known security vulnerabilities.
+        """
+        hsts_max_age = 60 * 60 * 24 * 365 * 2
+        return {
+            'Content-Security-Policy': cls.content_security_policy(),
+            'Referrer-Policy': 'strict-origin-when-cross-origin',
+            'Strict-Transport-Security': jw(f'max-age={hsts_max_age};',
+                                            'includeSubDomains;',
+                                            'preload'),
+            'X-Content-Type-Options': 'nosniff',
+            'X-Frame-Options': 'DENY',
+            'X-XSS-Protection': '1; mode=block'
+        }
 
     def _security_headers_middleware(self, event, get_response):
         """
         Add headers to the response
         """
         response = get_response(event)
-        response.headers.update(self.security_headers)
-        # FIXME: Add a CSP header with a nonce value to text/html responses
-        #        https://github.com/DataBiosphere/azul-private/issues/6
-        if response.headers.get('Content-Type') == 'text/html':
-            del response.headers['Content-Security-Policy']
+        # Add security headers to the response without overwriting any headers
+        # that might have been added already (e.g. Content-Security-Policy)
+        for key, val in self.security_headers().items():
+            response.headers.setdefault(key, val)
         view_function = self.routes[event.path][event.method].view_function
         cache_control = getattr(view_function, 'cache_control')
         response.headers['Cache-Control'] = cache_control
@@ -491,11 +507,14 @@ def _controller(self, controller_cls: Type[C], **kwargs) -> C:
         return controller_cls(app=self, **kwargs)
 
     def swagger_ui(self) -> Response:
-        swagger_ui_template = self.load_static_resource('swagger', 'swagger-ui.html.template.mustache')
+        file_name = 'swagger-ui.html.template.mustache'
+        template = self.load_static_resource('swagger', file_name)
         base_url = self.base_url
         redirect_url = furl(base_url).add(path='oauth2_redirect')
         deployment_url = furl(base_url).add(path='openapi')
-        swagger_ui_html = chevron.render(swagger_ui_template, {
+        nonce = secrets.token_urlsafe(32)
+        html = chevron.render(template, {
+            'CSP_NONCE': json.dumps(nonce),
             'DEPLOYMENT_PATH': json.dumps(str(deployment_url.path)),
             'OAUTH2_CLIENT_ID': json.dumps(config.google_oauth2_client_id),
             'OAUTH2_REDIRECT_URL': json.dumps(str(redirect_url)),
@@ -505,8 +524,11 @@ def swagger_ui(self) -> Response:
             ])
         })
         return Response(status_code=200,
-                        headers={'Content-Type': 'text/html'},
-                        body=swagger_ui_html)
+                        headers={
+                            'Content-Type': 'text/html',
+                            'Content-Security-Policy': self.content_security_policy(nonce)
+                        },
+                        body=html)
 
     def swagger_resource(self, file_name: str) -> Response:
         if os.sep in file_name:

src/azul/terraform.py

@@ -829,7 +829,7 @@ def tf_config(self, app_name):
                     # value, which that function would prohibit.
                     #
                     f'gatewayresponse.header.{k}': f"'{v}'"
-                    for k, v in AzulChaliceApp.security_headers.items()
+                    for k, v in AzulChaliceApp.security_headers().items()
                 }
             } for response_type in ['4XX', '5XX']
         }

swagger/oauth2-redirect.html → swagger/oauth2-redirect.html.template.mustache

@@ -2,10 +2,10 @@
 <!doctype html>
 <html lang="en-US">
 <title>Swagger UI: OAuth2 Redirect</title>
-<body onload="run()">
+<body>
 </body>
 </html>
-<script>
+<script nonce={{{CSP_NONCE}}}>
     'use strict';
     function run () {
         var oauth2 = window.opener.swaggerUIRedirectOauth2;
@@ -66,4 +66,15 @@
         }
         window.close();
     }
+    // The CSP blocks JS event handlers from inline HTML markup, so instead of
+    // calling run() from the body tag's onload property, as a workaround it
+    // is added here with an event listener.
+    //
+    // See also:
+    //
+    // https://github.com/swagger-api/swagger-ui/issues/5720
+    //
+    window.addEventListener('DOMContentLoaded', function () {
+        run();
+    });
 </script>

swagger/swagger-ui.html.template.mustache

@@ -5,7 +5,7 @@
     <meta charset="UTF-8">
     <title>Swagger UI</title>
     <link rel="stylesheet" href="static/swagger-ui.css" crossorigin="anonymous">
-    <style>
+    <style nonce={{{CSP_NONCE}}}>
       html
       {
         box-sizing: border-box;
@@ -30,7 +30,7 @@
     <div id="swagger-ui"></div>
     <script src="static/swagger-ui-bundle.js"></script>
     <script src="static/swagger-ui-standalone-preset.js"></script>
-    <script>
+    <script nonce={{{CSP_NONCE}}}>
     window.onload = function() {
       // Adapted from https://github.com/swagger-api/swagger-ui/issues/3725#issuecomment-334899276
       const DisableTryItOutPlugin = function() {

test/integration_test.py

@@ -2053,10 +2053,12 @@ def test_response_security_headers(self):
                     else:
                         response = requests.get(str(endpoint / path))
                         response.raise_for_status()
-                        expected = AzulChaliceApp.security_headers | expected_headers
-                        # FIXME: Add a CSP header with a nonce value to text/html responses
-                        #        https://github.com/DataBiosphere/azul-private/issues/6
+                        expected = AzulChaliceApp.security_headers() | expected_headers
                         if path in ['/', '/oauth2_redirect']:
+                            # Since these paths have a CSP that differs from the
+                            # default, the best we can do is assert the CSP has
+                            # a 'nonce' value.
+                            self.assertIn('nonce', response.headers['Content-Security-Policy'])
                             del expected['Content-Security-Policy']
                         self.assertIsSubset(expected.items(), response.headers.items())
 
@@ -2065,5 +2067,5 @@ def test_default_4xx_response_headers(self):
             with self.subTest(endpoint=endpoint):
                 response = requests.get(str(endpoint / 'does-not-exist'))
                 self.assertEqual(403, response.status_code)
-                self.assertIsSubset(AzulChaliceApp.security_headers.items(),
+                self.assertIsSubset(AzulChaliceApp.security_headers().items(),
                                     response.headers.items())

test/service/test_app_logging.py

@@ -51,14 +51,20 @@ def test_request_logs(self):
                         'connection': 'keep-alive',
                         **headers,
                     }
+                    self_ = sq('self')
+                    none = sq('none')
                     response_headers = {
                         'Access-Control-Allow-Origin': '*',
                         'Access-Control-Allow-Headers': 'Authorization,'
                                                         'Content-Type,'
                                                         'X-Amz-Date,'
                                                         'X-Amz-Security-Token,'
                                                         'X-Api-Key',
-                        'Content-Security-Policy': f"default-src {sq('self')}",
+                        'Content-Security-Policy': f"default-src {self_};"
+                                                   f"img-src {self_} data:;"
+                                                   f"script-src {self_};"
+                                                   f"style-src {self_};"
+                                                   f"frame-ancestors {none}",
                         'Referrer-Policy': 'strict-origin-when-cross-origin',
                         'Strict-Transport-Security': 'max-age=63072000;'
                                                      ' includeSubDomains;'

test/test_app_logging.py

@@ -105,9 +105,15 @@ def fail():
                         self.assertTrue(response.startswith(traceback_header))
                         self.assertIn(magic_message, response)
                         # … and the response is logged.
+                        self_ = sq('self')
+                        none = sq('none')
                         headers = {
                             'Content-Type': 'text/plain',
-                            'Content-Security-Policy': f"default-src {sq('self')}",
+                            'Content-Security-Policy': f"default-src {self_};"
+                                                       f"img-src {self_} data:;"
+                                                       f"script-src {self_};"
+                                                       f"style-src {self_};"
+                                                       f"frame-ancestors {none}",
                             'Referrer-Policy': 'strict-origin-when-cross-origin',
                             'Strict-Transport-Security': 'max-age=63072000;'
                                                          ' includeSubDomains;'