Security: rename WEBHOOK_AUTHORIZATION --> WEBHOOK_SECRET

This fixes a low severity security issue affecting Anymail v0.2--v1.3.

Django error reporting includes the value of your Anymail
WEBHOOK_AUTHORIZATION setting. In a properly-configured deployment,
this should not be cause for concern. But if you have somehow exposed
your Django error reports (e.g., by mis-deploying with DEBUG=True or by
sending error reports through insecure channels), anyone who gains
access to those reports could discover your webhook shared secret. An
attacker could use this to post fabricated or malicious Anymail
tracking/inbound events to your app, if you are using those Anymail
features.

The fix renames Anymail's webhook shared secret setting so that
Django's error reporting mechanism will [sanitize][0] it.

If you are using Anymail's event tracking and/or inbound webhooks, you
should upgrade to this release and change "WEBHOOK_AUTHORIZATION" to
"WEBHOOK_SECRET" in the ANYMAIL section of your settings.py. You may
also want to [rotate the shared secret][1] value, particularly if you
have ever exposed your Django error reports to untrusted individuals.

If you are only using Anymail's EmailBackends for sending email and
have not set up Anymail's webhooks, this issue does not affect you.

The old WEBHOOK_AUTHORIZATION setting is still allowed in this release,
but will issue a system-check warning when running most Django
management commands. It will be removed completely in a near-future
release, as a breaking change.

Thanks to Charlie DeTar (@yourcelf) for responsibly reporting this
security issue through private channels.

[0]: https://docs.djangoproject.com/en/stable/ref/settings/#debug
[1]: https://anymail.readthedocs.io/en/1.4/tips/securing_webhooks/#use-a-shared-authorization-secret
This commit is contained in:
medmunds
2018-02-07 13:25:48 -08:00
parent 4d34a181b6
commit 1a6086f2b5
14 changed files with 99 additions and 29 deletions

27
tests/test_checks.py Normal file
View File

@@ -0,0 +1,27 @@
from django.core import checks
from django.test import SimpleTestCase
from django.test.utils import override_settings
from anymail.checks import check_deprecated_settings
from .utils import AnymailTestMixin
class DeprecatedSettingsTests(SimpleTestCase, AnymailTestMixin):
@override_settings(ANYMAIL={"WEBHOOK_AUTHORIZATION": "abcde:12345"})
def test_webhook_authorization(self):
errors = check_deprecated_settings(None)
self.assertEqual(errors, [checks.Warning(
"The ANYMAIL setting 'WEBHOOK_AUTHORIZATION' has been renamed 'WEBHOOK_SECRET' to improve security.",
hint="You must update your settings.py. The old name will stop working in a near-future release.",
id="anymail.W001",
)])
@override_settings(ANYMAIL_WEBHOOK_AUTHORIZATION="abcde:12345", ANYMAIL={})
def test_anymail_webhook_authorization(self):
errors = check_deprecated_settings(None)
self.assertEqual(errors, [checks.Warning(
"The ANYMAIL_WEBHOOK_AUTHORIZATION setting has been renamed ANYMAIL_WEBHOOK_SECRET to improve security.",
hint="You must update your settings.py. The old name will stop working in a near-future release.",
id="anymail.W001",
)])

View File

@@ -87,7 +87,7 @@ class MandrillWebhookSecurityTestCase(WebhookTestCase, WebhookBasicAuthTestsMixi
response = self.client.post(**kwargs)
self.assertEqual(response.status_code, 400)
@override_settings(ANYMAIL={}) # clear WEBHOOK_AUTHORIZATION from WebhookTestCase
@override_settings(ANYMAIL={}) # clear WEBHOOK_SECRET from WebhookTestCase
def test_no_basic_auth(self):
# Signature validation should work properly if you're not using basic auth
self.clear_basic_auth()
@@ -99,7 +99,7 @@ class MandrillWebhookSecurityTestCase(WebhookTestCase, WebhookBasicAuthTestsMixi
ALLOWED_HOSTS=['127.0.0.1', '.example.com'],
ANYMAIL={
"MANDRILL_WEBHOOK_URL": "https://abcde:12345@example.com/anymail/mandrill/",
"WEBHOOK_AUTHORIZATION": "abcde:12345",
"WEBHOOK_SECRET": "abcde:12345",
})
def test_webhook_url_setting(self):
# If Django can't build_absolute_uri correctly (e.g., because your proxy
@@ -111,7 +111,7 @@ class MandrillWebhookSecurityTestCase(WebhookTestCase, WebhookBasicAuthTestsMixi
self.assertEqual(response.status_code, 200)
# override WebhookBasicAuthTestsMixin version of this test
@override_settings(ANYMAIL={'WEBHOOK_AUTHORIZATION': ['cred1:pass1', 'cred2:pass2']})
@override_settings(ANYMAIL={'WEBHOOK_SECRET': ['cred1:pass1', 'cred2:pass2']})
def test_supports_credential_rotation(self):
"""You can supply a list of basic auth credentials, and any is allowed"""
self.set_basic_auth('cred1', 'pass1')

View File

@@ -14,7 +14,7 @@ def event_handler(sender, event, esp_name, **kwargs):
pass
@override_settings(ANYMAIL={'WEBHOOK_AUTHORIZATION': 'username:password'})
@override_settings(ANYMAIL={'WEBHOOK_SECRET': 'username:password'})
class WebhookTestCase(AnymailTestMixin, SimpleTestCase):
"""Base for testing webhooks
@@ -111,7 +111,7 @@ class WebhookBasicAuthTestsMixin(object):
response = self.call_webhook()
self.assertEqual(response.status_code, 400)
@override_settings(ANYMAIL={'WEBHOOK_AUTHORIZATION': ['cred1:pass1', 'cred2:pass2']})
@override_settings(ANYMAIL={'WEBHOOK_SECRET': ['cred1:pass1', 'cred2:pass2']})
def test_supports_credential_rotation(self):
"""You can supply a list of basic auth credentials, and any is allowed"""
self.set_basic_auth('cred1', 'pass1')
@@ -125,3 +125,9 @@ class WebhookBasicAuthTestsMixin(object):
self.set_basic_auth('baduser', 'wrongpassword')
response = self.call_webhook()
self.assertEqual(response.status_code, 400)
@override_settings(ANYMAIL={'WEBHOOK_AUTHORIZATION': "username:password"})
def test_deprecated_setting(self):
"""The older WEBHOOK_AUTHORIZATION setting is still supported (for now)"""
response = self.call_webhook()
self.assertEqual(response.status_code, 200)