From 19def7045e170b0bb50f8d8d0090ff10a9c003cf Mon Sep 17 00:00:00 2001 From: Jeremy Stanley Date: Mon, 3 Jun 2019 22:25:22 +0000 Subject: [PATCH] Safely add Ansible password lookup plugin Ansible's password lookup plugin can be useful for generating random passwords, but has other uses which could allow a nefarious user to create new files or read the contents of existing files outside the workspace (as long as those paths are writeable or readable by the user under which Ansible is executing). To that end, short-circuit and return an error if an untrusted use attempts to escape the workspace in these ways. Also increase the wait timeout for the plugins test by 25% (from 4 to 5 minutes) since this change increases the number of jobs in it. Change-Id: I0e372dda3a0f0f74d9e343c439514317dceb4d24 --- .../playbooks/password_create_bad.yaml | 5 +++ .../playbooks/password_create_good.yaml | 5 +++ .../playbooks/password_null_good.yaml | 5 +++ .../playbooks/password_read_bad.yaml | 5 +++ .../playbooks/password_read_good.yaml | 10 ++++++ tests/unit/test_v3.py | 8 ++++- zuul/ansible/base/lookup/password.py | 31 ++++++++++++++++++- 7 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_create_bad.yaml create mode 100644 tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_create_good.yaml create mode 100644 tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_null_good.yaml create mode 100644 tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_read_bad.yaml create mode 100644 tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_read_good.yaml mode change 120000 => 100644 zuul/ansible/base/lookup/password.py diff --git a/tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_create_bad.yaml b/tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_create_bad.yaml new file mode 100644 index 0000000000..a346fdae96 --- /dev/null +++ b/tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_create_bad.yaml @@ -0,0 +1,5 @@ +- hosts: all + vars: + value: "{{ lookup('password', '/etc/passwd') }}" + tasks: + - debug: msg="value is {{ value }}" diff --git a/tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_create_good.yaml b/tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_create_good.yaml new file mode 100644 index 0000000000..5aa95d2470 --- /dev/null +++ b/tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_create_good.yaml @@ -0,0 +1,5 @@ +- hosts: all + vars: + value: "{{ lookup('password', '{{ zuul.executor.work_root }}/test.newpassword') }}" + tasks: + - debug: msg="value is {{ value }}" diff --git a/tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_null_good.yaml b/tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_null_good.yaml new file mode 100644 index 0000000000..8f42c1cf7e --- /dev/null +++ b/tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_null_good.yaml @@ -0,0 +1,5 @@ +- hosts: all + vars: + value: "{{ lookup('password', '/dev/null') }}" + tasks: + - debug: msg="value is {{ value }}" diff --git a/tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_read_bad.yaml b/tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_read_bad.yaml new file mode 100644 index 0000000000..a346fdae96 --- /dev/null +++ b/tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_read_bad.yaml @@ -0,0 +1,5 @@ +- hosts: all + vars: + value: "{{ lookup('password', '/etc/passwd') }}" + tasks: + - debug: msg="value is {{ value }}" diff --git a/tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_read_good.yaml b/tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_read_good.yaml new file mode 100644 index 0000000000..b40c465754 --- /dev/null +++ b/tests/fixtures/config/ansible/git/org_plugin-project/playbooks/password_read_good.yaml @@ -0,0 +1,10 @@ +- hosts: all + tasks: + - copy: + content: 'an_example_password' + dest: "{{zuul.executor.work_root}}/test.password" +- hosts: all + vars: + value: "{{ lookup('password', '{{zuul.executor.work_root}}/test.password') }}" + tasks: + - debug: msg="value is {{ value }}" diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index fb512c3455..b74153d96d 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -2556,7 +2556,7 @@ class TestAnsible25(AnsibleZuulTestCase): def test_playbook(self): # This test runs a bit long and needs extra time. - self.wait_timeout = 240 + self.wait_timeout = 300 # Keep the jobdir around so we can inspect contents if an # assert fails. self.executor_server.keep_jobdir = True @@ -2707,7 +2707,13 @@ class TestAnsible25(AnsibleZuulTestCase): ('file_local_good', 'SUCCESS'), ('file_local_bad', 'FAILURE'), ('zuul_return', 'SUCCESS'), + ('password_create_good', 'SUCCESS'), + ('password_null_good', 'SUCCESS'), + ('password_read_good', 'SUCCESS'), + ('password_create_bad', 'FAILURE'), + ('password_read_bad', 'FAILURE'), ] + for job_name, result in plugin_tests: count += 1 self._add_job(job_name) diff --git a/zuul/ansible/base/lookup/password.py b/zuul/ansible/base/lookup/password.py deleted file mode 120000 index d45b9c405d..0000000000 --- a/zuul/ansible/base/lookup/password.py +++ /dev/null @@ -1 +0,0 @@ -_banned.py \ No newline at end of file diff --git a/zuul/ansible/base/lookup/password.py b/zuul/ansible/base/lookup/password.py new file mode 100644 index 0000000000..0fdf898d4d --- /dev/null +++ b/zuul/ansible/base/lookup/password.py @@ -0,0 +1,30 @@ +# Copyright 2019 OpenStack Foundation +# +# This module is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This software is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this software. If not, see . + + +from zuul.ansible import paths +password = paths._import_ansible_lookup_plugin("password") + + +class LookupModule(password.LookupModule): + + def run(self, terms, variables, **kwargs): + for term in terms: + relpath = password._parse_parameters(term)[0] + # /dev/null is whitelisted because it's interpreted specially + if relpath != "/dev/null": + path = self._loader.path_dwim(relpath) + paths._fail_if_unsafe(path, allow_trusted=True) + return super(LookupModule, self).run(terms, variables, **kwargs)