Merge branch 'release_3.2.4' into release_3.3.0

awx/main/tests/unit/models/test_survey_models.py

@@ -1,5 +1,6 @@
 import tempfile
 import json
+import yaml
 
 import pytest
 from awx.main.utils.encryption import encrypt_value
@@ -10,6 +11,7 @@ from awx.main.models import (
     JobLaunchConfig,
     WorkflowJobTemplate
 )
+from awx.main.utils.safe_yaml import SafeLoader
 
 ENCRYPTED_SECRET = encrypt_value('secret')
 
@@ -122,7 +124,7 @@ def test_job_safe_args_redacted_passwords(job):
     safe_args = run_job.build_safe_args(job, **kwargs)
     ev_index = safe_args.index('-e') + 1
     extra_var_file = open(safe_args[ev_index][1:], 'r')
-    extra_vars = json.load(extra_var_file)
+    extra_vars = yaml.load(extra_var_file, SafeLoader)
     extra_var_file.close()
     assert extra_vars['secret_key'] == '$encrypted$'
 
@@ -133,7 +135,7 @@ def test_job_args_unredacted_passwords(job, tmpdir_factory):
     args = run_job.build_args(job, **kwargs)
     ev_index = args.index('-e') + 1
     extra_var_file = open(args[ev_index][1:], 'r')
-    extra_vars = json.load(extra_var_file)
+    extra_vars = yaml.load(extra_var_file, SafeLoader)
     extra_var_file.close()
     assert extra_vars['secret_key'] == 'my_password'
 

awx/main/tests/unit/test_tasks.py

@@ -28,6 +28,7 @@ from awx.main.models import (
     InventorySource,
     InventoryUpdate,
     Job,
+    JobTemplate,
     Notification,
     Project,
     ProjectUpdate,
@@ -40,7 +41,7 @@ from awx.main.models import (
 from awx.main import tasks
 from awx.main.queue import CallbackQueueDispatcher
 from awx.main.utils import encrypt_field, encrypt_value, OutputEventFilter
-
+from awx.main.utils.safe_yaml import SafeLoader
 
 
 @contextmanager
@@ -191,7 +192,7 @@ def parse_extra_vars(args):
     for chunk in args:
         if chunk.startswith('@/tmp/'):
             with open(chunk.strip('@'), 'r') as f:
-                extra_vars.update(json.load(f))
+                extra_vars.update(yaml.load(f, SafeLoader))
     return extra_vars
 
 
@@ -271,7 +272,8 @@ class TestJobExecution:
             cancel_flag=False,
             project=Project(),
             playbook='helloworld.yml',
-            verbosity=3
+            verbosity=3,
+            job_template=JobTemplate(extra_vars='')
         )
 
         # mock the job.credentials M2M relation so we can avoid DB access
@@ -297,6 +299,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_generic_failure(self):
@@ -473,6 +600,13 @@ 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')
 
@@ -584,6 +718,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.credentials.add(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.credentials.add(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(
@@ -1171,6 +1332,7 @@ class TestJobCredentials(TestJobExecution):
             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

awx/main/tests/unit/utils/test_safe_yaml.py

@@ -0,0 +1,97 @@
+# -*- 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_null():
+    assert safe_dump({'a': None}) == "!unsafe 'a': null\n"
+
+
+def test_kv_null_safe():
+    assert safe_dump({'a': None}, {'a': None}) == "a: null\n"
+
+
+def test_kv_null_unsafe():
+    assert safe_dump({'a': ''}, {'a': None}) == "!unsafe 'a': !unsafe ''\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