From 55da623f02ccfd762009352ce1b40a7447f1423c Mon Sep 17 00:00:00 2001 From: James Murty Date: Wed, 24 Jul 2019 10:53:34 +1000 Subject: [PATCH 1/5] Print name to real Bandit settings module --- ixc_django_docker/settings/email_bandit.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/ixc_django_docker/settings/email_bandit.py b/ixc_django_docker/settings/email_bandit.py index 6800027..7b9632c 100644 --- a/ixc_django_docker/settings/email_bandit.py +++ b/ixc_django_docker/settings/email_bandit.py @@ -1,5 +1,11 @@ import os + +# When loaded by django-split-settings __name__ gives us the *includer* file's +# name, not the name of this *included* file. +REAL_MODULE_NAME = ".".join([__package__, "email_bandit"]) + + # Hijack django-post-office backend if project is using that lib... if 'POST_OFFICE' in locals(): HIJACKED_EMAIL_BACKEND = POST_OFFICE['BACKENDS']['default'] @@ -22,7 +28,7 @@ ] else: BANDIT_EMAIL = None -print("%s: BANDIT_EMAIL = %r" % (__name__, BANDIT_EMAIL)) +print("%s: BANDIT_EMAIL = %r" % (REAL_MODULE_NAME, BANDIT_EMAIL)) # Whitelist outgoing emails to these specific addresses or domains to let # them through, instead of redirecting them to the BANDIT_EMAIL address. @@ -34,9 +40,9 @@ for wl in os.environ['BANDIT_WHITELIST'].split(',') if wl.strip() ] - print("%s: BANDIT_WHITELIST = %r" % (__name__, BANDIT_WHITELIST)) else: - print("%s: BANDIT_WHITELIST is not set" % __name__) + BANDIT_WHITELIST = [] +print("%s: BANDIT_WHITELIST = %r" % (REAL_MODULE_NAME, BANDIT_WHITELIST)) # Make it clear that emails have been hijacked and from which site. # NOTE: This only applies to emails sent with admin-specific methods: From f3c7594c285957af672e2c9f6ac51716ba49dfca Mon Sep 17 00:00:00 2001 From: James Murty Date: Wed, 24 Jul 2019 10:54:44 +1000 Subject: [PATCH 2/5] Raise exception if BANDIT_EMAIL is unset or empty --- ixc_django_docker/settings/email_bandit.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ixc_django_docker/settings/email_bandit.py b/ixc_django_docker/settings/email_bandit.py index 7b9632c..9df5083 100644 --- a/ixc_django_docker/settings/email_bandit.py +++ b/ixc_django_docker/settings/email_bandit.py @@ -44,6 +44,15 @@ BANDIT_WHITELIST = [] print("%s: BANDIT_WHITELIST = %r" % (REAL_MODULE_NAME, BANDIT_WHITELIST)) + +# Ensure that BANDIT_EMAIL is set appropriately: it is always required and +# must contain at least one value +if not BANDIT_EMAIL: + raise ValueError( + "BANDIT_EMAIL environment variable must be set with at least one" + " email address. If you do not want to hijack email, remove" + " 'email_bandit.py' from the BASE_SETTINGS environment variable") + # Make it clear that emails have been hijacked and from which site. # NOTE: This only applies to emails sent with admin-specific methods: # https://docs.djangoproject.com/en/2.2/ref/settings/#email-subject-prefix From 8f3f7fe131bba8bb4d9d66f9aa52410934c4eb12 Mon Sep 17 00:00:00 2001 From: James Murty Date: Wed, 24 Jul 2019 10:55:58 +1000 Subject: [PATCH 3/5] Print emails automatically whitelisted by Bandit --- ixc_django_docker/settings/email_bandit.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ixc_django_docker/settings/email_bandit.py b/ixc_django_docker/settings/email_bandit.py index 9df5083..03afc7a 100644 --- a/ixc_django_docker/settings/email_bandit.py +++ b/ixc_django_docker/settings/email_bandit.py @@ -1,5 +1,7 @@ import os +from django.conf import settings + # When loaded by django-split-settings __name__ gives us the *includer* file's # name, not the name of this *included* file. @@ -44,6 +46,16 @@ BANDIT_WHITELIST = [] print("%s: BANDIT_WHITELIST = %r" % (REAL_MODULE_NAME, BANDIT_WHITELIST)) +# Print the additional emails whitelisted by Bandit by default, to make it +# clearer that this is what Bandit does. See logic in +# `bandit.backends.base:HijackBackendMixin.send_messages()` +admin_emails = [email for name, email in getattr(settings, 'ADMINS', [])] +server_email = getattr(settings, 'SERVER_EMAIL', 'root@localhost') +extra_whitelisted = admin_emails + [server_email] +print( + "%s: Emails automatically whitelisted by Bandit, from `settings.ADMINS` and" + " `settings.SERVER_EMAIL` = %r" % (REAL_MODULE_NAME, extra_whitelisted) +) # Ensure that BANDIT_EMAIL is set appropriately: it is always required and # must contain at least one value From 0f7d0ff82ec9ae3aaea7927554e4ea7187be3d5e Mon Sep 17 00:00:00 2001 From: James Murty Date: Wed, 24 Jul 2019 15:00:04 +1000 Subject: [PATCH 4/5] Avoid potentially risky import of django.conf.settings Read setting variables directly from `locals()` instead, where they should be present. If not, the lookups fall back to default values. Using `locals()` is a hack, but less hacky than wrapping each setting lookup in a try/catch block to guard against them being unset in general, or unset by the time this composable setting file is loaded. --- ixc_django_docker/settings/email_bandit.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ixc_django_docker/settings/email_bandit.py b/ixc_django_docker/settings/email_bandit.py index 03afc7a..25220a0 100644 --- a/ixc_django_docker/settings/email_bandit.py +++ b/ixc_django_docker/settings/email_bandit.py @@ -1,7 +1,5 @@ import os -from django.conf import settings - # When loaded by django-split-settings __name__ gives us the *includer* file's # name, not the name of this *included* file. @@ -49,8 +47,8 @@ # Print the additional emails whitelisted by Bandit by default, to make it # clearer that this is what Bandit does. See logic in # `bandit.backends.base:HijackBackendMixin.send_messages()` -admin_emails = [email for name, email in getattr(settings, 'ADMINS', [])] -server_email = getattr(settings, 'SERVER_EMAIL', 'root@localhost') +admin_emails = [email for name, email in locals().get('ADMINS', [])] +server_email = locals().get('SERVER_EMAIL', 'root@localhost') extra_whitelisted = admin_emails + [server_email] print( "%s: Emails automatically whitelisted by Bandit, from `settings.ADMINS` and" From a22cb40daa9b06fa4e7786a37dc4ca765ff3c3b2 Mon Sep 17 00:00:00 2001 From: James Murty Date: Tue, 30 Jul 2019 11:40:20 +1000 Subject: [PATCH 5/5] Drop use of locals() --- ixc_django_docker/settings/email_bandit.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ixc_django_docker/settings/email_bandit.py b/ixc_django_docker/settings/email_bandit.py index 25220a0..d540cf0 100644 --- a/ixc_django_docker/settings/email_bandit.py +++ b/ixc_django_docker/settings/email_bandit.py @@ -7,12 +7,13 @@ # Hijack django-post-office backend if project is using that lib... -if 'POST_OFFICE' in locals(): +try: + # Lookup of POST_OFFICE setting should fail if post-office isn't used HIJACKED_EMAIL_BACKEND = POST_OFFICE['BACKENDS']['default'] POST_OFFICE['BACKENDS']['default'] = \ 'ixc_django_docker.bandit.HijackedEmailBackend' # ...otherwise hijack default Django backend -else: +except NameError: HIJACKED_EMAIL_BACKEND = EMAIL_BACKEND EMAIL_BACKEND = 'ixc_django_docker.bandit.HijackedEmailBackend' @@ -47,9 +48,8 @@ # Print the additional emails whitelisted by Bandit by default, to make it # clearer that this is what Bandit does. See logic in # `bandit.backends.base:HijackBackendMixin.send_messages()` -admin_emails = [email for name, email in locals().get('ADMINS', [])] -server_email = locals().get('SERVER_EMAIL', 'root@localhost') -extra_whitelisted = admin_emails + [server_email] +admin_emails = [email for name, email in ADMINS] +extra_whitelisted = admin_emails + [SERVER_EMAIL] print( "%s: Emails automatically whitelisted by Bandit, from `settings.ADMINS` and" " `settings.SERVER_EMAIL` = %r" % (REAL_MODULE_NAME, extra_whitelisted)