From 20fafa4c2039adf49979010314d538f6a450159d Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 11 Jul 2018 12:03:51 -0300 Subject: [PATCH 1/3] Encrypt SSHKey.private_key database field FIELD_ENCRYPTION_KEY could be a list to support key rotation. This commit adds one as example, but the production value is stored in a private repository. --- readthedocs/settings/base.py | 6 ++++++ readthedocs/ssh/migrations/0001_initial.py | 3 ++- readthedocs/ssh/models.py | 3 ++- requirements/pip.txt | 1 + 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 518f9a8b5..b988fd528 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -65,6 +65,11 @@ class CommunityBaseSettings(Settings): CSRF_COOKIE_HTTPONLY = True CSRF_COOKIE_AGE = 30 * 24 * 60 * 60 + # Encryption + FIELD_ENCRYPTION_KEY = [ + 'P98DEYWoSuLmgXdRGzJkbo1JWeiHf4ghpFI8QvV3hRI=', # replace this please + ] + # Application classes @property def INSTALLED_APPS(self): # noqa @@ -90,6 +95,7 @@ class CommunityBaseSettings(Settings): 'django_extensions', 'messages_extends', 'tastypie', + 'encrypted_model_fields', # our apps 'readthedocs.projects', diff --git a/readthedocs/ssh/migrations/0001_initial.py b/readthedocs/ssh/migrations/0001_initial.py index a28bd60a0..69d2a19d5 100644 --- a/readthedocs/ssh/migrations/0001_initial.py +++ b/readthedocs/ssh/migrations/0001_initial.py @@ -5,6 +5,7 @@ from __future__ import unicode_literals from django.db import migrations, models import django.db.models.deletion import readthedocs.ssh.mixins +import encrypted_model_fields.fields class Migration(migrations.Migration): @@ -22,7 +23,7 @@ class Migration(migrations.Migration): ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('create_date', models.DateTimeField(auto_now_add=True, verbose_name='Creation date')), ('public_key', models.TextField(help_text='Add this to your version control to give us access.', verbose_name='Public SSH Key')), - ('private_key', models.TextField(verbose_name='Private SSH Key')), + ('private_key', encrypted_model_fields.fields.EncryptedTextField(verbose_name='Private SSH Key')), ('json', models.TextField(blank=True, null=True, verbose_name='Serialized API response')), ('project', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='sshkeys', to='projects.Project')), ], diff --git a/readthedocs/ssh/models.py b/readthedocs/ssh/models.py index c48530b0d..6dc0acde3 100644 --- a/readthedocs/ssh/models.py +++ b/readthedocs/ssh/models.py @@ -15,6 +15,7 @@ import hashlib import json from builtins import zip # pylint: disable=redefined-builtin +from encrypted_model_fields.fields import EncryptedTextField from django.db import models from django.utils.encoding import python_2_unicode_compatible from django.utils.translation import ugettext_lazy as _ @@ -41,7 +42,7 @@ class SSHKey(SSHKeyGenMixin, models.Model): _('Public SSH Key'), help_text='Add this to your version control to give us access.', ) - private_key = models.TextField(_('Private SSH Key')) + private_key = EncryptedTextField(_('Private SSH Key')) project = models.ForeignKey(Project, related_name='sshkeys') json = models.TextField(_('Serialized API response'), blank=True, null=True) diff --git a/requirements/pip.txt b/requirements/pip.txt index deb6b2f28..2b3f89f9a 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -38,6 +38,7 @@ defusedxml==0.5.0 redis==2.10.6 celery==4.1.1 cryptography==2.2.2 +django-encrypted-model-fields==0.5.5 # django-allauth 0.33.0 dropped support for Django 1.9 # https://django-allauth.readthedocs.io/en/latest/release-notes.html#backwards-incompatible-changes From f57d6bf3e14b06f567ebc1e118a96aaf869cc886 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 18 Jul 2018 20:30:43 -0300 Subject: [PATCH 2/3] Move the default key to development settings --- readthedocs/settings/base.py | 5 ----- readthedocs/settings/dev.py | 5 +++++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index b988fd528..280d3202d 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -65,11 +65,6 @@ class CommunityBaseSettings(Settings): CSRF_COOKIE_HTTPONLY = True CSRF_COOKIE_AGE = 30 * 24 * 60 * 60 - # Encryption - FIELD_ENCRYPTION_KEY = [ - 'P98DEYWoSuLmgXdRGzJkbo1JWeiHf4ghpFI8QvV3hRI=', # replace this please - ] - # Application classes @property def INSTALLED_APPS(self): # noqa diff --git a/readthedocs/settings/dev.py b/readthedocs/settings/dev.py index 7fa4dafe9..88224d171 100644 --- a/readthedocs/settings/dev.py +++ b/readthedocs/settings/dev.py @@ -48,6 +48,11 @@ class CommunityDevSettings(CommunityBaseSettings): 'test:8000', ) + # Encryption + FIELD_ENCRYPTION_KEY = [ + 'P98DEYWoSuLmgXdRGzJkbo1JWeiHf4ghpFI8QvV3hRI=', # only for development + ] + @property def LOGGING(self): # noqa - avoid pep8 N802 logging = super(CommunityDevSettings, self).LOGGING From a3369c4e3adde0da1cf1c330d30675bf4d54b9eb Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 18 Jul 2018 23:17:34 -0300 Subject: [PATCH 3/3] Use a system check to check for a valid encryption key Register a system check that checks the encryption key is defined and is not the default value used for development. --- readthedocs/ssh/__init__.py | 1 + readthedocs/ssh/apps.py | 47 +++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/readthedocs/ssh/__init__.py b/readthedocs/ssh/__init__.py index e69de29bb..de6863b79 100644 --- a/readthedocs/ssh/__init__.py +++ b/readthedocs/ssh/__init__.py @@ -0,0 +1 @@ +default_app_config = 'readthedocs.ssh.apps.SshConfig' diff --git a/readthedocs/ssh/apps.py b/readthedocs/ssh/apps.py index d9fe35b20..5778c6088 100644 --- a/readthedocs/ssh/apps.py +++ b/readthedocs/ssh/apps.py @@ -3,6 +3,8 @@ from __future__ import division, print_function, unicode_literals from django.apps import AppConfig +from django.core.checks import Error, register +from django.conf import settings class SshConfig(AppConfig): @@ -11,3 +13,48 @@ class SshConfig(AppConfig): name = 'readthedocs.ssh' verbose_name = 'SSH Keys' + + +@register() +def check_valid_encryption_key(app_configs, **kwargs): + from encrypted_model_fields.fields import parse_key + + errors = [] + keys = getattr(settings, 'FIELD_ENCRYPTION_KEY', None) + if not keys: + errors.append( + Error( + 'FIELD_ENCRYPTION_KEY must be defined in settings', + hint='Use the "generate_encryption_key" management command to generate a valid one ', # noqa + id='ssh.E001', + ), + ) + else: + default_key = 'P98DEYWoSuLmgXdRGzJkbo1JWeiHf4ghpFI8QvV3hRI=' + if not isinstance(keys, (tuple, list)): + keys = [keys] + + for key in keys: + try: + parse_key(key) + except Exception: + errors.append( + Error( + 'FIELD_ENCRYPTION_KEY setting has an incorrect padding', + hint='Use the "generate_encryption_key" management command to generate a valid one ', # noqa + id='ssh.E002', + ), + ) + + # Check against the default value to prevent users to run the + # instance in production with a shared encryption key + if key == default_key and not settings.DEBUG: + errors.append( + Error( + 'You are using the default value for FIELD_ENCRYPTION_KEY', + hint='Change it by running the "generate_encryption_key" management command', + id='ssh.E003', + ), + ) + + return errors