From ea0f4ce59dbcc713f640ab783adab2a6f3de37e8 Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Wed, 31 May 2017 14:16:26 -0400 Subject: [PATCH 1/2] properly validate SSH key data for SCM, Net, GCE, and Azure Classic see: #6384 --- awx/main/models/credential.py | 4 +++ .../tests/functional/api/test_credential.py | 26 ++++++++++--------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/awx/main/models/credential.py b/awx/main/models/credential.py index 660c00b75f..585d57ae8e 100644 --- a/awx/main/models/credential.py +++ b/awx/main/models/credential.py @@ -644,6 +644,7 @@ def scm(cls): 'id': 'ssh_key_data', 'label': 'SCM Private Key', 'type': 'string', + 'format': 'ssh_private_key', 'secret': True, 'multiline': True }, { @@ -694,6 +695,7 @@ def net(cls): 'id': 'ssh_key_data', 'label': 'SSH Private Key', 'type': 'string', + 'format': 'ssh_private_key', 'secret': True, 'multiline': True }, { @@ -871,6 +873,7 @@ def gce(cls): 'id': 'ssh_key_data', 'label': 'RSA Private Key', 'type': 'string', + 'format': 'ssh_private_key', 'secret': True, 'multiline': True }] @@ -893,6 +896,7 @@ def azure(cls): 'id': 'ssh_key_data', 'label': 'Management Certificate', 'type': 'string', + 'format': 'ssh_private_key', 'secret': True, 'multiline': True }] diff --git a/awx/main/tests/functional/api/test_credential.py b/awx/main/tests/functional/api/test_credential.py index 1b9b2ec610..bcc1f8f3a7 100644 --- a/awx/main/tests/functional/api/test_credential.py +++ b/awx/main/tests/functional/api/test_credential.py @@ -5,6 +5,8 @@ from awx.main.models.credential import Credential, CredentialType from awx.main.utils.common import decrypt_field from awx.api.versioning import reverse +EXAMPLE_PRIVATE_KEY = '-----BEGIN PRIVATE KEY-----\nxyz==\n-----END PRIVATE KEY-----' + @pytest.mark.django_db @pytest.mark.parametrize('kind, total', [ @@ -664,7 +666,7 @@ def test_inputs_cannot_contain_extra_fields(get, post, organization, admin, cred 'name': 'Best credential ever', 'username': 'some_username', 'password': 'some_password', - 'ssh_key_data': 'some_key_data', + 'ssh_key_data': EXAMPLE_PRIVATE_KEY, 'ssh_key_unlock': 'some_key_unlock', }], ['v2', { @@ -673,7 +675,7 @@ def test_inputs_cannot_contain_extra_fields(get, post, organization, admin, cred 'inputs': { 'username': 'some_username', 'password': 'some_password', - 'ssh_key_data': 'some_key_data', + 'ssh_key_data': EXAMPLE_PRIVATE_KEY, 'ssh_key_unlock': 'some_key_unlock', } }] @@ -693,7 +695,7 @@ def test_scm_create_ok(post, organization, admin, version, params): cred = Credential.objects.all()[:1].get() assert cred.inputs['username'] == 'some_username' assert decrypt_field(cred, 'password') == 'some_password' - assert decrypt_field(cred, 'ssh_key_data') == 'some_key_data' + assert decrypt_field(cred, 'ssh_key_data') == EXAMPLE_PRIVATE_KEY assert decrypt_field(cred, 'ssh_key_unlock') == 'some_key_unlock' @@ -796,7 +798,7 @@ def test_vault_create_ok(post, organization, admin, version, params): 'name': 'Best credential ever', 'username': 'some_username', 'password': 'some_password', - 'ssh_key_data': 'some_key_data', + 'ssh_key_data': EXAMPLE_PRIVATE_KEY, 'ssh_key_unlock': 'some_key_unlock', 'authorize': True, 'authorize_password': 'some_authorize_password', @@ -807,7 +809,7 @@ def test_vault_create_ok(post, organization, admin, version, params): 'inputs': { 'username': 'some_username', 'password': 'some_password', - 'ssh_key_data': 'some_key_data', + 'ssh_key_data': EXAMPLE_PRIVATE_KEY, 'ssh_key_unlock': 'some_key_unlock', 'authorize': True, 'authorize_password': 'some_authorize_password', @@ -829,7 +831,7 @@ def test_net_create_ok(post, organization, admin, version, params): cred = Credential.objects.all()[:1].get() assert cred.inputs['username'] == 'some_username' assert decrypt_field(cred, 'password') == 'some_password' - assert decrypt_field(cred, 'ssh_key_data') == 'some_key_data' + assert decrypt_field(cred, 'ssh_key_data') == EXAMPLE_PRIVATE_KEY assert decrypt_field(cred, 'ssh_key_unlock') == 'some_key_unlock' assert decrypt_field(cred, 'authorize_password') == 'some_authorize_password' assert cred.inputs['authorize'] is True @@ -885,7 +887,7 @@ def test_cloudforms_create_ok(post, organization, admin, version, params): 'name': 'Best credential ever', 'username': 'some_username', 'project': 'some_project', - 'ssh_key_data': 'XYZ' + 'ssh_key_data': EXAMPLE_PRIVATE_KEY, }], ['v2', { 'credential_type': 1, @@ -893,7 +895,7 @@ def test_cloudforms_create_ok(post, organization, admin, version, params): 'inputs': { 'username': 'some_username', 'project': 'some_project', - 'ssh_key_data': 'XYZ' + 'ssh_key_data': EXAMPLE_PRIVATE_KEY, } }] ]) @@ -912,7 +914,7 @@ def test_gce_create_ok(post, organization, admin, version, params): cred = Credential.objects.all()[:1].get() assert cred.inputs['username'] == 'some_username' assert cred.inputs['project'] == 'some_project' - assert decrypt_field(cred, 'ssh_key_data') == 'XYZ' + assert decrypt_field(cred, 'ssh_key_data') == EXAMPLE_PRIVATE_KEY # @@ -924,14 +926,14 @@ def test_gce_create_ok(post, organization, admin, version, params): 'kind': 'azure', 'name': 'Best credential ever', 'username': 'some_username', - 'ssh_key_data': 'XYZ' + 'ssh_key_data': EXAMPLE_PRIVATE_KEY }], ['v2', { 'credential_type': 1, 'name': 'Best credential ever', 'inputs': { 'username': 'some_username', - 'ssh_key_data': 'XYZ' + 'ssh_key_data': EXAMPLE_PRIVATE_KEY } }] ]) @@ -949,7 +951,7 @@ def test_azure_create_ok(post, organization, admin, version, params): assert Credential.objects.count() == 1 cred = Credential.objects.all()[:1].get() assert cred.inputs['username'] == 'some_username' - assert decrypt_field(cred, 'ssh_key_data') == 'XYZ' + assert decrypt_field(cred, 'ssh_key_data') == EXAMPLE_PRIVATE_KEY # From 28ad576c909f097ab0a587d4cbb200541872da27 Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Tue, 6 Jun 2017 11:08:06 -0400 Subject: [PATCH 2/2] properly validate ssh_key_unlock for Net and SCM credentials see: #6460 --- awx/main/fields.py | 5 ++++- awx/main/tests/functional/api/test_credential.py | 13 +++++++------ awx/main/tests/functional/test_credential.py | 7 +++++-- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/awx/main/fields.py b/awx/main/fields.py index ee5fb13f74..5c06ba35d9 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -501,7 +501,10 @@ class CredentialInputField(JSONSchemaField): # `ssh_key_unlock` requirements are very specific and can't be # represented without complicated JSON schema - if model_instance.credential_type.kind == 'ssh': + if ( + model_instance.credential_type.managed_by_tower is True and + 'ssh_key_unlock' in model_instance.credential_type.defined_fields + ): if model_instance.has_encrypted_ssh_key_data and not value.get('ssh_key_unlock'): errors['ssh_key_unlock'] = [_('must be set when SSH key is encrypted.')] if not model_instance.has_encrypted_ssh_key_data and value.get('ssh_key_unlock'): diff --git a/awx/main/tests/functional/api/test_credential.py b/awx/main/tests/functional/api/test_credential.py index bcc1f8f3a7..c552763772 100644 --- a/awx/main/tests/functional/api/test_credential.py +++ b/awx/main/tests/functional/api/test_credential.py @@ -6,6 +6,7 @@ from awx.main.utils.common import decrypt_field from awx.api.versioning import reverse EXAMPLE_PRIVATE_KEY = '-----BEGIN PRIVATE KEY-----\nxyz==\n-----END PRIVATE KEY-----' +EXAMPLE_ENCRYPTED_PRIVATE_KEY = '-----BEGIN PRIVATE KEY-----\nProc-Type: 4,ENCRYPTED\nxyz==\n-----END PRIVATE KEY-----' @pytest.mark.django_db @@ -666,7 +667,7 @@ def test_inputs_cannot_contain_extra_fields(get, post, organization, admin, cred 'name': 'Best credential ever', 'username': 'some_username', 'password': 'some_password', - 'ssh_key_data': EXAMPLE_PRIVATE_KEY, + 'ssh_key_data': EXAMPLE_ENCRYPTED_PRIVATE_KEY, 'ssh_key_unlock': 'some_key_unlock', }], ['v2', { @@ -675,7 +676,7 @@ def test_inputs_cannot_contain_extra_fields(get, post, organization, admin, cred 'inputs': { 'username': 'some_username', 'password': 'some_password', - 'ssh_key_data': EXAMPLE_PRIVATE_KEY, + 'ssh_key_data': EXAMPLE_ENCRYPTED_PRIVATE_KEY, 'ssh_key_unlock': 'some_key_unlock', } }] @@ -695,7 +696,7 @@ def test_scm_create_ok(post, organization, admin, version, params): cred = Credential.objects.all()[:1].get() assert cred.inputs['username'] == 'some_username' assert decrypt_field(cred, 'password') == 'some_password' - assert decrypt_field(cred, 'ssh_key_data') == EXAMPLE_PRIVATE_KEY + assert decrypt_field(cred, 'ssh_key_data') == EXAMPLE_ENCRYPTED_PRIVATE_KEY assert decrypt_field(cred, 'ssh_key_unlock') == 'some_key_unlock' @@ -798,7 +799,7 @@ def test_vault_create_ok(post, organization, admin, version, params): 'name': 'Best credential ever', 'username': 'some_username', 'password': 'some_password', - 'ssh_key_data': EXAMPLE_PRIVATE_KEY, + 'ssh_key_data': EXAMPLE_ENCRYPTED_PRIVATE_KEY, 'ssh_key_unlock': 'some_key_unlock', 'authorize': True, 'authorize_password': 'some_authorize_password', @@ -809,7 +810,7 @@ def test_vault_create_ok(post, organization, admin, version, params): 'inputs': { 'username': 'some_username', 'password': 'some_password', - 'ssh_key_data': EXAMPLE_PRIVATE_KEY, + 'ssh_key_data': EXAMPLE_ENCRYPTED_PRIVATE_KEY, 'ssh_key_unlock': 'some_key_unlock', 'authorize': True, 'authorize_password': 'some_authorize_password', @@ -831,7 +832,7 @@ def test_net_create_ok(post, organization, admin, version, params): cred = Credential.objects.all()[:1].get() assert cred.inputs['username'] == 'some_username' assert decrypt_field(cred, 'password') == 'some_password' - assert decrypt_field(cred, 'ssh_key_data') == EXAMPLE_PRIVATE_KEY + assert decrypt_field(cred, 'ssh_key_data') == EXAMPLE_ENCRYPTED_PRIVATE_KEY assert decrypt_field(cred, 'ssh_key_unlock') == 'some_key_unlock' assert decrypt_field(cred, 'authorize_password') == 'some_authorize_password' assert cred.inputs['authorize'] is True diff --git a/awx/main/tests/functional/test_credential.py b/awx/main/tests/functional/test_credential.py index 16d80e6a3c..bd2fdbcdae 100644 --- a/awx/main/tests/functional/test_credential.py +++ b/awx/main/tests/functional/test_credential.py @@ -211,6 +211,7 @@ def test_credential_creation_validation_failure(organization_factory, inputs): @pytest.mark.django_db +@pytest.mark.parametrize('kind', ['ssh', 'net', 'scm']) @pytest.mark.parametrize('ssh_key_data, ssh_key_unlock, valid', [ [EXAMPLE_PRIVATE_KEY, None, True], # unencrypted key, no unlock pass [EXAMPLE_PRIVATE_KEY, 'super-secret', False], # unencrypted key, unlock pass @@ -221,14 +222,16 @@ def test_credential_creation_validation_failure(organization_factory, inputs): ['INVALID-KEY-DATA', None, False], # invalid key data [EXAMPLE_PRIVATE_KEY.replace('=', '\u003d'), None, True], # automatically fix JSON-encoded GCE keys ]) -def test_ssh_key_data_validation(credentialtype_ssh, organization, ssh_key_data, ssh_key_unlock, valid): +def test_ssh_key_data_validation(organization, kind, ssh_key_data, ssh_key_unlock, valid): inputs = {} if ssh_key_data: inputs['ssh_key_data'] = ssh_key_data if ssh_key_unlock: inputs['ssh_key_unlock'] = ssh_key_unlock + cred_type = CredentialType.defaults[kind]() + cred_type.save() cred = Credential( - credential_type=credentialtype_ssh, + credential_type=cred_type, name="Best credential ever", inputs=inputs, organization=organization