diff --git a/settings.py b/settings.py index 7d35bae793..d1513ea209 100644 --- a/settings.py +++ b/settings.py @@ -124,6 +124,7 @@ ALLOWED_FXA_CONFIGS = ['default', 'amo', 'local', 'code-manager'] # CSP report endpoint which returns a 204 from addons-nginx in local dev. CSP_REPORT_URI = '/csp-report' +RESTRICTED_DOWNLOAD_CSP['REPORT_URI'] = CSP_REPORT_URI # Allow GA over http + www subdomain in local development. HTTP_GA_SRC = 'http://www.google-analytics.com' diff --git a/src/olympia/files/tests/test_views.py b/src/olympia/files/tests/test_views.py index 55056dd374..0c57d80fb0 100644 --- a/src/olympia/files/tests/test_views.py +++ b/src/olympia/files/tests/test_views.py @@ -493,7 +493,47 @@ class TestFileViewer(FilesBase, TestCase): res = self.client.get(self.files_serve(binary) + '?token=' + token) assert res.status_code == 200 assert res['Content-Type'] == 'application/octet-stream' - assert 'X-Sendfile' in res + + # Not sending through nginx but directly using djangos FileResponse + # to be able to restrict and test CSP related settings. + assert 'X-Sendfile' not in res + assert ( + res['Content-Disposition'] == + 'attachment; filename="ar.dic"') + + content = res.getvalue().decode('utf-8') + assert content.startswith( + '52726\n####الأدوات(واستثناءات)+الأفعال+الأسماء') + + @override_settings(CSP_REPORT_ONLY=False) + def test_serve_respects_csp(self): + self.file_viewer.extract() + res = self.client.get(self.files_redirect(binary)) + assert res.status_code == 302 + url = res['Location'] + token = urlparse(url).query.split('=')[1] + + res = self.client.get(self.files_serve(binary) + '?token=' + token) + assert res.status_code == 200 + assert res['Content-Type'] == 'application/octet-stream' + + # Make sure a default-src is set. + assert "default-src 'none'" in res['content-security-policy'] + # Make sure things are as locked down as possible, + # as per https://bugzilla.mozilla.org/show_bug.cgi?id=1566954 + assert "object-src 'none'" in res['content-security-policy'] + assert "base-uri 'none'" in res['content-security-policy'] + assert "form-action 'none'" in res['content-security-policy'] + assert "frame-ancestors 'none'" in res['content-security-policy'] + + # The report-uri should be set. + assert "report-uri" in res['content-security-policy'] + + # Other properties that we defined by default aren't set + assert "style-src" not in res['content-security-policy'] + assert "font-src" not in res['content-security-policy'] + assert "frame-src" not in res['content-security-policy'] + assert "child-src" not in res['content-security-policy'] def test_serve_obj_not_exist(self): self.file_viewer.extract() @@ -519,13 +559,6 @@ class TestFileViewer(FilesBase, TestCase): res = self.client.get(url) assert res.status_code == 403 - def test_bounce(self): - self.file_viewer.extract() - res = self.client.get(self.files_redirect(binary), follow=True) - assert res.status_code == 200 - assert res[settings.XSENDFILE_HEADER] == ( - self.file_viewer.get_files().get(binary)['full']) - @patch.object(settings, 'FILE_VIEWER_SIZE_LIMIT', 5) def test_file_size(self): self.file_viewer.extract() diff --git a/src/olympia/files/views.py b/src/olympia/files/views.py index ae67efccee..06a493b38e 100644 --- a/src/olympia/files/views.py +++ b/src/olympia/files/views.py @@ -1,3 +1,4 @@ +from django.conf import settings from django import http, shortcuts from django.db.transaction import non_atomic_requests from django.utils.translation import ugettext @@ -5,13 +6,15 @@ from django.views.decorators.cache import never_cache from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import condition +from csp.decorators import csp as set_csp + import olympia.core.logger from olympia.access import acl from olympia.lib.cache import Message, Token from olympia.amo.decorators import json_view from olympia.amo.urlresolvers import reverse -from olympia.amo.utils import HttpResponseSendFile, render, urlparams +from olympia.amo.utils import render, urlparams from olympia.files.decorators import ( compare_file_view, etag, file_view, file_view_token, last_modified) from olympia.files.file_viewer import extract_file @@ -179,6 +182,7 @@ def redirect(request, viewer, key): return http.HttpResponseRedirect(url) +@set_csp(**settings.RESTRICTED_DOWNLOAD_CSP) @file_view_token @non_atomic_requests def serve(request, viewer, key): @@ -193,5 +197,10 @@ def serve(request, viewer, key): key, list(files.keys())[:10], len(files.keys()), viewer.file.id)) raise http.Http404 - return HttpResponseSendFile(request, obj['full'], - content_type=obj['mimetype']) + + fobj = open(obj['full'], 'rb') + + response = http.FileResponse( + fobj, as_attachment=True, content_type=obj['mimetype']) + + return response diff --git a/src/olympia/lib/settings_base.py b/src/olympia/lib/settings_base.py index ab87aa3230..bc1cc3e520 100644 --- a/src/olympia/lib/settings_base.py +++ b/src/olympia/lib/settings_base.py @@ -1409,6 +1409,15 @@ CSP_STYLE_SRC = ( PROD_CDN_HOST, ) +RESTRICTED_DOWNLOAD_CSP = { + 'DEFAULT_SRC': "'none'", + 'BASE_URI': "'none'", + 'FORM_ACTION': "'none'", + 'OBJECT_SRC': "'none'", + 'FRAME_ANCESTORS': "'none'", + 'REPORT_URI': CSP_REPORT_URI +} + # Should robots.txt deny everything or disallow a calculated list of URLs we # don't want to be crawled? Default is true, allow everything, toggled to # False on -dev and stage. diff --git a/src/olympia/reviewers/tests/test_views.py b/src/olympia/reviewers/tests/test_views.py index f6a8de0b1e..a30166a5b3 100644 --- a/src/olympia/reviewers/tests/test_views.py +++ b/src/olympia/reviewers/tests/test_views.py @@ -6339,6 +6339,39 @@ class TestDownloadGitFileView(TestCase): assert content.startswith('{') assert '"manifest_version": 2' in content + @override_settings(CSP_REPORT_ONLY=False) + def test_download_respects_csp(self): + user = UserProfile.objects.create(username='reviewer') + self.grant_permission(user, 'Addons:Review') + self.client.login(email=user.email) + + url = reverse('reviewers.download_git_file', kwargs={ + 'version_id': self.version.pk, + 'filename': 'manifest.json' + }) + + response = self.client.get(url) + + assert response.status_code == 200 + + # Make sure a default-src is set. + assert "default-src 'none'" in response['content-security-policy'] + # Make sure things are as locked down as possible, + # as per https://bugzilla.mozilla.org/show_bug.cgi?id=1566954 + assert "object-src 'none'" in response['content-security-policy'] + assert "base-uri 'none'" in response['content-security-policy'] + assert "form-action 'none'" in response['content-security-policy'] + assert "frame-ancestors 'none'" in response['content-security-policy'] + + # The report-uri should be set. + assert "report-uri" in response['content-security-policy'] + + # Other properties that we defined by default aren't set + assert "style-src" not in response['content-security-policy'] + assert "font-src" not in response['content-security-policy'] + assert "frame-src" not in response['content-security-policy'] + assert "child-src" not in response['content-security-policy'] + def test_download_emoji_filename(self): new_version = version_factory( addon=self.addon, file_kw={'filename': 'webextension_no_id.xpi'}) diff --git a/src/olympia/reviewers/views.py b/src/olympia/reviewers/views.py index 190546814b..3623bcdb99 100644 --- a/src/olympia/reviewers/views.py +++ b/src/olympia/reviewers/views.py @@ -26,6 +26,8 @@ from rest_framework.mixins import ( ListModelMixin, RetrieveModelMixin, CreateModelMixin, DestroyModelMixin, UpdateModelMixin) +from csp.decorators import csp as set_csp + import olympia.core.logger from olympia import amo @@ -1159,6 +1161,7 @@ def theme_background_images(request, version_id): @login_required +@set_csp(**settings.RESTRICTED_DOWNLOAD_CSP) def download_git_stored_file(request, version_id, filename): version = get_object_or_404(Version.unfiltered, id=int(version_id))