Browse Source

Automatically set CSP when serving images and documents (#12672)

Addresses #12617
Jake Howard 3 months ago
parent
commit
076af8bec8

+ 1 - 0
CHANGELOG.txt

@@ -16,6 +16,7 @@ Changelog
  * Add `on_serve_page` hook to modify the serving chain of pages (Krystian Magdziarz, Dawid Bugajewski)
  * Add support for `WAGTAIL_GRAVATAR_PROVIDER_URL` URLs with query string parameters (Ayaan Qadri, Guilhem Saurel)
  * Add `get_avatar_url` hook to customise user avatars (jhrr)
+ * Set content security policy (CSP) headers to block embedded content when serving images and documents (Jake Howard, with thanks to Ali İltizar for the initial report)
  * Fix: Improve handling of translations for bulk page action confirmation messages (Matt Westcott)
  * Fix: Ensure custom rich text feature icons are correctly handled when provided as a list of SVG paths (Temidayo Azeez, Joel William, LB (Ben) Johnston)
  * Fix: Ensure manual edits to `StreamField` values do not throw an error (Stefan Hammer)

+ 2 - 0
docs/deployment/under_the_hood.md

@@ -64,6 +64,8 @@ If a remote ("cloud") storage backend is used, the serve method will default to
 
 If these limitations are not acceptable, you may set the `WAGTAILDOCS_SERVE_METHOD` to `serve_view` and ensure that the documents are not publicly accessible using the cloud service's file url.
 
+The steps required to set headers for specific responses will vary, depending on how your Wagtail application is deployed and which storage backend is used. For the `'serve_view` method, a `Content-Security-Policy` header is automatically set for you (unless disabled via [](wagtaildocs_block_embedded_content)) to prevent the execution of scripts embedded in documents.
+
 #### Cloud storage
 
 Be aware that setting up remote storage will not entirely offload file handling tasks from the application server - some Wagtail functionality requires files to be read back by the application server.

+ 14 - 0
docs/reference/settings.md

@@ -501,6 +501,20 @@ WAGTAILDOCS_INLINE_CONTENT_TYPES = ['application/pdf', 'text/plain']
 
 A list of MIME content types that will be shown inline in the browser (by serving the HTTP header `Content-Disposition: inline`) rather than served as a download, when using the `serve_view` method. Defaults to `application/pdf`.
 
+(wagtaildocs_block_embedded_content)=
+
+### `WAGTAILDOCS_BLOCK_EMBEDDED_CONTENT`
+
+```python
+WAGTAILDOCS_BLOCK_EMBEDDED_CONTENT = True
+```
+
+Wagtail serves a restrictive Content-Security policy for documents which ensures embedded content (such as the Javascript in a HTML file) is not executed. This functionality can be disabled by setting this to `False`.
+
+This does not affect Javascript embedded in PDFs, however this is already executed in an isolated environment.
+
+Unless absolutely necessary, it's strongly recommended not to change this setting.
+
 (wagtaildocs_extensions)=
 
 ### `WAGTAILDOCS_EXTENSIONS`

+ 1 - 0
docs/releases/6.4.md

@@ -25,6 +25,7 @@ depth: 1
  * Add [`on_serve_page`](on_serve_page) hook to modify the serving chain of pages (Krystian Magdziarz, Dawid Bugajewski)
  * Add support for [`WAGTAIL_GRAVATAR_PROVIDER_URL`](wagtail_gravatar_provider_url) URLs with query string parameters (Ayaan Qadri, Guilhem Saurel)
  * Add [`get_avatar_url`](get_avatar_url) hook to customise user avatars (jhrr)
+ * Set content security policy (CSP) headers to block embedded content when serving images and documents (Jake Howard, with thanks to Ali İltizar for the initial report)
 
 ### Bug fixes
 

+ 1 - 1
docs/topics/images.md

@@ -565,7 +565,7 @@ If a user navigates directly to the URL of the SVG file embedded scripts may be
 -   setting `Content-Security-Policy: default-src 'none'` will prevent scripts from being loaded or executed (as well as other resources - a more relaxed policy of `script-src 'none'` may also be suitable); and
 -   setting `Content-Disposition: attachment` will cause the file to be downloaded rather than being immediately rendered in the browser, meaning scripts will not be executed (note: this will not prevent scripts from running if a user downloads and subsequently opens the SVG file in their browser).
 
-The steps required to set headers for specific responses will vary, depending on how your Wagtail application is deployed.
+The steps required to set headers for specific responses will vary, depending on how your Wagtail application is deployed. For the built-in [](using_images_outside_wagtail), a Content-Security-Policy header is automatically set for you.
 
 (heic_heif_images)=
 

+ 21 - 12
wagtail/documents/tests/test_views.py

@@ -58,15 +58,18 @@ class TestServeView(TestCase):
             f'inline; filename="{self.pdf_document.filename}"',
         )
 
+    def test_content_security_policy(self):
+        self.assertEqual(self.get()["Content-Security-Policy"], "default-src 'none'")
+
+        with self.settings(WAGTAILDOCS_BLOCK_EMBEDDED_CONTENT=False):
+            self.assertNotIn("Content-Security-Policy", self.get().headers)
+
+    def test_no_sniff_content_type(self):
+        self.assertEqual(self.get()["X-Content-Type-Options"], "nosniff")
+
     @mock.patch("wagtail.documents.views.serve.hooks")
     @mock.patch("wagtail.documents.views.serve.get_object_or_404")
-    def test_non_local_filesystem_content_disposition_header(
-        self, mock_get_object_or_404, mock_hooks
-    ):
-        """
-        Tests the 'Content-Disposition' header in a response when using a
-        storage backend that doesn't expose filesystem paths.
-        """
+    def test_non_local_filesystem_headers(self, mock_get_object_or_404, mock_hooks):
         # Create a mock document with no local file to hit the correct code path
         mock_doc = mock.Mock()
         mock_doc.filename = self.document.filename
@@ -90,16 +93,14 @@ class TestServeView(TestCase):
                 urllib.parse.quote(self.document.filename)
             ),
         )
+        self.assertEqual(response["Content-Security-Policy"], "default-src 'none'")
+        self.assertEqual(response["X-Content-Type-Options"], "nosniff")
 
     @mock.patch("wagtail.documents.views.serve.hooks")
     @mock.patch("wagtail.documents.views.serve.get_object_or_404")
-    def test_non_local_filesystem_inline_content_disposition_header(
+    def test_non_local_filesystem_inline_headers(
         self, mock_get_object_or_404, mock_hooks
     ):
-        """
-        Tests the 'Content-Disposition' header in a response when using a
-        storage backend that doesn't expose filesystem paths.
-        """
         # Create a mock document with no local file to hit the correct code path
         mock_doc = mock.Mock()
         mock_doc.filename = self.pdf_document.filename
@@ -118,6 +119,8 @@ class TestServeView(TestCase):
         self.assertEqual(response.status_code, 200)
 
         self.assertEqual(response["Content-Disposition"], "inline")
+        self.assertEqual(response["Content-Security-Policy"], "default-src 'none'")
+        self.assertEqual(response["X-Content-Type-Options"], "nosniff")
 
     def test_content_length_header(self):
         self.assertEqual(self.get()["Content-Length"], "25")
@@ -346,6 +349,12 @@ class TestServeViewWithSendfile(TestCase):
             os.path.join(settings.MEDIA_URL, self.document.file.name),
         )
 
+    def test_content_security_policy(self):
+        self.assertEqual(self.get()["Content-Security-Policy"], "default-src 'none'")
+
+    def test_no_sniff_content_type(self):
+        self.assertEqual(self.get()["X-Content-Type-Options"], "nosniff")
+
 
 @override_settings(WAGTAILDOCS_SERVE_METHOD=None)
 class TestServeWithUnicodeFilename(TestCase):

+ 9 - 2
wagtail/documents/views/serve.py

@@ -90,7 +90,7 @@ def serve(request, document_id, document_filename):
             # Fallback to streaming backend if user hasn't specified SENDFILE_BACKEND
             sendfile_opts["backend"] = sendfile_streaming_backend.sendfile
 
-        return sendfile(request, local_path, **sendfile_opts)
+        response = sendfile(request, local_path, **sendfile_opts)
 
     else:
         # We are using a storage backend which does not expose filesystem paths
@@ -107,7 +107,14 @@ def serve(request, document_id, document_filename):
         # FIXME: storage backends are not guaranteed to implement 'size'
         response["Content-Length"] = doc.file.size
 
-        return response
+    # Add a CSP header to prevent inline execution
+    if getattr(settings, "WAGTAILDOCS_BLOCK_EMBEDDED_CONTENT", True):
+        response["Content-Security-Policy"] = "default-src 'none'"
+
+    # Prevent browsers from auto-detecting the content-type of a document
+    response["X-Content-Type-Options"] = "nosniff"
+
+    return response
 
 
 def authenticate_with_password(request, restriction_id):

+ 10 - 0
wagtail/images/tests/tests.py

@@ -366,6 +366,8 @@ class TestFrontendServeView(TestCase):
         self.assertEqual(response.status_code, 200)
         self.assertTrue(response.streaming)
         self.assertEqual(response["Content-Type"], "image/png")
+        self.assertEqual(response["Content-Security-Policy"], "default-src 'none'")
+        self.assertEqual(response["X-Content-Type-Options"], "nosniff")
         # Ensure the file can actually be read
         image = willow.Image.open(b"".join(response.streaming_content))
         self.assertIsInstance(image, PNGImageFile)
@@ -385,6 +387,8 @@ class TestFrontendServeView(TestCase):
         self.assertEqual(response.status_code, 200)
         self.assertTrue(response.streaming)
         self.assertEqual(response["Content-Type"], "image/svg+xml")
+        self.assertEqual(response["Content-Security-Policy"], "default-src 'none'")
+        self.assertEqual(response["X-Content-Type-Options"], "nosniff")
         # Ensure the file can actually be read
         image = willow.Image.open(BytesIO(b"".join(response.streaming_content)))
         self.assertIsInstance(image, SvgImageFile)
@@ -462,6 +466,8 @@ class TestFrontendServeView(TestCase):
         self.assertEqual(response.status_code, 200)
         self.assertTrue(response.streaming)
         self.assertEqual(response["Content-Type"], "image/png")
+        self.assertEqual(response["Content-Security-Policy"], "default-src 'none'")
+        self.assertEqual(response["X-Content-Type-Options"], "nosniff")
         # Ensure the file can actually be read
         image = willow.Image.open(b"".join(response.streaming_content))
         self.assertIsInstance(image, PNGImageFile)
@@ -627,6 +633,8 @@ class TestFrontendSendfileView(TestCase):
 
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response["Content-Type"], "image/png")
+        self.assertEqual(response["Content-Security-Policy"], "default-src 'none'")
+        self.assertEqual(response["X-Content-Type-Options"], "nosniff")
 
     @override_settings(SENDFILE_BACKEND="sendfile.backends.development")
     def test_sendfile_dummy_backend(self):
@@ -640,6 +648,8 @@ class TestFrontendSendfileView(TestCase):
 
         self.assertEqual(response.status_code, 200)
         self.assertTrue(response.content, msg="Dummy backend response")
+        self.assertEqual(response["Content-Security-Policy"], "default-src 'none'")
+        self.assertEqual(response["X-Content-Type-Options"], "nosniff")
 
 
 class TestRect(TestCase):

+ 18 - 2
wagtail/images/views/serve.py

@@ -66,7 +66,15 @@ class ServeView(View):
 
         # Serve the file
         rendition.file.open("rb")
-        return FileResponse(rendition.file, content_type=mime_type)
+        response = FileResponse(rendition.file, content_type=mime_type)
+
+        # Add a CSP header to prevent inline execution
+        response["Content-Security-Policy"] = "default-src 'none'"
+
+        # Prevent browsers from auto-detecting the content-type of a document
+        response["X-Content-Type-Options"] = "nosniff"
+
+        return response
 
     def redirect(self, rendition):
         # Redirect to the file's public location
@@ -80,4 +88,12 @@ class SendFileView(ServeView):
     backend = None
 
     def serve(self, rendition):
-        return sendfile(self.request, rendition.file.path, backend=self.backend)
+        response = sendfile(self.request, rendition.file.path, backend=self.backend)
+
+        # Add a CSP header to prevent inline execution
+        response["Content-Security-Policy"] = "default-src 'none'"
+
+        # Prevent browsers from auto-detecting the content-type of a document
+        response["X-Content-Type-Options"] = "nosniff"
+
+        return response