From adc371fcf8e455486372f08f1d2dc8b69fd6dc9e Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 15 Jun 2017 09:56:43 -0400 Subject: [PATCH 1/5] remove errant encryption migration call ensure credential migration was using the proper algorithm --- awx/main/migrations/0038_v320_release.py | 2 -- awx/main/migrations/_credentialtypes.py | 2 +- awx/main/migrations/_reencrypt.py | 7 ++++--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/awx/main/migrations/0038_v320_release.py b/awx/main/migrations/0038_v320_release.py index ddb15aee57..585b42ce04 100644 --- a/awx/main/migrations/0038_v320_release.py +++ b/awx/main/migrations/0038_v320_release.py @@ -9,7 +9,6 @@ from psycopg2.extensions import AsIs from django.db import migrations, models # AWX -from awx.main.migrations import _reencrypt as reencrypt import awx.main.fields from awx.main.models import Host @@ -277,7 +276,6 @@ class Migration(migrations.Migration): name='kind', field=models.CharField(default=b'', help_text='Kind of inventory being represented.', max_length=32, blank=True, choices=[(b'', 'Hosts have a direct link to this inventory.'), (b'smart', 'Hosts for inventory generated using the host_filter property.')]), ), - migrations.RunPython(reencrypt.replace_aesecb_fernet), # Timeout help text update migrations.AlterField( diff --git a/awx/main/migrations/_credentialtypes.py b/awx/main/migrations/_credentialtypes.py index a0fc1aac1e..0f71af81fe 100644 --- a/awx/main/migrations/_credentialtypes.py +++ b/awx/main/migrations/_credentialtypes.py @@ -1,6 +1,6 @@ from awx.main import utils from awx.main.models import CredentialType -from awx.main.utils import encrypt_field, decrypt_field +from awx.conf.migrations._reencrypt import encrypt_field, decrypt_field from django.db.models import Q diff --git a/awx/main/migrations/_reencrypt.py b/awx/main/migrations/_reencrypt.py index 29ae5867d1..ae5dc5e76d 100644 --- a/awx/main/migrations/_reencrypt.py +++ b/awx/main/migrations/_reencrypt.py @@ -26,12 +26,13 @@ def _credentials(apps): Credential = apps.get_model('main', 'Credential') for credential in Credential.objects.all(): for field_name, value in credential.inputs.items(): - if field_name in credential.credential_type.secret_fields: + if field_name in credential.credential_type.inputs.get('fields', []): value = getattr(credential, field_name) if value.startswith('$encrypted$AESCBC$'): continue - value = decrypt_field(credential, field_name) - credential.inputs[field_name] = value + elif value.startswith('$encrypted$AES$'): + value = decrypt_field(credential, field_name) + credential.inputs[field_name] = value credential.save() From 2c1a39233e660b6db522bf148024121b47f86967 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 15 Jun 2017 11:00:20 -0400 Subject: [PATCH 2/5] monkey-patch get_current_apps so that resolve_role_field works correctly --- awx/main/migrations/_reencrypt.py | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/awx/main/migrations/_reencrypt.py b/awx/main/migrations/_reencrypt.py index ae5dc5e76d..2aa1d2fbb0 100644 --- a/awx/main/migrations/_reencrypt.py +++ b/awx/main/migrations/_reencrypt.py @@ -1,3 +1,4 @@ +from awx.main import utils from awx.conf.migrations._reencrypt import decrypt_field @@ -23,17 +24,25 @@ def _notification_templates(apps): def _credentials(apps): - Credential = apps.get_model('main', 'Credential') - for credential in Credential.objects.all(): - for field_name, value in credential.inputs.items(): - if field_name in credential.credential_type.inputs.get('fields', []): - value = getattr(credential, field_name) - if value.startswith('$encrypted$AESCBC$'): - continue - elif value.startswith('$encrypted$AES$'): - value = decrypt_field(credential, field_name) - credential.inputs[field_name] = value - credential.save() + # this monkey-patch is necessary to make the implicit role generation save + # signal use the correct Role model (the version active at this point in + # migration, not the one at HEAD) + orig_current_apps = utils.get_current_apps + try: + utils.get_current_apps = lambda: apps + for credential in apps.get_model('main', 'Credential').objects.all(): + for field_name, value in credential.inputs.items(): + if field_name in credential.credential_type.inputs.get('fields', []): + value = getattr(credential, field_name) + if value.startswith('$encrypted$AESCBC$'): + continue + elif value.startswith('$encrypted$AES$'): + value = decrypt_field(credential, field_name) + credential.inputs[field_name] = value + credential.save() + finally: + utils.get_current_apps = orig_current_apps + def _unified_jobs(apps): From c554c10c5c5df8dbae5fef75e2f83ae814e1f2d2 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 15 Jun 2017 11:08:22 -0400 Subject: [PATCH 3/5] remove credential_type iteration, explicitly check for encyption with startswith --- awx/main/migrations/_reencrypt.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/awx/main/migrations/_reencrypt.py b/awx/main/migrations/_reencrypt.py index 2aa1d2fbb0..39b00d85e3 100644 --- a/awx/main/migrations/_reencrypt.py +++ b/awx/main/migrations/_reencrypt.py @@ -32,13 +32,11 @@ def _credentials(apps): utils.get_current_apps = lambda: apps for credential in apps.get_model('main', 'Credential').objects.all(): for field_name, value in credential.inputs.items(): - if field_name in credential.credential_type.inputs.get('fields', []): - value = getattr(credential, field_name) - if value.startswith('$encrypted$AESCBC$'): - continue - elif value.startswith('$encrypted$AES$'): - value = decrypt_field(credential, field_name) - credential.inputs[field_name] = value + if value.startswith('$encrypted$AESCBC$'): + continue + elif value.startswith('$encrypted$AES$'): + value = decrypt_field(credential, field_name) + credential.inputs[field_name] = value credential.save() finally: utils.get_current_apps = orig_current_apps From 9014e8c612d0c037fa74e49911b4f25c550e4be5 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 15 Jun 2017 11:19:46 -0400 Subject: [PATCH 4/5] remove if AESCBC overhead --- awx/main/migrations/_reencrypt.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/awx/main/migrations/_reencrypt.py b/awx/main/migrations/_reencrypt.py index 39b00d85e3..20eab67c86 100644 --- a/awx/main/migrations/_reencrypt.py +++ b/awx/main/migrations/_reencrypt.py @@ -32,9 +32,7 @@ def _credentials(apps): utils.get_current_apps = lambda: apps for credential in apps.get_model('main', 'Credential').objects.all(): for field_name, value in credential.inputs.items(): - if value.startswith('$encrypted$AESCBC$'): - continue - elif value.startswith('$encrypted$AES$'): + if value.startswith('$encrypted$AES$'): value = decrypt_field(credential, field_name) credential.inputs[field_name] = value credential.save() From f9b412419c4ca2a1adc62c52abcad439926a09ec Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 15 Jun 2017 12:04:54 -0400 Subject: [PATCH 5/5] update encryption migration checks and tests --- awx/conf/migrations/_reencrypt.py | 11 ++++--- .../functional/test_reencrypt_migration.py | 18 +++++++--- awx/main/migrations/_reencrypt.py | 23 ++++++------- .../functional/test_reencrypt_migration.py | 33 +++++++++++++------ 4 files changed, 55 insertions(+), 30 deletions(-) diff --git a/awx/conf/migrations/_reencrypt.py b/awx/conf/migrations/_reencrypt.py index 013c96d33b..ca19a9234c 100644 --- a/awx/conf/migrations/_reencrypt.py +++ b/awx/conf/migrations/_reencrypt.py @@ -9,7 +9,7 @@ from awx.conf import settings_registry __all__ = ['replace_aesecb_fernet', 'get_encryption_key', 'encrypt_field', - 'decrypt_value', 'decrypt_value'] + 'decrypt_value', 'decrypt_value', 'should_decrypt_field'] def replace_aesecb_fernet(apps, schema_editor): @@ -17,9 +17,8 @@ def replace_aesecb_fernet(apps, schema_editor): for setting in Setting.objects.filter().order_by('pk'): if settings_registry.is_setting_encrypted(setting.key): - if setting.value.startswith('$encrypted$AESCBC$'): - continue - setting.value = decrypt_field(setting, 'value') + if should_decrypt_field(setting.value): + setting.value = decrypt_field(setting, 'value') setting.save() @@ -100,3 +99,7 @@ def encrypt_field(instance, field_name, ask=False, subfield=None, skip_utf8=Fals # know to decode the data when it's decrypted later tokens.insert(1, 'UTF8') return '$'.join(tokens) + + +def should_decrypt_field(value): + return value.startswith('$encrypted$') and '$AESCBC$' not in value diff --git a/awx/conf/tests/functional/test_reencrypt_migration.py b/awx/conf/tests/functional/test_reencrypt_migration.py index 78cbf734a9..e138ddb77f 100644 --- a/awx/conf/tests/functional/test_reencrypt_migration.py +++ b/awx/conf/tests/functional/test_reencrypt_migration.py @@ -1,3 +1,7 @@ +# -*- coding: utf-8 -*- + +# Copyright (c) 2017 Ansible, Inc. +# All Rights Reserved. import pytest import mock @@ -12,17 +16,21 @@ from awx.main.utils import decrypt_field as new_decrypt_field @pytest.mark.django_db -def test_settings(): +@pytest.mark.parametrize("old_enc, new_enc, value", [ + ('$encrypted$UTF8$AES', '$encrypted$UTF8$AESCBC$', u'Iñtërnâtiônàlizætiøn'), + ('$encrypted$AES$', '$encrypted$AESCBC$', 'test'), +]) +def test_settings(old_enc, new_enc, value): with mock.patch('awx.conf.models.encrypt_field', encrypt_field): with mock.patch('awx.conf.settings.decrypt_field', decrypt_field): - setting = Setting.objects.create(key='SOCIAL_AUTH_GITHUB_SECRET', value='test') - assert setting.value.startswith('$encrypted$AES$') + setting = Setting.objects.create(key='SOCIAL_AUTH_GITHUB_SECRET', value=value) + assert setting.value.startswith(old_enc) replace_aesecb_fernet(apps, None) setting.refresh_from_db() - assert setting.value.startswith('$encrypted$AESCBC$') - assert new_decrypt_field(setting, 'value') == 'test' + assert setting.value.startswith(new_enc) + assert new_decrypt_field(setting, 'value') == value # This is here for a side-effect. # Exception if the encryption type of AESCBC is not properly skipped, ensures diff --git a/awx/main/migrations/_reencrypt.py b/awx/main/migrations/_reencrypt.py index 20eab67c86..b246ea64c8 100644 --- a/awx/main/migrations/_reencrypt.py +++ b/awx/main/migrations/_reencrypt.py @@ -1,5 +1,8 @@ from awx.main import utils -from awx.conf.migrations._reencrypt import decrypt_field +from awx.conf.migrations._reencrypt import ( + decrypt_field, + should_decrypt_field, +) __all__ = ['replace_aesecb_fernet'] @@ -16,10 +19,9 @@ def _notification_templates(apps): for nt in NotificationTemplate.objects.all(): for field in filter(lambda x: nt.notification_class.init_parameters[x]['type'] == "password", nt.notification_class.init_parameters): - if nt.notification_configuration[field].startswith('$encrypted$AESCBC$'): - continue - value = decrypt_field(nt, 'notification_configuration', subfield=field) - nt.notification_configuration[field] = value + if should_decrypt_field(nt.notification_configuration[field]): + value = decrypt_field(nt, 'notification_configuration', subfield=field) + nt.notification_configuration[field] = value nt.save() @@ -32,7 +34,7 @@ def _credentials(apps): utils.get_current_apps = lambda: apps for credential in apps.get_model('main', 'Credential').objects.all(): for field_name, value in credential.inputs.items(): - if value.startswith('$encrypted$AES$'): + if should_decrypt_field(value): value = decrypt_field(credential, field_name) credential.inputs[field_name] = value credential.save() @@ -45,8 +47,7 @@ def _unified_jobs(apps): UnifiedJob = apps.get_model('main', 'UnifiedJob') for uj in UnifiedJob.objects.all(): if uj.start_args is not None: - if uj.start_args.startswith('$encrypted$AESCBC$'): - continue - start_args = decrypt_field(uj, 'start_args') - uj.start_args = start_args - uj.save() + if should_decrypt_field(uj.start_args): + start_args = decrypt_field(uj, 'start_args') + uj.start_args = start_args + uj.save() diff --git a/awx/main/tests/functional/test_reencrypt_migration.py b/awx/main/tests/functional/test_reencrypt_migration.py index de594b8638..3201866893 100644 --- a/awx/main/tests/functional/test_reencrypt_migration.py +++ b/awx/main/tests/functional/test_reencrypt_migration.py @@ -1,3 +1,7 @@ +# -*- coding: utf-8 -*- + +# Copyright (c) 2017 Ansible, Inc. +# All Rights Reserved. import json import pytest import mock @@ -23,6 +27,7 @@ from awx.main.utils import decrypt_field @pytest.mark.django_db def test_notification_template_migration(): + # Doesn't get tagged as UTF8 because the the internal save call explicitly sets skip_utf8=True with mock.patch('awx.main.models.notifications.encrypt_field', encrypt_field): nt = NotificationTemplate.objects.create(notification_type='slack', notification_configuration=dict(token='test')) @@ -42,20 +47,24 @@ def test_notification_template_migration(): @pytest.mark.django_db -def test_credential_migration(): +@pytest.mark.parametrize("old_enc, new_enc, value", [ + ('$encrypted$UTF8$AES', '$encrypted$UTF8$AESCBC$', u'Iñtërnâtiônàlizætiøn'), + ('$encrypted$AES$', '$encrypted$AESCBC$', 'test'), +]) +def test_credential_migration(old_enc, new_enc, value): with mock.patch('awx.main.models.credential.encrypt_field', encrypt_field): cred_type = ssh() cred_type.save() - cred = Credential.objects.create(credential_type=cred_type, inputs=dict(password='test')) + cred = Credential.objects.create(credential_type=cred_type, inputs=dict(password=value)) - assert cred.password.startswith('$encrypted$AES$') + assert cred.password.startswith(old_enc) _credentials(apps) cred.refresh_from_db() - assert cred.password.startswith('$encrypted$AESCBC$') - assert decrypt_field(cred, 'password') == 'test' + assert cred.password.startswith(new_enc) + assert decrypt_field(cred, 'password') == value # This is here for a side-effect. # Exception if the encryption type of AESCBC is not properly skipped, ensures @@ -64,17 +73,21 @@ def test_credential_migration(): @pytest.mark.django_db -def test_unified_job_migration(): +@pytest.mark.parametrize("old_enc, new_enc, value", [ + ('$encrypted$AES$', '$encrypted$AESCBC$', u'Iñtërnâtiônàlizætiøn'), + ('$encrypted$AES$', '$encrypted$AESCBC$', 'test'), +]) +def test_unified_job_migration(old_enc, new_enc, value): with mock.patch('awx.main.models.base.encrypt_field', encrypt_field): - uj = UnifiedJob.objects.create(launch_type='manual', start_args=json.dumps({'test':'value'})) + uj = UnifiedJob.objects.create(launch_type='manual', start_args=json.dumps({'test':value})) - assert uj.start_args.startswith('$encrypted$AES$') + assert uj.start_args.startswith(old_enc) _unified_jobs(apps) uj.refresh_from_db() - assert uj.start_args.startswith('$encrypted$AESCBC$') - assert json.loads(decrypt_field(uj, 'start_args')) == {'test':'value'} + assert uj.start_args.startswith(new_enc) + assert json.loads(decrypt_field(uj, 'start_args')) == {'test':value} # This is here for a side-effect. # Exception if the encryption type of AESCBC is not properly skipped, ensures