fix(csp): parse effective-directive from violated-directive (#12272)

To handle Firefox CSP violation reports naively parse
'effective-directive' as up to the first whitespace in
'violated-directive'.

Fixes GH-3542

src/sentry/interfaces/security.py

@@ -334,6 +334,17 @@ class Csp(SecurityReport):
 
     @classmethod
     def from_raw(cls, raw):
+        # Firefox doesn't send effective-directive, so parse it from
+        # violated-directive but prefer effective-directive when present
+        #
+        # refs: https://bugzil.la/1192684#c8
+        try:
+            report = raw['csp-report']
+            report['effective-directive'] = report.get('effective-directive',
+                                                       report['violated-directive'].split(None, 1)[0])
+        except (KeyError, IndexError):
+            pass
+
         # Validate the raw data against the input schema (raises on failure)
         schema = INPUT_SCHEMAS[cls.path]
         jsonschema.validate(raw, schema)

tests/integration/fixtures/csp/firefox_blocked_asset_input.json

@@ -1,7 +1,7 @@
 {
     "csp-report": {
-        "blocked-uri": "http://localhost:8000/lol.css",
-        "document-uri": "http://localhost:8000/",
+        "blocked-uri": "http://notlocalhost:8000/lol.css",
+        "document-uri": "http://notlocalhost:8000/",
         "original-policy": "default-src 'none'; style-src http://cdn.example.com; report-uri http://requestb.in/1im8m061",
         "referrer": "",
         "violated-directive": "style-src http://cdn.example.com"

tests/integration/fixtures/csp/firefox_blocked_asset_output.json

@@ -0,0 +1,23 @@
+{
+  "message": "Blocked 'style' from 'notlocalhost:8000'",
+  "tags": {
+    "logger": "csp",
+    "effective-directive": "style-src",
+    "blocked-uri": "http://notlocalhost:8000/lol.css"
+  },
+  "data": {
+    "sentry.interfaces.User": {"ip_address": "127.0.0.1"},
+    "sentry.interfaces.Csp": {
+      "blocked_uri": "http://notlocalhost:8000/lol.css",
+      "violated_directive": "style-src http://cdn.example.com",
+      "document_uri": "http://notlocalhost:8000/",
+      "original_policy": "default-src 'none'; style-src http://cdn.example.com; report-uri http://requestb.in/1im8m061",
+      "effective_directive": "style-src",
+      "referrer": ""
+    },
+    "sentry.interfaces.Http": {
+      "url": "http://notlocalhost:8000/",
+      "headers": [["User-Agent", "Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2228.0 Safari/537.36"]]
+    }
+  }
+}

tests/integration/tests.py

@@ -561,9 +561,11 @@ class CspReportTest(TestCase):
         resp = self._postCspWithHeader(input)
         assert resp.status_code in (400, 403), resp.content
 
+    def test_invalid_report(self):
+        self.assertReportRejected('')
+
     def test_chrome_blocked_asset(self):
         self.assertReportCreated(*get_fixtures('chrome_blocked_asset'))
 
     def test_firefox_missing_effective_uri(self):
-        input, _ = get_fixtures('firefox_blocked_asset')
-        self.assertReportRejected(input)
+        self.assertReportCreated(*get_fixtures('firefox_blocked_asset'))

tests/sentry/event_manager/test_validate_csp.py

@@ -14,7 +14,7 @@ def validate_and_normalize(report, client_ip='198.51.100.0',
     return manager.get_data()
 
 
-def test_csp_validate_basic():
+def _build_test_report(effective_directive, violated_directive):
     report = {
         "release": "abc123",
         "environment": "production",
@@ -23,21 +23,44 @@ def test_csp_validate_basic():
             "csp-report": {
                 "document-uri": "http://45.55.25.245:8123/csp",
                 "referrer": "http://example.com",
-                "violated-directive": "img-src https://45.55.25.245:8123/",
-                "effective-directive": "img-src",
+                "violated-directive": violated_directive,
+                "effective-directive": effective_directive,
                 "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,
             }
         }
     }
+    if violated_directive is None:
+        del report['report']['csp-report']['violated-directive']
+    if effective_directive is None:
+        del report['report']['csp-report']['effective-directive']
+
+    return report
+
+
+@pytest.mark.parametrize(
+    'effective_directive,violated_directive,culprit_element', (
+        ("img-src", "img-src https://45.55.25.245:8123/", "img-src"),
+        ("img-src", "default-src https://45.55.25.245:8123/", "default-src"),
+        # build a report without the effective-directive key
+        (None, "img-src https://45.55.25.245:8123/", "img-src"),
+    ),
+    ids=(
+        'directives match',
+        'prefer effective-directive',
+        'parse effective-directive from violated-directive',
+    )
+)
+def test_csp_validate(effective_directive, violated_directive, culprit_element):
+    report = _build_test_report(effective_directive, violated_directive)
     result = validate_and_normalize(report)
     assert result['logger'] == 'csp'
     assert result['release'] == 'abc123'
     assert result['environment'] == 'production'
     assert "errors" not in result
     assert 'logentry' in result
-    assert result['culprit'] == "img-src 'self'"
+    assert result['culprit'] == culprit_element + " 'self'"
     assert map(tuple, result['tags']) == [
         ('effective-directive', 'img-src'),
         ('blocked-uri', 'http://google.com'),
@@ -50,19 +73,26 @@ def test_csp_validate_basic():
     }
 
 
-def test_csp_validate_failure():
-    report = {
-        "release": "abc123",
-        "interface": 'csp',
-        "report": {}
-    }
-
+@pytest.mark.parametrize(
+    'report', (
+        {},
+        {"release": "abc123", "interface": 'csp', "report": {}},
+        _build_test_report(effective_directive=None, violated_directive=None),
+        _build_test_report(effective_directive=None, violated_directive=""),
+        _build_test_report(effective_directive=None, violated_directive="blink-src"),
+    ),
+    ids=(
+        'empty dict',
+        'no csp-report',
+        'no violated-directive to parse (expect KeyError)',
+        'unsplittable violated-directive (expect IndexError)',
+        'invalid violated-directive (not in schema enum)',
+    )
+)
+def test_csp_validate_failure(report):
     with pytest.raises(APIError):
         validate_and_normalize(report)
 
-    with pytest.raises(APIError):
-        validate_and_normalize({})
-
 
 def test_csp_tags_out_of_bounds():
     report = {

tests/sentry/utils/test_csp.py

@@ -10,6 +10,10 @@ from sentry.interfaces.security import Csp
     'report', (
         {}, {
             'effective_directive': 'lolnotreal'
+        }, {
+            'violated_directive': 'lolnotreal'
+        }, {
+            'violated_directive': ' '
         },
     )
 )