Browse Source

Initial code parts for CSP implementation for sentry and self-hosted (#48699)

Another attempt of https://github.com/getsentry/sentry/pull/47980 and
https://github.com/getsentry/sentry/pull/48507 (which were reverted due
to bugs).

This PR adds some preliminary code for adding a
Content-Security-Policy-Report-Only header with minimal required
permissions (at least I could not find any violations on sentry
devserver and self-hosted).

The CSP middleware is disabled (commented in the MIDDLEWARE)
There is no report collecting enabled by default (CSP_REPORT_URI is not
set), the intent is to customize it depending on the use case.
Alexander Tarasov 1 year ago
parent
commit
4ba5bb7e65

+ 1 - 0
requirements-base.txt

@@ -11,6 +11,7 @@ croniter>=1.3.7
 cssselect>=1.0.3
 datadog>=0.29.3
 django-crispy-forms>=1.14.0
+django-csp>=3.7
 django-pg-zero-downtime-migrations>=0.11
 Django>=2.2.28
 djangorestframework>=3.12.4

+ 1 - 0
requirements-dev-frozen.txt

@@ -40,6 +40,7 @@ dictpath==0.1.3
 distlib==0.3.4
 django==2.2.28
 django-crispy-forms==1.14.0
+django-csp==3.7
 django-pg-zero-downtime-migrations==0.11
 djangorestframework==3.12.4
 docker==3.7.0

+ 1 - 0
requirements-frozen.txt

@@ -34,6 +34,7 @@ datadog==0.29.3
 decorator==5.1.1
 django==2.2.28
 django-crispy-forms==1.14.0
+django-csp==3.7
 django-pg-zero-downtime-migrations==0.11
 djangorestframework==3.12.4
 drf-spectacular==0.22.1

+ 58 - 0
src/sentry/conf/server.py

@@ -301,6 +301,8 @@ USE_TZ = True
 # response modifying middleware reset the Content-Length header.
 # This is because CommonMiddleware Sets the Content-Length header for non-streaming responses.
 MIDDLEWARE = (
+    # Uncomment to enable Content Security Policy on this Sentry installation (experimental)
+    # "csp.middleware.CSPMiddleware",
     "sentry.middleware.health.HealthCheck",
     "sentry.middleware.security.SecurityHeadersMiddleware",
     "sentry.middleware.env.SentryEnvMiddleware",
@@ -407,6 +409,62 @@ SILENCED_SYSTEM_CHECKS = (
     "urls.E007",
 )
 
+CSP_INCLUDE_NONCE_IN = [
+    "script-src",
+]
+
+CSP_DEFAULT_SRC = [
+    "'none'",
+]
+CSP_SCRIPT_SRC = [
+    "'self'",
+    "'unsafe-inline'",
+]
+CSP_FONT_SRC = [
+    "'self'",
+    "data:",
+]
+CSP_CONNECT_SRC = [
+    "'self'",
+]
+CSP_FRAME_ANCESTORS = [
+    "'none'",
+]
+CSP_OBJECT_SRC = [
+    "'none'",
+]
+CSP_BASE_URI = [
+    "'none'",
+]
+CSP_STYLE_SRC = [
+    "'self'",
+    "'unsafe-inline'",
+]
+CSP_IMG_SRC = [
+    "'self'",
+    "blob:",
+    "data:",
+    "https://secure.gravatar.com",
+]
+
+if ENVIRONMENT == "development":
+    CSP_SCRIPT_SRC += [
+        "'unsafe-eval'",
+    ]
+    CSP_CONNECT_SRC += [
+        "ws://127.0.0.1:8000",
+    ]
+
+# Before enforcing Content Security Policy, we recommend creating a separate
+# Sentry project and collecting CSP violations in report only mode:
+# https://docs.sentry.io/product/security-policy-reporting/
+
+# Point this parameter to your Sentry installation:
+# CSP_REPORT_URI = "https://example.com/api/{PROJECT_ID}/security/?sentry_key={SENTRY_KEY}"
+
+# To enforce CSP (block violated resources), update the following parameter to False
+CSP_REPORT_ONLY = True
+
 STATIC_ROOT = os.path.realpath(os.path.join(PROJECT_ROOT, "static"))
 STATIC_URL = "/_static/{version}/"
 # webpack assets live at a different URL that is unversioned

+ 32 - 3
src/sentry/integrations/jira/views/base.py

@@ -1,3 +1,5 @@
+from csp.middleware import CSPMiddleware
+from django.conf import settings
 from django.views.generic import View
 
 from sentry import options
@@ -16,11 +18,38 @@ class JiraSentryUIBaseView(View):
         """
 
         context["ac_js_src"] = "https://connect-cdn.atl-paas.net/all.js"
-        response = render_to_response(self.html_file, context, self.request)
         sources = [
             self.request.GET.get("xdm_e"),
             options.get("system.url-prefix"),
         ]
-        sources_string = " ".join(s for s in sources if s)  # Filter out None
-        response["Content-Security-Policy"] = f"frame-ancestors 'self' {sources_string}"
+        sources = [s for s in sources if s and ";" not in s]  # Filter out None and invalid sources
+
+        if "csp.middleware.CSPMiddleware" in settings.MIDDLEWARE:
+            settings.CSP_FRAME_ANCESTORS = [
+                "'self'",
+            ] + sources
+            settings.CSP_STYLE_SRC = [
+                # same as default (server.py)
+                "'self'",
+                "'unsafe-inline'",
+            ]
+
+            if settings.STATIC_FRONTEND_APP_URL.startswith("https://"):
+                origin = "/".join(settings.STATIC_FRONTEND_APP_URL.split("/")[0:3])
+                settings.CSP_STYLE_SRC.append(origin)
+
+            header = "Content-Security-Policy"
+            if getattr(settings, "CSP_REPORT_ONLY", False):
+                header += "-Report-Only"
+
+            middleware = CSPMiddleware()
+            middleware.process_request(self.request)  # adds nonce
+
+            response = render_to_response(self.html_file, context, self.request)
+
+            response[header] = middleware.build_policy(request=self.request, response=response)
+        else:
+            response = render_to_response(self.html_file, context, self.request)
+            sources_string = " ".join(sources)
+            response["Content-Security-Policy"] = f"frame-ancestors 'self' {sources_string}"
         return response

+ 48 - 0
tests/sentry/integrations/jira/test_csp.py

@@ -0,0 +1,48 @@
+from django.conf import settings
+from django.test.utils import override_settings
+
+from sentry.testutils import APITestCase
+from sentry.utils.http import absolute_uri
+
+
+def provision_middleware():
+    return ["csp.middleware.CSPMiddleware"] + list(settings.MIDDLEWARE)
+
+
+class JiraCSPTest(APITestCase):
+    def setUp(self):
+        super().setUp()
+        self.issue_key = "APP-123"
+        self.path = absolute_uri(f"extensions/jira/issue/{self.issue_key}/") + "?xdm_e=base_url"
+        self.middleware = provision_middleware()
+
+    def _split_csp_policy(self, policy):
+        csp = {}
+        for directive in policy.split("; "):
+            parts = directive.split(" ")
+            csp[parts[0]] = parts[1:]
+        return csp
+
+    def test_csp_frame_ancestors(self):
+        with override_settings(MIDDLEWARE=tuple(self.middleware)):
+            response = self.client.get(self.path)
+            assert "Content-Security-Policy-Report-Only" in response
+
+            csp = self._split_csp_policy(response["Content-Security-Policy-Report-Only"])
+            assert "base_url" in csp["frame-ancestors"]
+            assert "http://testserver" in csp["frame-ancestors"]
+
+    @override_settings(STATIC_FRONTEND_APP_URL="https://sentry.io/_static/dist/")
+    def test_csp_remote_style(self):
+        with override_settings(MIDDLEWARE=tuple(self.middleware)):
+            response = self.client.get(self.path)
+            assert "Content-Security-Policy-Report-Only" in response
+
+            csp = self._split_csp_policy(response["Content-Security-Policy-Report-Only"])
+            assert "https://sentry.io" in csp["style-src"]
+
+    @override_settings(CSP_REPORT_ONLY=False)
+    def test_csp_enforce(self):
+        with override_settings(MIDDLEWARE=tuple(self.middleware)):
+            response = self.client.get(self.path)
+            assert "Content-Security-Policy" in response