From 6cb68fb6dbb829b8643a508c6702e789bf4891f2 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Tue, 18 Jun 2019 20:36:26 +0200 Subject: [PATCH] whitelist send_registration_email_next_url using HMAC signature (#34115) --- src/authentic2/constants.py | 1 + src/authentic2/crypto.py | 17 +++++++++++++++++ src/authentic2/utils.py | 18 ++++++++++++++---- src/authentic2/views.py | 2 +- tests/test_api.py | 4 +++- 5 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/authentic2/constants.py b/src/authentic2/constants.py index 7d86849d..f32c226c 100644 --- a/src/authentic2/constants.py +++ b/src/authentic2/constants.py @@ -21,3 +21,4 @@ AUTHENTICATION_EVENTS_SESSION_KEY = 'authentication-events' SWITCH_USER_SESSION_KEY = '_switch_user' LAST_LOGIN_SESSION_KEY = '_last_login' SERVICE_FIELD_NAME = 'service' +NEXT_URL_SIGNATURE = 'next-signature' diff --git a/src/authentic2/crypto.py b/src/authentic2/crypto.py index 465f0f6b..9091a872 100644 --- a/src/authentic2/crypto.py +++ b/src/authentic2/crypto.py @@ -16,6 +16,7 @@ import base64 import hashlib +import hmac import struct from Crypto.Cipher import AES @@ -24,6 +25,8 @@ from Crypto.Hash import SHA256 from Crypto.Hash import HMAC from Crypto import Random +from django.utils.crypto import constant_time_compare + class DecryptionError(Exception): pass @@ -177,3 +180,17 @@ def aes_base64url_deterministic_decrypt(key, urlencoded, salt, raise_on_error=Tr if not raise_on_error: return None raise + + +def hmac_url(key, url): + if hasattr(key, 'encode'): + key = key.encode('utf-8') + if hasattr(url, 'decode'): + url = url.encode('ascii') + return base64.b32encode(hmac.HMAC(key=key, msg=url, digestmod=hashlib.sha256).digest()).strip('=') + + +def check_hmac_url(key, url, signature): + if hasattr(signature, 'decode'): + signature = signature.decode('ascii') + return constant_time_compare(signature, hmac_url(key, url)) diff --git a/src/authentic2/utils.py b/src/authentic2/utils.py index b8c39321..5509603a 100644 --- a/src/authentic2/utils.py +++ b/src/authentic2/utils.py @@ -62,7 +62,7 @@ except ImportError: from authentic2.saml.saml2utils import filter_attribute_private_key, \ filter_element_private_key -from . import plugins, app_settings, constants +from . import plugins, app_settings, constants, crypto class CleanLogMessage(logging.Filter): @@ -731,7 +731,7 @@ def send_account_deletion_mail(request, user): logger.info(u'account deletion mail sent to %s', user.email) -def build_reset_password_url(user, request=None, next_url=None, set_random_password=True): +def build_reset_password_url(user, request=None, next_url=None, set_random_password=True, sign_next_url=True): '''Build a reset password URL''' from .compat import default_token_generator @@ -744,7 +744,10 @@ def build_reset_password_url(user, request=None, next_url=None, set_random_passw if request: reset_url = request.build_absolute_uri(reset_url) if next_url: - reset_url += '?' + urlparse.urlencode({'next': next_url}) + params = {'next': next_url} + if sign_next_url: + params[constants.NEXT_URL_SIGNATURE] = crypto.hmac_url(settings.SECRET_KEY, next_url) + reset_url += '?' + urlparse.urlencode(params) return reset_url, token @@ -754,6 +757,7 @@ def send_password_reset_mail(user, template_names=None, request=None, legacy_subject_templates=['registration/password_reset_subject.txt'], legacy_body_templates=['registration/password_reset_email.html'], set_random_password=True, + sign_next_url=True, **kwargs): from . import middleware @@ -775,7 +779,8 @@ def send_password_reset_mail(user, template_names=None, request=None, # Build reset URL ctx['reset_url'], token = build_reset_password_url(user, request=request, next_url=next_url, - set_random_password=set_random_password) + set_random_password=set_random_password, + sign_next_url=sign_next_url) send_templated_mail(user.email, template_names, ctx, request=request, legacy_subject_templates=legacy_subject_templates, @@ -894,6 +899,11 @@ def good_next_url(request, next_url): return True if same_origin(request.build_absolute_uri(), next_url): return True + signature = request.POST.get(constants.NEXT_URL_SIGNATURE) or request.GET.get(constants.NEXT_URL_SIGNATURE) + + if signature: + return crypto.check_hmac_url(settings.SECRET_KEY, next_url, signature) + for origin in app_settings.A2_REDIRECT_WHITELIST: if same_origin(next_url, origin): return True diff --git a/src/authentic2/views.py b/src/authentic2/views.py index 0370d2a6..d9575c8b 100644 --- a/src/authentic2/views.py +++ b/src/authentic2/views.py @@ -644,7 +644,7 @@ class PasswordResetView(cbv.NextURLViewMixin, FormView): def get_form_kwargs(self, **kwargs): kwargs = super(PasswordResetView, self).get_form_kwargs(**kwargs) initial = kwargs.setdefault('initial', {}) - initial['next_url'] = self.request.GET.get(REDIRECT_FIELD_NAME, '') + initial['next_url'] = utils.select_next_url(self.request, '') return kwargs def get_context_data(self, **kwargs): diff --git a/tests/test_api.py b/tests/test_api.py index cf3be3cb..3ed7ae2a 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -575,6 +575,7 @@ def test_api_users_create_send_mail(app, settings, superuser): 'email': 'john.doe@example.net', 'title': 'Mr', 'send_registration_email': True, + 'send_registration_email_next_url': 'http://example.com/', } assert len(mail.outbox) == 0 resp = app.post_json('/api/users/', params=payload, status=201) @@ -586,9 +587,10 @@ def test_api_users_create_send_mail(app, settings, superuser): resp = app.get(relative_url, status=200) resp.form.set('new_password1', '1234==aA') resp.form.set('new_password2', '1234==aA') - resp = resp.form.submit().follow() + resp = resp.form.submit() # Check user was properly logged in assert str(app.session['_auth_user_id']) == str(user_id) + assert resp.location == 'http://example.com/' def test_api_users_create_force_password_reset(app, client, settings, superuser): -- 2.20.1