jazzband / django-fernet-encrypted-fields

MIT License
42 stars 9 forks source link

Updating Django's SECRET_KEY makes encrypted fields inaccessible #13

Open hendrikschneider opened 1 year ago

hendrikschneider commented 1 year ago

I've just tested rotating Django's SECRET_KEY and my field's couldn't be encrypted anymore even though a SALT_KEY is set:

How to replicate: 1) Create a new django project 2) Create a new app with an EncryptedTextField 3) Start Django shell 4) Create an instance with some example text and save 5) Stop Django shell 6) Update secret key 7) Load earlier data from db and print content of the text field

# Settings
# orignal key
SECRET_KEY = 'django-insecure-i40*4wu4erqc24em97k&fy#bj^rg%25)mtf2_cj)=2u7eg(pht'

# rotated key
# SECRET_KEY = 'django-insecure-i40*4wu4erqc24em97k&fy#bj^rg%25)mtf2_cj)=2u7eg(ph3'

SALT_KEY = '0123456789abcdefghijklmnopqrstuvwxyz'

Model

from encrypted_fields.fields import EncryptedTextField

from django.db import models

class MyModel(models.Model):
    text_field = EncryptedTextField()

What I did.

Console output
In [3]: MyModel.objects.create(text_field="test1")
Out[3]: <MyModel: MyModel object (1)>

In [4]: m = MyModel.objects.last()

In [5]: m.text_field
Out[5]: 'test1'

###########
# Rotate key #
###########

In [2]: from test_app.models import MyModel

In [3]: m = MyModel.objects.last()

In [4]: m.text_field
Out[4]: 'gAAAAABj8is-Hi0WTGpbcC5Xg1J8CYO8Zh5ErsWW631UZ2m3LimZkRrCZrII83VJAy831W9ISWCja1SQecwAA7eZ0panQuNY4g==

I think, it should be changed that it does not depend on the secret key, as this will easily break applications. Developers should only have one setting that affects the encryption.

hendrikschneider commented 1 year ago

So, I found the issue. The secret key as well as the salt_keys are both used for encryption. A secret key is something that can be rotated from time to time, which leads to not decryptable entries in the db.

What I would propose is, to add a new section to the settings like DJANGO_FERNET_FIELD_ENCRYPTION_KEYS = ['key1', 'key2', ...], which will be used together with the salt to encrypt/decrypt data. If this setting is not used, the secret key can still be used to also keep the module backwards compatible.

@frgmt What do you think about this?

naohide commented 1 year ago

@StevenMapes Do you know why this happens?

StevenMapes commented 1 year ago

@StevenMapes Do you know why this happens?

For the reason @hendrikschneider has said, it's used in the function. I hadn't noticed it before and I'd agree about changing it to use another value first and then falling back to the secret key that way it could be rotated much like how I changed this project to allow for salt rotation. What we may want to do though I'd to limit it to a maximum of two values effectively old and new as otherwise if you had 6 salts and 6 keys it would produce 36 potential attempts to decrypt which runs the risk of becoming slow. Or perhaps leaving or open and having a warning on the docs.

hendrikschneider commented 1 year ago

@StevenMapes @naohide I think limiting is a good idea. Actually, only two keys are needed to support key rotation and it would offer a clear way to document how to rotate the encryption secret. Having this approach would lead people to have only key active during normal operations and two in the moment they want to rotate the key and afterwards again only one key. No way to get lazy while rotating.

hendrikschneider commented 1 year ago

@StevenMapes @naohide Actually what I am thinking about is to extend this library with a command that automatically detects all models, which use an encrypted field to rotate the saved value to the new key. Just imagine, how easy rotations would be and developers wouldn't forget to rotate some models.

If we decide on how to handle the keys, I could take a look on implementing this.

StevenMapes commented 1 year ago

I like the idea of the management command to check and rotate values.

Using DJANGO_FERNET_FIELD_ENCRYPTION_KEYS as a list makes sense to me as, whilst long, it's explicit and verbose.

blag commented 8 months ago

As of Django 4.1, Django use a SECRET_KEY_FALLBACKS setting, which is used to rotate secret keys. If users use that properly, this project should be able to use that instead of DJANGO_FERNET_FIELD_ENCRYPTION_KEYS.