feat(api): encrypt-then-authenticate action codes, closes #250

api/desecapi/crypto.py

@@ -0,0 +1,37 @@
+from base64 import urlsafe_b64encode
+from enum import Enum
+
+from cryptography.fernet import Fernet, InvalidToken
+from cryptography.hazmat.primitives import hashes
+from cryptography.hazmat.primitives.kdf.kbkdf import CounterLocation, KBKDFHMAC, Mode
+from cryptography.hazmat.backends import default_backend
+from django.conf import settings
+from django.utils.encoding import force_bytes
+
+
+def _derive_urlsafe_key(*, label, context):
+    backend = default_backend()
+    kdf = KBKDFHMAC(algorithm=hashes.SHA256(), mode=Mode.CounterMode, length=32, rlen=4, llen=4,
+                    location=CounterLocation.BeforeFixed, label=label, context=context, fixed=None, backend=backend)
+    key = kdf.derive(settings.SECRET_KEY.encode())
+    return urlsafe_b64encode(key)
+
+
+def retrieve_key(*, label, context):
+    # Keeping this function separate from key derivation gives us freedom to implement look-ups later, e.g. from cache
+    label = force_bytes(label, strings_only=True)
+    context = force_bytes(context, strings_only=True)
+    return _derive_urlsafe_key(label=label, context=context)
+
+
+def encrypt(data, *, context):
+    key = retrieve_key(label=b'crypt', context=context)
+    return Fernet(key=key).encrypt(data)
+
+
+def decrypt(token, *, context, ttl=None):
+    key = retrieve_key(label=b'crypt', context=context)
+    try:
+        return Fernet(key=key).decrypt(token, ttl=ttl)
+    except InvalidToken:
+        raise ValueError

api/desecapi/models.py

@@ -8,7 +8,8 @@ import string
 import time
 import uuid
 from base64 import urlsafe_b64encode
-from datetime import datetime, timedelta
+from datetime import timedelta
+from hashlib import sha256
 from os import urandom
 
 import psl_dns
@@ -18,13 +19,11 @@ from django.contrib.auth.hashers import make_password
 from django.contrib.auth.models import BaseUserManager, AbstractBaseUser, AnonymousUser
 from django.core.exceptions import ValidationError
 from django.core.mail import EmailMessage, get_connection
-from django.core.signing import Signer
 from django.core.validators import RegexValidator
 from django.db import models
 from django.db.models import Manager, Q
 from django.template.loader import get_template
 from django.utils import timezone
-from django.utils.crypto import constant_time_compare
 from rest_framework.exceptions import APIException
 
 from desecapi import pdns
@@ -429,91 +428,84 @@ class AuthenticatedAction(models.Model):
     Represents a procedure call on a defined set of arguments.
 
     Subclasses can define additional arguments by adding Django model fields and must define the action to be taken by
-    implementing the `act` method.
+    implementing the `_act` method.
 
-    AuthenticatedAction provides the `mac` property that returns a Message Authentication Code (MAC) based on the
-    state. By default, the state contains the action's name (defined by the `name` property) and a timestamp; the
-    state can be extended by (carefully) overriding the `_mac_state` property. Any AuthenticatedAction instance of
-    the same subclass and state will deterministically have the same MAC, effectively allowing authenticated
-    procedure calls by third parties according to the following protocol:
+    AuthenticatedAction provides the `state` property which by default is a hash of the action type (defined by the
+    action's class path). Other information such as user state can be included in the state hash by (carefully)
+    overriding the `_state_fields` property. Instantiation of the model, if given a `state` kwarg, will raise an error
+    if the given state argument does not match the state computed from `_state_fields` at the moment of instantiation.
+    The same applies to the `act` method: If called on an object that was instantiated without a `state` kwargs, an
+    error will be raised.
 
-    (1) Instantiate the AuthenticatedAction subclass representing the action to be taken with the desired state,
-    (2) provide information on how to instantiate the instance and the MAC to a third party,
-    (3) when provided with data that allows instantiation and a valid MAC, take the defined action, possibly with
+    This effectively allows hash-authenticated procedure calls by third parties as long as the server-side state is
+    unaltered, according to the following protocol:
+
+    (1) Instantiate the AuthenticatedAction subclass representing the action to be taken (no `state` kwarg here),
+    (2) provide information on how to instantiate the instance, and the state hash, to a third party,
+    (3) when provided with data that allows instantiation and a valid state hash, take the defined action, possibly with
         additional parameters chosen by the third party that do not belong to the verified state.
     """
-    created = models.PositiveIntegerField(default=lambda: int(datetime.timestamp(timezone.now())))
+    _validated = False
 
     class Meta:
         managed = False
 
     def __init__(self, *args, **kwargs):
-        # silently ignore any value supplied for the mac value, that makes it easier to use with DRF serializers
-        kwargs.pop('mac', None)
+        state = kwargs.pop('state', None)
         super().__init__(*args, **kwargs)
 
-    @property
-    def mac(self):
-        """
-        Deterministically generates a message authentication code (MAC) for this action, based on the state as defined
-        by `self._mac_state`. Identical state is guaranteed to yield identical MAC.
-        :return:
-        """
-        return Signer().signature(json.dumps(self._mac_state))
-
-    def validate_mac(self, mac):
-        """
-        Checks if the message authentication code (MAC) provided by the first argument matches the MAC of this action.
-        Note that expiration is not verified by this method.
-        :param mac: Message Authentication Code
-        :return: True, if MAC is valid; False otherwise.
-        """
-        return constant_time_compare(
-            mac,
-            self.mac,
-        )
-
-    def is_expired(self):
-        """
-        Checks if the action's timestamp is older than the given validity period. Note that the message
-        authentication code itself is not verified by this method.
-        :return: True if expired, False otherwise.
-        """
-        created = datetime.fromtimestamp(self.created, tz=timezone.utc)
-        return timezone.now() - created > settings.VALIDITY_PERIOD_VERIFICATION_SIGNATURE
+        if state is not None:
+            self._validated = self.validate_state(state)
+            if not self._validated:
+                raise ValueError
 
     @property
-    def _mac_state(self):
+    def _state_fields(self):
         """
-        Returns a list that defines the state of this action (used for MAC calculation).
+        Returns a list that defines the state of this action (used for authentication of this action).
 
         Return value must be JSON-serializable.
 
-        Values not included in the return value will not be used for MAC calculation, i.e. the MAC will be independent
-        of them.
+        Values not included in the return value will not be used for authentication, i.e. those values can be varied
+        freely and function as unauthenticated action input parameters.
 
         Use caution when overriding this method. You will usually want to append a value to the list returned by the
         parent. Overriding the behavior altogether could result in reducing the state to fewer variables, resulting
         in valid signatures when they were intended to be invalid. The suggested method for overriding is
 
             @property
-            def _mac_state:
-                return super()._mac_state + [self.important_value, self.another_added_value]
+            def _state_fields:
+                return super()._state_fields + [self.important_value, self.another_added_value]
 
         :return: List of values to be signed.
         """
         # TODO consider adding a "last change" attribute of the user to the state to avoid code
         #  re-use after the the state has been changed and changed back.
         name = '.'.join([self.__module__, self.__class__.__qualname__])
-        return [name, self.created]
+        return [name]
 
-    def act(self):
+    @property
+    def state(self):
+        state = json.dumps(self._state_fields).encode()
+        hash = sha256()
+        hash.update(state)
+        return hash.hexdigest()
+
+    def validate_state(self, value):
+        return value == self.state
+
+    def _act(self):
         """
         Conduct the action represented by this class.
         :return: None
         """
         raise NotImplementedError
 
+    def act(self):
+        if not self._validated:
+            raise RuntimeError('Action state could not be verified.')
+        return self._act()
+
 
 class AuthenticatedUserAction(AuthenticatedAction):
     """
@@ -526,10 +518,10 @@ class AuthenticatedUserAction(AuthenticatedAction):
         managed = False
 
     @property
-    def _mac_state(self):
-        return super()._mac_state + [str(self.user.id), self.user.email, self.user.password, self.user.is_active]
+    def _state_fields(self):
+        return super()._state_fields + [str(self.user.id), self.user.email, self.user.password, self.user.is_active]
 
-    def act(self):
+    def _act(self):
         raise NotImplementedError
 
 
@@ -540,10 +532,10 @@ class AuthenticatedActivateUserAction(AuthenticatedUserAction):
         managed = False
 
     @property
-    def _mac_state(self):
-        return super()._mac_state + [self.domain]
+    def _state_fields(self):
+        return super()._state_fields + [self.domain]
 
-    def act(self):
+    def _act(self):
         self.user.activate()
 
 
@@ -554,10 +546,10 @@ class AuthenticatedChangeEmailUserAction(AuthenticatedUserAction):
         managed = False
 
     @property
-    def _mac_state(self):
-        return super()._mac_state + [self.new_email]
+    def _state_fields(self):
+        return super()._state_fields + [self.new_email]
 
-    def act(self):
+    def _act(self):
         self.user.change_email(self.new_email)
 
 
@@ -567,7 +559,7 @@ class AuthenticatedResetPasswordUserAction(AuthenticatedUserAction):
     class Meta:
         managed = False
 
-    def act(self):
+    def _act(self):
         self.user.change_password(self.new_password)
 
 
@@ -576,7 +568,7 @@ class AuthenticatedDeleteUserAction(AuthenticatedUserAction):
     class Meta:
         managed = False
 
-    def act(self):
+    def _act(self):
         self.user.delete()
 
 

api/desecapi/serializers.py

@@ -13,7 +13,7 @@ from rest_framework.settings import api_settings
 from rest_framework.validators import UniqueTogetherValidator, UniqueValidator, qs_filter
 
 from api import settings
-from desecapi import models
+from desecapi import crypto, models
 
 
 class CaptchaSerializer(serializers.ModelSerializer):
@@ -584,20 +584,25 @@ class CustomFieldNameUniqueValidator(UniqueValidator):
 
 
 class AuthenticatedActionSerializer(serializers.ModelSerializer):
-    mac = serializers.CharField()  # serializer read-write, but model read-only field
+    state = serializers.CharField()  # serializer read-write, but model read-only field
 
     class Meta:
         model = models.AuthenticatedAction
-        fields = ('mac', 'created')
+        fields = ('state',)
 
     @classmethod
-    def _pack_code(cls, unpacked_data):
-        return urlsafe_b64encode(json.dumps(unpacked_data).encode()).decode()
+    def _pack_code(cls, data):
+        payload = json.dumps(data).encode()
+        payload_enc = crypto.encrypt(payload, context='desecapi.serializers.AuthenticatedActionSerializer')
+        return urlsafe_b64encode(payload_enc).decode()
 
     @classmethod
-    def _unpack_code(cls, packed_data):
+    def _unpack_code(cls, code):
         try:
-            return json.loads(urlsafe_b64decode(packed_data.encode()).decode())
+            payload_enc = urlsafe_b64decode(code.encode())
+            payload = crypto.decrypt(payload_enc, context='desecapi.serializers.AuthenticatedActionSerializer',
+                                     ttl=settings.VALIDITY_PERIOD_VERIFICATION_SIGNATURE.total_seconds())
+            return json.loads(payload.decode())
         except (TypeError, UnicodeDecodeError, UnicodeEncodeError, json.JSONDecodeError, binascii.Error):
             raise ValueError
 
@@ -624,20 +629,6 @@ class AuthenticatedActionSerializer(serializers.ModelSerializer):
         # do the regular business
         return super().to_internal_value(unpacked_data)
 
-    def validate(self, attrs):
-        if not self.instance:
-            self.instance = self.Meta.model(**attrs)  # TODO This creates an attribute on self. Side-effect intended?
-
-        # check if expired
-        if self.instance.is_expired():
-            raise ValidationError(detail='Code expired, please restart the process.', code='expired')
-
-        # check if MAC valid
-        if not self.instance.validate_mac(attrs['mac']):
-            raise ValidationError(detail='Bad signature.', code='bad_sig')
-
-        return attrs
-
     def act(self):
         self.instance.act()
         return self.instance

api/desecapi/tests/test_crypto.py

@@ -0,0 +1,56 @@
+from math import log
+
+from django.test import TestCase
+
+from desecapi import crypto
+
+
+class CryptoTestCase(TestCase):
+    context = 'desecapi.tests.test_crypto'
+
+    def test_retrieved_key_is_reproducible(self):
+        keys = (crypto.retrieve_key(label='test', context=self.context) for _ in range(2))
+        self.assertEqual(*keys)
+
+    def test_retrieved_key_depends_on_secret(self):
+        keys = []
+        for secret in ['abcdefgh', 'hgfedcba']:
+            with self.settings(SECRET_KEY=secret):
+                keys.append(crypto.retrieve_key(label='test', context=self.context))
+        self.assertNotEqual(*keys)
+
+    def test_retrieved_key_depends_on_label(self):
+        keys = (crypto.retrieve_key(label=f'test_{i}', context=self.context) for i in range(2))
+        self.assertNotEqual(*keys)
+
+    def test_retrieved_key_depends_on_context(self):
+        keys = (crypto.retrieve_key(label='test', context=f'{self.context}_{i}') for i in range(2))
+        self.assertNotEqual(*keys)
+
+    def test_encrypt_has_high_entropy(self):
+        def entropy(value: str):
+            result = 0
+            counts = [value.count(char) for char in set(value)]
+            for count in counts:
+                count /= len(value)
+                result -= count * log(count, 2)
+            return result * len(value)
+
+        ciphertext = crypto.encrypt(b'test', context=self.context)
+        self.assertGreater(entropy(ciphertext), 100)  # arbitrary
+
+    def test_encrypt_decrypt(self):
+        plain = b'test'
+        ciphertext = crypto.encrypt(plain, context=self.context)
+        self.assertEqual(plain, crypto.decrypt(ciphertext, context=self.context))
+
+    def test_encrypt_decrypt_raises_on_tampering(self):
+        ciphertext = crypto.encrypt(b'test', context=self.context)
+
+        with self.assertRaises(ValueError):
+            ciphertext_decoded = ciphertext.decode()
+            ciphertext_tampered = (ciphertext_decoded[:30] + 'TAMPERBEEF' + ciphertext_decoded[40:]).encode()
+            crypto.decrypt(ciphertext_tampered, context=self.context)
+
+        with self.assertRaises(ValueError):
+            crypto.decrypt(ciphertext, context=f'{self.context}2')

api/desecapi/tests/test_user_management.py

@@ -14,13 +14,13 @@ This involves testing five separate endpoints:
 """
 import random
 import re
+import time
 from unittest import mock
 from urllib.parse import urlparse
 
 from django.contrib.auth.hashers import is_password_usable
 from django.core import mail
 from django.urls import resolve
-from django.utils import timezone
 from rest_framework import status
 from rest_framework.reverse import reverse
 from rest_framework.test import APIClient
@@ -353,14 +353,14 @@ class UserManagementTestCase(DesecTestCase, PublicSuffixMockMixin):
     def assertVerificationFailureInvalidCodeResponse(self, response):
         return self.assertContains(
             response=response,
-            text="Bad signature.",
+            text="Invalid input.",
             status_code=status.HTTP_400_BAD_REQUEST
         )
 
     def assertVerificationFailureExpiredCodeResponse(self, response):
         return self.assertContains(
             response=response,
-            text="Code expired",
+            text="Invalid verification code.",
             status_code=status.HTTP_400_BAD_REQUEST
         )
 
@@ -808,8 +808,8 @@ class HasUserAccountTestCase(UserManagementTestCase):
         self.assertResetPasswordSuccessResponse(self.reset_password(self.email))
         confirmation_link = self.assertResetPasswordEmail(self.email)
 
-        with mock.patch('desecapi.models.timezone.now',
-                        return_value=timezone.now() + settings.VALIDITY_PERIOD_VERIFICATION_SIGNATURE):
+        with mock.patch('time.time',
+                        return_value=time.time() + settings.VALIDITY_PERIOD_VERIFICATION_SIGNATURE.total_seconds() + 1):
             response = self.client.verify(confirmation_link, new_password=self.random_password())
         self.assertVerificationFailureExpiredCodeResponse(response)
 

api/desecapi/views.py

@@ -541,7 +541,10 @@ class AuthenticatedActionView(generics.GenericAPIView):
             data = {**request.data, 'code': self.view.kwargs['code']}  # order crucial to avoid override from payload!
             serializer = self.view.serializer_class(data=data, context=self.view.get_serializer_context())
             serializer.is_valid(raise_exception=True)
-            self.view.authenticated_action = serializer.instance
+            try:
+                self.view.authenticated_action = serializer.Meta.model(**serializer.validated_data)
+            except ValueError:
+                raise ValidationError()
 
             return self.view.authenticated_action.user, None
 

api/requirements.txt

@@ -1,5 +1,7 @@
+captcha~=0.3.0
 celery~=4.3.0
 coverage~=4.5.3
+cryptography~=2.8
 Django~=2.2.0
 django-cors-headers~=3.1.1
 djangorestframework~=3.10.3
@@ -9,4 +11,3 @@ mysqlclient~=1.4.0
 psl-dns~=1.0rc2
 requests~=2.21.0
 uwsgi~=2.0.0
-captcha~=0.3.0