Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating Django's SECRET_KEY makes encrypted fields inaccessible #13

Open
hendrikschneider opened this issue Feb 19, 2023 · 7 comments
Open

Comments

@hendrikschneider
Copy link
Contributor

hendrikschneider commented Feb 19, 2023

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
Copy link
Contributor Author

hendrikschneider commented Feb 21, 2023

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
Copy link
Contributor

naohide commented Mar 3, 2023

@StevenMapes Do you know why this happens?

@StevenMapes
Copy link
Contributor

@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
Copy link
Contributor Author

@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
Copy link
Contributor Author

@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
Copy link
Contributor

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
Copy link

blag commented Oct 17, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants