feat(api): store tokens in hashed format and with UUID

Token values are now stored in hashed format. For now, it is
PBKDF2-HMAC-SHA256 with one iteration and a static salt.

Rationale:

- We should not store secrets in the clear, including tokens. What we
  don't have in plaintext, we can't loose. Potential loss vectors can
  be: a) backup compromise, b) API compromise (e.g. through code
  injection vulnerability), c) dbapi container compromise via MariaDB
  vulnerability, d) coding mistake in view authentication so that we
  return tokens we don't want to return, e) deSEC staff accessing
  tokens directly in the database, f) probably more things I forgot.

- It's also a good practice to not show secrets to the user multiple
  times, so that somebody accessing the user's account cannot quickly
  list API tokens. (Amazon AWS takes this approach, for example.)
  However, if we show secrets only once, there's no reason for being
  able to access them.

- There is no practical timing penalty for this implementation: A quick
  survey using timeit showed that about 250,000 tokens can be hashed on
  my laptop per second, sequentially. Most likely, each API request
  comes with other much more expensive operations (e.g. connecting to
  the database). In another test, running our Django test suite on
  master vs. this branch three times each took (9.032s, 8.941s, 9.063s)
  vs. (9.173s, 9.014s, 8.963s).

Implementation choices:

- Why one iteration? - Our tokens have high entropy (168 bits), so an
  attacker who is in possession of a database copy can only use brute
  force. The size of the combinatorial space makes that impractical. As
  a consequence, there's no point in adding a time penalty (unlike with
  user passwords!). However, we don't want to spend unnecessary time in
  verifying tokens, so keeping the number of iterations low is a good
  thing. If we think our tokens should be slower to break, we should
  increase entropy.

- Why static salt? - The purpose of salt is to make rainbow table
  generation infeasible, by enlarging the space using some extra entropy
  (separate from the secret, usually a password). However, as our tokens
  have high entropy, rainbow tables are not feasible in the first place.
  Another way to look at it: One could consider the salt's entropy to be
  contained in the token itself. On the other hand, if the salt was not
  static, then it would not be clear which row to look up from the
  database.
  Salt is also used to mask the fact when two user passwords are the
  same (given the salt has reasonable entropy). As we have a uniqueness
  constraint on this database field, this aspect does not apply to us.

- Why PBKDF2-HMAC-SHA256? - It's what NIST recommends. The advantage
  over using plain SHA256 is that length attacks "by adding to the
  digest" are not possible. (That's more of an attack scenario for
  integrity checks than for secret storage, but whatever.) Django
  supports it, so I used their existing interface (password-management
  tooling).

api/api/settings.py

@@ -12,6 +12,8 @@ https://docs.djangoproject.com/en/1.7/ref/settings/
 import os
 from datetime import timedelta
 
+from django.conf.global_settings import PASSWORD_HASHERS as DEFAULT_PASSWORD_HASHERS
+
 BASE_DIR = os.path.dirname(os.path.dirname(__file__))
 
 
@@ -97,6 +99,9 @@ REST_FRAMEWORK = {
     'ALLOWED_VERSIONS': ['v1', 'v2'],
 }
 
+PASSWORD_HASHER_TOKEN = 'desecapi.authentication.TokenHasher'
+PASSWORD_HASHERS = DEFAULT_PASSWORD_HASHERS + [PASSWORD_HASHER_TOKEN]
+
 # CORS
 # No need to add Authorization to CORS_ALLOW_HEADERS (included by default)
 CORS_ORIGIN_ALLOW_ALL = True

api/api/settings_quick_test.py

@@ -16,6 +16,7 @@ DATABASES = {
 # avoid computationally expensive password hashing for tests
 PASSWORD_HASHERS = [
     'django.contrib.auth.hashers.MD5PasswordHasher',
+    PASSWORD_HASHER_TOKEN,
 ]
 
 REST_FRAMEWORK['PAGE_SIZE'] = 20

api/desecapi/authentication.py

@@ -1,5 +1,6 @@
 import base64
 
+from django.contrib.auth.hashers import PBKDF2PasswordHasher
 from rest_framework import exceptions, HTTP_HEADER_ENCODING
 from rest_framework.authentication import (
     BaseAuthentication,
@@ -14,6 +15,10 @@ from desecapi.serializers import EmailPasswordSerializer
 class TokenAuthentication(RestFrameworkTokenAuthentication):
     model = Token
 
+    def authenticate_credentials(self, key):
+        key = Token.make_hash(key)
+        return super().authenticate_credentials(key)
+
 
 class BasicTokenAuthentication(BaseAuthentication):
     """
@@ -53,6 +58,7 @@ class BasicTokenAuthentication(BaseAuthentication):
         invalid_token_message = 'Invalid basic auth token'
         try:
             user, key = base64.b64decode(basic).decode(HTTP_HEADER_ENCODING).split(':')
+            key = Token.make_hash(key)
             token = self.model.objects.get(key=key)
             domain_names = token.user.domains.values_list('name', flat=True)
             if user not in ['', token.user.email] and not user.lower() in domain_names:
@@ -91,6 +97,7 @@ class URLParamAuthentication(BaseAuthentication):
         return self.authenticate_credentials(request.query_params['username'], request.query_params['password'])
 
     def authenticate_credentials(self, _, key):
+        key = Token.make_hash(key)
         try:
             token = self.model.objects.get(key=key)
         except self.model.DoesNotExist:
@@ -109,3 +116,8 @@ class EmailPasswordPayloadAuthentication(BaseAuthentication):
         serializer = EmailPasswordSerializer(data=request.data)
         serializer.is_valid(raise_exception=True)
         return self.authenticate_credentials(serializer.data['email'], serializer.data['password'], request)
+
+
+class TokenHasher(PBKDF2PasswordHasher):
+    algorithm = 'pbkdf2_sha256_iter1'
+    iterations = 1

api/desecapi/migrations/0010_hash_tokens_and_switch_to_uuid.py

@@ -0,0 +1,52 @@
+# Generated by Django 2.2.7 on 2019-11-11 20:14
+
+from django.contrib.auth.hashers import make_password
+from django.db import migrations, models, transaction
+import uuid
+
+
+def migrate_data(apps, schema_editor):
+    Token = apps.get_model('desecapi', 'Token')
+    tokens = Token.objects.exclude(key__contains='$').all()
+    with transaction.atomic():
+        for token in tokens:
+            hashed = make_password(token.key, salt='static', hasher='pbkdf2_sha256_iter1')
+            Token.objects.filter(key=token.key).update(id=uuid.uuid4().hex, key=hashed)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('desecapi', '0009_domain_minimum_ttl_default'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='token',
+            options={'verbose_name': 'Token', 'verbose_name_plural': 'Tokens'},
+        ),
+        migrations.AlterUniqueTogether(
+            name='token',
+            unique_together=set(),
+        ),
+        migrations.RemoveField(
+            model_name='token',
+            name='user_specific_id',
+        ),
+        migrations.AlterField(
+            model_name='token',
+            name='key',
+            field=models.CharField(db_index=True, max_length=128, unique=True, verbose_name='Key'),
+        ),
+        migrations.AlterField(
+            model_name='token',
+            name='id',
+            field=models.CharField(default=uuid.uuid4, max_length=32, primary_key=True, serialize=False),
+        ),
+        migrations.RunPython(migrate_data, reverse_code=migrations.RunPython.noop),
+        migrations.AlterField(
+            model_name='token',
+            name='id',
+            field=models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False),
+        ),
+    ]

api/desecapi/models.py

@@ -14,6 +14,7 @@ from os import urandom
 import psl_dns
 import rest_framework.authtoken.models
 from django.conf import settings
+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
@@ -163,26 +164,23 @@ class User(AbstractBaseUser):
 
 
 class Token(rest_framework.authtoken.models.Token):
-    key = models.CharField("Key", max_length=40, db_index=True, unique=True)
-    # relation to user is a ForeignKey, so each user can have more than one token
+    id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
+    key = models.CharField("Key", max_length=128, db_index=True, unique=True)
     user = models.ForeignKey(
         User, related_name='auth_tokens',
         on_delete=models.CASCADE, verbose_name="User"
     )
     name = models.CharField("Name", max_length=64, default="")
-    user_specific_id = models.BigIntegerField("User-Specific ID")
-
-    def save(self, *args, **kwargs):
-        if not self.user_specific_id:
-            self.user_specific_id = random.randrange(16 ** 8)
-        super().save(*args, **kwargs)  # Call the "real" save() method.
+    plain = None
 
     def generate_key(self):
-        return urlsafe_b64encode(urandom(21)).decode()
+        self.plain = urlsafe_b64encode(urandom(21)).decode()
+        self.key = Token.make_hash(self.plain)
+        return self.key
 
-    class Meta:
-        abstract = False
-        unique_together = (('user', 'user_specific_id'),)
+    @staticmethod
+    def make_hash(plain):
+        return make_password(plain, salt='static', hasher='pbkdf2_sha256_iter1')
 
 
 validate_domain_name = [

api/desecapi/serializers.py

@@ -45,9 +45,7 @@ class CaptchaSolutionSerializer(serializers.Serializer):
 
 
 class TokenSerializer(serializers.ModelSerializer):
-    token = serializers.ReadOnlyField(source='key')
-    # note this overrides the original "id" field, which is the db primary key
-    id = serializers.ReadOnlyField(source='user_specific_id')
+    token = serializers.ReadOnlyField(source='plain')
 
     class Meta:
         model = models.Token

api/desecapi/signals.py

@@ -12,5 +12,5 @@ def domain_handler(sender, instance: models.Domain, created, raw, using, update_
             'domain': instance.name,
             'url': f'https://update.{instance.parent_domain_name}/',
             'username': instance.name,
-            'password': models.Token.objects.create(user=instance.owner, name='dyndns')
+            'password': models.Token.objects.create(user=instance.owner, name='dyndns').plain
         })

api/desecapi/tests/base.py

@@ -10,6 +10,7 @@ from typing import Union, List, Dict
 from unittest import mock
 
 from django.conf import settings
+from django.contrib.auth.hashers import check_password
 from django.db import connection
 from django.utils import timezone
 from httpretty import httpretty, core as hr_core
@@ -545,6 +546,12 @@ class MockPDNSTestCase(APITestCase):
         if body:
             self.assertJSONEqual(response.content, body)
 
+    def assertToken(self, plain, user=None):
+        user = user or self.owner
+        self.assertTrue(any(check_password(plain, hashed, preferred='pbkdf2_sha256_iter1')
+                            for hashed in Token.objects.filter(user=user).values_list('key', flat=True)))
+        self.assertEqual(len(Token.make_hash(plain).split('$')), 4)
+
     @classmethod
     def setUpTestData(cls):
         httpretty.enable(allow_net_connect=False)
@@ -689,7 +696,7 @@ class DesecTestCase(MockPDNSTestCase):
     def create_token(cls, user):
         token = Token.objects.create(user=user)
         token.save()
-        return token.key
+        return token.plain
 
     @classmethod
     def create_user(cls, **kwargs):

api/desecapi/tests/test_domains.py

@@ -1,3 +1,5 @@
+import re
+
 from django.conf import settings
 from django.core import mail
 from django.core.exceptions import ValidationError
@@ -398,7 +400,8 @@ class AutoDelegationDomainOwnerTests(DomainOwnerTestCase):
                 self.assertEqual(len(mail.outbox), i + 1)
                 email = str(mail.outbox[0].message())
                 self.assertTrue(name in email)
-                self.assertTrue(any(token.key in email for token in Token.objects.filter(user=self.owner).all()))
+                password_plain = re.search('password: (.*)', email).group(1)
+                self.assertToken(password_plain)
                 self.assertFalse(self.user.plain_password in email)
 
     def test_domain_limit(self):

api/desecapi/tests/test_user_management.py

@@ -256,7 +256,8 @@ class UserManagementTestCase(DesecTestCase, PublicSuffixMockMixin):
     def assertRegistrationWithDomainVerificationSuccessResponse(self, response, domain=None, email=None):
         if domain and self.has_local_suffix(domain):
             body = self.assertEmailSent('', body_contains=domain, recipient=email)
-            self.assertTrue(any(token.key in body for token in Token.objects.filter(user__email=email).all()))
+            password_plain = re.search('password: (.*)', body).group(1)
+            self.assertToken(password_plain, user=User.objects.get(email=email))
             text = 'Success! Here is the secret token'
         else:
             self.assertNoEmailSent()

api/desecapi/views.py

@@ -55,7 +55,6 @@ class TokenViewSet(IdempotentDestroy,
                    GenericViewSet):
     serializer_class = serializers.TokenSerializer
     permission_classes = (IsAuthenticated, )
-    lookup_field = 'user_specific_id'
 
     def get_queryset(self):
         return self.request.user.auth_tokens.all()

docs/authentication.rst

@@ -378,7 +378,7 @@ To retrieve a list of currently valid tokens, issue a ``GET`` request::
 
 The server will respond with a list of token objects, each containing a
 timestamp when the token was created (note the ``Z`` indicating the UTC
-timezone) and an ID to identify that token. Furthermore, each token can
+timezone) and a UUID to identify that token. Furthermore, each token can
 carry a name that is of no operational relevance to the API (it is meant
 for user reference only). Certain API operations (such as login) will
 automatically populate the ``name`` field with values such as "login" or
@@ -389,14 +389,12 @@ automatically populate the ``name`` field with values such as "login" or
     [
         {
             "created": "2018-09-06T07:05:54.080564Z",
-            "id": 14423,
-            "value": "4yScSMFFNdAlk6WZuLIwYBVYnXPF",
+            "id": "3159e485-5499-46c0-ae2b-aeb84d627a8e",
             "name": "login"
         },
         {
             "created": "2018-09-06T08:53:26.428396Z",
-            "id": 36483,
-            "value": "mu4W4MHuSc0HyrGD1h/dnKuZBond",
+            "id": "76d6e39d-65bc-4ab2-a1b7-6e94eee0a534",
             "name": ""
         }
     ]
@@ -418,8 +416,8 @@ will reply with ``201 Created`` and the created token in the response body::
 
     {
         "created": "2018-09-06T09:08:43.762697Z",
-        "id": 73658,
-        "value": "4pnk7u+NHvrEkFzrhFDRTjGFyX+S",
+        "id": "3a6b94b5-d20e-40bd-a7cc-521f5c79fab3",
+        "token": "4pnk7u+NHvrEkFzrhFDRTjGFyX+S",
         "name": "my new token"
     }
 
@@ -443,18 +441,17 @@ any action. We may implement specialized tokens in the future.
 Token Security Considerations
 `````````````````````````````
 
-This section is for information only. Token length and encoding may be subject
-to change in the future.
+This section is for information only. Token length and encoding may change in
+the future.
 
-Any token is generated from 168 bits of true randomness at the server. Guessing
-the token correctly is hence practically impossible. The value corresponds to 21
-bytes and is represented by 28 characters in Base64-like encoding. That is, any token
-will only consist of URL-safe characters ``A-Z``, ``a-z``, ``0-9``, ``-``, and ``_``.
-(We do not have any padding at the end because the string length is a multiple of 4.)
+Any token is generated from 168 bits of randomness at the server and stored in
+hashed format (PBKDF2-HMAC-SHA256). Guessing the token correctly or reversing
+the hash is hence practically impossible.
 
-As all tokens are stored in plain text on the server, the user may not choose
-the token value individually to prevent re-using passwords as tokens at deSEC.
+The token value is represented by 28 characters using a URL-safe variant of
+base64 encoding. It comprises only the characters ``A-Z``, ``a-z``, ``0-9``, ``-``,
+and ``_``. (Base64 padding is not needed as the string length is a multiple of 4.)
 
-Old versions of deSEC encoded 20-byte tokens in 40 characters with hexadecimal
+Old versions of the API encoded 20-byte tokens in 40 characters with hexadecimal
 representation. Such tokens will not be issued anymore, but remain valid until
 invalidated by the user.

test/e2e/schemas.js

@@ -89,7 +89,10 @@ exports.token = {
     properties: {
         name: { type: "string" },
         created: { type: "string" },
-        id: { type: "integer" },
+        id: {
+            type: "string",
+            format: "uuid"
+        },
     },
     required: ["name", "created", "id"]
 };