From a4dfd96a8d0437a16964f0f0fcf6dee4fea00ac3 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 3 Oct 2018 09:09:07 -0400 Subject: [PATCH 1/2] Validate ANSIBLE_ injectors on save and increase verbosity --- awx/main/constants.py | 8 ++++++ awx/main/fields.py | 25 +++++++++++++++++-- awx/main/models/credential/__init__.py | 17 ++++++------- .../functional/api/test_credential_type.py | 9 +++---- awx/main/tests/unit/test_fields.py | 8 +++++- 5 files changed, 49 insertions(+), 18 deletions(-) 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_) From b9279ebd5ea8a40bd46d27f86be85ba9180b8fa9 Mon Sep 17 00:00:00 2001 From: Shane McDonald Date: Tue, 9 Oct 2018 14:38:18 -0400 Subject: [PATCH 2/2] Port downstream installer changes --- installer/roles/kubernetes/defaults/main.yml | 8 ++--- installer/roles/kubernetes/tasks/backup.yml | 1 + installer/roles/kubernetes/tasks/main.yml | 3 +- installer/roles/kubernetes/tasks/restore.yml | 7 ++-- .../kubernetes/templates/deployment.yml.j2 | 36 ++++--------------- .../templates/management-pod.yml.j2 | 2 +- 6 files changed, 18 insertions(+), 39 deletions(-) diff --git a/installer/roles/kubernetes/defaults/main.yml b/installer/roles/kubernetes/defaults/main.yml index 052224d241..bc3d6851e1 100644 --- a/installer/roles/kubernetes/defaults/main.yml +++ b/installer/roles/kubernetes/defaults/main.yml @@ -1,13 +1,11 @@ --- -dockerhub_version: "{{ lookup('file', playbook_dir + '/../VERSION') }}" - admin_user: 'admin' admin_email: 'root@localhost' -admin_password: 'password' +admin_password: '' rabbitmq_user: 'awx' -rabbitmq_password: 'password' -rabbitmq_erlang_cookie: 'cookiemonster' +rabbitmq_password: '' +rabbitmq_erlang_cookie: '' kubernetes_base_path: "{{ local_base_config_path|default('/tmp') }}/{{ kubernetes_deployment_name }}-config" diff --git a/installer/roles/kubernetes/tasks/backup.yml b/installer/roles/kubernetes/tasks/backup.yml index e01b740dba..dd392d8212 100644 --- a/installer/roles/kubernetes/tasks/backup.yml +++ b/installer/roles/kubernetes/tasks/backup.yml @@ -33,6 +33,7 @@ register: result until: result.stdout == "Running" retries: 60 + delay: 10 - name: Create directory for backup file: diff --git a/installer/roles/kubernetes/tasks/main.yml b/installer/roles/kubernetes/tasks/main.yml index 1bbccc55fb..4e560247e7 100644 --- a/installer/roles/kubernetes/tasks/main.yml +++ b/installer/roles/kubernetes/tasks/main.yml @@ -24,7 +24,7 @@ kubectl_or_oc: "{{ openshift_oc_bin if openshift_oc_bin is defined else 'kubectl' }}" - set_fact: - deployment_object: "{{ 'dc' if openshift_host is defined else 'deployment' }}" + deployment_object: "sts" - name: Record deployment size shell: | @@ -156,6 +156,7 @@ register: result until: result.stdout == "Running" retries: 60 + delay: 10 - name: Migrate database shell: | diff --git a/installer/roles/kubernetes/tasks/restore.yml b/installer/roles/kubernetes/tasks/restore.yml index 84967896d4..766701ff74 100644 --- a/installer/roles/kubernetes/tasks/restore.yml +++ b/installer/roles/kubernetes/tasks/restore.yml @@ -26,7 +26,7 @@ extra_opts: [--strip-components=1] - set_fact: - deployment_object: "{{ 'dc' if openshift_host is defined else 'deployment' }}" + deployment_object: "sts" - name: Record deployment size shell: | @@ -70,6 +70,7 @@ register: result until: result.stdout == "Running" retries: 60 + delay: 10 - name: Temporarily grant createdb role shell: | @@ -79,7 +80,7 @@ --host={{ pg_hostname | default('postgresql') }} \ --port={{ pg_port | default('5432') }} \ --username=postgres \ - --dbname=template1 -c 'ALTER USER tower CREATEDB;'" + --dbname=template1 -c 'ALTER USER {{ pg_username }} CREATEDB;'" no_log: true when: pg_hostname is not defined or pg_hostname == '' @@ -102,7 +103,7 @@ --host={{ pg_hostname | default('postgresql') }} \ --port={{ pg_port | default('5432') }} \ --username=postgres \ - --dbname=template1 -c 'ALTER USER tower NOCREATEDB;'" + --dbname=template1 -c 'ALTER USER {{ pg_username }} NOCREATEDB;'" no_log: true when: pg_hostname is not defined or pg_hostname == '' diff --git a/installer/roles/kubernetes/templates/deployment.yml.j2 b/installer/roles/kubernetes/templates/deployment.yml.j2 index 88485205f0..ddc8947c2e 100644 --- a/installer/roles/kubernetes/templates/deployment.yml.j2 +++ b/installer/roles/kubernetes/templates/deployment.yml.j2 @@ -12,7 +12,7 @@ metadata: namespace: {{ kubernetes_namespace }} name: rabbitmq labels: - app: rabbitmq + app: {{ kubernetes_deployment_name }} type: LoadBalancer spec: type: NodePort @@ -26,7 +26,7 @@ spec: port: 5672 targetPort: 5672 selector: - app: rabbitmq + app: {{ kubernetes_deployment_name }} --- apiVersion: v1 @@ -109,13 +109,8 @@ userNames: {% endif %} --- -{% if openshift_host is defined %} -apiVersion: v1 -kind: DeploymentConfig -{% else %} -apiVersion: extensions/v1beta1 -kind: Deployment -{% endif %} +apiVersion: apps/v1beta1 +kind: StatefulSet metadata: name: {{ kubernetes_deployment_name }} namespace: {{ kubernetes_namespace }} @@ -126,31 +121,14 @@ spec: labels: name: {{ kubernetes_deployment_name }}-web-deploy service: django - app: rabbitmq + app: {{ kubernetes_deployment_name }} spec: serviceAccountName: awx + terminationGracePeriodSeconds: 10 containers: - name: {{ kubernetes_deployment_name }}-web image: "{{ kubernetes_web_image }}:{{ kubernetes_web_version }}" imagePullPolicy: Always - env: - - name: DATABASE_USER - value: {{ pg_username }} - - name: DATABASE_NAME - value: {{ pg_database }} - - name: DATABASE_HOST - value: {{ pg_hostname|default('postgresql') }} - - name: DATABASE_PORT - value: "{{ pg_port|default('5432') }}" - - name: DATABASE_PASSWORD - valueFrom: - secretKeyRef: - name: "{{ kubernetes_deployment_name }}-secrets" - key: pg_password - - name: MEMCACHED_HOST - value: {{ memcached_hostname|default('localhost') }} - - name: RABBITMQ_HOST - value: {{ rabbitmq_hostname|default('localhost') }} ports: - containerPort: 8052 volumeMounts: @@ -341,7 +319,7 @@ spec: port: targetPort: http tls: - insecureEdgeTerminationPolicy: Allow + insecureEdgeTerminationPolicy: Redirect termination: edge to: kind: Service diff --git a/installer/roles/kubernetes/templates/management-pod.yml.j2 b/installer/roles/kubernetes/templates/management-pod.yml.j2 index c808b72620..618b66a3ae 100644 --- a/installer/roles/kubernetes/templates/management-pod.yml.j2 +++ b/installer/roles/kubernetes/templates/management-pod.yml.j2 @@ -7,7 +7,7 @@ metadata: spec: containers: - name: ansible-tower-management - image: {{ kubernetes_task_image }} + image: "{{ kubernetes_task_image }}:{{ kubernetes_task_version }}" command: ["sleep", "infinity"] volumeMounts: - name: {{ kubernetes_deployment_name }}-application-config