diff --git a/awx/main/constants.py b/awx/main/constants.py index 3a92dfc18f..637b5d7646 100644 --- a/awx/main/constants.py +++ b/awx/main/constants.py @@ -29,3 +29,11 @@ STANDARD_INVENTORY_UPDATE_ENV = { CAN_CANCEL = ('new', 'pending', 'waiting', 'running') ACTIVE_STATES = CAN_CANCEL CENSOR_VALUE = '************' +ENV_BLACKLIST = frozenset(( + 'VIRTUAL_ENV', 'PATH', 'PYTHONPATH', 'PROOT_TMP_DIR', 'JOB_ID', + 'INVENTORY_ID', 'INVENTORY_SOURCE_ID', 'INVENTORY_UPDATE_ID', + 'AD_HOC_COMMAND_ID', 'REST_API_URL', 'REST_API_TOKEN', 'MAX_EVENT_RES', + 'CALLBACK_QUEUE', 'CALLBACK_CONNECTION', 'CACHE', + 'JOB_CALLBACK_DEBUG', 'INVENTORY_HOSTVARS', 'FACT_QUEUE', + 'AWX_HOST', 'PROJECT_REVISION' +)) diff --git a/awx/main/fields.py b/awx/main/fields.py index 0747a32b4d..5bed9407bf 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -46,7 +46,7 @@ from awx.main.utils.filters import SmartFilter from awx.main.utils.encryption import encrypt_value, decrypt_value, get_encryption_key from awx.main.validators import validate_ssh_private_key from awx.main.models.rbac import batch_role_ancestor_rebuilding, Role -from awx.main.constants import CHOICES_PRIVILEGE_ESCALATION_METHODS +from awx.main.constants import CHOICES_PRIVILEGE_ESCALATION_METHODS, ENV_BLACKLIST from awx.main import utils @@ -767,7 +767,12 @@ class CredentialTypeInjectorField(JSONSchemaField): # of underscores, digits, and alphabetics from the portable # character set. The first character of a name is not # a digit. - '^[a-zA-Z_]+[a-zA-Z0-9_]*$': {'type': 'string'}, + '^[a-zA-Z_]+[a-zA-Z0-9_]*$': { + 'type': 'string', + # The environment variable _value_ can be any ascii, + # but pexpect will choke on any unicode + 'pattern': '^[\x00-\x7F]*$' + }, }, 'additionalProperties': False, }, @@ -783,6 +788,19 @@ class CredentialTypeInjectorField(JSONSchemaField): 'additionalProperties': False } + def validate_env_var_allowed(self, env_var): + if env_var.startswith('ANSIBLE_'): + raise django_exceptions.ValidationError( + _('Environment variable {} may affect Ansible configuration so its ' + 'use is not allowed in credentials.').format(env_var), + code='invalid', params={'value': env_var}, + ) + if env_var in ENV_BLACKLIST: + raise django_exceptions.ValidationError( + _('Environment variable {} is blacklisted from use in credentials.').format(env_var), + code='invalid', params={'value': env_var}, + ) + def validate(self, value, model_instance): super(CredentialTypeInjectorField, self).validate( value, model_instance @@ -834,6 +852,9 @@ class CredentialTypeInjectorField(JSONSchemaField): setattr(valid_namespace['tower'].filename, template_name, 'EXAMPLE_FILENAME') for type_, injector in value.items(): + if type_ == 'env': + for key in injector.keys(): + self.validate_env_var_allowed(key) for key, tmpl in injector.items(): try: Environment( diff --git a/awx/main/models/credential/__init__.py b/awx/main/models/credential/__init__.py index a31131c198..a4c4774824 100644 --- a/awx/main/models/credential/__init__.py +++ b/awx/main/models/credential/__init__.py @@ -439,15 +439,6 @@ class CredentialType(CommonModelNameNotUnique): defaults = OrderedDict() - ENV_BLACKLIST = set(( - 'VIRTUAL_ENV', 'PATH', 'PYTHONPATH', 'PROOT_TMP_DIR', 'JOB_ID', - 'INVENTORY_ID', 'INVENTORY_SOURCE_ID', 'INVENTORY_UPDATE_ID', - 'AD_HOC_COMMAND_ID', 'REST_API_URL', 'REST_API_TOKEN', 'MAX_EVENT_RES', - 'CALLBACK_QUEUE', 'CALLBACK_CONNECTION', 'CACHE', - 'JOB_CALLBACK_DEBUG', 'INVENTORY_HOSTVARS', 'FACT_QUEUE', - 'AWX_HOST', 'PROJECT_REVISION' - )) - class Meta: app_label = 'main' ordering = ('kind', 'name') @@ -648,8 +639,14 @@ class CredentialType(CommonModelNameNotUnique): file_label = file_label.split('.')[1] setattr(tower_namespace.filename, file_label, path) + injector_field = self._meta.get_field('injectors') for env_var, tmpl in self.injectors.get('env', {}).items(): - if env_var.startswith('ANSIBLE_') or env_var in self.ENV_BLACKLIST: + try: + injector_field.validate_env_var_allowed(env_var) + except ValidationError as e: + logger.error(six.text_type( + 'Ignoring prohibited env var {}, reason: {}' + ).format(env_var, e)) continue env[env_var] = Template(tmpl).render(**namespace) safe_env[env_var] = Template(tmpl).render(**safe_namespace) diff --git a/awx/main/tests/functional/api/test_credential_type.py b/awx/main/tests/functional/api/test_credential_type.py index 38c8998f30..e05649ed98 100644 --- a/awx/main/tests/functional/api/test_credential_type.py +++ b/awx/main/tests/functional/api/test_credential_type.py @@ -359,18 +359,17 @@ def test_create_with_valid_injectors(get, post, admin): }, 'injectors': { 'env': { - 'ANSIBLE_MY_CLOUD_TOKEN': '{{api_token}}' + 'AWX_MY_CLOUD_TOKEN': '{{api_token}}' } } - }, admin) - assert response.status_code == 201 + }, admin, expect=201) response = get(reverse('api:credential_type_list'), admin) assert response.data['count'] == 1 injectors = response.data['results'][0]['injectors'] assert len(injectors) == 1 assert injectors['env'] == { - 'ANSIBLE_MY_CLOUD_TOKEN': '{{api_token}}' + 'AWX_MY_CLOUD_TOKEN': '{{api_token}}' } @@ -388,7 +387,7 @@ def test_create_with_undefined_template_variable_xfail(post, admin): }] }, 'injectors': { - 'env': {'ANSIBLE_MY_CLOUD_TOKEN': '{{api_tolkien}}'} + 'env': {'AWX_MY_CLOUD_TOKEN': '{{api_tolkien}}'} } }, admin) assert response.status_code == 400 diff --git a/awx/main/tests/unit/test_fields.py b/awx/main/tests/unit/test_fields.py index 79a163b840..77f08ddcb9 100644 --- a/awx/main/tests/unit/test_fields.py +++ b/awx/main/tests/unit/test_fields.py @@ -1,4 +1,6 @@ +# -*- coding: utf-8 -*- import pytest +import six from django.core.exceptions import ValidationError from rest_framework.serializers import ValidationError as DRFValidationError @@ -123,6 +125,9 @@ def test_cred_type_input_schema_validity(input_, valid): ({'env': {'AWX_SECRET_99': '{{awx_secret}}'}}, True), ({'env': {'99': '{{awx_secret}}'}}, False), ({'env': {'AWX_SECRET=': '{{awx_secret}}'}}, False), + ({'env': {'ANSIBLE_SETTING': '{{awx_secret}}'}}, False), + ({'env': {'DRAGON': u'🐉'}}, False), + ({'env': {u'🐉': 'DRAGON'}}, False), ({'extra_vars': 123}, False), ({'extra_vars': {}}, True), ({'extra_vars': {'hostname': '{{host}}'}}, True), @@ -147,7 +152,8 @@ def test_cred_type_injectors_schema(injectors, valid): ) field = CredentialType._meta.get_field('injectors') if valid is False: - with pytest.raises(ValidationError): + with pytest.raises(ValidationError, message=six.text_type( + "Injector was supposed to throw a validation error, data: {}").format(injectors)): field.clean(injectors, type_) else: field.clean(injectors, type_)