Skip to content

Commit

Permalink
Fixed #30360 -- Added support for secret key rotation.
Browse files Browse the repository at this point in the history
Thanks Florian Apolloner for the implementation idea.

Co-authored-by: Andreas Pelme <andreas@pelme.se>
Co-authored-by: Carlton Gibson <carlton.gibson@noumenal.es>
Co-authored-by: Vuyisile Ndlovu <terrameijar@gmail.com>
  • Loading branch information
4 people authored and felixxm committed Feb 1, 2022
1 parent ba4a688 commit 0dcd549
Show file tree
Hide file tree
Showing 18 changed files with 364 additions and 56 deletions.
1 change: 1 addition & 0 deletions django/conf/__init__.py
Expand Up @@ -188,6 +188,7 @@ def __init__(self, settings_module):
"INSTALLED_APPS",
"TEMPLATE_DIRS",
"LOCALE_PATHS",
"SECRET_KEY_FALLBACKS",
)
self._explicit_settings = set()
for setting in dir(mod):
Expand Down
4 changes: 4 additions & 0 deletions django/conf/global_settings.py
Expand Up @@ -272,6 +272,10 @@ def gettext_noop(s):
# loudly.
SECRET_KEY = ''

# List of secret keys used to verify the validity of signatures. This allows
# secret key rotation.
SECRET_KEY_FALLBACKS = []

# Default file storage mechanism that holds media.
DEFAULT_FILE_STORAGE = 'django.core.files.storage.FileSystemStorage'

Expand Down
29 changes: 25 additions & 4 deletions django/contrib/auth/tokens.py
Expand Up @@ -13,6 +13,7 @@ class PasswordResetTokenGenerator:
key_salt = "django.contrib.auth.tokens.PasswordResetTokenGenerator"
algorithm = None
_secret = None
_secret_fallbacks = None

def __init__(self):
self.algorithm = self.algorithm or 'sha256'
Expand All @@ -25,12 +26,26 @@ def _set_secret(self, secret):

secret = property(_get_secret, _set_secret)

def _get_fallbacks(self):
if self._secret_fallbacks is None:
return settings.SECRET_KEY_FALLBACKS
return self._secret_fallbacks

def _set_fallbacks(self, fallbacks):
self._secret_fallbacks = fallbacks

secret_fallbacks = property(_get_fallbacks, _set_fallbacks)

def make_token(self, user):
"""
Return a token that can be used once to do a password reset
for the given user.
"""
return self._make_token_with_timestamp(user, self._num_seconds(self._now()))
return self._make_token_with_timestamp(
user,
self._num_seconds(self._now()),
self.secret,
)

def check_token(self, user, token):
"""
Expand All @@ -50,7 +65,13 @@ def check_token(self, user, token):
return False

# Check that the timestamp/uid has not been tampered with
if not constant_time_compare(self._make_token_with_timestamp(user, ts), token):
for secret in [self.secret, *self.secret_fallbacks]:
if constant_time_compare(
self._make_token_with_timestamp(user, ts, secret),
token,
):
break
else:
return False

# Check the timestamp is within limit.
Expand All @@ -59,14 +80,14 @@ def check_token(self, user, token):

return True

def _make_token_with_timestamp(self, user, timestamp):
def _make_token_with_timestamp(self, user, timestamp, secret):
# timestamp is number of seconds since 2001-1-1. Converted to base 36,
# this gives us a 6 digit string until about 2069.
ts_b36 = int_to_base36(timestamp)
hash_string = salted_hmac(
self.key_salt,
self._make_hash_value(user, timestamp),
secret=self.secret,
secret=secret,
algorithm=self.algorithm,
).hexdigest()[::2] # Limit to shorten the URL.
return "%s-%s" % (ts_b36, hash_string)
Expand Down
53 changes: 39 additions & 14 deletions django/core/checks/security/base.py
Expand Up @@ -16,6 +16,15 @@
SECRET_KEY_MIN_LENGTH = 50
SECRET_KEY_MIN_UNIQUE_CHARACTERS = 5

SECRET_KEY_WARNING_MSG = (
f"Your %s has less than {SECRET_KEY_MIN_LENGTH} characters, less than "
f"{SECRET_KEY_MIN_UNIQUE_CHARACTERS} unique characters, or it's prefixed "
f"with '{SECRET_KEY_INSECURE_PREFIX}' indicating that it was generated "
f"automatically by Django. Please generate a long and random value, "
f"otherwise many of Django's security-critical features will be "
f"vulnerable to attack."
)

W001 = Warning(
"You do not have 'django.middleware.security.SecurityMiddleware' "
"in your MIDDLEWARE so the SECURE_HSTS_SECONDS, "
Expand Down Expand Up @@ -72,15 +81,7 @@
)

W009 = Warning(
"Your SECRET_KEY has less than %(min_length)s characters, less than "
"%(min_unique_chars)s unique characters, or it's prefixed with "
"'%(insecure_prefix)s' indicating that it was generated automatically by "
"Django. Please generate a long and random SECRET_KEY, otherwise many of "
"Django's security-critical features will be vulnerable to attack." % {
'min_length': SECRET_KEY_MIN_LENGTH,
'min_unique_chars': SECRET_KEY_MIN_UNIQUE_CHARACTERS,
'insecure_prefix': SECRET_KEY_INSECURE_PREFIX,
},
SECRET_KEY_WARNING_MSG % 'SECRET_KEY',
id='security.W009',
)

Expand Down Expand Up @@ -131,6 +132,8 @@
id='security.E024',
)

W025 = Warning(SECRET_KEY_WARNING_MSG, id='security.W025')


def _security_middleware():
return 'django.middleware.security.SecurityMiddleware' in settings.MIDDLEWARE
Expand Down Expand Up @@ -196,21 +199,43 @@ def check_ssl_redirect(app_configs, **kwargs):
return [] if passed_check else [W008]


def _check_secret_key(secret_key):
return (
len(set(secret_key)) >= SECRET_KEY_MIN_UNIQUE_CHARACTERS and
len(secret_key) >= SECRET_KEY_MIN_LENGTH and
not secret_key.startswith(SECRET_KEY_INSECURE_PREFIX)
)


@register(Tags.security, deploy=True)
def check_secret_key(app_configs, **kwargs):
try:
secret_key = settings.SECRET_KEY
except (ImproperlyConfigured, AttributeError):
passed_check = False
else:
passed_check = (
len(set(secret_key)) >= SECRET_KEY_MIN_UNIQUE_CHARACTERS and
len(secret_key) >= SECRET_KEY_MIN_LENGTH and
not secret_key.startswith(SECRET_KEY_INSECURE_PREFIX)
)
passed_check = _check_secret_key(secret_key)
return [] if passed_check else [W009]


@register(Tags.security, deploy=True)
def check_secret_key_fallbacks(app_configs, **kwargs):
warnings = []
try:
fallbacks = settings.SECRET_KEY_FALLBACKS
except (ImproperlyConfigured, AttributeError):
warnings.append(
Warning(W025.msg % 'SECRET_KEY_FALLBACKS', id=W025.id)
)
else:
for index, key in enumerate(fallbacks):
if not _check_secret_key(key):
warnings.append(
Warning(W025.msg % f'SECRET_KEY_FALLBACKS[{index}]', id=W025.id)
)
return warnings


@register(Tags.security, deploy=True)
def check_debug(app_configs, **kwargs):
passed_check = not settings.DEBUG
Expand Down
51 changes: 42 additions & 9 deletions django/core/signing.py
Expand Up @@ -97,10 +97,18 @@ def base64_hmac(salt, value, key, algorithm='sha1'):
return b64_encode(salted_hmac(salt, value, key, algorithm=algorithm).digest()).decode()


def _cookie_signer_key(key):
# SECRET_KEYS items may be str or bytes.
return b'django.http.cookies' + force_bytes(key)


def get_cookie_signer(salt='django.core.signing.get_cookie_signer'):
Signer = import_string(settings.SIGNING_BACKEND)
key = force_bytes(settings.SECRET_KEY) # SECRET_KEY may be str or bytes.
return Signer(b'django.http.cookies' + key, salt=salt)
return Signer(
key=_cookie_signer_key(settings.SECRET_KEY),
fallback_keys=map(_cookie_signer_key, settings.SECRET_KEY_FALLBACKS),
salt=salt,
)


class JSONSerializer:
Expand Down Expand Up @@ -135,18 +143,41 @@ def dumps(obj, key=None, salt='django.core.signing', serializer=JSONSerializer,
return TimestampSigner(key, salt=salt).sign_object(obj, serializer=serializer, compress=compress)


def loads(s, key=None, salt='django.core.signing', serializer=JSONSerializer, max_age=None):
def loads(
s,
key=None,
salt='django.core.signing',
serializer=JSONSerializer,
max_age=None,
fallback_keys=None,
):
"""
Reverse of dumps(), raise BadSignature if signature fails.
The serializer is expected to accept a bytestring.
"""
return TimestampSigner(key, salt=salt).unsign_object(s, serializer=serializer, max_age=max_age)
return TimestampSigner(key, salt=salt, fallback_keys=fallback_keys).unsign_object(
s,
serializer=serializer,
max_age=max_age,
)


class Signer:
def __init__(self, key=None, sep=':', salt=None, algorithm=None):
def __init__(
self,
key=None,
sep=':',
salt=None,
algorithm=None,
fallback_keys=None,
):
self.key = key or settings.SECRET_KEY
self.fallback_keys = (
fallback_keys
if fallback_keys is not None
else settings.SECRET_KEY_FALLBACKS
)
self.sep = sep
if _SEP_UNSAFE.match(self.sep):
raise ValueError(
Expand All @@ -156,8 +187,9 @@ def __init__(self, key=None, sep=':', salt=None, algorithm=None):
self.salt = salt or '%s.%s' % (self.__class__.__module__, self.__class__.__name__)
self.algorithm = algorithm or 'sha256'

def signature(self, value):
return base64_hmac(self.salt + 'signer', value, self.key, algorithm=self.algorithm)
def signature(self, value, key=None):
key = key or self.key
return base64_hmac(self.salt + 'signer', value, key, algorithm=self.algorithm)

def sign(self, value):
return '%s%s%s' % (value, self.sep, self.signature(value))
Expand All @@ -166,8 +198,9 @@ def unsign(self, signed_value):
if self.sep not in signed_value:
raise BadSignature('No "%s" found in value' % self.sep)
value, sig = signed_value.rsplit(self.sep, 1)
if constant_time_compare(sig, self.signature(value)):
return value
for key in [self.key, *self.fallback_keys]:
if constant_time_compare(sig, self.signature(value, key)):
return value
raise BadSignature('Signature "%s" does not match' % sig)

def sign_object(self, obj, serializer=JSONSerializer, compress=False):
Expand Down
16 changes: 16 additions & 0 deletions docs/howto/deployment/checklist.txt
Expand Up @@ -59,6 +59,22 @@ or from a file::
with open('/etc/secret_key.txt') as f:
SECRET_KEY = f.read().strip()

If rotating secret keys, you may use :setting:`SECRET_KEY_FALLBACKS`::

import os
SECRET_KEY = os.environ['CURRENT_SECRET_KEY']
SECRET_KEY_FALLBACKS = [
os.environ['OLD_SECRET_KEY'],
]

Ensure that old secret keys are removed from ``SECRET_KEY_FALLBACKS`` in a
timely manner.

.. versionchanged:: 4.1

The ``SECRET_KEY_FALLBACKS`` setting was added to support rotating secret
keys.

:setting:`DEBUG`
----------------

Expand Down
10 changes: 8 additions & 2 deletions docs/ref/checks.txt
Expand Up @@ -457,8 +457,8 @@ The following checks are run if you use the :option:`check --deploy` option:
* **security.W009**: Your :setting:`SECRET_KEY` has less than 50 characters,
less than 5 unique characters, or it's prefixed with ``'django-insecure-'``
indicating that it was generated automatically by Django. Please generate a
long and random ``SECRET_KEY``, otherwise many of Django's security-critical
features will be vulnerable to attack.
long and random value, otherwise many of Django's security-critical features
will be vulnerable to attack.
* **security.W010**: You have :mod:`django.contrib.sessions` in your
:setting:`INSTALLED_APPS` but you have not set
:setting:`SESSION_COOKIE_SECURE` to ``True``. Using a secure-only session
Expand Down Expand Up @@ -511,6 +511,12 @@ The following checks are run if you use the :option:`check --deploy` option:
to an invalid value.
* **security.E024**: You have set the
:setting:`SECURE_CROSS_ORIGIN_OPENER_POLICY` setting to an invalid value.
* **security.W025**: Your
:setting:`SECRET_KEY_FALLBACKS[n] <SECRET_KEY_FALLBACKS>` has less than 50
characters, less than 5 unique characters, or it's prefixed with
``'django-insecure-'`` indicating that it was generated automatically by
Django. Please generate a long and random value, otherwise many of Django's
security-critical features will be vulnerable to attack.

The following checks verify that your security-related settings are correctly
configured:
Expand Down
39 changes: 36 additions & 3 deletions docs/ref/settings.txt
Expand Up @@ -2291,16 +2291,48 @@ The secret key is used for:
* Any usage of :doc:`cryptographic signing </topics/signing>`, unless a
different key is provided.

If you rotate your secret key, all of the above will be invalidated.
Secret keys are not used for passwords of users and key rotation will not
affect them.
When a secret key is no longer set as :setting:`SECRET_KEY` or contained within
:setting:`SECRET_KEY_FALLBACKS` all of the above will be invalidated. When
rotating your secret key, you should move the old key to
:setting:`SECRET_KEY_FALLBACKS` temporarily. Secret keys are not used for
passwords of users and key rotation will not affect them.

.. note::

The default :file:`settings.py` file created by :djadmin:`django-admin
startproject <startproject>` creates a unique ``SECRET_KEY`` for
convenience.

.. setting:: SECRET_KEY_FALLBACKS

``SECRET_KEY_FALLBACKS``
------------------------

.. versionadded:: 4.1

Default: ``[]``

A list of fallback secret keys for a particular Django installation. These are
used to allow rotation of the ``SECRET_KEY``.

In order to rotate your secret keys, set a new ``SECRET_KEY`` and move the
previous value to the beginning of ``SECRET_KEY_FALLBACKS``. Then remove the
old values from the end of the ``SECRET_KEY_FALLBACKS`` when you are ready to
expire the sessions, password reset tokens, and so on, that make use of them.

.. note::

Signing operations are computationally expensive. Having multiple old key
values in ``SECRET_KEY_FALLBACKS`` adds additional overhead to all checks
that don't match an earlier key.

As such, fallback values should be removed after an appropriate period,
allowing for key rotation.

Uses of the secret key values shouldn't assume that they are text or bytes.
Every use should go through :func:`~django.utils.encoding.force_str` or
:func:`~django.utils.encoding.force_bytes` to convert it to the desired type.

.. setting:: SECURE_CONTENT_TYPE_NOSNIFF

``SECURE_CONTENT_TYPE_NOSNIFF``
Expand Down Expand Up @@ -3725,6 +3757,7 @@ Security
* :setting:`CSRF_USE_SESSIONS`

* :setting:`SECRET_KEY`
* :setting:`SECRET_KEY_FALLBACKS`
* :setting:`X_FRAME_OPTIONS`

Serialization
Expand Down
3 changes: 2 additions & 1 deletion docs/releases/4.1.txt
Expand Up @@ -254,7 +254,8 @@ Requests and Responses
Security
~~~~~~~~

* ...
* The new :setting:`SECRET_KEY_FALLBACKS` setting allows providing a list of
values for secret key rotation.

Serialization
~~~~~~~~~~~~~
Expand Down
5 changes: 3 additions & 2 deletions docs/topics/auth/default.txt
Expand Up @@ -912,8 +912,9 @@ function.

Since
:meth:`~django.contrib.auth.models.AbstractBaseUser.get_session_auth_hash()`
is based on :setting:`SECRET_KEY`, updating your site to use a new secret
will invalidate all existing sessions.
is based on :setting:`SECRET_KEY`, secret key values must be
rotated to avoid invalidating existing sessions when updating your site to
use a new secret. See :setting:`SECRET_KEY_FALLBACKS` for details.

.. _built-in-auth-views:

Expand Down

0 comments on commit 0dcd549

Please sign in to comment.