search

georgemarshall / django-cryptography

Easily encrypt data in Django
https://django-cryptography.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
371 stars 69 forks source link

Allow to rotate SECRET_KEY #75

Open tex0l opened 2 years ago

tex0l commented 2 years ago

Hi,

In a django project, I might need to rotate the SECRET_KEY.

However, when using the encrypt wrapper for the field, I can only override the CRYPTOGRAPHY_KEY which is used for encryption in FernetBytes, but not the SECRET_KEY used for the HMAC in FernetSigner.

Would it be possible to add an argument to the encrypt wrapper to be able to override the HMAC key ?

tex0l commented 2 years ago

I hadn't seen this related issue here: https://github.com/georgemarshall/django-cryptography/issues/36.

However, the proposal of just adding an extra argument to the encrypt wrapper, and leave the rest up to the developer seems more flexible and much easier to implement. If needed, I can do a PR.

georgemarshall commented 2 years ago

https://django-cryptography.readthedocs.io/en/latest/fields.html#django_cryptography.fields.encrypt

tex0l commented 2 years ago

As I understand the code & the docs, the key argument of the encrypt field wrapper is then used as the first argument of FernetBytes, which is in turn used to override settings.CRYPTOGRAPHY_KEY.

However, the signing key cannot be set through the arguments of the encrypt field wrapper. When using the encrypt field wrapper, it uses the FernetSigner with no option, and the default key used within FernetSigner is settings.SECRET_KEY.

To rotate only the CRYPTOGRAPHY_KEY, I could use the key argument of the encrypt to set it manually to the value of the derived CRYPTOGRAPHY_KEY through the same PBKDF2 parameters as you do in the configuration.

However, I don't see a clear path to rotating SECRET_KEY properly, as it is used as the signing key in FernetSigner, and it seems it is not possible to override the signing key.

Am I mistaken?

If I wanted to properly rotate the SECRET_KEY with the current version of django-cryptography, I would need to do a migration in three steps:

  1. decrypt the data contained in the encrypted field and set it in clear text in a temporary field;
  2. rotate the SECRET_KEY setting;
  3. re-set the clear text data contained in the temporary field in the new encrypted field.

This would leave the data in clear text temporarily in the database, which is a security issue.

What I suggest is to allow setting from the encrypt field wrapper a signing key, which would be used to override the SECRET_KEY in FernetSigner. This could then be used to:

  1. modify the existing field to use the current SECRET_KEY as the signing key, the current CRYPTOGRAPHY_KEY as the seed for the encryption key (with your PBKDF2 parameters);
  2. rotate the keys in the settings & create a new encrypted field;
  3. make a migration where the data is decrypted from a field and re-encrypted in the other, without writing onto the databse the clear text data.
abradaric commented 2 years ago

Key rotation via management command / settings config would be a very useful feature :heavy_plus_sign:
@tex0l were you planning on opening a PR in near future?

tex0l commented 2 years ago

I was actually hoping @georgemarshall would agree with my suggestion first, and then if he does not have the time to make the modification himself, I would indeed make a PR. I don't want to fork without being sure it will be merged.

My PR would be dead simple: it would allow setting manually a key used in the FernetSigner as an argument of the encrypt wrapper (very much like the current key argument, used only in FernetBytes), a migration would be possible from that, but I wasn't planning to make a management command to do the rotation automatically.

@georgemarshall what do you think of my proposal?

vaimdev commented 9 months ago

@tex0l , do you think this help? https://cryptography.io/en/latest/fernet/#cryptography.fernet.MultiFernet

Key rotation is an enterprise requirement, if you can release a PR, that will be the best.

tex0l commented 9 months ago

I need to deep dive again to reply to you. We solved the issue differently, I'll re-open the issue internally to see if we can prioritize resources on this in the coming weeks, but I doubt it.

zaeem-maqsood commented 2 months ago

Hey guys, sorry I couldn't wait for a release on this. I Just forked and made a hacky solution in case anyone is interested for the time being. I am fairly new to Python/Django but I had to implement something for a project so excuse the quality of the code. Hopefully, it can be used in the time being until a better fix is implemented.

https://github.com/zaeem-maqsood/reencrypt-fields