From ffe7278c08e6e36bf8b18f732c764e00ff51551e Mon Sep 17 00:00:00 2001 From: Tobias Henkel Date: Fri, 8 Jun 2018 15:25:50 +0200 Subject: [PATCH] Fix information disclosure caused by unreachable nodes Currently we can leak secrets if we encounter unreachable nodes combined with a task using with_items and no_log. In this case the item variables are written to both the job-output.json and job-output.txt. Upstream Ansible has the same issue [1]. The text log can be fixed by defining the v2_runner_on_unreachable callback the same as v2_runner_on_failed. The json log can be fixed the same way as the upstream Ansible issue. [1] https://bugzilla.redhat.com/show_bug.cgi?id=1588855 Change-Id: Ie5dd2a6b11e8e276da65fe470f364107f3dd07ef --- .../playbooks/no-log-unreachable.yaml | 28 +++++++++++++++++++ .../ansible-no-log/git/org_project/zuul.yaml | 26 +++++++++++++++++ .../fixtures/config/ansible-no-log/main.yaml | 6 ++++ tests/unit/test_v3.py | 27 ++++++++++++++++++ zuul/ansible/callback/zuul_json.py | 2 +- zuul/ansible/callback/zuul_stream.py | 2 ++ 6 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/config/ansible-no-log/git/org_project/playbooks/no-log-unreachable.yaml create mode 100644 tests/fixtures/config/ansible-no-log/git/org_project/zuul.yaml create mode 100644 tests/fixtures/config/ansible-no-log/main.yaml diff --git a/tests/fixtures/config/ansible-no-log/git/org_project/playbooks/no-log-unreachable.yaml b/tests/fixtures/config/ansible-no-log/git/org_project/playbooks/no-log-unreachable.yaml new file mode 100644 index 0000000000..3d3ddf5830 --- /dev/null +++ b/tests/fixtures/config/ansible-no-log/git/org_project/playbooks/no-log-unreachable.yaml @@ -0,0 +1,28 @@ +- hosts: localhost + gather_facts: no + tasks: + - name: Add a fake host + add_host: + hostname: fake + ansible_host: notexisting.example.notexisting + +- hosts: fake + gather_facts: no + tasks: + - name: Run a lineinfile task + vars: + logins: + - machine: foo + login: bar + password: my-very-secret-password-1 + - machine: two + login: three + password: my-very-secret-password-2 + lineinfile: + path: /tmp/.netrc + mode: 0600 + create: true + insertafter: EOF + line: "machine {{ item.machine }} login {{ item.login }} password {{ item.password }}" + with_items: "{{ logins }}" + no_log: true diff --git a/tests/fixtures/config/ansible-no-log/git/org_project/zuul.yaml b/tests/fixtures/config/ansible-no-log/git/org_project/zuul.yaml new file mode 100644 index 0000000000..38501b21fa --- /dev/null +++ b/tests/fixtures/config/ansible-no-log/git/org_project/zuul.yaml @@ -0,0 +1,26 @@ +- pipeline: + name: check + manager: independent + post-review: true + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + +- job: + name: base + parent: null + +- job: + name: no-log-unreachable + run: playbooks/no-log-unreachable.yaml + +- project: + check: + jobs: + - no-log-unreachable diff --git a/tests/fixtures/config/ansible-no-log/main.yaml b/tests/fixtures/config/ansible-no-log/main.yaml new file mode 100644 index 0000000000..73787886f0 --- /dev/null +++ b/tests/fixtures/config/ansible-no-log/main.yaml @@ -0,0 +1,6 @@ +- tenant: + name: tenant-one + source: + gerrit: + config-projects: + - org/project diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index 87adc601f9..bf17729a58 100755 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -3844,3 +3844,30 @@ class TestPlugins(AnsibleZuulTestCase): roles="roles: [{zuul: 'org/project2'}]") self._run_job('filter-plugin-shared-bare-role', roles="roles: [{zuul: 'org/project3', name: 'shared'}]") + + +class TestNoLog(AnsibleZuulTestCase): + tenant_config_file = 'config/ansible-no-log/main.yaml' + + def _get_file(self, build, path): + p = os.path.join(build.jobdir.root, path) + with open(p) as f: + return f.read() + + def test_no_log_unreachable(self): + # Output extra ansible info so we might see errors. + self.executor_server.verbose = True + self.executor_server.keep_jobdir = True + + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') + + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + json_log = self._get_file(self.history[0], 'work/logs/job-output.json') + text_log = self._get_file(self.history[0], 'work/logs/job-output.txt') + + self.assertNotIn('my-very-secret-password-1', json_log) + self.assertNotIn('my-very-secret-password-2', json_log) + self.assertNotIn('my-very-secret-password-1', text_log) + self.assertNotIn('my-very-secret-password-2', text_log) diff --git a/zuul/ansible/callback/zuul_json.py b/zuul/ansible/callback/zuul_json.py index dc61fcec4c..818fa6d0b1 100644 --- a/zuul/ansible/callback/zuul_json.py +++ b/zuul/ansible/callback/zuul_json.py @@ -137,7 +137,7 @@ class CallbackModule(CallbackBase): def v2_runner_on_ok(self, result, **kwargs): host = result._host - if result._result.get('_ansible_no_log', False): + if result._result.get('_ansible_no_log', False) or result._task.no_log: self.results[-1]['tasks'][-1]['hosts'][host.name] = dict( censored="the output has been hidden due to the fact that" " 'no_log: true' was specified for this result") diff --git a/zuul/ansible/callback/zuul_stream.py b/zuul/ansible/callback/zuul_stream.py index 9fdfca89cc..17ba6b4c86 100644 --- a/zuul/ansible/callback/zuul_stream.py +++ b/zuul/ansible/callback/zuul_stream.py @@ -606,3 +606,5 @@ class CallbackModule(default.CallbackModule): delegated_host=delegated_vars['ansible_host']) else: return result._host.get_name() + + v2_runner_on_unreachable = v2_runner_on_failed