From 6fbc19fad3e6679c700e513128dcee6527cb96f5 Mon Sep 17 00:00:00 2001 From: Alexander Tarasov Date: Fri, 5 May 2023 10:51:08 +0200 Subject: [PATCH] Initial code parts for CSP implementation for sentry and self-hosted (#48507) Another attempt of https://github.com/getsentry/sentry/pull/47980 (which was 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. --- requirements-base.txt | 1 + requirements-dev-frozen.txt | 1 + requirements-frozen.txt | 1 + src/sentry/conf/server.py | 58 ++++++++++++++++++++++ src/sentry/integrations/jira/views/base.py | 31 ++++++++++-- tests/sentry/integrations/jira/test_csp.py | 39 +++++++++++++++ 6 files changed, 127 insertions(+), 4 deletions(-) create mode 100644 tests/sentry/integrations/jira/test_csp.py diff --git a/requirements-base.txt b/requirements-base.txt index 943e3a5cb43f6f..c5c3a42cc78208 100644 --- a/requirements-base.txt +++ b/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 diff --git a/requirements-dev-frozen.txt b/requirements-dev-frozen.txt index 16e13af11e7fb8..d617906774f72c 100644 --- a/requirements-dev-frozen.txt +++ b/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 diff --git a/requirements-frozen.txt b/requirements-frozen.txt index 42758a04c71d09..3cf82165fdcc7f 100644 --- a/requirements-frozen.txt +++ b/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 diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index 91f7927edc697e..78da40626cf84e 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -301,6 +301,8 @@ def env( # 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 @@ def env( "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 diff --git a/src/sentry/integrations/jira/views/base.py b/src/sentry/integrations/jira/views/base.py index c60b5d6acb2f8b..0df8a489f18c32 100644 --- a/src/sentry/integrations/jira/views/base.py +++ b/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 @@ -15,12 +17,33 @@ def get_response(self, context): add the requisite CSP headers before returning it. """ - 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}" + + settings.CSP_FRAME_ANCESTORS = [ + "'self'", + ] + [s for s in sources if s and ";" not in s] + 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 + + context["ac_js_src"] = "https://connect-cdn.atl-paas.net/all.js" + response = render_to_response(self.html_file, context, self.request) + + response[header] = middleware.build_policy(request=self.request, response=response) return response diff --git a/tests/sentry/integrations/jira/test_csp.py b/tests/sentry/integrations/jira/test_csp.py new file mode 100644 index 00000000000000..4a6363aa5a1e2f --- /dev/null +++ b/tests/sentry/integrations/jira/test_csp.py @@ -0,0 +1,39 @@ +from django.test.utils import override_settings + +from sentry.testutils import APITestCase +from sentry.utils.http import absolute_uri + + +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" + + 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): + 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): + 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): + response = self.client.get(self.path) + assert "Content-Security-Policy" in response