From 714cd2ad2eff7f003d728414afcb91591fad5d9a Mon Sep 17 00:00:00 2001 From: Sloane Hertel Date: Thu, 27 Aug 2020 14:41:14 -0400 Subject: [PATCH] copy - redact 'content' from invocation in check mode (#71033) (#71067) * sanitize copy module invocation secrets in check mode (cherry picked from commit 991714b9d1e878a4c2fda67ffd829724fa7ac67e) --- ...y-sanitize-check-mode-invocation-args.yaml | 7 ++ lib/ansible/plugins/action/copy.py | 9 +- test/integration/targets/copy/tasks/main.yml | 2 + .../integration/targets/copy/tasks/no_log.yml | 82 +++++++++++++++++++ 4 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 changelogs/fragments/copy-sanitize-check-mode-invocation-args.yaml create mode 100644 test/integration/targets/copy/tasks/no_log.yml diff --git a/changelogs/fragments/copy-sanitize-check-mode-invocation-args.yaml b/changelogs/fragments/copy-sanitize-check-mode-invocation-args.yaml new file mode 100644 index 00000000000..894a17f4fd0 --- /dev/null +++ b/changelogs/fragments/copy-sanitize-check-mode-invocation-args.yaml @@ -0,0 +1,7 @@ +security_fixes: +- > + **security issue** - copy - Redact the value of the no_log 'content' + parameter in the result's invocation.module_args in check mode. + Previously when used with check mode and with '-vvv', the module + would not censor the content if a change would be made to the + destination path. (CVE-2020-14332) diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py index e56ef99ea11..cb3d15b3837 100644 --- a/lib/ansible/plugins/action/copy.py +++ b/lib/ansible/plugins/action/copy.py @@ -209,6 +209,8 @@ class ActionModule(ActionBase): # NOTE: adding invocation arguments here needs to be kept in sync with # any no_log specified in the argument_spec in the module. # This is not automatic. + # NOTE: do not add to this. This should be made a generic function for action plugins. + # This should also use the same argspec as the module instead of keeping it in sync. if 'invocation' not in result: if self._play_context.no_log: result['invocation'] = "CENSORED: no_log is set" @@ -218,8 +220,11 @@ class ActionModule(ActionBase): result['invocation'] = self._task.args.copy() result['invocation']['module_args'] = self._task.args.copy() - if isinstance(result['invocation'], dict) and 'content' in result['invocation']: - result['invocation']['content'] = 'CENSORED: content is a no_log parameter' + if isinstance(result['invocation'], dict): + if 'content' in result['invocation']: + result['invocation']['content'] = 'CENSORED: content is a no_log parameter' + if result['invocation'].get('module_args', {}).get('content') is not None: + result['invocation']['module_args']['content'] = 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER' return result diff --git a/test/integration/targets/copy/tasks/main.yml b/test/integration/targets/copy/tasks/main.yml index fbe7a1b2459..33a92bf9ada 100644 --- a/test/integration/targets/copy/tasks/main.yml +++ b/test/integration/targets/copy/tasks/main.yml @@ -74,6 +74,8 @@ - import_tasks: acls.yml when: ansible_system == 'Linux' + - import_tasks: no_log.yml + - import_tasks: check_mode.yml # https://github.com/ansible/ansible/issues/57618 diff --git a/test/integration/targets/copy/tasks/no_log.yml b/test/integration/targets/copy/tasks/no_log.yml new file mode 100644 index 00000000000..980c31778de --- /dev/null +++ b/test/integration/targets/copy/tasks/no_log.yml @@ -0,0 +1,82 @@ +- block: + + - set_fact: + dest: "{{ local_temp_dir }}/test_no_log" + + - name: ensure playbook and dest files don't exist yet + file: + path: "{{ item }}" + state: absent + loop: + - "{{ local_temp_dir }}/test_no_log.yml" + - "{{ dest }}" + + - name: create a playbook to run with command + copy: + dest: "{{local_temp_dir}}/test_no_log.yml" + content: !unsafe | + --- + - hosts: localhost + gather_facts: no + tasks: + - copy: + dest: "{{ dest }}" + content: "{{ secret }}" + + - name: copy the secret while using -vvv and check mode + command: "ansible-playbook {{local_temp_dir}}/test_no_log.yml -vvv -e secret=SECRET -e dest={{dest}} --check" + register: result + + - assert: + that: + - "'SECRET' not in result.stdout" + + - name: copy the secret while using -vvv + command: "ansible-playbook {{local_temp_dir}}/test_no_log.yml -vvv -e secret=SECRET -e dest={{dest}}" + register: result + + - assert: + that: + - "'SECRET' not in result.stdout" + + - name: copy the secret while using -vvv and check mode again + command: "ansible-playbook {{local_temp_dir}}/test_no_log.yml -vvv -e secret=SECRET -e dest={{dest}} --check" + register: result + + - assert: + that: + - "'SECRET' not in result.stdout" + + - name: copy the secret while using -vvv again + command: "ansible-playbook {{local_temp_dir}}/test_no_log.yml -vvv -e secret=SECRET -e dest={{dest}}" + register: result + + - assert: + that: + - "'SECRET' not in result.stdout" + + - name: copy a new secret while using -vvv and check mode + command: "ansible-playbook {{local_temp_dir}}/test_no_log.yml -vvv -e secret=NEWSECRET -e dest={{dest}} --check" + register: result + + - assert: + that: + - "'NEWSECRET' not in result.stdout" + + - name: copy a new secret while using -vvv + command: "ansible-playbook {{local_temp_dir}}/test_no_log.yml -vvv -e secret=NEWSECRET -e dest={{dest}}" + register: result + + - assert: + that: + - "'NEWSECRET' not in result.stdout" + + always: + + - name: remove temp test files + file: + path: "{{ item }}" + state: absent + loop: + - "{{ local_temp_dir }}/test_no_log.yml" + - "{{ dest }}"