diff --git a/awx/main/conf.py b/awx/main/conf.py index 63714ab4bb..3dc3453fb1 100644 --- a/awx/main/conf.py +++ b/awx/main/conf.py @@ -132,6 +132,27 @@ register( required=False, ) +register( + 'ALLOW_JINJA_IN_EXTRA_VARS', + field_class=fields.ChoiceField, + choices=[ + ('always', _('Always')), + ('never', _('Never')), + ('template', _('Only On Job Template Definitions')), + ], + required=True, + label=_('When can extra variables contain Jinja templates?'), + help_text=_( + 'Ansible allows variable substitution via the Jinja2 templating ' + 'language for --extra-vars. This poses a potential security ' + 'risk where Tower users with the ability to specify extra vars at job ' + 'launch time can use Jinja2 templates to run arbitrary Python. It is ' + 'recommended that this value be set to "template" or "never".' + ), + category=_('Jobs'), + category_slug='jobs', +) + register( 'AWX_PROOT_ENABLED', field_class=fields.BooleanField, diff --git a/awx/main/models/credential.py b/awx/main/models/credential.py index f33ed07ee4..5eddacd225 100644 --- a/awx/main/models/credential.py +++ b/awx/main/models/credential.py @@ -2,7 +2,6 @@ # All Rights Reserved. from collections import OrderedDict import functools -import json import logging import operator import os @@ -25,6 +24,7 @@ from awx.main.fields import (ImplicitRoleField, CredentialInputField, CredentialTypeInputField, CredentialTypeInjectorField) from awx.main.utils import decrypt_field +from awx.main.utils.safe_yaml import safe_dump from awx.main.validators import validate_ssh_private_key from awx.main.models.base import * # noqa from awx.main.models.mixins import ResourceMixin @@ -581,16 +581,21 @@ class CredentialType(CommonModelNameNotUnique): safe_env[env_var] = Template(tmpl).render(**safe_namespace) extra_vars = {} - safe_extra_vars = {} for var_name, tmpl in self.injectors.get('extra_vars', {}).items(): extra_vars[var_name] = Template(tmpl).render(**namespace) - safe_extra_vars[var_name] = Template(tmpl).render(**safe_namespace) + def build_extra_vars_file(vars, private_dir): + handle, path = tempfile.mkstemp(dir = private_dir) + f = os.fdopen(handle, 'w') + f.write(safe_dump(vars)) + f.close() + os.chmod(path, stat.S_IRUSR) + return path + + path = build_extra_vars_file(extra_vars, private_data_dir) if extra_vars: - args.extend(['-e', json.dumps(extra_vars)]) - - if safe_extra_vars: - safe_args.extend(['-e', json.dumps(safe_extra_vars)]) + args.extend(['-e', '@%s' % path]) + safe_args.extend(['-e', '@%s' % path]) @CredentialType.default diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 52d70f51d3..ce85ab4d83 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -58,6 +58,7 @@ from awx.main.utils import (get_ansible_version, get_ssh_version, decrypt_field, wrap_args_with_proot, get_system_task_capacity, OutputEventFilter, parse_yaml_or_json, ignore_inventory_computed_fields, ignore_inventory_group_removal, get_type_for_model, extract_ansible_vars) +from awx.main.utils.safe_yaml import safe_dump, sanitize_jinja from awx.main.utils.reload import restart_local_services, stop_local_services from awx.main.utils.handlers import configure_external_logger from awx.main.consumers import emit_channel_notification @@ -622,6 +623,17 @@ class BaseTask(LogErrorsTask): '': '', } + def build_extra_vars_file(self, vars, **kwargs): + handle, path = tempfile.mkstemp(dir=kwargs.get('private_data_dir', None)) + f = os.fdopen(handle, 'w') + if settings.ALLOW_JINJA_IN_EXTRA_VARS == 'always': + f.write(yaml.safe_dump(vars)) + else: + f.write(safe_dump(vars, kwargs.get('safe_dict', {}) or None)) + f.close() + os.chmod(path, stat.S_IRUSR) + return path + def add_ansible_venv(self, env, add_awx_lib=True): env['VIRTUAL_ENV'] = settings.ANSIBLE_VENV_PATH env['PATH'] = os.path.join(settings.ANSIBLE_VENV_PATH, "bin") + ":" + env['PATH'] @@ -900,8 +912,7 @@ class BaseTask(LogErrorsTask): except Exception: if status != 'canceled': tb = traceback.format_exc() - if settings.DEBUG: - logger.exception('%s Exception occurred while running task', instance.log_format) + logger.exception('%s Exception occurred while running task', instance.log_format) finally: try: stdout_handle.flush() @@ -1142,7 +1153,7 @@ class RunJob(BaseTask): args = ['ansible-playbook', '-i', self.build_inventory(job, **kwargs)] if job.job_type == 'check': args.append('--check') - args.extend(['-u', ssh_username]) + args.extend(['-u', sanitize_jinja(ssh_username)]) if 'ssh_password' in kwargs.get('passwords', {}): args.append('--ask-pass') if job.become_enabled: @@ -1150,9 +1161,9 @@ class RunJob(BaseTask): if job.diff_mode: args.append('--diff') if become_method: - args.extend(['--become-method', become_method]) + args.extend(['--become-method', sanitize_jinja(become_method)]) if become_username: - args.extend(['--become-user', become_username]) + args.extend(['--become-user', sanitize_jinja(become_username)]) if 'become_password' in kwargs.get('passwords', {}): args.append('--ask-become-pass') # Support prompting for a vault password. @@ -1205,7 +1216,21 @@ class RunJob(BaseTask): extra_vars.update(json.loads(job.display_extra_vars())) else: extra_vars.update(json.loads(job.decrypted_extra_vars())) - args.extend(['-e', json.dumps(extra_vars)]) + + # By default, all extra vars disallow Jinja2 template usage for + # security reasons; top level key-values defined in JT.extra_vars, however, + # are whitelisted as "safe" (because they can only be set by users with + # higher levels of privilege - those that have the ability create and + # edit Job Templates) + safe_dict = {} + if job.job_template and settings.ALLOW_JINJA_IN_EXTRA_VARS == 'template': + safe_dict = job.job_template.extra_vars_dict + extra_vars_path = self.build_extra_vars_file( + vars=extra_vars, + safe_dict=safe_dict, + **kwargs + ) + args.extend(['-e', '@%s' % (extra_vars_path)]) # Add path to playbook (relative to project.local_path). args.append(job.playbook) @@ -1460,7 +1485,8 @@ class RunProjectUpdate(BaseTask): 'scm_revision_output': self.revision_path, 'scm_revision': project_update.project.scm_revision, }) - args.extend(['-e', json.dumps(extra_vars)]) + extra_vars_path = self.build_extra_vars_file(vars=extra_vars, **kwargs) + args.extend(['-e', '@%s' % (extra_vars_path)]) args.append('project_update.yml') return args @@ -2179,7 +2205,7 @@ class RunAdHocCommand(BaseTask): args = ['ansible', '-i', self.build_inventory(ad_hoc_command, **kwargs)] if ad_hoc_command.job_type == 'check': args.append('--check') - args.extend(['-u', ssh_username]) + args.extend(['-u', sanitize_jinja(ssh_username)]) if 'ssh_password' in kwargs.get('passwords', {}): args.append('--ask-pass') # We only specify sudo/su user and password if explicitly given by the @@ -2187,9 +2213,9 @@ class RunAdHocCommand(BaseTask): if ad_hoc_command.become_enabled: args.append('--become') if become_method: - args.extend(['--become-method', become_method]) + args.extend(['--become-method', sanitize_jinja(become_method)]) if become_username: - args.extend(['--become-user', become_username]) + args.extend(['--become-user', sanitize_jinja(become_username)]) if 'become_password' in kwargs.get('passwords', {}): args.append('--ask-become-pass') @@ -2220,10 +2246,11 @@ class RunAdHocCommand(BaseTask): "{} are prohibited from use in ad hoc commands." ).format(", ".join(removed_vars))) extra_vars.update(ad_hoc_command.extra_vars_dict) - args.extend(['-e', json.dumps(extra_vars)]) + extra_vars_path = self.build_extra_vars_file(vars=extra_vars, **kwargs) + args.extend(['-e', '@%s' % (extra_vars_path)]) args.extend(['-m', ad_hoc_command.module_name]) - args.extend(['-a', ad_hoc_command.module_args]) + args.extend(['-a', sanitize_jinja(ad_hoc_command.module_args)]) if ad_hoc_command.limit: args.append(ad_hoc_command.limit) diff --git a/awx/main/tests/unit/models/test_survey_models.py b/awx/main/tests/unit/models/test_survey_models.py index abd0b5fbba..a235984ed8 100644 --- a/awx/main/tests/unit/models/test_survey_models.py +++ b/awx/main/tests/unit/models/test_survey_models.py @@ -1,6 +1,7 @@ import tempfile import pytest import json +import yaml from awx.main.utils.encryption import encrypt_value from awx.main.tasks import RunJob @@ -9,6 +10,7 @@ from awx.main.models import ( JobTemplate, WorkflowJobTemplate ) +from awx.main.utils.safe_yaml import SafeLoader ENCRYPTED_SECRET = encrypt_value('secret') @@ -71,7 +73,9 @@ def test_job_safe_args_redacted_passwords(job): run_job = RunJob() safe_args = run_job.build_safe_args(job, **kwargs) ev_index = safe_args.index('-e') + 1 - extra_vars = json.loads(safe_args[ev_index]) + extra_var_file = open(safe_args[ev_index][1:], 'r') + extra_vars = yaml.load(extra_var_file, SafeLoader) + extra_var_file.close() assert extra_vars['secret_key'] == '$encrypted$' @@ -80,7 +84,9 @@ def test_job_args_unredacted_passwords(job, tmpdir_factory): run_job = RunJob() args = run_job.build_args(job, **kwargs) ev_index = args.index('-e') + 1 - extra_vars = json.loads(args[ev_index]) + extra_var_file = open(args[ev_index][1:], 'r') + extra_vars = yaml.load(extra_var_file, SafeLoader) + extra_var_file.close() assert extra_vars['secret_key'] == 'my_password' diff --git a/awx/main/tests/unit/test_tasks.py b/awx/main/tests/unit/test_tasks.py index 20a051ec30..e81fe794a5 100644 --- a/awx/main/tests/unit/test_tasks.py +++ b/awx/main/tests/unit/test_tasks.py @@ -23,6 +23,7 @@ from awx.main.models import ( InventorySource, InventoryUpdate, Job, + JobTemplate, Notification, Project, ProjectUpdate, @@ -32,7 +33,7 @@ from awx.main.models import ( from awx.main import tasks from awx.main.utils import encrypt_field, encrypt_value - +from awx.main.utils.safe_yaml import SafeLoader @contextmanager @@ -172,6 +173,15 @@ def pytest_generate_tests(metafunc): ) +def parse_extra_vars(args): + extra_vars = {} + for chunk in args: + if chunk.startswith('@/tmp/'): + with open(chunk.strip('@'), 'r') as f: + extra_vars.update(yaml.load(f, SafeLoader)) + return extra_vars + + class TestJobExecution: """ For job runs, test that `ansible-playbook` is invoked with the proper @@ -234,7 +244,8 @@ class TestJobExecution: cancel_flag=False, project=Project(), playbook='helloworld.yml', - verbosity=3 + verbosity=3, + job_template=JobTemplate(extra_vars='') ) # mock the job.extra_credentials M2M relation so we can avoid DB access @@ -254,6 +265,131 @@ class TestJobExecution: return self.instance.pk +class TestExtraVarSanitation(TestJobExecution): + # By default, extra vars are marked as `!unsafe` in the generated yaml + # _unless_ they've been specified on the JobTemplate's extra_vars (which + # are deemed trustable, because they can only be added by users w/ enough + # privilege to add/modify a Job Template) + + UNSAFE = '{{ lookup(''pipe'',''ls -la'') }}' + + def test_vars_unsafe_by_default(self): + self.instance.created_by = User(pk=123, username='angry-spud') + + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + + # ensure that strings are marked as unsafe + for unsafe in ['awx_job_template_name', 'tower_job_template_name', + 'awx_user_name', 'tower_job_launch_type', + 'awx_project_revision', + 'tower_project_revision', 'tower_user_name', + 'awx_job_launch_type']: + assert hasattr(extra_vars[unsafe], '__UNSAFE__') + + # ensure that non-strings are marked as safe + for safe in ['awx_job_template_id', 'awx_job_id', 'awx_user_id', + 'tower_user_id', 'tower_job_template_id', + 'tower_job_id']: + assert not hasattr(extra_vars[safe], '__UNSAFE__') + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) + + def test_launchtime_vars_unsafe(self): + self.instance.extra_vars = json.dumps({'msg': self.UNSAFE}) + + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars['msg'] == self.UNSAFE + assert hasattr(extra_vars['msg'], '__UNSAFE__') + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) + + def test_nested_launchtime_vars_unsafe(self): + self.instance.extra_vars = json.dumps({'msg': {'a': [self.UNSAFE]}}) + + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars['msg'] == {'a': [self.UNSAFE]} + assert hasattr(extra_vars['msg']['a'][0], '__UNSAFE__') + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) + + def test_whitelisted_jt_extra_vars(self): + self.instance.job_template.extra_vars = self.instance.extra_vars = json.dumps({'msg': self.UNSAFE}) + + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars['msg'] == self.UNSAFE + assert not hasattr(extra_vars['msg'], '__UNSAFE__') + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) + + def test_nested_whitelisted_vars(self): + self.instance.extra_vars = json.dumps({'msg': {'a': {'b': [self.UNSAFE]}}}) + self.instance.job_template.extra_vars = self.instance.extra_vars + + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars['msg'] == {'a': {'b': [self.UNSAFE]}} + assert not hasattr(extra_vars['msg']['a']['b'][0], '__UNSAFE__') + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) + + def test_sensitive_values_dont_leak(self): + # JT defines `msg=SENSITIVE`, the job *should not* be able to do + # `other_var=SENSITIVE` + self.instance.job_template.extra_vars = json.dumps({'msg': self.UNSAFE}) + self.instance.extra_vars = json.dumps({ + 'msg': 'other-value', + 'other_var': self.UNSAFE + }) + + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + + assert extra_vars['msg'] == 'other-value' + assert hasattr(extra_vars['msg'], '__UNSAFE__') + + assert extra_vars['other_var'] == self.UNSAFE + assert hasattr(extra_vars['other_var'], '__UNSAFE__') + + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) + + def test_overwritten_jt_extra_vars(self): + self.instance.job_template.extra_vars = json.dumps({'msg': 'SAFE'}) + self.instance.extra_vars = json.dumps({'msg': self.UNSAFE}) + + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars['msg'] == self.UNSAFE + assert hasattr(extra_vars['msg'], '__UNSAFE__') + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) + + class TestGenericRun(TestJobExecution): def test_cancel_flag(self): @@ -296,15 +432,18 @@ class TestGenericRun(TestJobExecution): def test_created_by_extra_vars(self): self.instance.created_by = User(pk=123, username='angry-spud') - self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, _ = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - assert '"tower_user_id": 123,' in ' '.join(args) - assert '"tower_user_name": "angry-spud"' in ' '.join(args) - assert '"awx_user_id": 123,' in ' '.join(args) - assert '"awx_user_name": "angry-spud"' in ' '.join(args) + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars['tower_user_id'] == 123 + assert extra_vars['tower_user_name'] == "angry-spud" + assert extra_vars['awx_user_id'] == 123 + assert extra_vars['awx_user_name'] == "angry-spud" + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) def test_survey_extra_vars(self): self.instance.extra_vars = json.dumps({ @@ -313,12 +452,15 @@ class TestGenericRun(TestJobExecution): self.instance.survey_passwords = { 'super_secret': '$encrypted$' } - self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, _ = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - assert '"super_secret": "CLASSIFIED"' in ' '.join(args) + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars['super_secret'] == "CLASSIFIED" + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) def test_awx_task_env(self): patch = mock.patch('awx.main.tasks.settings.AWX_TASK_ENV', {'FOO': 'BAR'}) @@ -383,18 +525,28 @@ class TestAdhocRun(TestJobExecution): extra_vars={'awx_foo': 'awx-bar'} ) + def test_options_jinja_usage(self): + self.instance.module_args = '{{ ansible_ssh_pass }}' + with pytest.raises(Exception): + self.task.run(self.pk) + update_model_call = self.task.update_model.call_args[1] + assert 'Jinja variables are not allowed' in update_model_call['result_traceback'] + def test_created_by_extra_vars(self): self.instance.created_by = User(pk=123, username='angry-spud') - self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, _ = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - assert '"tower_user_id": 123,' in ' '.join(args) - assert '"tower_user_name": "angry-spud"' in ' '.join(args) - assert '"awx_user_id": 123,' in ' '.join(args) - assert '"awx_user_name": "angry-spud"' in ' '.join(args) - assert '"awx_foo": "awx-bar' in ' '.join(args) + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars['tower_user_id'] == 123 + assert extra_vars['tower_user_name'] == "angry-spud" + assert extra_vars['awx_user_id'] == 123 + assert extra_vars['awx_user_name'] == "angry-spud" + assert extra_vars['awx_foo'] == "awx-bar" + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) class TestIsolatedExecution(TestJobExecution): @@ -502,6 +654,33 @@ class TestJobCredentials(TestJobExecution): ] } + def test_username_jinja_usage(self): + ssh = CredentialType.defaults['ssh']() + credential = Credential( + pk=1, + credential_type=ssh, + inputs = {'username': '{{ ansible_ssh_pass }}'} + ) + self.instance.credential = credential + with pytest.raises(Exception): + self.task.run(self.pk) + update_model_call = self.task.update_model.call_args[1] + assert 'Jinja variables are not allowed' in update_model_call['result_traceback'] + + @pytest.mark.parametrize("flag", ['become_username', 'become_method']) + def test_become_jinja_usage(self, flag): + ssh = CredentialType.defaults['ssh']() + credential = Credential( + pk=1, + credential_type=ssh, + inputs = {'username': 'joe', flag: '{{ ansible_ssh_pass }}'} + ) + self.instance.credential = credential + with pytest.raises(Exception): + self.task.run(self.pk) + update_model_call = self.task.update_model.call_args[1] + assert 'Jinja variables are not allowed' in update_model_call['result_traceback'] + def test_ssh_passwords(self, field, password_name, expected_flag): ssh = CredentialType.defaults['ssh']() credential = Credential( @@ -986,14 +1165,17 @@ class TestJobCredentials(TestJobExecution): inputs = {'api_token': 'ABC123'} ) self.instance.extra_credentials.add(credential) + + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars["api_token"] == "ABC123" + assert hasattr(extra_vars["api_token"], '__UNSAFE__') + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, _ = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - - assert '-e {"api_token": "ABC123"}' in ' '.join(args) - def test_custom_environment_injectors_with_boolean_extra_vars(self): some_cloud = CredentialType( kind='cloud', @@ -1018,12 +1200,15 @@ class TestJobCredentials(TestJobExecution): inputs={'turbo_button': True} ) self.instance.extra_credentials.add(credential) - self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, _ = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - assert '-e {"turbo_button": "True"}' in ' '.join(args) + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars["turbo_button"] == "True" + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) def test_custom_environment_injectors_with_complicated_boolean_template(self): some_cloud = CredentialType( @@ -1049,12 +1234,15 @@ class TestJobCredentials(TestJobExecution): inputs={'turbo_button': True} ) self.instance.extra_credentials.add(credential) - self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, _ = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - assert '-e {"turbo_button": "FAST!"}' in ' '.join(args) + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars["turbo_button"] == "FAST!" + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) def test_custom_environment_injectors_with_secret_extra_vars(self): """ @@ -1085,13 +1273,16 @@ class TestJobCredentials(TestJobExecution): ) credential.inputs['password'] = encrypt_field(credential, 'password') self.instance.extra_credentials.add(credential) + + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars["password"] == "SUPER-SECRET-123" + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, _ = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - - assert '-e {"password": "SUPER-SECRET-123"}' in ' '.join(args) assert 'SUPER-SECRET-123' not in json.dumps(self.task.update_model.call_args_list) def test_custom_environment_injectors_with_file(self): @@ -1217,20 +1408,22 @@ class TestProjectUpdateCredentials(TestJobExecution): pk=1, credential_type=ssh, ) + + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert ' '.join(args).startswith('bwrap') + assert ' '.join([ + '--bind', + os.path.realpath(settings.PROJECTS_ROOT), + os.path.realpath(settings.PROJECTS_ROOT) + ]) in ' '.join(args) + assert extra_vars["scm_revision_output"].startswith(settings.PROJECTS_ROOT) + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, call_kwargs = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - - assert ' '.join(args).startswith('bwrap') - ' '.join([ - '--bind', - settings.PROJECTS_ROOT, - settings.PROJECTS_ROOT, - ]) in ' '.join(args) - assert '"scm_revision_output": "/projects/tmp' in ' '.join(args) - def test_username_and_password_auth(self, scm_type): ssh = CredentialType.defaults['ssh']() self.instance.scm_type = scm_type diff --git a/awx/main/tests/unit/utils/test_safe_yaml.py b/awx/main/tests/unit/utils/test_safe_yaml.py new file mode 100644 index 0000000000..7f528ea595 --- /dev/null +++ b/awx/main/tests/unit/utils/test_safe_yaml.py @@ -0,0 +1,85 @@ +# -*- coding: utf-8 -*- + +from copy import deepcopy +import pytest +import yaml +from awx.main.utils.safe_yaml import safe_dump + + +@pytest.mark.parametrize('value', [None, 1, 1.5, []]) +def test_native_types(value): + # Native non-string types should dump the same way that `yaml.safe_dump` does + assert safe_dump(value) == yaml.safe_dump(value) + + +def test_empty(): + assert safe_dump({}) == '' + + +def test_raw_string(): + assert safe_dump('foo') == "!unsafe 'foo'\n" + + +def test_kv_int(): + assert safe_dump({'a': 1}) == "!unsafe 'a': 1\n" + + +def test_kv_float(): + assert safe_dump({'a': 1.5}) == "!unsafe 'a': 1.5\n" + + +def test_kv_unsafe(): + assert safe_dump({'a': 'b'}) == "!unsafe 'a': !unsafe 'b'\n" + + +def test_kv_unsafe_unicode(): + assert safe_dump({'a': u'🐉'}) == '!unsafe \'a\': !unsafe "\\U0001F409"\n' + + +def test_kv_unsafe_in_list(): + assert safe_dump({'a': ['b']}) == "!unsafe 'a':\n- !unsafe 'b'\n" + + +def test_kv_unsafe_in_mixed_list(): + assert safe_dump({'a': [1, 'b']}) == "!unsafe 'a':\n- 1\n- !unsafe 'b'\n" + + +def test_kv_unsafe_deep_nesting(): + yaml = safe_dump({'a': [1, [{'b': {'c': [{'d': 'e'}]}}]]}) + for x in ('a', 'b', 'c', 'd', 'e'): + assert "!unsafe '{}'".format(x) in yaml + + +def test_kv_unsafe_multiple(): + assert safe_dump({'a': 'b', 'c': 'd'}) == '\n'.join([ + "!unsafe 'a': !unsafe 'b'", + "!unsafe 'c': !unsafe 'd'", + "" + ]) + + +def test_safe_marking(): + assert safe_dump({'a': 'b'}, safe_dict={'a': 'b'}) == "a: b\n" + + +def test_safe_marking_mixed(): + assert safe_dump({'a': 'b', 'c': 'd'}, safe_dict={'a': 'b'}) == '\n'.join([ + "a: b", + "!unsafe 'c': !unsafe 'd'", + "" + ]) + + +def test_safe_marking_deep_nesting(): + deep = {'a': [1, [{'b': {'c': [{'d': 'e'}]}}]]} + yaml = safe_dump(deep, deepcopy(deep)) + for x in ('a', 'b', 'c', 'd', 'e'): + assert "!unsafe '{}'".format(x) not in yaml + + +def test_deep_diff_unsafe_marking(): + deep = {'a': [1, [{'b': {'c': [{'d': 'e'}]}}]]} + jt_vars = deepcopy(deep) + deep['a'][1][0]['b']['z'] = 'not safe' + yaml = safe_dump(deep, jt_vars) + assert "!unsafe 'z'" in yaml diff --git a/awx/main/utils/safe_yaml.py b/awx/main/utils/safe_yaml.py new file mode 100644 index 0000000000..1b958b69d0 --- /dev/null +++ b/awx/main/utils/safe_yaml.py @@ -0,0 +1,87 @@ +import re +import six +import yaml + + +__all__ = ['safe_dump', 'SafeLoader'] + + +class SafeStringDumper(yaml.SafeDumper): + + def represent_data(self, value): + if isinstance(value, six.string_types): + return self.represent_scalar('!unsafe', value) + return super(SafeStringDumper, self).represent_data(value) + + +class SafeLoader(yaml.Loader): + + def construct_yaml_unsafe(self, node): + class UnsafeText(six.text_type): + __UNSAFE__ = True + node = UnsafeText(self.construct_scalar(node)) + return node + + +SafeLoader.add_constructor( + u'!unsafe', + SafeLoader.construct_yaml_unsafe +) + + +def safe_dump(x, safe_dict=None): + """ + Used to serialize an extra_vars dict to YAML + + By default, extra vars are marked as `!unsafe` in the generated yaml + _unless_ they've been deemed "trusted" (meaning, they likely were set/added + by a user with a high level of privilege). + + This function allows you to pass in a trusted `safe_dict` to whitelist + certain extra vars so that they are _not_ marked as `!unsafe` in the + resulting YAML. Anything _not_ in this dict will automatically be + `!unsafe`. + + safe_dump({'a': 'b', 'c': 'd'}) -> + !unsafe 'a': !unsafe 'b' + !unsafe 'c': !unsafe 'd' + + safe_dump({'a': 'b', 'c': 'd'}, safe_dict={'a': 'b'}) + a: b + !unsafe 'c': !unsafe 'd' + """ + if isinstance(x, dict): + yamls = [] + safe_dict = safe_dict or {} + + # Compare the top level keys so that we can find values that have + # equality matches (and consider those branches safe) + for k, v in x.items(): + dumper = yaml.SafeDumper + if safe_dict.get(k) != v: + dumper = SafeStringDumper + yamls.append(yaml.dump_all( + [{k: v}], + None, + Dumper=dumper, + default_flow_style=False, + )) + return ''.join(yamls) + else: + return yaml.dump_all([x], None, Dumper=SafeStringDumper, default_flow_style=False) + + +def sanitize_jinja(arg): + """ + For some string, prevent usage of Jinja-like flags + """ + if isinstance(arg, six.string_types): + # If the argument looks like it contains Jinja expressions + # {{ x }} ... + if re.search('\{\{[^}]+}}', arg) is not None: + raise ValueError('Inline Jinja variables are not allowed.') + # If the argument looks like it contains Jinja statements/control flow... + # {% if x.foo() %} ... + if re.search('\{%[^%]+%}', arg) is not None: + raise ValueError('Inline Jinja variables are not allowed.') + return arg diff --git a/awx/settings/defaults.py b/awx/settings/defaults.py index 89d1727188..54b6110812 100644 --- a/awx/settings/defaults.py +++ b/awx/settings/defaults.py @@ -585,6 +585,9 @@ CAPTURE_JOB_EVENT_HOSTS = False # Rebuild Host Smart Inventory memberships. AWX_REBUILD_SMART_MEMBERSHIP = False +# By default, allow arbitrary Jinja templating in extra_vars defined on a Job Template +ALLOW_JINJA_IN_EXTRA_VARS = 'template' + # Enable bubblewrap support for running jobs (playbook runs only). # Note: This setting may be overridden by database settings. AWX_PROOT_ENABLED = True