Matt Robenolt 9 лет назад
Родитель
Сommit
22a0553f9d

+ 5 - 5
src/sentry/coreapi.py

@@ -573,13 +573,13 @@ class ClientApiHelper(object):
 
 
 
 
 class CspApiHelper(ClientApiHelper):
 class CspApiHelper(ClientApiHelper):
-    def validate_data(self, project, data):
-        report = data.get('csp-report')
-        if not report:
-            raise APIForbidden('Missing csp-report')
+    def origin_from_request(self, request):
+        # We don't use an origin here
+        return None
 
 
+    def validate_data(self, project, data):
         # All keys are sent with hyphens, so we want to conver to underscores
         # All keys are sent with hyphens, so we want to conver to underscores
-        report = dict(map(lambda v: (v[0].replace('-', '_'), v[1]), report.iteritems()))
+        report = dict(map(lambda v: (v[0].replace('-', '_'), v[1]), data.iteritems()))
         inst = Csp.to_python(report)
         inst = Csp.to_python(report)
 
 
         # Construct a faux Http interface based on the little information we have
         # Construct a faux Http interface based on the little information we have

+ 25 - 8
src/sentry/interfaces/csp.py

@@ -20,6 +20,9 @@ REPORT_KEYS = frozenset((
     'blocked_uri', 'document_uri', 'effective_directive', 'original_policy',
     'blocked_uri', 'document_uri', 'effective_directive', 'original_policy',
     'referrer', 'status_code', 'violated_directive', 'source_file',
     'referrer', 'status_code', 'violated_directive', 'source_file',
     'line_number', 'column_number',
     'line_number', 'column_number',
+
+    # FireFox specific keys
+    'script_sample',
 ))
 ))
 
 
 KEYWORDS = frozenset((
 KEYWORDS = frozenset((
@@ -47,6 +50,11 @@ class Csp(Interface):
     @classmethod
     @classmethod
     def to_python(cls, data):
     def to_python(cls, data):
         kwargs = {k: trim(data.get(k, None), 1024) for k in REPORT_KEYS}
         kwargs = {k: trim(data.get(k, None), 1024) for k in REPORT_KEYS}
+        # Inline script violations are confusing and don't say what uri blocked them
+        # because they're inline. FireFox sends along "blocked-uri": "self", which is
+        # vastly more useful, so we want to emulate that
+        if kwargs['effective_directive'] == 'script-src' and not kwargs['blocked_uri']:
+            kwargs['blocked_uri'] = 'self'
         return cls(**kwargs)
         return cls(**kwargs)
 
 
     def get_hash(self):
     def get_hash(self):
@@ -54,21 +62,30 @@ class Csp(Interface):
         # This normalization has to be done for FireFox because they send
         # This normalization has to be done for FireFox because they send
         # weird stuff compared to Safari and Chrome.
         # weird stuff compared to Safari and Chrome.
         # NOTE: this may or may not be great, not sure until we see it in the wild
         # NOTE: this may or may not be great, not sure until we see it in the wild
-        bits = filter(None, self.violated_directive.split(' '))
-        return [bits[0]] + map(self._normalize_value, bits[1:])
+        return [':'.join(self.get_violated_directive()), ':'.join(self.get_culprit_directive())]
+
+    def get_violated_directive(self):
+        return 'violated-directive', self._normalize_directive(self.violated_directive)
+
+    def get_culprit_directive(self):
+        if self.blocked_uri:
+            return 'blocked-uri', self.blocked_uri
+        return 'effective-directive', self._normalize_directive(self.effective_directive)
 
 
     def get_path(self):
     def get_path(self):
         return 'sentry.interfaces.Csp'
         return 'sentry.interfaces.Csp'
 
 
     def get_message(self):
     def get_message(self):
-        return 'CSP Violation: %r' % ' '.join(self.get_hash())
+        return 'CSP Violation: %s %r' % self.get_culprit_directive()
 
 
     def get_culprit(self):
     def get_culprit(self):
-        if self.blocked_uri:
-            return 'blocked uri: %r' % self.blocked_uri
-        if self.effective_directive:
-            return 'effective directive: %r' % self.effective_directive
-        return 'violated directive: %r' % self.violated_directive
+        return '%s in %r' % self.get_violated_directive()
+
+    def _normalize_directive(self, directive):
+        if not directive:
+            return directive
+        bits = filter(None, directive.split(' '))
+        return ' '.join([bits[0]] + map(self._normalize_value, bits[1:]))
 
 
     def _normalize_value(self, value):
     def _normalize_value(self, value):
         # > If no scheme is specified, the same scheme as the one used to
         # > If no scheme is specified, the same scheme as the one used to

+ 8 - 3
src/sentry/web/api.py

@@ -393,7 +393,7 @@ class CspReportView(StoreView):
         # inside of the JSON body of the request. This 'document-uri' value
         # inside of the JSON body of the request. This 'document-uri' value
         # should be treated as an origin check since it refers to the page
         # should be treated as an origin check since it refers to the page
         # that triggered the report. The Content-Type is supposed to be
         # that triggered the report. The Content-Type is supposed to be
-        # `application/csp-report`, but FireFix sends it as `application/json`.
+        # `application/csp-report`, but FireFox sends it as `application/json`.
         if request.method != 'POST':
         if request.method != 'POST':
             return HttpResponseNotAllowed(['POST'])
             return HttpResponseNotAllowed(['POST'])
 
 
@@ -431,7 +431,12 @@ class CspReportView(StoreView):
 
 
         # Do origin check based on the `document-uri` key as explained
         # Do origin check based on the `document-uri` key as explained
         # in `_dispatch`.
         # in `_dispatch`.
-        origin = data['csp-report'].get('document-uri')
+        try:
+            report = data['csp-report']
+        except KeyError:
+            raise APIError('Missing csp-report')
+
+        origin = report.get('document-uri')
         if not is_valid_origin(origin, project):
         if not is_valid_origin(origin, project):
             raise APIForbidden('Invalid document-uri')
             raise APIForbidden('Invalid document-uri')
 
 
@@ -440,7 +445,7 @@ class CspReportView(StoreView):
             project=project,
             project=project,
             auth=auth,
             auth=auth,
             helper=helper,
             helper=helper,
-            data=data,
+            data=report,
             **kwargs
             **kwargs
         )
         )
         if isinstance(response_or_event_id, HttpResponse):
         if isinstance(response_or_event_id, HttpResponse):

+ 31 - 2
tests/sentry/coreapi/tests.py

@@ -9,19 +9,21 @@ from uuid import UUID
 
 
 from sentry.coreapi import (
 from sentry.coreapi import (
     APIError, APIUnauthorized, Auth, ClientApiHelper, InvalidFingerprint,
     APIError, APIUnauthorized, Auth, ClientApiHelper, InvalidFingerprint,
-    InvalidTimestamp, get_interface
+    InvalidTimestamp, get_interface, CspApiHelper,
 )
 )
 from sentry.testutils import TestCase
 from sentry.testutils import TestCase
 
 
 
 
 class BaseAPITest(TestCase):
 class BaseAPITest(TestCase):
+    helper_cls = ClientApiHelper
+
     def setUp(self):
     def setUp(self):
         self.user = self.create_user('coreapi@example.com')
         self.user = self.create_user('coreapi@example.com')
         self.team = self.create_team(name='Foo')
         self.team = self.create_team(name='Foo')
         self.project = self.create_project(team=self.team)
         self.project = self.create_project(team=self.team)
         self.pm = self.project.team.member_set.get_or_create(user=self.user)[0]
         self.pm = self.project.team.member_set.get_or_create(user=self.user)[0]
         self.pk = self.project.key_set.get_or_create()[0]
         self.pk = self.project.key_set.get_or_create()[0]
-        self.helper = ClientApiHelper()
+        self.helper = self.helper_cls(agent='Awesome Browser', ip_address='69.69.69.69')
 
 
 
 
 class AuthFromRequestTest(BaseAPITest):
 class AuthFromRequestTest(BaseAPITest):
@@ -332,3 +334,30 @@ class EnsureHasIpTest(BaseAPITest):
         out = {}
         out = {}
         self.helper.ensure_has_ip(out, '127.0.0.1')
         self.helper.ensure_has_ip(out, '127.0.0.1')
         assert out['sentry.interfaces.User']['ip_address'] == '127.0.0.1'
         assert out['sentry.interfaces.User']['ip_address'] == '127.0.0.1'
+
+
+class CspApiHelperTest(BaseAPITest):
+    helper_cls = CspApiHelper
+
+    def test_validate_basic(self):
+        report = {
+            "document-uri": "http://45.55.25.245:8123/csp",
+            "referrer": "http://example.com",
+            "violated-directive": "child-src https://45.55.25.245:8123/",
+            "effective-directive": "frame-src",
+            "original-policy": "default-src  https://45.55.25.245:8123/; child-src  https://45.55.25.245:8123/; connect-src  https://45.55.25.245:8123/; font-src  https://45.55.25.245:8123/; img-src  https://45.55.25.245:8123/; media-src  https://45.55.25.245:8123/; object-src  https://45.55.25.245:8123/; script-src  https://45.55.25.245:8123/; style-src  https://45.55.25.245:8123/; form-action  https://45.55.25.245:8123/; frame-ancestors 'none'; plugin-types 'none'; report-uri http://45.55.25.245:8123/csp-report?os=OS%20X&device=&browser_version=43.0&browser=chrome&os_version=Lion",
+            "blocked-uri": "http://google.com",
+            "status-code": 200
+        }
+        result = self.helper.validate_data(self.project, report)
+        assert result['project'] == self.project.id
+        assert 'message' in result
+        assert 'culprit' in result
+        assert result['sentry.interfaces.User'] == {'ip_address': '69.69.69.69'}
+        assert result['sentry.interfaces.Http'] == {
+            'url': 'http://45.55.25.245:8123/csp',
+            'headers': {
+                'User-Agent': 'Awesome Browser',
+                'Referer': 'http://example.com'
+            }
+        }

+ 98 - 0
tests/sentry/interfaces/test_csp.py

@@ -0,0 +1,98 @@
+# -*- coding: utf-8 -*-
+
+from __future__ import absolute_import
+
+from mock import patch
+from exam import fixture
+
+from sentry.interfaces.csp import Csp
+from sentry.testutils import TestCase
+
+
+class CspTest(TestCase):
+    @fixture
+    def interface(self):
+        return Csp.to_python(dict(
+            document_uri='http://example.com',
+            violated_directive='style-src cdn.example.com',
+            blocked_uri='http://example.com/lol.css',
+        ))
+
+    def test_path(self):
+        assert self.interface.get_path() == 'sentry.interfaces.Csp'
+
+    def test_serialize_unserialize_behavior(self):
+        result = type(self.interface).to_python(self.interface.to_json())
+        assert result.to_json() == self.interface.to_json()
+
+    def test_basic(self):
+        result = self.interface
+        assert result.document_uri == 'http://example.com'
+        assert result.violated_directive == 'style-src cdn.example.com'
+        assert result.blocked_uri == 'http://example.com/lol.css'
+
+    def test_coerce_blocked_uri_if_script_src(self):
+        result = Csp.to_python(dict(
+            effective_directive='script-src'
+        ))
+        assert result.blocked_uri == 'self'
+
+    def test_violated_directive(self):
+        result = Csp.to_python(dict(
+            document_uri='http://example.com/foo',
+            violated_directive='style-src http://cdn.example.com',
+        ))
+        assert result.get_violated_directive() == ('violated-directive', 'style-src http://cdn.example.com')
+
+        result = Csp.to_python(dict(
+            document_uri='http://example.com/foo',
+            violated_directive='style-src cdn.example.com',
+        ))
+        assert result.get_violated_directive() == ('violated-directive', 'style-src http://cdn.example.com')
+
+        result = Csp.to_python(dict(
+            document_uri='https://example.com/foo',
+            violated_directive='style-src cdn.example.com',
+        ))
+        assert result.get_violated_directive() == ('violated-directive', 'style-src https://cdn.example.com')
+
+        result = Csp.to_python(dict(
+            document_uri='http://example.com/foo',
+            violated_directive='style-src https://cdn.example.com',
+        ))
+        assert result.get_violated_directive() == ('violated-directive', 'style-src https://cdn.example.com')
+
+        result = Csp.to_python(dict(
+            document_uri='blob:example.com/foo',
+            violated_directive='style-src cdn.example.com',
+        ))
+        assert result.get_violated_directive() == ('violated-directive', 'style-src blob:cdn.example.com')
+
+    def test_get_culprit_directive(self):
+        result = Csp.to_python(dict(
+            document_uri='http://example.com/foo',
+            blocked_uri='http://example.com/lol.css',
+            effective_directive='style-src'
+        ))
+        assert result.get_culprit_directive() == ('blocked-uri', 'http://example.com/lol.css')
+
+        result = Csp.to_python(dict(
+            document_uri='http://example.com/foo',
+            blocked_uri='',
+            effective_directive='style-src',
+        ))
+        assert result.get_culprit_directive() == ('effective-directive', 'style-src')
+
+        result = Csp.to_python(dict(
+            document_uri='http://example.com/foo',
+            effective_directive='script-src',
+            blocked_uri='',
+        ))
+        assert result.get_culprit_directive() == ('blocked-uri', 'self')
+
+    @patch('sentry.interfaces.csp.Csp.get_culprit_directive')
+    @patch('sentry.interfaces.csp.Csp.get_violated_directive')
+    def test_get_hash(self, get_culprit, get_violated):
+        get_culprit.return_value = ('a', 'b')
+        get_violated.return_value = ('c', 'd')
+        assert self.interface.get_hash() == ['a:b', 'c:d']

+ 50 - 0
tests/sentry/web/api/tests.py

@@ -12,6 +12,56 @@ from sentry.testutils import TestCase
 from sentry.utils import json
 from sentry.utils import json
 
 
 
 
+class CspReportViewTest(TestCase):
+    @fixture
+    def path(self):
+        path = reverse('sentry-api-csp-report', kwargs={'project_id': self.project.id})
+        return path + '?sentry_key=%s&sentry_version=5' % self.projectkey.public_key
+
+    def _postWithHeader(self, data, key=None, **extra):
+        body = json.dumps({'csp-report': data})
+        with self.tasks():
+            return self.client.post(
+                self.path, data=body,
+                content_type='application/csp-report',
+                HTTP_USER_AGENT='awesome',
+                **extra
+            )
+
+    def test_get_response(self):
+        resp = self.client.get(self.path)
+        assert resp.status_code == 405, resp.content
+
+    def test_invalid_content_type(self):
+        resp = self.client.post(self.path, content_type='text/plain')
+        assert resp.status_code == 400, resp.content
+
+    def test_missing_csp_report(self):
+        resp = self.client.post(self.path,
+            content_type='application/csp-report',
+            data='{"lol":1}',
+            HTTP_USER_AGENT='awesome',
+        )
+        assert resp.status_code == 400, resp.content
+
+    @mock.patch('sentry.utils.http.get_origins')
+    def test_bad_origin(self, get_origins):
+        get_origins.return_value = ['example.com']
+        resp = self.client.post(self.path,
+            content_type='application/csp-report',
+            data='{"csp-report":{"document_uri":"http://lolnope.com"}}',
+            HTTP_USER_AGENT='awesome',
+        )
+        assert resp.status_code == 403, resp.content
+
+    @mock.patch('sentry.web.api.is_valid_origin', mock.Mock(return_value=True))
+    @mock.patch('sentry.web.api.CspReportView.process')
+    def test_post_success(self, process):
+        process.return_value = 'ok'
+        resp = self._postWithHeader(dict(document_uri='http://example.com'))
+        assert resp.status_code == 201, resp.content
+
+
 class StoreViewTest(TestCase):
 class StoreViewTest(TestCase):
     @fixture
     @fixture
     def path(self):
     def path(self):