diff options
| -rw-r--r-- | CHANGES.rst | 11 | ||||
| -rw-r--r-- | docs/narr/security.rst | 7 | ||||
| -rw-r--r-- | src/pyramid/config/security.py | 17 | ||||
| -rw-r--r-- | src/pyramid/csrf.py | 148 | ||||
| -rw-r--r-- | src/pyramid/viewderivers.py | 9 | ||||
| -rw-r--r-- | tests/test_config/test_security.py | 7 | ||||
| -rw-r--r-- | tests/test_csrf.py | 42 | ||||
| -rw-r--r-- | tests/test_viewderivers.py | 22 |
8 files changed, 186 insertions, 77 deletions
diff --git a/CHANGES.rst b/CHANGES.rst index 0cf66d16f..987d5c3d4 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -43,8 +43,17 @@ Features ``pyramid.csrf.check_csrf_origin``. This option controls whether a request is rejected if it has no ``Origin`` or ``Referer`` header - often the result of a user configuring their browser not to send a - ``Referer`` header for privacy reasons. + ``Referer`` header for privacy reasons even on same-domain requests. + The default is to reject requests without a known origin. It is also + possible to allow the special ``Origin: null`` header by adding it to the + ``pyramid.csrf_trusted_origins`` list in the settings. See https://github.com/Pylons/pyramid/pull/3512 + and https://github.com/Pylons/pyramid/pull/3518 + +- A new parameter, ``check_origin``, was added to + ``pyramid.config.Configurator.set_default_csrf_options`` which disables + origin checking entirely. + See https://github.com/Pylons/pyramid/pull/3518 - Added ``pyramid.interfaces.IPredicateInfo`` which defines the object passed to predicate factories as their second argument. diff --git a/docs/narr/security.rst b/docs/narr/security.rst index 9d45dfb6a..f1bb37c69 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -885,7 +885,12 @@ is the current host, however additional origins may be configured by setting are non-standard). If a host in the list of domains starts with a ``.`` then that will allow all subdomains as well as the domain without the ``.``. If no ``Referer`` or ``Origin`` header is present in an HTTPS request, the CSRF check -will fail unless ``allow_no_origin`` is set. +will fail unless ``allow_no_origin`` is set. The special ``Origin: null`` can +be allowed by adding ``null`` to the ``pyramid.csrf_trusted_origins`` list. + +It is possible to opt out of checking the origin by passing +``check_origin=False``. This is useful if the :term:`CSRF storage policy` is +known to be secure such that the token cannot be easily used by an attacker. If CSRF checks fail then a :class:`pyramid.exceptions.BadCSRFToken` or :class:`pyramid.exceptions.BadCSRFOrigin` exception will be raised. This diff --git a/src/pyramid/config/security.py b/src/pyramid/config/security.py index 32b4db03c..99eb5792c 100644 --- a/src/pyramid/config/security.py +++ b/src/pyramid/config/security.py @@ -254,6 +254,7 @@ class SecurityConfiguratorMixin(object): token='csrf_token', header='X-CSRF-Token', safe_methods=('GET', 'HEAD', 'OPTIONS', 'TRACE'), + check_origin=True, allow_no_origin=False, callback=None, ): @@ -279,8 +280,13 @@ class SecurityConfiguratorMixin(object): never be automatically checked for CSRF tokens. Default: ``('GET', 'HEAD', 'OPTIONS', TRACE')``. - ``allow_no_origin`` is a boolean. If false, a request lacking both an - ``Origin`` and ``Referer`` header will fail the CSRF check. + ``check_origin`` is a boolean. If ``False``, the ``Origin`` and + ``Referer`` headers will not be validated as part of automated + CSRF checks. + + ``allow_no_origin`` is a boolean. If ``True``, a request lacking both + an ``Origin`` and ``Referer`` header will pass the CSRF check. This + option has no effect if ``check_origin`` is ``False``. If ``callback`` is set, it must be a callable accepting ``(request)`` and returning ``True`` if the request should be checked for a valid @@ -298,7 +304,7 @@ class SecurityConfiguratorMixin(object): Added the ``callback`` option. .. versionchanged:: 2.0 - Added the ``allow_no_origin`` option. + Added the ``allow_no_origin`` and ``check_origin`` options. """ options = DefaultCSRFOptions( @@ -306,6 +312,7 @@ class SecurityConfiguratorMixin(object): token=token, header=header, safe_methods=safe_methods, + check_origin=check_origin, allow_no_origin=allow_no_origin, callback=callback, ) @@ -323,6 +330,8 @@ class SecurityConfiguratorMixin(object): intr['token'] = token intr['header'] = header intr['safe_methods'] = as_sorted_tuple(safe_methods) + intr['check_origin'] = allow_no_origin + intr['allow_no_origin'] = check_origin intr['callback'] = callback self.action( @@ -362,6 +371,7 @@ class DefaultCSRFOptions(object): token, header, safe_methods, + check_origin, allow_no_origin, callback, ): @@ -369,5 +379,6 @@ class DefaultCSRFOptions(object): self.token = token self.header = header self.safe_methods = frozenset(safe_methods) + self.check_origin = check_origin self.allow_no_origin = allow_no_origin self.callback = callback diff --git a/src/pyramid/csrf.py b/src/pyramid/csrf.py index b352ada71..f9914e852 100644 --- a/src/pyramid/csrf.py +++ b/src/pyramid/csrf.py @@ -248,7 +248,7 @@ def check_csrf_token( def check_csrf_origin( - request, trusted_origins=None, allow_no_origin=False, raises=True + request, *, trusted_origins=None, allow_no_origin=False, raises=True ): """ Check the ``Origin`` of the request to see if it is a cross site request or @@ -266,6 +266,10 @@ def check_csrf_origin( (the default) this list of additional domains will be pulled from the ``pyramid.csrf_trusted_origins`` setting. + ``allow_no_origin`` determines whether to return ``True`` when the + origin cannot be determined via either the ``Referer`` or ``Origin`` + header. The default is ``False`` which will reject the check. + Note that this function will do nothing if ``request.scheme`` is not ``https``. @@ -274,78 +278,90 @@ def check_csrf_origin( .. versionchanged:: 1.9 Moved from :mod:`pyramid.session` to :mod:`pyramid.csrf` + .. versionchanged:: 2.0 + Added the ``allow_no_origin`` option. + """ def _fail(reason): if raises: - raise BadCSRFOrigin(reason) + raise BadCSRFOrigin("Origin checking failed - " + reason) else: return False - if request.scheme == "https": - # Suppose user visits http://example.com/ - # An active network attacker (man-in-the-middle, MITM) sends a - # POST form that targets https://example.com/detonate-bomb/ and - # submits it via JavaScript. - # - # The attacker will need to provide a CSRF cookie and token, but - # that's no problem for a MITM when we cannot make any assumptions - # about what kind of session storage is being used. So the MITM can - # circumvent the CSRF protection. This is true for any HTTP connection, - # but anyone using HTTPS expects better! For this reason, for - # https://example.com/ we need additional protection that treats - # http://example.com/ as completely untrusted. Under HTTPS, - # Barth et al. found that the Referer header is missing for - # same-domain requests in only about 0.2% of cases or less, so - # we can use strict Referer checking. - - # Determine the origin of this request - origin = request.headers.get("Origin") - if origin is None: - origin = request.referrer - - # If we can't find an origin, fail or pass immediately depending on - # ``allow_no_origin`` - if not origin: - if allow_no_origin: - return True - else: - return _fail("Origin checking failed - no Origin or Referer.") - - # Parse our origin so we we can extract the required information from - # it. - originp = urlparse(origin) - - # Ensure that our Referer is also secure. - if originp.scheme != "https": - return _fail( - "Referer checking failed - Referer is insecure while host is " - "secure." - ) - - # Determine which origins we trust, which by default will include the - # current origin. - if trusted_origins is None: - trusted_origins = aslist( - request.registry.settings.get( - "pyramid.csrf_trusted_origins", [] - ) - ) - - if request.host_port not in set(["80", "443"]): - trusted_origins.append("{0.domain}:{0.host_port}".format(request)) + # Origin checks are only trustworthy / useful on HTTPS requests. + if request.scheme != "https": + return True + + # Suppose user visits http://example.com/ + # An active network attacker (man-in-the-middle, MITM) sends a + # POST form that targets https://example.com/detonate-bomb/ and + # submits it via JavaScript. + # + # The attacker will need to provide a CSRF cookie and token, but + # that's no problem for a MITM when we cannot make any assumptions + # about what kind of session storage is being used. So the MITM can + # circumvent the CSRF protection. This is true for any HTTP connection, + # but anyone using HTTPS expects better! For this reason, for + # https://example.com/ we need additional protection that treats + # http://example.com/ as completely untrusted. Under HTTPS, + # Barth et al. found that the Referer header is missing for + # same-domain requests in only about 0.2% of cases or less, so + # we can use strict Referer checking. + + # Determine the origin of this request + origin = request.headers.get("Origin") + origin_is_referrer = False + if origin is None: + origin = request.referrer + origin_is_referrer = True + + else: + # use the last origin in the list under the assumption that the + # server generally appends values and we want the origin closest + # to us + origin = origin.split(' ')[-1] + + # If we can't find an origin, fail or pass immediately depending on + # ``allow_no_origin`` + if not origin: + if allow_no_origin: + return True else: - trusted_origins.append(request.domain) - - # Actually check to see if the request's origin matches any of our - # trusted origins. - if not any( - is_same_domain(originp.netloc, host) for host in trusted_origins - ): - reason = ( - "Referer checking failed - {0} does not match any trusted " - "origins." - ) - return _fail(reason.format(origin)) + return _fail("missing Origin or Referer.") + + # Determine which origins we trust, which by default will include the + # current origin. + if trusted_origins is None: + trusted_origins = aslist( + request.registry.settings.get("pyramid.csrf_trusted_origins", []) + ) + + if request.host_port not in set(["80", "443"]): + trusted_origins.append("{0.domain}:{0.host_port}".format(request)) + else: + trusted_origins.append(request.domain) + + # Check "Origin: null" against trusted_origins + if not origin_is_referrer and origin == 'null': + if origin in trusted_origins: + return True + else: + return _fail("null does not match any trusted origins.") + + # Parse our origin so we we can extract the required information from + # it. + originp = urlparse(origin) + + # Ensure that our Referer is also secure. + if originp.scheme != "https": + return _fail("Origin is insecure while host is secure.") + + # Actually check to see if the request's origin matches any of our + # trusted origins. + if not any( + is_same_domain(originp.netloc, host) for host in trusted_origins + ): + return _fail("{0} does not match any trusted origins.".format(origin)) return True diff --git a/src/pyramid/viewderivers.py b/src/pyramid/viewderivers.py index 95c223e61..35f9a08d2 100644 --- a/src/pyramid/viewderivers.py +++ b/src/pyramid/viewderivers.py @@ -484,6 +484,7 @@ def csrf_view(view, info): token = 'csrf_token' header = 'X-CSRF-Token' safe_methods = frozenset(["GET", "HEAD", "OPTIONS", "TRACE"]) + check_origin = True allow_no_origin = False callback = None else: @@ -491,6 +492,7 @@ def csrf_view(view, info): token = defaults.token header = defaults.header safe_methods = defaults.safe_methods + check_origin = defaults.check_origin allow_no_origin = defaults.allow_no_origin callback = defaults.callback @@ -510,9 +512,10 @@ def csrf_view(view, info): if request.method not in safe_methods and ( callback is None or callback(request) ): - check_csrf_origin( - request, raises=True, allow_no_origin=allow_no_origin - ) + if check_origin: + check_csrf_origin( + request, raises=True, allow_no_origin=allow_no_origin + ) check_csrf_token(request, token, header, raises=True) return view(context, request) diff --git a/tests/test_config/test_security.py b/tests/test_config/test_security.py index 0ae199239..9a9ea9f7e 100644 --- a/tests/test_config/test_security.py +++ b/tests/test_config/test_security.py @@ -158,6 +158,7 @@ class ConfiguratorSecurityMethodsTests(unittest.TestCase): list(sorted(result.safe_methods)), ['GET', 'HEAD', 'OPTIONS', 'TRACE'], ) + self.assertTrue(result.check_origin) self.assertFalse(result.allow_no_origin) self.assertTrue(result.callback is None) @@ -174,7 +175,8 @@ class ConfiguratorSecurityMethodsTests(unittest.TestCase): token='DUMMY', header=None, safe_methods=('PUT',), - allow_no_origin=True, + check_origin=False, + allow_no_origin=False, callback=callback, ) result = config.registry.getUtility(IDefaultCSRFOptions) @@ -182,5 +184,6 @@ class ConfiguratorSecurityMethodsTests(unittest.TestCase): self.assertEqual(result.token, 'DUMMY') self.assertEqual(result.header, None) self.assertEqual(list(sorted(result.safe_methods)), ['PUT']) - self.assertTrue(result.allow_no_origin) + self.assertFalse(result.check_origin) + self.assertFalse(result.allow_no_origin) self.assertTrue(result.callback is callback) diff --git a/tests/test_csrf.py b/tests/test_csrf.py index f93a1afde..ae998ec95 100644 --- a/tests/test_csrf.py +++ b/tests/test_csrf.py @@ -387,8 +387,48 @@ class Test_check_csrf_origin(unittest.TestCase): request = testing.DummyRequest() request.scheme = "https" request.referrer = None - self.assertRaises(BadCSRFOrigin, self._callFUT, request) + self.assertRaises( + BadCSRFOrigin, self._callFUT, request, allow_no_origin=False + ) + self.assertFalse( + self._callFUT(request, raises=False, allow_no_origin=False) + ) + + def test_fail_with_null_origin(self): + from pyramid.exceptions import BadCSRFOrigin + + request = testing.DummyRequest() + request.scheme = "https" + request.host = "example.com" + request.host_port = "443" + request.referrer = None + request.headers = {'Origin': 'null'} + request.registry.settings = {} self.assertFalse(self._callFUT(request, raises=False)) + self.assertRaises(BadCSRFOrigin, self._callFUT, request) + + def test_success_with_null_origin_and_setting(self): + request = testing.DummyRequest() + request.scheme = "https" + request.host = "example.com" + request.host_port = "443" + request.referrer = None + request.headers = {'Origin': 'null'} + request.registry.settings = {"pyramid.csrf_trusted_origins": ["null"]} + self.assertTrue(self._callFUT(request, raises=False)) + + def test_success_with_multiple_origins(self): + request = testing.DummyRequest() + request.scheme = "https" + request.host = "example.com" + request.host_port = "443" + request.headers = { + 'Origin': 'https://google.com https://not-example.com' + } + request.registry.settings = { + "pyramid.csrf_trusted_origins": ["not-example.com"] + } + self.assertTrue(self._callFUT(request, raises=False)) def test_fails_when_http_to_https(self): from pyramid.exceptions import BadCSRFOrigin diff --git a/tests/test_viewderivers.py b/tests/test_viewderivers.py index 12a903eaa..e47296b50 100644 --- a/tests/test_viewderivers.py +++ b/tests/test_viewderivers.py @@ -1414,6 +1414,28 @@ class TestDeriveView(unittest.TestCase): result = view(None, request) self.assertTrue(result is response) + def test_csrf_view_disables_origin_check(self): + response = DummyResponse() + + def inner_view(request): + return response + + self.config.set_default_csrf_options( + require_csrf=True, check_origin=False + ) + request = self._makeRequest() + request.scheme = "https" + request.domain = "example.com" + request.host_port = "443" + request.referrer = None + request.method = 'POST' + request.headers = {"Origin": "https://evil-example.com"} + request.session = DummySession({'csrf_token': 'foo'}) + request.POST = {'csrf_token': 'foo'} + view = self.config._derive_view(inner_view, require_csrf=True) + result = view(None, request) + self.assertTrue(result is response) + def test_csrf_view_allow_no_origin(self): response = DummyResponse() |