Skip to content

Commit

Permalink
Security: rename WEBHOOK_AUTHORIZATION --> WEBHOOK_SECRET
Browse files Browse the repository at this point in the history
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
  • Loading branch information
medmunds committed Feb 8, 2018
1 parent 4d34a18 commit 1a6086f
Show file tree
Hide file tree
Showing 14 changed files with 99 additions and 29 deletions.
5 changes: 4 additions & 1 deletion anymail/apps.py
@@ -1,9 +1,12 @@
from django.apps import AppConfig
from django.core import checks

from .checks import check_deprecated_settings


class AnymailBaseConfig(AppConfig):
name = 'anymail'
verbose_name = "Anymail"

def ready(self):
pass
checks.register(check_deprecated_settings)
24 changes: 24 additions & 0 deletions anymail/checks.py
@@ -0,0 +1,24 @@
from django.conf import settings
from django.core import checks


def check_deprecated_settings(app_configs, **kwargs):
errors = []

anymail_settings = getattr(settings, "ANYMAIL", {})

# anymail.W001: rename WEBHOOK_AUTHORIZATION to WEBHOOK_SECRET
if "WEBHOOK_AUTHORIZATION" in anymail_settings:
errors.append(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",
))
if hasattr(settings, "ANYMAIL_WEBHOOK_AUTHORIZATION"):
errors.append(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",
))

return errors
8 changes: 6 additions & 2 deletions anymail/webhooks/base.py
Expand Up @@ -24,15 +24,19 @@ class AnymailBasicAuthMixin(object):
basic_auth = None # (Declaring class attr allows override by kwargs in View.as_view.)

def __init__(self, **kwargs):
self.basic_auth = get_anymail_setting('webhook_authorization', default=[],
self.basic_auth = get_anymail_setting('webhook_secret', default=[],
kwargs=kwargs) # no esp_name -- auth is shared between ESPs
if not self.basic_auth:
# Temporarily allow deprecated WEBHOOK_AUTHORIZATION setting
self.basic_auth = get_anymail_setting('webhook_authorization', default=[], kwargs=kwargs)

# Allow a single string:
if isinstance(self.basic_auth, six.string_types):
self.basic_auth = [self.basic_auth]
if self.warn_if_no_basic_auth and len(self.basic_auth) < 1:
warnings.warn(
"Your Anymail webhooks are insecure and open to anyone on the web. "
"You should set WEBHOOK_AUTHORIZATION in your ANYMAIL settings. "
"You should set WEBHOOK_SECRET in your ANYMAIL settings. "
"See 'Securing webhooks' in the Anymail docs.",
AnymailInsecureWebhookWarning)
# noinspection PyArgumentList
Expand Down
4 changes: 2 additions & 2 deletions docs/esps/mailgun.rst
Expand Up @@ -197,7 +197,7 @@ for all events you want to receive:

:samp:`https://{random}:{random}@{yoursite.example.com}/anymail/mailgun/tracking/`

* *random:random* is an :setting:`ANYMAIL_WEBHOOK_AUTHORIZATION` shared secret
* *random:random* is an :setting:`ANYMAIL_WEBHOOK_SECRET` shared secret
* *yoursite.example.com* is your Django site

If you use multiple Mailgun sending domains, you'll need to enter the webhook
Expand Down Expand Up @@ -232,7 +232,7 @@ The *action* for your route will be either:
:samp:`forward("https://{random}:{random}@{yoursite.example.com}/anymail/mailgun/inbound/")`
:samp:`forward("https://{random}:{random}@{yoursite.example.com}/anymail/mailgun/inbound_mime/")`

* *random:random* is an :setting:`ANYMAIL_WEBHOOK_AUTHORIZATION` shared secret
* *random:random* is an :setting:`ANYMAIL_WEBHOOK_SECRET` shared secret
* *yoursite.example.com* is your Django site

Anymail accepts either of Mailgun's "fully-parsed" (.../inbound/) and "raw MIME" (.../inbound_mime/)
Expand Down
4 changes: 2 additions & 2 deletions docs/esps/mailjet.rst
Expand Up @@ -232,7 +232,7 @@ the url in your Mailjet account REST API settings under `Event tracking (trigger

:samp:`https://{random}:{random}@{yoursite.example.com}/anymail/mailjet/tracking/`

* *random:random* is an :setting:`ANYMAIL_WEBHOOK_AUTHORIZATION` shared secret
* *random:random* is an :setting:`ANYMAIL_WEBHOOK_SECRET` shared secret
* *yoursite.example.com* is your Django site

Be sure to enter the URL in the Mailjet settings for all the event types you want to receive.
Expand Down Expand Up @@ -263,7 +263,7 @@ The parseroute Url parameter will be:

:samp:`https://{random}:{random}@{yoursite.example.com}/anymail/mailjet/inbound/`

* *random:random* is an :setting:`ANYMAIL_WEBHOOK_AUTHORIZATION` shared secret
* *random:random* is an :setting:`ANYMAIL_WEBHOOK_SECRET` shared secret
* *yoursite.example.com* is your Django site

Once you've done Mailjet's "basic setup" to configure the Parse API webhook, you can skip
Expand Down
2 changes: 1 addition & 1 deletion docs/esps/mandrill.rst
Expand Up @@ -206,7 +206,7 @@ requires deploying your Django project twice:

:samp:`https://{random}:{random}@{yoursite.example.com}/anymail/mandrill/`

* *random:random* is an :setting:`ANYMAIL_WEBHOOK_AUTHORIZATION` shared secret
* *random:random* is an :setting:`ANYMAIL_WEBHOOK_SECRET` shared secret
* *yoursite.example.com* is your Django site
* (Note: Unlike Anymail's other supported ESPs, the Mandrill webhook uses this
single url for both tracking and inbound events.)
Expand Down
4 changes: 2 additions & 2 deletions docs/esps/postmark.rst
Expand Up @@ -181,7 +181,7 @@ want to receive all these types of events):

:samp:`https://{random}:{random}@{yoursite.example.com}/anymail/postmark/tracking/`

* *random:random* is an :setting:`ANYMAIL_WEBHOOK_AUTHORIZATION` shared secret
* *random:random* is an :setting:`ANYMAIL_WEBHOOK_SECRET` shared secret
* *yoursite.example.com* is your Django site

Anymail doesn't care about the "include bounce content" and "post only on first open"
Expand Down Expand Up @@ -216,7 +216,7 @@ The InboundHookUrl setting will be:

:samp:`https://{random}:{random}@{yoursite.example.com}/anymail/postmark/inbound/`

* *random:random* is an :setting:`ANYMAIL_WEBHOOK_AUTHORIZATION` shared secret
* *random:random* is an :setting:`ANYMAIL_WEBHOOK_SECRET` shared secret
* *yoursite.example.com* is your Django site

Anymail handles the "parse an email" part of Postmark's instructions for you, but you'll
Expand Down
4 changes: 2 additions & 2 deletions docs/esps/sendgrid.rst
Expand Up @@ -284,7 +284,7 @@ the url in your `SendGrid mail settings`_, under "Event Notification":

:samp:`https://{random}:{random}@{yoursite.example.com}/anymail/sendgrid/tracking/`

* *random:random* is an :setting:`ANYMAIL_WEBHOOK_AUTHORIZATION` shared secret
* *random:random* is an :setting:`ANYMAIL_WEBHOOK_SECRET` shared secret
* *yoursite.example.com* is your Django site

Be sure to check the boxes in the SendGrid settings for the event types you want to receive.
Expand Down Expand Up @@ -315,7 +315,7 @@ The Destination URL setting will be:

:samp:`https://{random}:{random}@{yoursite.example.com}/anymail/sendgrid/inbound/`

* *random:random* is an :setting:`ANYMAIL_WEBHOOK_AUTHORIZATION` shared secret
* *random:random* is an :setting:`ANYMAIL_WEBHOOK_SECRET` shared secret
* *yoursite.example.com* is your Django site

Be sure the URL has a trailing slash. (SendGrid's inbound processing won't follow Django's
Expand Down
4 changes: 2 additions & 2 deletions docs/esps/sparkpost.rst
Expand Up @@ -197,7 +197,7 @@ webhook in your `SparkPost account settings under "Webhooks"`_:

* Target URL: :samp:`https://{yoursite.example.com}/anymail/sparkpost/tracking/`
* Authentication: choose "Basic Auth." For username and password enter the two halves of the
*random:random* shared secret you created for your :setting:`ANYMAIL_WEBHOOK_AUTHORIZATION`
*random:random* shared secret you created for your :setting:`ANYMAIL_WEBHOOK_SECRET`
Django setting. (Anymail doesn't support OAuth webhook auth.)
* Events: click "Select" and then *clear* the checkbox for "Relay Events" category (which is for
inbound email). You can leave all the other categories of events checked, or disable
Expand Down Expand Up @@ -235,7 +235,7 @@ The target parameter for the Relay Webhook will be:

:samp:`https://{random}:{random}@{yoursite.example.com}/anymail/sparkpost/inbound/`

* *random:random* is an :setting:`ANYMAIL_WEBHOOK_AUTHORIZATION` shared secret
* *random:random* is an :setting:`ANYMAIL_WEBHOOK_SECRET` shared secret
* *yoursite.example.com* is your Django site

.. _Enabling Inbound Email Relaying:
Expand Down
20 changes: 13 additions & 7 deletions docs/installation.rst
Expand Up @@ -98,22 +98,22 @@ Skip this section if you won't be using Anymail's webhooks.
or subject your app to malicious input data.

At a minimum, your site should **use https** and you should
configure **webhook authorization** as described below.
configure a **webhook secret** as described below.

See :ref:`securing-webhooks` for additional information.


If you want to use Anymail's inbound or tracking webhooks:

1. In your :file:`settings.py`, add
:setting:`WEBHOOK_AUTHORIZATION <ANYMAIL_WEBHOOK_AUTHORIZATION>`
:setting:`WEBHOOK_SECRET <ANYMAIL_WEBHOOK_SECRET>`
to the ``ANYMAIL`` block:

.. code-block:: python
ANYMAIL = {
...
'WEBHOOK_AUTHORIZATION': '<a random string>:<another random string>',
'WEBHOOK_SECRET': '<a random string>:<another random string>',
}
This setting should be a string with two sequences of random characters,
Expand All @@ -133,7 +133,7 @@ If you want to use Anymail's inbound or tracking webhooks:
(This setting is actually an HTTP basic auth string. You can also set it
to a list of auth strings, to simplify credential rotation or use different auth
with different ESPs. See :setting:`ANYMAIL_WEBHOOK_AUTHORIZATION` in the
with different ESPs. See :setting:`ANYMAIL_WEBHOOK_SECRET` in the
:ref:`securing-webhooks` docs for more details.)


Expand All @@ -160,7 +160,7 @@ If you want to use Anymail's inbound or tracking webhooks:
:samp:`https://{random}:{random}@{yoursite.example.com}/anymail/{esp}/{type}/`

* "https" (rather than http) is *strongly recommended*
* *random:random* is the WEBHOOK_AUTHORIZATION string you created in step 1
* *random:random* is the WEBHOOK_SECRET string you created in step 1
* *yoursite.example.com* is your Django site
* "anymail" is the url prefix (from step 2)
* *esp* is the lowercase name of your ESP (e.g., "sendgrid" or "mailgun")
Expand Down Expand Up @@ -266,20 +266,26 @@ Set to `True` to ignore these problems and send the email anyway. See
:ref:`unsupported-features`. (Default `False`.)


.. rubric:: WEBHOOK_AUTHORIZATION
.. rubric:: WEBHOOK_SECRET

A `'random:random'` shared secret string. Anymail will reject incoming webhook calls
from your ESP that don't include this authorization. You can also give a list of
shared secret strings, and Anymail will allow ESP webhook calls that match any of them
(to facilitate credential rotation). See :ref:`securing-webhooks`.

Default is unset, which leaves your webhooks insecure. Anymail
will warn if you try to use webhooks with setting up authorization.
will warn if you try to use webhooks without a shared secret.

This is actually implemented using HTTP basic authorization, and the string is
technically a "username:password" format. But you should *not* use any real
username or password for this shared secret.

.. versionchanged:: 1.4

The earlier WEBHOOK_AUTHORIZATION setting was renamed WEBHOOK_SECRET, so that
Django error reporting sanitizes it. The old name is still allowed in v1.4,
but will be removed in a near-future release. You should update your settings.


.. setting:: ANYMAIL_REQUESTS_TIMEOUT

Expand Down
6 changes: 3 additions & 3 deletions docs/tips/securing_webhooks.rst
Expand Up @@ -29,7 +29,7 @@ If you aren't able to use https on your Django site, then you should
not set up your ESP's webhooks.


.. setting:: ANYMAIL_WEBHOOK_AUTHORIZATION
.. setting:: ANYMAIL_WEBHOOK_SECRET

Use a shared authorization secret
---------------------------------
Expand All @@ -41,7 +41,7 @@ with webhook data, to prove the post is coming from your ESP.

Most ESPs recommend using HTTP basic authorization as this shared
secret. Anymail includes support for this, via the
:setting:`!ANYMAIL_WEBHOOK_AUTHORIZATION` setting.
:setting:`!ANYMAIL_WEBHOOK_SECRET` setting.
Basic usage is covered in the
:ref:`webhooks configuration <webhooks-configuration>` docs.

Expand All @@ -60,7 +60,7 @@ any of the authorization strings:
ANYMAIL = {
...
'WEBHOOK_AUTHORIZATION': [
'WEBHOOK_SECRET': [
'abcdefghijklmnop:qrstuvwxyz0123456789',
'ZYXWVUTSRQPONMLK:JIHGFEDCBA9876543210',
],
Expand Down
27 changes: 27 additions & 0 deletions tests/test_checks.py
@@ -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",
)])
6 changes: 3 additions & 3 deletions tests/test_mandrill_webhooks.py
Expand Up @@ -87,7 +87,7 @@ def test_verifies_bad_signature(self):
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()
Expand All @@ -99,7 +99,7 @@ def test_no_basic_auth(self):
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
Expand All @@ -111,7 +111,7 @@ def test_webhook_url_setting(self):
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')
Expand Down
10 changes: 8 additions & 2 deletions tests/webhook_cases.py
Expand Up @@ -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
Expand Down Expand Up @@ -111,7 +111,7 @@ def test_verifies_missing_auth(self):
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')
Expand All @@ -125,3 +125,9 @@ def test_supports_credential_rotation(self):
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)

0 comments on commit 1a6086f

Please sign in to comment.