From 63c9a5b4ace7b0a2c5446464f9181f0eccb0894d Mon Sep 17 00:00:00 2001 From: Chris Church Date: Mon, 9 Sep 2013 20:27:28 -0400 Subject: [PATCH] AC-132. Mask passwords in project update args and stdout. --- awx/main/tasks.py | 88 ++++++++++++++++++++++++++++---- awx/main/tests/projects.py | 41 +++++++++++++-- awx/playbooks/project_update.yml | 11 ++-- 3 files changed, 119 insertions(+), 21 deletions(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 33524b11ef..51862c8855 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -44,10 +44,14 @@ class BaseTask(Task): ''' Reload model from database and update the given fields. ''' + output_replacements = updates.pop('output_replacements', None) or [] instance = self.model.objects.get(pk=pk) if updates: update_fields = [] for field, value in updates.items(): + if field in ('result_stdout', 'result_traceback'): + for srch, repl in output_replacements: + value = value.replace(srch, repl) setattr(instance, field, value) update_fields.append(field) if field == 'status': @@ -114,9 +118,15 @@ class BaseTask(Task): def build_args(self, instance, **kwargs): raise NotImplementedError + def build_safe_args(self, instance, **kwargs): + return self.build_args(instance, **kwargs) + def build_cwd(self, instance, **kwargs): raise NotImplementedError + def build_output_replacements(self, instance, **kwargs): + return [] + def get_password_prompts(self): ''' Return a dictionary of prompt regular expressions and password lookup @@ -127,7 +137,8 @@ class BaseTask(Task): r'Bad passphrase, try again for .*:': '', } - def run_pexpect(self, instance, args, cwd, env, passwords): + def run_pexpect(self, instance, args, cwd, env, passwords, + output_replacements=None): ''' Run the given command using pexpect to capture output and provide passwords when requested. @@ -149,7 +160,8 @@ class BaseTask(Task): result_id = child.expect(expect_list, timeout=5) if result_id in expect_passwords: child.sendline(expect_passwords[result_id]) - updates = {'status': 'running'} + updates = {'status': 'running', + 'output_replacements': output_replacements} if logfile_pos != logfile.tell(): logfile_pos = logfile.tell() updates['result_stdout'] = logfile.getvalue() @@ -193,9 +205,11 @@ class BaseTask(Task): kwargs['ssh_key_path'] = self.build_ssh_key_path(instance, **kwargs) kwargs['passwords'] = self.build_passwords(instance, **kwargs) args = self.build_args(instance, **kwargs) + safe_args = self.build_safe_args(instance, **kwargs) + output_replacements = self.build_output_replacements(instance, **kwargs) cwd = self.build_cwd(instance, **kwargs) env = self.build_env(instance, **kwargs) - instance = self.update_model(pk, job_args=json.dumps(args), + instance = self.update_model(pk, job_args=json.dumps(safe_args), job_cwd=cwd, job_env=env) status, stdout = self.run_pexpect(instance, args, cwd, env, kwargs['passwords']) @@ -208,7 +222,8 @@ class BaseTask(Task): except IOError: pass instance = self.update_model(pk, status=status, result_stdout=stdout, - result_traceback=tb) + result_traceback=tb, + output_replacements=output_replacements) class RunJob(BaseTask): ''' @@ -422,13 +437,11 @@ class RunProjectUpdate(BaseTask): return urlparse.urlunsplit([parts.scheme, netloc, parts.path, parts.query, parts.fragment]) - def build_args(self, project_update, **kwargs): + def _build_scm_url_extra_vars(self, project_update, **kwargs): ''' - Build command line argument list for running ansible-playbook, - optionally using ssh-agent for public/private key authentication. + Helper method to build SCM url and extra vars with parameters needed + for authentication. ''' - args = ['ansible-playbook', '-i', 'localhost,'] - args.append('-%s' % ('v' * 3)) extra_vars = {} project = project_update.project scm_url = project.scm_url @@ -446,7 +459,18 @@ class RunProjectUpdate(BaseTask): extra_vars['scm_username'] = scm_username else: scm_url = self.update_url_auth(scm_url, scm_username) - # FIXME: Need to hide password in saved job_args and result_stdout! + return scm_url, extra_vars + + def build_args(self, project_update, **kwargs): + ''' + Build command line argument list for running ansible-playbook, + optionally using ssh-agent for public/private key authentication. + ''' + args = ['ansible-playbook', '-i', 'localhost,'] + args.append('-%s' % ('v' * 3)) + project = project_update.project + scm_url, extra_vars = self._build_scm_url_extra_vars(project_update, + **kwargs) scm_branch = project.scm_branch or {'hg': 'tip'}.get(project.scm_type, 'HEAD') scm_delete_on_update = project.scm_delete_on_update or project.scm_delete_on_next_update extra_vars.update({ @@ -467,9 +491,53 @@ class RunProjectUpdate(BaseTask): args = ['ssh-agent', 'sh', '-c', cmd] return args + def build_safe_args(self, project_update, **kwargs): + pwdict = dict(kwargs.get('passwords', {}).items()) + for pw_name, pw_val in pwdict.items(): + if pw_name in ('', 'yes', 'no', 'scm_username'): + continue + pwdict[pw_name] = '*'*len(pw_val) + kwargs['passwords'] = pwdict + return self.build_args(project_update, **kwargs) + def build_cwd(self, project_update, **kwargs): return self.get_path_to('..', 'playbooks') + def build_output_replacements(self, project_update, **kwargs): + ''' + Return search/replace strings to prevent output URLs from showing + sensitive passwords. + ''' + output_replacements = [] + before_url = self._build_scm_url_extra_vars(project_update, + **kwargs)[0] + scm_password = kwargs.get('passwords', {}).get('scm_password', '') + pwdict = dict(kwargs.get('passwords', {}).items()) + for pw_name, pw_val in pwdict.items(): + if pw_name in ('', 'yes', 'no', 'scm_username'): + continue + pwdict[pw_name] = '*'*len(pw_val) + kwargs['passwords'] = pwdict + after_url = self._build_scm_url_extra_vars(project_update, + **kwargs)[0] + if after_url != before_url: + output_replacements.append((before_url, after_url)) + project = project_update.project + if project.scm_type == 'svn' and project.scm_username and scm_password: + d_before = { + 'username': project.scm_username, + 'password': scm_password, + } + d_after = { + 'username': project.scm_username, + 'password': '*'*len(scm_password), + } + pattern1 = "username=\"%(username)s\" password=\"%(password)s\"" + pattern2 = "--username '%(username)s' --password '%(password)s'" + output_replacements.append((pattern1 % d_before, pattern1 % d_after)) + output_replacements.append((pattern2 % d_before, pattern2 % d_after)) + return output_replacements + def get_password_prompts(self): d = super(RunProjectUpdate, self).get_password_prompts() d.update({ diff --git a/awx/main/tests/projects.py b/awx/main/tests/projects.py index 72efef629d..044b47b939 100644 --- a/awx/main/tests/projects.py +++ b/awx/main/tests/projects.py @@ -21,6 +21,7 @@ from django.utils.timezone import now from awx.main.models import * from awx.main.tests.base import BaseTest, BaseTransactionTest from awx.main.tests.tasks import TEST_SSH_KEY_DATA_LOCKED, TEST_SSH_KEY_DATA_UNLOCK +from awx.main.utils import decrypt_field TEST_PLAYBOOK = '''- hosts: mygroup gather_facts: false @@ -631,13 +632,35 @@ class ProjectUpdatesTest(BaseTransactionTest): return project def check_project_update(self, project, should_fail=False, **kwargs): - pu = project.update(**kwargs) - self.assertTrue(pu) + pu = kwargs.pop('project_update', None) + if not pu: + pu = project.update(**kwargs) + self.assertTrue(pu) pu = ProjectUpdate.objects.get(pk=pu.pk) if should_fail: self.assertEqual(pu.status, 'failed', pu.result_stdout) else: self.assertEqual(pu.status, 'successful', pu.result_stdout) + scm_password = kwargs.get('scm_password', + decrypt_field(project, 'scm_password')) + if scm_password not in ('', 'ASK'): + self.assertFalse(scm_password in pu.job_args, pu.job_args) + self.assertFalse(scm_password in json.dumps(pu.job_env), + json.dumps(pu.job_env)) + self.assertFalse(scm_password in pu.result_stdout, + pu.result_stdout) + self.assertFalse(scm_password in pu.result_traceback, + pu.result_traceback) + scm_key_unlock = kwargs.get('scm_key_unlock', + decrypt_field(project, 'scm_key_unlock')) + if scm_key_unlock not in ('', 'ASK'): + self.assertFalse(scm_key_unlock in pu.job_args, pu.job_args) + self.assertFalse(scm_key_unlock in json.dumps(pu.job_env), + json.dumps(pu.job_env)) + self.assertFalse(scm_key_unlock in pu.result_stdout, + pu.result_stdout) + self.assertFalse(scm_key_unlock in pu.result_traceback, + pu.result_traceback) project = Project.objects.get(pk=project.pk) self.assertEqual(project.last_update, pu) self.assertEqual(project.last_update_failed, pu.failed) @@ -737,7 +760,7 @@ class ProjectUpdatesTest(BaseTransactionTest): self.check_project_update(project, should_fail=True) # Try invalid username. project = Project.objects.get(pk=project.pk) - project.scm_username = 'notavalidusername' + project.scm_username = 'not a\\ valid\' user" name' project.save() self.check_project_update(project, should_fail=True) # Clear username and password. @@ -753,7 +776,7 @@ class ProjectUpdatesTest(BaseTransactionTest): self.check_project_update(project, should_fail=True) # Set username, with invalid password. project = Project.objects.get(pk=project.pk) - project.scm_password = 'notavalidpassword' + project.scm_password = 'not a\\ valid\' "password' project.save() self.check_project_update(project, should_fail=True) @@ -885,7 +908,11 @@ class ProjectUpdatesTest(BaseTransactionTest): response = self.post(url, {}, expect=400) self.assertTrue('scm_password' in response['passwords_needed_to_update']) with self.current_user(self.super_django_user): - response = self.post(url, {'scm_password': 'blah'}, expect=202) + response = self.post(url, {'scm_password': 'blah1234'}, expect=202) + project_update = project.project_updates.order_by('-pk')[0] + self.check_project_update(project, should_fail=False, + scm_password='blah1234', + project_update=project_update) def test_prompt_for_scm_key_unlock_on_update(self): scm_url = 'git@github.com:ansible/ansible.github.com.git' @@ -906,6 +933,10 @@ class ProjectUpdatesTest(BaseTransactionTest): self.assertTrue('scm_key_unlock' in response['passwords_needed_to_update']) with self.current_user(self.super_django_user): response = self.post(url, {'scm_key_unlock': TEST_SSH_KEY_DATA_UNLOCK}, expect=202) + project_update = project.project_updates.order_by('-pk')[0] + self.check_project_update(project, should_fail=False, + scm_key_unlock=TEST_SSH_KEY_DATA_UNLOCK, + project_update=project_update) def create_test_job_template(self, **kwargs): opts = { diff --git a/awx/playbooks/project_update.yml b/awx/playbooks/project_update.yml index e132011c4f..29046cace3 100644 --- a/awx/playbooks/project_update.yml +++ b/awx/playbooks/project_update.yml @@ -16,22 +16,21 @@ tasks: - name: delete project directory before update - file: path={{project_path}} state=absent + file: path="{{project_path}}" state=absent when: scm_delete_on_update|default('') - name: update project using git - git: dest={{project_path}} repo={{scm_url}} version={{scm_branch}} force={{scm_clean}} + git: dest="{{project_path}}" repo="{{scm_url}}" version="{{scm_branch}}" force={{scm_clean}} when: scm_type == 'git' - name: update project using hg - hg: dest={{project_path}} repo={{scm_url}} revision={{scm_branch}} force={{scm_clean}} + hg: dest="{{project_path}}" repo="{{scm_url}}" revision="{{scm_branch}}" force={{scm_clean}} when: scm_type == 'hg' - name: update project using svn - subversion: dest={{project_path}} repo={{scm_url}} revision={{scm_branch}} force={{scm_clean}} + subversion: dest="{{project_path}}" repo="{{scm_url}}" revision="{{scm_branch}}" force={{scm_clean}} when: scm_type == 'svn' and not scm_username|default('') - name: update project using svn with auth - subversion: dest={{project_path}} repo={{scm_url}} revision={{scm_branch}} force={{scm_clean}} username={{scm_username}} password={{scm_password}} + subversion: dest="{{project_path}}" repo="{{scm_url}}" revision="{{scm_branch}}" force={{scm_clean}} username="{{scm_username}}" password="{{scm_password}}" when: scm_type == 'svn' and scm_username|default('') -