From 181d5886ebcfaf601391f713b56e970574af860f Mon Sep 17 00:00:00 2001 From: medmunds Date: Sun, 7 Jul 2019 13:43:08 -0700 Subject: [PATCH] Add MAILGUN_WEBHOOK_SIGNING_KEY setting. Fixes #153. --- CHANGELOG.rst | 19 +++++++++++ anymail/webhooks/mailgun.py | 14 +++++--- docs/esps/mailgun.rst | 58 ++++++++++++++++++++++++++++------ tests/test_mailgun_inbound.py | 4 +-- tests/test_mailgun_webhooks.py | 56 ++++++++++++++++++++++++-------- 5 files changed, 122 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 3d90e29..a712f00 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -25,6 +25,22 @@ Release history ^^^^^^^^^^^^^^^ .. This extra heading level keeps the ToC from becoming unmanageably long +vNext +----- + +*UNRELEASED* + +Fixes +~~~~~ + +* **Mailgun:** Add new `MAILGUN_WEBHOOK_SIGNING_KEY` setting for verifying tracking and + inbound webhook calls. Mailgun's webhook signing key can become different from your + `MAILGUN_API_KEY` if you have ever rotated either key. + See `docs `__. + (More in `#153`_. Thanks to `@dominik-lekse`_ for reporting the problem and Mailgun's + `@mbk-ok`_ for identifying the cause.) + + v6.0.1 ------ @@ -945,17 +961,20 @@ Features .. _#115: https://github.com/anymail/issues/115 .. _#147: https://github.com/anymail/issues/147 .. _#148: https://github.com/anymail/issues/148 +.. _#153: https://github.com/anymail/issues/153 .. _@ailionx: https://github.com/ailionx .. _@calvin: https://github.com/calvin .. _@costela: https://github.com/costela .. _@decibyte: https://github.com/decibyte +.. _@dominik-lekse: https://github.com/dominik-lekse .. _@ewingrj: https://github.com/ewingrj .. _@fdemmer: https://github.com/fdemmer .. _@janneThoft: https://github.com/janneThoft .. _@joshkersey: https://github.com/joshkersey .. _@Lekensteyn: https://github.com/Lekensteyn .. _@lewistaylor: https://github.com/lewistaylor +.. _@mbk-ok: https://github.com/mbk-ok .. _@RignonNoel: https://github.com/RignonNoel .. _@sebbacon: https://github.com/sebbacon .. _@varche1: https://github.com/varche1 diff --git a/anymail/webhooks/mailgun.py b/anymail/webhooks/mailgun.py index ec4ab51..123fb75 100644 --- a/anymail/webhooks/mailgun.py +++ b/anymail/webhooks/mailgun.py @@ -10,7 +10,7 @@ from .base import AnymailBaseWebhookView from ..exceptions import AnymailConfigurationError, AnymailWebhookValidationFailure, AnymailInvalidAddress from ..inbound import AnymailInboundMessage from ..signals import inbound, tracking, AnymailInboundEvent, AnymailTrackingEvent, EventType, RejectReason -from ..utils import get_anymail_setting, combine, querydict_getfirst, parse_single_address +from ..utils import get_anymail_setting, combine, querydict_getfirst, parse_single_address, UNSET class MailgunBaseWebhookView(AnymailBaseWebhookView): @@ -19,12 +19,18 @@ class MailgunBaseWebhookView(AnymailBaseWebhookView): esp_name = "Mailgun" warn_if_no_basic_auth = False # because we validate against signature + webhook_signing_key = None # (Declaring class attr allows override by kwargs in View.as_view.) + + # The `api_key` attribute name is still allowed for compatibility with earlier Anymail releases. api_key = None # (Declaring class attr allows override by kwargs in View.as_view.) def __init__(self, **kwargs): + # webhook_signing_key: falls back to api_key if webhook_signing_key not provided api_key = get_anymail_setting('api_key', esp_name=self.esp_name, - kwargs=kwargs, allow_bare=True) - self.api_key = api_key.encode('ascii') # hmac.new requires bytes key in python 3 + kwargs=kwargs, allow_bare=True, default=None) + webhook_signing_key = get_anymail_setting('webhook_signing_key', esp_name=self.esp_name, + kwargs=kwargs, default=UNSET if api_key is None else api_key) + self.webhook_signing_key = webhook_signing_key.encode('ascii') # hmac.new requires bytes key in python 3 super(MailgunBaseWebhookView, self).__init__(**kwargs) def validate_request(self, request): @@ -52,7 +58,7 @@ class MailgunBaseWebhookView(AnymailBaseWebhookView): except KeyError: raise AnymailWebhookValidationFailure("Mailgun webhook called without required security fields") - expected_signature = hmac.new(key=self.api_key, msg='{}{}'.format(timestamp, token).encode('ascii'), + expected_signature = hmac.new(key=self.webhook_signing_key, msg='{}{}'.format(timestamp, token).encode('ascii'), digestmod=hashlib.sha256).hexdigest() if not constant_time_compare(signature, expected_signature): raise AnymailWebhookValidationFailure("Mailgun webhook called with incorrect signature") diff --git a/docs/esps/mailgun.rst b/docs/esps/mailgun.rst index 850e104..5b58074 100644 --- a/docs/esps/mailgun.rst +++ b/docs/esps/mailgun.rst @@ -26,7 +26,8 @@ in your settings.py. .. rubric:: MAILGUN_API_KEY -Required. Your Mailgun API key: +Required for sending. Your Mailgun "Private API key" from the Mailgun +`API security settings`_: .. code-block:: python @@ -54,6 +55,27 @@ Mailgun sender domain, this setting is not needed. See :ref:`mailgun-sender-domain` below for examples. +.. setting:: ANYMAIL_MAILGUN_WEBHOOK_SIGNING_KEY + +.. rubric:: MAILGUN_WEBHOOK_SIGNING_KEY + +.. versionadded:: 6.1 + +Required for tracking or inbound webhooks. Your "HTTP webhook signing key" from the +Mailgun `API security settings`_: + + .. code-block:: python + + ANYMAIL = { + ... + "MAILGUN_WEBHOOK_SIGNING_KEY": "", + } + +If not provided, Anymail will attempt to validate webhooks using the +:setting:`MAILGUN_API_KEY ` setting instead. (These two keys have +the same values for new Mailgun users, but will diverge if you ever rotate either key.) + + .. setting:: ANYMAIL_MAILGUN_API_URL .. rubric:: MAILGUN_API_URL @@ -75,6 +97,9 @@ region: } +.. _API security settings: https://app.mailgun.com/app/account/security/api_keys + + .. _mailgun-sender-domain: Email sender domain @@ -260,9 +285,14 @@ Status tracking webhooks Added support for Mailgun's June, 2018 (non-"legacy") webhook format. +.. versionchanged:: 6.1 + + Added support for a new :setting:`MAILGUN_WEBHOOK_SIGNING_KEY ` + setting, separate from your MAILGUN_API_KEY. + If you are using Anymail's normalized :ref:`status tracking `, enter -the url in the `Mailgun webhooks dashboard`_. (Be sure to select the correct sending -domain---Mailgun's sandbox and production domains have separate webhook settings.) +the url in the Mailgun webhooks config for your domain. (Be sure to select the correct +sending domain---Mailgun's sandbox and production domains have separate webhook settings.) Mailgun allows you to enter a different URL for each event type: just enter this same Anymail tracking URL for all events you want to receive: @@ -273,8 +303,9 @@ Anymail tracking URL for all events you want to receive: * *yoursite.example.com* is your Django site Mailgun implements a limited form of webhook signing, and Anymail will verify -these signatures (based on your :setting:`MAILGUN_API_KEY ` -Anymail setting). By default, Mailgun's webhook signature provides similar security +these signatures against your +:setting:`MAILGUN_WEBHOOK_SIGNING_KEY ` +Anymail setting. By default, Mailgun's webhook signature provides similar security to Anymail's shared webhook secret, so it's acceptable to omit the :setting:`ANYMAIL_WEBHOOK_SECRET` setting (and "{random}:{random}@" portion of the webhook url) with Mailgun webhooks. @@ -321,7 +352,6 @@ Mailgun's other event APIs.) newer, non-legacy webhooks.) -.. _Mailgun webhooks dashboard: https://mailgun.com/app/webhooks .. _Mailgun webhook payload: https://documentation.mailgun.com/en/latest/user_manual.html#webhooks @@ -333,7 +363,7 @@ Inbound webhook If you want to receive email from Mailgun through Anymail's normalized :ref:`inbound ` handling, follow Mailgun's `Receiving, Storing and Fowarding Messages`_ guide to set up an inbound route that forwards to Anymail's inbound webhook. (You can configure routes -using Mailgun's API, or simply using the `Mailgun routes dashboard`_.) +using Mailgun's API, or simply using the `Mailgun receiving config`_.) The *action* for your route will be either: @@ -352,9 +382,17 @@ received email (including complex forms like multi-message mailing list digests) If you want to use Anymail's normalized :attr:`~anymail.inbound.AnymailInboundMessage.spam_detected` and :attr:`~anymail.inbound.AnymailInboundMessage.spam_score` attributes, you'll need to set your Mailgun domain's inbound spam filter to "Deliver spam, but add X-Mailgun-SFlag and X-Mailgun-SScore headers" -(in the `Mailgun domains dashboard`_). +(in the `Mailgun domains config`_). + +Anymail will verify Mailgun inbound message events using your +:setting:`MAILGUN_WEBHOOK_SIGNING_KEY ` +Anymail setting. By default, Mailgun's webhook signature provides similar security +to Anymail's shared webhook secret, so it's acceptable to omit the +:setting:`ANYMAIL_WEBHOOK_SECRET` setting (and "{random}:{random}@" portion of the +action) with Mailgun inbound routing. + .. _Receiving, Storing and Fowarding Messages: https://documentation.mailgun.com/en/latest/user_manual.html#receiving-forwarding-and-storing-messages -.. _Mailgun routes dashboard: https://app.mailgun.com/app/routes -.. _Mailgun domains dashboard: https://app.mailgun.com/app/domains +.. _Mailgun receiving config: https://app.mailgun.com/app/receiving/routes +.. _Mailgun domains config: https://app.mailgun.com/app/sending/domains diff --git a/tests/test_mailgun_inbound.py b/tests/test_mailgun_inbound.py index cbd64bf..23345ce 100644 --- a/tests/test_mailgun_inbound.py +++ b/tests/test_mailgun_inbound.py @@ -13,14 +13,14 @@ from anymail.signals import AnymailInboundEvent from anymail.webhooks.mailgun import MailgunInboundWebhookView from .test_mailgun_webhooks import ( - TEST_API_KEY, mailgun_sign_payload, + TEST_WEBHOOK_SIGNING_KEY, mailgun_sign_payload, mailgun_sign_legacy_payload, querydict_to_postdict) from .utils import sample_image_content, sample_email_content from .webhook_cases import WebhookTestCase @tag('mailgun') -@override_settings(ANYMAIL_MAILGUN_API_KEY=TEST_API_KEY) +@override_settings(ANYMAIL_MAILGUN_WEBHOOK_SIGNING_KEY=TEST_WEBHOOK_SIGNING_KEY) class MailgunInboundTestCase(WebhookTestCase): def test_inbound_basics(self): raw_event = mailgun_sign_legacy_payload({ diff --git a/tests/test_mailgun_webhooks.py b/tests/test_mailgun_webhooks.py index 766979e..6225daa 100644 --- a/tests/test_mailgun_webhooks.py +++ b/tests/test_mailgun_webhooks.py @@ -14,19 +14,19 @@ from anymail.webhooks.mailgun import MailgunTrackingWebhookView from .webhook_cases import WebhookTestCase, WebhookBasicAuthTestsMixin -TEST_API_KEY = 'TEST_API_KEY' +TEST_WEBHOOK_SIGNING_KEY = 'TEST_WEBHOOK_SIGNING_KEY' -def mailgun_signature(timestamp, token, api_key): +def mailgun_signature(timestamp, token, webhook_signing_key): """Generates a Mailgun webhook signature""" # https://documentation.mailgun.com/en/latest/user_manual.html#securing-webhooks return hmac.new( - key=api_key.encode('ascii'), + key=webhook_signing_key.encode('ascii'), msg='{timestamp}{token}'.format(timestamp=timestamp, token=token).encode('ascii'), digestmod=hashlib.sha256).hexdigest() -def mailgun_sign_payload(data, api_key=TEST_API_KEY): +def mailgun_sign_payload(data, webhook_signing_key=TEST_WEBHOOK_SIGNING_KEY): """Add or complete Mailgun webhook signature block in data dict""" # Modifies the dict in place event_data = data.get('event-data', {}) @@ -34,16 +34,16 @@ def mailgun_sign_payload(data, api_key=TEST_API_KEY): token = signature.setdefault('token', '1234567890abcdef1234567890abcdef') timestamp = signature.setdefault('timestamp', str(int(float(event_data.get('timestamp', '1234567890.123'))))) - signature['signature'] = mailgun_signature(timestamp, token, api_key=api_key) + signature['signature'] = mailgun_signature(timestamp, token, webhook_signing_key=webhook_signing_key) return data -def mailgun_sign_legacy_payload(data, api_key=TEST_API_KEY): +def mailgun_sign_legacy_payload(data, webhook_signing_key=TEST_WEBHOOK_SIGNING_KEY): """Add a Mailgun webhook signature to data dict""" # Modifies the dict in place data.setdefault('timestamp', '1234567890') data.setdefault('token', '1234567890abcdef1234567890abcdef') - data['signature'] = mailgun_signature(data['timestamp'], data['token'], api_key=api_key) + data['signature'] = mailgun_signature(data['timestamp'], data['token'], webhook_signing_key=webhook_signing_key) return data @@ -61,14 +61,44 @@ def querydict_to_postdict(qd): @tag('mailgun') class MailgunWebhookSettingsTestCase(WebhookTestCase): - def test_requires_api_key(self): - with self.assertRaises(ImproperlyConfigured): + def test_requires_webhook_signing_key(self): + with self.assertRaisesMessage(ImproperlyConfigured, "MAILGUN_WEBHOOK_SIGNING_KEY"): self.client.post('/anymail/mailgun/tracking/', content_type="application/json", data=json.dumps(mailgun_sign_payload({'event-data': {'event': 'delivered'}}))) + @override_settings( + ANYMAIL_MAILGUN_API_KEY='TEST_API_KEY', + ANYMAIL_MAILGUN_WEBHOOK_SIGNING_KEY='TEST_WEBHOOK_SIGNING_KEY', + ) + def test_webhook_signing_is_different_from_api_key(self): + """Webhooks should use MAILGUN_WEBHOOK_SIGNING_KEY, not MAILGUN_API_KEY, if both provided""" + payload = json.dumps(mailgun_sign_payload({'event-data': {'event': 'delivered'}}, + webhook_signing_key='TEST_WEBHOOK_SIGNING_KEY')) + response = self.client.post('/anymail/mailgun/tracking/', content_type="application/json", data=payload) + self.assertEqual(response.status_code, 200) + + @override_settings(ANYMAIL_MAILGUN_API_KEY='TEST_API_KEY') + def test_defaults_webhook_signing_to_api_key(self): + """Webhooks should default to MAILGUN_API_KEY if MAILGUN_WEBHOOK_SIGNING_KEY not provided""" + payload = json.dumps(mailgun_sign_payload({'event-data': {'event': 'delivered'}}, + webhook_signing_key='TEST_API_KEY')) + response = self.client.post('/anymail/mailgun/tracking/', content_type="application/json", data=payload) + self.assertEqual(response.status_code, 200) + + def test_webhook_signing_key_view_params(self): + """Webhook signing key can be provided as a view param""" + view = MailgunTrackingWebhookView.as_view(webhook_signing_key='VIEW_SIGNING_KEY') + view_instance = view.view_class(**view.view_initkwargs) + self.assertEqual(view_instance.webhook_signing_key, b'VIEW_SIGNING_KEY') + + # Can also use `api_key` param for backwards compatiblity with earlier Anymail versions + view = MailgunTrackingWebhookView.as_view(api_key='VIEW_API_KEY') + view_instance = view.view_class(**view.view_initkwargs) + self.assertEqual(view_instance.webhook_signing_key, b'VIEW_API_KEY') + @tag('mailgun') -@override_settings(ANYMAIL_MAILGUN_API_KEY=TEST_API_KEY) +@override_settings(ANYMAIL_MAILGUN_WEBHOOK_SIGNING_KEY=TEST_WEBHOOK_SIGNING_KEY) class MailgunWebhookSecurityTestCase(WebhookTestCase, WebhookBasicAuthTestsMixin): should_warn_if_no_auth = False # because we check webhook signature @@ -90,14 +120,14 @@ class MailgunWebhookSecurityTestCase(WebhookTestCase, WebhookBasicAuthTestsMixin def test_verifies_bad_signature(self): data = mailgun_sign_payload({'event-data': {'event': 'delivered'}}, - api_key="wrong API key") + webhook_signing_key="wrong signing key") response = self.client.post('/anymail/mailgun/tracking/', content_type="application/json", data=json.dumps(data)) self.assertEqual(response.status_code, 400) @tag('mailgun') -@override_settings(ANYMAIL_MAILGUN_API_KEY=TEST_API_KEY) +@override_settings(ANYMAIL_MAILGUN_WEBHOOK_SIGNING_KEY=TEST_WEBHOOK_SIGNING_KEY) class MailgunTestCase(WebhookTestCase): # Tests for Mailgun's new webhooks (announced 2018-06-29) @@ -449,7 +479,7 @@ class MailgunTestCase(WebhookTestCase): @tag('mailgun') -@override_settings(ANYMAIL_MAILGUN_API_KEY=TEST_API_KEY) +@override_settings(ANYMAIL_MAILGUN_WEBHOOK_SIGNING_KEY=TEST_WEBHOOK_SIGNING_KEY) class MailgunLegacyTestCase(WebhookTestCase): # Tests for Mailgun's "legacy" webhooks # (which were the only webhooks available prior to Anymail 4.0)