diff --git a/src/pretix/base/migrations/0264_order_internal_secret.py b/src/pretix/base/migrations/0264_order_internal_secret.py new file mode 100644 index 0000000000..f7aae4dc71 --- /dev/null +++ b/src/pretix/base/migrations/0264_order_internal_secret.py @@ -0,0 +1,24 @@ +# Generated by Django 4.2.11 on 2024-05-16 11:07 + +from django.db import migrations, models + +import pretix.base.models.orders + + +class Migration(migrations.Migration): + + dependencies = [ + ("pretixbase", "0263_auto_20240409_0732"), + ] + + operations = [ + migrations.AddField( + model_name="order", + name="internal_secret", + field=models.CharField( + default=None, + max_length=32, + null=True, + ), + ), + ] diff --git a/src/pretix/base/models/orders.py b/src/pretix/base/models/orders.py index 465ce45ca7..ef7b979035 100644 --- a/src/pretix/base/models/orders.py +++ b/src/pretix/base/models/orders.py @@ -35,6 +35,7 @@ import copy import hashlib +import hmac import json import logging import operator @@ -59,7 +60,7 @@ from django.db.models.functions import Coalesce, Greatest from django.db.models.signals import post_delete from django.dispatch import receiver from django.urls import reverse -from django.utils.crypto import get_random_string +from django.utils.crypto import get_random_string, salted_hmac from django.utils.encoding import escape_uri_path from django.utils.formats import date_format from django.utils.functional import cached_property @@ -105,6 +106,35 @@ def generate_position_secret(): raise TypeError("Function no longer exists, use secret generators") +class OrderQuerySet(models.QuerySet): + def get_with_secret_check(self, code, received_secret, tag, secret_length=64): + dummy = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"[:secret_length] + try: + order = self.get(code=code) + if not hmac.compare_digest( + order.tagged_secret(tag, secret_length) if tag else order.secret, + received_secret[:secret_length].lower() if tag else received_secret.lower() + ) and not ( + # TODO: remove this clause after a while (compatibility with old secrets currently in flight) + tag and hmac.compare_digest( + hashlib.sha1(order.secret.lower().encode()).hexdigest(), + received_secret.lower() + ) + ): + raise Order.DoesNotExist + return order + except Order.DoesNotExist: + # Do a hash comparison as well to harden against timing attacks + if hmac.compare_digest( + salted_hmac(key_salt=b"", value=tag, algorithm="sha256", + secret=dummy).hexdigest()[:secret_length], + received_secret[:secret_length] + ): + raise Order.DoesNotExist + else: + raise Order.DoesNotExist + + class Order(LockModel, LoggedModel): """ An order is created when a user clicks 'buy' on his cart. It holds @@ -223,6 +253,7 @@ class Order(LockModel, LoggedModel): verbose_name=_('Locale') ) secret = models.CharField(max_length=32, default=generate_secret) + internal_secret = models.CharField(null=True, blank=True, max_length=32, default=generate_secret) datetime = models.DateTimeField( verbose_name=_("Date"), db_index=False ) @@ -285,7 +316,7 @@ class Order(LockModel, LoggedModel): default=False, ) - objects = ScopedManager(organizer='event__organizer') + objects = ScopedManager(OrderQuerySet.as_manager().__class__, organizer='event__organizer') class Meta: verbose_name = _("Order") @@ -1223,6 +1254,10 @@ class Order(LockModel, LoggedModel): _transactions_mark_order_clean(self.pk) return create + def tagged_secret(self, tag, secret_length=64): + return salted_hmac(value=tag, key_salt=b"", algorithm="sha256", + secret=self.internal_secret or self.secret).hexdigest()[:secret_length] + def answerfile_name(instance, filename: str) -> str: secret = get_random_string(length=32, allowed_chars=string.ascii_letters + string.digits) diff --git a/src/pretix/plugins/paypal2/payment.py b/src/pretix/plugins/paypal2/payment.py index 2ae1ea6f7c..85cb1a8c58 100644 --- a/src/pretix/plugins/paypal2/payment.py +++ b/src/pretix/plugins/paypal2/payment.py @@ -19,7 +19,6 @@ # You should have received a copy of the GNU Affero General Public License along with this program. If not, see # . # -import hashlib import json import logging import urllib.parse @@ -1096,5 +1095,5 @@ class PaypalAPM(PaypalMethod): return eventreverse(self.event, 'plugins:paypal2:pay', kwargs={ 'order': payment.order.code, 'payment': payment.pk, - 'hash': hashlib.sha1(payment.order.secret.lower().encode()).hexdigest(), + 'hash': payment.order.tagged_secret('plugins:paypal2:pay'), }) diff --git a/src/pretix/plugins/paypal2/views.py b/src/pretix/plugins/paypal2/views.py index c89dd5c687..e92226b234 100644 --- a/src/pretix/plugins/paypal2/views.py +++ b/src/pretix/plugins/paypal2/views.py @@ -31,7 +31,6 @@ # Unless required by applicable law or agreed to in writing, software distributed under the Apache License 2.0 is # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations under the License. -import hashlib import json import logging from decimal import Decimal @@ -81,15 +80,11 @@ logger = logging.getLogger('pretix.plugins.paypal2') class PaypalOrderView: def dispatch(self, request, *args, **kwargs): try: - self.order = request.event.orders.get(code=kwargs['order']) - if hashlib.sha1(self.order.secret.lower().encode()).hexdigest() != kwargs['hash'].lower(): - raise Http404('Unknown order') + self.order = request.event.orders.get_with_secret_check( + code=kwargs['order'], received_secret=kwargs['hash'].lower(), tag='plugins:paypal2:pay' + ) except Order.DoesNotExist: - # Do a hash comparison as well to harden timing attacks - if 'abcdefghijklmnopq'.lower() == hashlib.sha1('abcdefghijklmnopq'.encode()).hexdigest(): - raise Http404('Unknown order') - else: - raise Http404('Unknown order') + raise Http404('Unknown order') return super().dispatch(request, *args, **kwargs) @cached_property diff --git a/src/pretix/plugins/stripe/payment.py b/src/pretix/plugins/stripe/payment.py index 6dc0ce022a..cc20efe5aa 100644 --- a/src/pretix/plugins/stripe/payment.py +++ b/src/pretix/plugins/stripe/payment.py @@ -32,7 +32,6 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations under the License. -import hashlib import json import logging import re @@ -636,7 +635,7 @@ class StripeMethod(BasePaymentProvider): 'order': payment.order, 'payment': payment, 'payment_info': payment_info, - 'payment_hash': hashlib.sha1(payment.order.secret.lower().encode()).hexdigest() + 'payment_hash': payment.order.tagged_secret('plugins:stripe') } return template.render(ctx) @@ -890,7 +889,7 @@ class StripeMethod(BasePaymentProvider): return_url=build_absolute_uri(self.event, 'plugins:stripe:sca.return', kwargs={ 'order': payment.order.code, 'payment': payment.pk, - 'hash': hashlib.sha1(payment.order.secret.lower().encode()).hexdigest(), + 'hash': payment.order.tagged_secret('plugins:stripe'), }), expand=['latest_charge'], **params @@ -988,7 +987,7 @@ class StripeMethod(BasePaymentProvider): url = build_absolute_uri(self.event, 'plugins:stripe:sca', kwargs={ 'order': payment.order.code, 'payment': payment.pk, - 'hash': hashlib.sha1(payment.order.secret.lower().encode()).hexdigest(), + 'hash': payment.order.tagged_secret('plugins:stripe'), }) if not self.redirect_in_widget_allowed and request.session.get('iframe_session', False): return build_absolute_uri(self.event, 'plugins:stripe:redirect') + '?data=' + signing.dumps({ @@ -1009,7 +1008,7 @@ class StripeMethod(BasePaymentProvider): return_url=build_absolute_uri(self.event, 'plugins:stripe:sca.return', kwargs={ 'order': payment.order.code, 'payment': payment.pk, - 'hash': hashlib.sha1(payment.order.secret.lower().encode()).hexdigest(), + 'hash': payment.order.tagged_secret('plugins:stripe'), }), expand=["latest_charge"], **self.api_kwargs @@ -1829,7 +1828,7 @@ class StripeMultibanco(StripeSourceMethod): 'return_url': build_absolute_uri(self.event, 'plugins:stripe:return', kwargs={ 'order': payment.order.code, 'payment': payment.pk, - 'hash': hashlib.sha1(payment.order.secret.lower().encode()).hexdigest(), + 'hash': payment.order.tagged_secret('plugins:stripe'), }) }, **self.api_kwargs diff --git a/src/pretix/plugins/stripe/views.py b/src/pretix/plugins/stripe/views.py index a6605b6900..fbd3656d42 100644 --- a/src/pretix/plugins/stripe/views.py +++ b/src/pretix/plugins/stripe/views.py @@ -32,7 +32,6 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations under the License. -import hashlib import json import logging import urllib.parse @@ -486,15 +485,11 @@ def oauth_disconnect(request, **kwargs): class StripeOrderView: def dispatch(self, request, *args, **kwargs): try: - self.order = request.event.orders.get(code=kwargs['order']) - if hashlib.sha1(self.order.secret.lower().encode()).hexdigest() != kwargs['hash'].lower(): - raise Http404('') + self.order = request.event.orders.get_with_secret_check( + code=kwargs['order'], received_secret=kwargs['hash'].lower(), tag='plugins:stripe' + ) except Order.DoesNotExist: - # Do a hash comparison as well to harden timing attacks - if 'abcdefghijklmnopq'.lower() == hashlib.sha1('abcdefghijklmnopq'.encode()).hexdigest(): - raise Http404('') - else: - raise Http404('') + raise Http404('Unknown order') self.payment = get_object_or_404( self.order.payments, pk=self.kwargs['payment'], diff --git a/src/pretix/presale/views/order.py b/src/pretix/presale/views/order.py index 03c6710aa0..d6f35f80ff 100644 --- a/src/pretix/presale/views/order.py +++ b/src/pretix/presale/views/order.py @@ -33,6 +33,7 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations under the License. import copy +import hmac import inspect import json import mimetypes @@ -102,18 +103,12 @@ class OrderDetailMixin(NoSearchIndexViewMixin): @cached_property def order(self): - order = self.request.event.orders.filter(code=self.kwargs['order']).select_related('event').first() - if order: - if order.secret.lower() == self.kwargs['secret'].lower(): - return order - else: - return None - else: - # Do a comparison as well to harden timing attacks - if 'abcdefghijklmnopq'.lower() == self.kwargs['secret'].lower(): - return None - else: - return None + try: + return self.request.event.orders.filter().select_related('event').get_with_secret_check( + code=self.kwargs['order'], received_secret=self.kwargs['secret'], tag=None, + ) + except Order.DoesNotExist: + return None def get_order_url(self): return eventreverse(self.request.event, 'presale:event.order', kwargs={ @@ -133,13 +128,13 @@ class OrderPositionDetailMixin(NoSearchIndexViewMixin): ).select_related('order', 'order__event') p = qs.first() if p: - if p.web_secret.lower() == self.kwargs['secret'].lower(): + if hmac.compare_digest(p.web_secret.lower(), self.kwargs['secret'].lower()): return p else: return None else: # Do a comparison as well to harden timing attacks - if 'abcdefghijklmnopq'.lower() == self.kwargs['secret'].lower(): + if hmac.compare_digest('abcdefghijklmnopq'.lower(), self.kwargs['secret'].lower()): return None else: return None diff --git a/src/tests/base/test_ordersecrets.py b/src/tests/base/test_ordersecrets.py new file mode 100644 index 0000000000..9dee37d9ed --- /dev/null +++ b/src/tests/base/test_ordersecrets.py @@ -0,0 +1,167 @@ +# +# This file is part of pretix (Community Edition). +# +# Copyright (C) 2014-2020 Raphael Michel and contributors +# Copyright (C) 2020-2021 rami.io GmbH and contributors +# +# This program is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General +# Public License as published by the Free Software Foundation in version 3 of the License. +# +# ADDITIONAL TERMS APPLY: Pursuant to Section 7 of the GNU Affero General Public License, additional terms are +# applicable granting you additional permissions and placing additional restrictions on your usage of this software. +# Please refer to the pretix LICENSE file to obtain the full terms applicable to this work. If you did not receive +# this file, see . +# +# This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied +# warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along with this program. If not, see +# . +# +import hashlib +from datetime import timedelta +from decimal import Decimal + +import pytest +from django.utils.timezone import now +from django_scopes import scope + +from pretix.base.models import ( + Event, Order, OrderPosition, Organizer, generate_secret, +) + + +@pytest.fixture(scope='function') +def event(): + o = Organizer.objects.create(name='Dummy', slug='dummy') + event = Event.objects.create( + organizer=o, name='Dummy', slug='dummy', + date_from=now(), + plugins='pretix.plugins.banktransfer' + ) + with scope(organizer=o): + yield event + + +@pytest.fixture +def item(event): + return event.items.create( + name='Early-bird ticket', + category=None, default_price=23, + admission=True + ) + + +@pytest.fixture +def order(event, item): + o = Order.objects.create( + code='FOO', event=event, email='dummy@dummy.test', + status=Order.STATUS_PENDING, + datetime=now(), expires=now() + timedelta(days=10), + total=14, locale='en' + ) + OrderPosition.objects.create( + order=o, + item=item, + variation=None, + price=Decimal("14"), + ) + return o + + +@pytest.mark.django_db +def test_order_untagged_secret_compare(order): + found = Order.objects.get_with_secret_check(order.code, order.secret, tag=None) + assert found.code == order.code + + found = Order.objects.get_with_secret_check(order.code, order.secret.upper(), tag=None) + assert found.code == order.code + + with pytest.raises(Order.DoesNotExist): + Order.objects.get_with_secret_check(order.code, order.secret + "X", tag=None) + + with pytest.raises(Order.DoesNotExist): + Order.objects.get_with_secret_check(order.code, order.secret, tag='foo') + + +@pytest.mark.django_db +def test_order_tagged_secret_compare(order): + tagged_secret = order.tagged_secret('my_tag_123') + + found = Order.objects.get_with_secret_check(order.code, tagged_secret, tag='my_tag_123') + assert found.code == order.code + + with pytest.raises(Order.DoesNotExist): + Order.objects.get_with_secret_check(order.code, 'X' + tagged_secret, tag='my_tag_123') + + with pytest.raises(Order.DoesNotExist): + Order.objects.get_with_secret_check(order.code, tagged_secret, tag=None) + + with pytest.raises(Order.DoesNotExist): + Order.objects.get_with_secret_check(order.code, tagged_secret, tag='some_other_tag') + + +@pytest.mark.django_db +def test_order_tagged_secret_allows_legacy_hashes(order): + # TODO: remove this test when support for legacy hashes is removed, and enable the test below + legacy_hash = hashlib.sha1(order.secret.encode('utf-8')).hexdigest() + + found = Order.objects.get_with_secret_check(order.code, legacy_hash, tag='my_tag_123') + assert found.code == order.code + + +@pytest.mark.skip(reason="support for legacy hashes") # TODO: enable this test when support for legacy hashes is removed +@pytest.mark.django_db +def test_order_tagged_secret_doesnt_allow_legacy_hashes(order): + legacy_hash = hashlib.sha1(order.secret.encode('utf-8')).hexdigest() + + with pytest.raises(Order.DoesNotExist): + Order.objects.get_with_secret_check(order.code, legacy_hash, tag='my_tag_123') + + +@pytest.mark.django_db +def test_order_untagged_secret_doesnt_allow_legacy_hashes(order): + legacy_hash = hashlib.sha1(order.secret.encode('utf-8')).hexdigest() + + with pytest.raises(Order.DoesNotExist): + Order.objects.get_with_secret_check(order.code, legacy_hash, tag=None) + + +@pytest.mark.django_db +def test_order_tagged_secret_independent(order): + tagged_secret = order.tagged_secret('my_tag_123') + + found = Order.objects.get_with_secret_check(order.code, tagged_secret, tag='my_tag_123') + assert found.code == order.code + + # a) still valid after order.secret change + order.secret = generate_secret() + order.save() + + found = Order.objects.get_with_secret_check(order.code, tagged_secret, tag='my_tag_123') + assert found.code == order.code + + # b) invalidated after order.internal_secret change + order.internal_secret = generate_secret() + order.save() + + with pytest.raises(Order.DoesNotExist): + Order.objects.get_with_secret_check(order.code, tagged_secret, tag='my_tag_123') + + +@pytest.mark.django_db +def test_order_tagged_secret_uses_regular_secret_if_internal_secret_missing(order): + order.internal_secret = None + order.save() + + tagged_secret = order.tagged_secret('my_tag_123') + + found = Order.objects.get_with_secret_check(order.code, tagged_secret, tag='my_tag_123') + assert found.code == order.code + + order.secret = generate_secret() + order.save() + + with pytest.raises(Order.DoesNotExist): + Order.objects.get_with_secret_check(order.code, tagged_secret, tag='my_tag_123')