From 9cbb681446d4c958e9e6f312f6efcfd3642962aa Mon Sep 17 00:00:00 2001 From: Tobias Henkel Date: Wed, 14 Mar 2018 09:19:52 +0100 Subject: [PATCH] Fix plugin injection vulnerability Currently it is possible to inject speculative plugins into untrusted jobs. These plugins are run locally on the executor and make it possible to run arbitraty code within the bwrap context. There are two problems here. First the path check is broken such it never matches a plugin dir. Further we don't check paths residing within playbook dirs. Change-Id: Idf1b940de2be7819afeb2dbad943fad2ae7ebc55 --- .../git/common-config/zuul.yaml | 17 +++++++ .../git/org_project/README | 1 + .../bare-role/filter_plugins/main.py | 14 ++++++ .../bare-role/tasks/main.yaml | 3 ++ .../filter-plugin-bare-role/test.yaml | 4 ++ .../filter_plugins | 1 + .../filter-plugin-playbook-symlink/test.yaml | 5 +++ .../filter_plugins/main.py | 14 ++++++ .../filter-plugin-playbook/test.yaml | 5 +++ .../roles/local-role/filter_plugins/main.py | 14 ++++++ .../roles/local-role/tasks/main.yaml | 3 ++ .../playbooks/filter-plugin-role/test.yaml | 4 ++ .../filter-plugin-shared-bare-role/test.yaml | 4 ++ .../filter-plugin-shared-role/test.yaml | 4 ++ .../git/org_project/zuul.yaml | 4 ++ .../roles/shared-role/filter_plugins/main.py | 14 ++++++ .../roles/shared-role/tasks/main.yaml | 3 ++ .../git/org_project3/filter_plugins/main.py | 14 ++++++ .../git/org_project3/tasks/main.yaml | 3 ++ .../filter-plugin-repo-role/test.yaml | 4 ++ .../roles/project-role/filter_plugins/main.py | 14 ++++++ .../roles/project-role/tasks/main.yaml | 3 ++ .../config/speculative-plugins/main.yaml | 11 +++++ tests/unit/test_v3.py | 45 +++++++++++++++++++ zuul/executor/server.py | 35 ++++++++++++++- 25 files changed, 242 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/config/speculative-plugins/git/common-config/zuul.yaml create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project/README create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-bare-role/bare-role/filter_plugins/main.py create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-bare-role/bare-role/tasks/main.yaml create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-bare-role/test.yaml create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook-symlink/filter_plugins create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook-symlink/test.yaml create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook/filter_plugins/main.py create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook/test.yaml create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-role/roles/local-role/filter_plugins/main.py create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-role/roles/local-role/tasks/main.yaml create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-role/test.yaml create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-shared-bare-role/test.yaml create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-shared-role/test.yaml create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project/zuul.yaml create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project2/roles/shared-role/filter_plugins/main.py create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project2/roles/shared-role/tasks/main.yaml create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project3/filter_plugins/main.py create mode 100644 tests/fixtures/config/speculative-plugins/git/org_project3/tasks/main.yaml create mode 100644 tests/fixtures/config/speculative-plugins/git/org_projectrole/playbooks/filter-plugin-repo-role/test.yaml create mode 100644 tests/fixtures/config/speculative-plugins/git/org_projectrole/roles/project-role/filter_plugins/main.py create mode 100644 tests/fixtures/config/speculative-plugins/git/org_projectrole/roles/project-role/tasks/main.yaml create mode 100644 tests/fixtures/config/speculative-plugins/main.yaml diff --git a/tests/fixtures/config/speculative-plugins/git/common-config/zuul.yaml b/tests/fixtures/config/speculative-plugins/git/common-config/zuul.yaml new file mode 100644 index 0000000000..a07342e2ec --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/common-config/zuul.yaml @@ -0,0 +1,17 @@ +- 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 diff --git a/tests/fixtures/config/speculative-plugins/git/org_project/README b/tests/fixtures/config/speculative-plugins/git/org_project/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-bare-role/bare-role/filter_plugins/main.py b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-bare-role/bare-role/filter_plugins/main.py new file mode 100644 index 0000000000..1b81dc8d6a --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-bare-role/bare-role/filter_plugins/main.py @@ -0,0 +1,14 @@ +import subprocess + + +def my_cool_test(string): + shell_output = subprocess.check_output(['hostname']) + return 'hostname: %s' % shell_output.decode('utf-8') + + +class FilterModule(object): + + def filters(self): + return { + 'my_cool_test': my_cool_test + } diff --git a/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-bare-role/bare-role/tasks/main.yaml b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-bare-role/bare-role/tasks/main.yaml new file mode 100644 index 0000000000..2e49d416e6 --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-bare-role/bare-role/tasks/main.yaml @@ -0,0 +1,3 @@ +- name: Test filter plugin + debug: + msg: "{{ 'ignore me' | my_cool_test }}" diff --git a/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-bare-role/test.yaml b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-bare-role/test.yaml new file mode 100644 index 0000000000..f25bb560df --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-bare-role/test.yaml @@ -0,0 +1,4 @@ +- hosts: all + roles: + - bare-role + diff --git a/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook-symlink/filter_plugins b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook-symlink/filter_plugins new file mode 100644 index 0000000000..56b0433cc3 --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook-symlink/filter_plugins @@ -0,0 +1 @@ +symlink: ../filter-plugin-playbook/filter_plugins diff --git a/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook-symlink/test.yaml b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook-symlink/test.yaml new file mode 100644 index 0000000000..e190da0d9e --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook-symlink/test.yaml @@ -0,0 +1,5 @@ +- hosts: all + tasks: + - name: Test filter plugin + debug: + msg: "{{ 'ignore me' | my_cool_test }}" diff --git a/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook/filter_plugins/main.py b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook/filter_plugins/main.py new file mode 100644 index 0000000000..1b81dc8d6a --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook/filter_plugins/main.py @@ -0,0 +1,14 @@ +import subprocess + + +def my_cool_test(string): + shell_output = subprocess.check_output(['hostname']) + return 'hostname: %s' % shell_output.decode('utf-8') + + +class FilterModule(object): + + def filters(self): + return { + 'my_cool_test': my_cool_test + } diff --git a/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook/test.yaml b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook/test.yaml new file mode 100644 index 0000000000..e190da0d9e --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-playbook/test.yaml @@ -0,0 +1,5 @@ +- hosts: all + tasks: + - name: Test filter plugin + debug: + msg: "{{ 'ignore me' | my_cool_test }}" diff --git a/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-role/roles/local-role/filter_plugins/main.py b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-role/roles/local-role/filter_plugins/main.py new file mode 100644 index 0000000000..1b81dc8d6a --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-role/roles/local-role/filter_plugins/main.py @@ -0,0 +1,14 @@ +import subprocess + + +def my_cool_test(string): + shell_output = subprocess.check_output(['hostname']) + return 'hostname: %s' % shell_output.decode('utf-8') + + +class FilterModule(object): + + def filters(self): + return { + 'my_cool_test': my_cool_test + } diff --git a/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-role/roles/local-role/tasks/main.yaml b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-role/roles/local-role/tasks/main.yaml new file mode 100644 index 0000000000..2e49d416e6 --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-role/roles/local-role/tasks/main.yaml @@ -0,0 +1,3 @@ +- name: Test filter plugin + debug: + msg: "{{ 'ignore me' | my_cool_test }}" diff --git a/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-role/test.yaml b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-role/test.yaml new file mode 100644 index 0000000000..f31611a7dd --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-role/test.yaml @@ -0,0 +1,4 @@ +- hosts: all + roles: + - local-role + diff --git a/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-shared-bare-role/test.yaml b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-shared-bare-role/test.yaml new file mode 100644 index 0000000000..32e72c865f --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-shared-bare-role/test.yaml @@ -0,0 +1,4 @@ +- hosts: all + roles: + - shared-bare-role + diff --git a/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-shared-role/test.yaml b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-shared-role/test.yaml new file mode 100644 index 0000000000..d9557f6a74 --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project/playbooks/filter-plugin-shared-role/test.yaml @@ -0,0 +1,4 @@ +- hosts: all + roles: + - shared-role + diff --git a/tests/fixtures/config/speculative-plugins/git/org_project/zuul.yaml b/tests/fixtures/config/speculative-plugins/git/org_project/zuul.yaml new file mode 100644 index 0000000000..2a5dca97f2 --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project/zuul.yaml @@ -0,0 +1,4 @@ +- project: + check: + jobs: + - noop diff --git a/tests/fixtures/config/speculative-plugins/git/org_project2/roles/shared-role/filter_plugins/main.py b/tests/fixtures/config/speculative-plugins/git/org_project2/roles/shared-role/filter_plugins/main.py new file mode 100644 index 0000000000..1b81dc8d6a --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project2/roles/shared-role/filter_plugins/main.py @@ -0,0 +1,14 @@ +import subprocess + + +def my_cool_test(string): + shell_output = subprocess.check_output(['hostname']) + return 'hostname: %s' % shell_output.decode('utf-8') + + +class FilterModule(object): + + def filters(self): + return { + 'my_cool_test': my_cool_test + } diff --git a/tests/fixtures/config/speculative-plugins/git/org_project2/roles/shared-role/tasks/main.yaml b/tests/fixtures/config/speculative-plugins/git/org_project2/roles/shared-role/tasks/main.yaml new file mode 100644 index 0000000000..2e49d416e6 --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project2/roles/shared-role/tasks/main.yaml @@ -0,0 +1,3 @@ +- name: Test filter plugin + debug: + msg: "{{ 'ignore me' | my_cool_test }}" diff --git a/tests/fixtures/config/speculative-plugins/git/org_project3/filter_plugins/main.py b/tests/fixtures/config/speculative-plugins/git/org_project3/filter_plugins/main.py new file mode 100644 index 0000000000..1b81dc8d6a --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project3/filter_plugins/main.py @@ -0,0 +1,14 @@ +import subprocess + + +def my_cool_test(string): + shell_output = subprocess.check_output(['hostname']) + return 'hostname: %s' % shell_output.decode('utf-8') + + +class FilterModule(object): + + def filters(self): + return { + 'my_cool_test': my_cool_test + } diff --git a/tests/fixtures/config/speculative-plugins/git/org_project3/tasks/main.yaml b/tests/fixtures/config/speculative-plugins/git/org_project3/tasks/main.yaml new file mode 100644 index 0000000000..2e49d416e6 --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_project3/tasks/main.yaml @@ -0,0 +1,3 @@ +- name: Test filter plugin + debug: + msg: "{{ 'ignore me' | my_cool_test }}" diff --git a/tests/fixtures/config/speculative-plugins/git/org_projectrole/playbooks/filter-plugin-repo-role/test.yaml b/tests/fixtures/config/speculative-plugins/git/org_projectrole/playbooks/filter-plugin-repo-role/test.yaml new file mode 100644 index 0000000000..d680d3bcc6 --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_projectrole/playbooks/filter-plugin-repo-role/test.yaml @@ -0,0 +1,4 @@ +- hosts: all + roles: + - project-role + diff --git a/tests/fixtures/config/speculative-plugins/git/org_projectrole/roles/project-role/filter_plugins/main.py b/tests/fixtures/config/speculative-plugins/git/org_projectrole/roles/project-role/filter_plugins/main.py new file mode 100644 index 0000000000..1b81dc8d6a --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_projectrole/roles/project-role/filter_plugins/main.py @@ -0,0 +1,14 @@ +import subprocess + + +def my_cool_test(string): + shell_output = subprocess.check_output(['hostname']) + return 'hostname: %s' % shell_output.decode('utf-8') + + +class FilterModule(object): + + def filters(self): + return { + 'my_cool_test': my_cool_test + } diff --git a/tests/fixtures/config/speculative-plugins/git/org_projectrole/roles/project-role/tasks/main.yaml b/tests/fixtures/config/speculative-plugins/git/org_projectrole/roles/project-role/tasks/main.yaml new file mode 100644 index 0000000000..2e49d416e6 --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/git/org_projectrole/roles/project-role/tasks/main.yaml @@ -0,0 +1,3 @@ +- name: Test filter plugin + debug: + msg: "{{ 'ignore me' | my_cool_test }}" diff --git a/tests/fixtures/config/speculative-plugins/main.yaml b/tests/fixtures/config/speculative-plugins/main.yaml new file mode 100644 index 0000000000..4b16676e4b --- /dev/null +++ b/tests/fixtures/config/speculative-plugins/main.yaml @@ -0,0 +1,11 @@ +- tenant: + name: tenant-one + source: + gerrit: + config-projects: + - common-config + untrusted-projects: + - org/project + - org/project2 + - org/project3 + - org/projectrole diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index f019ead382..aa86896f79 100755 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -3429,3 +3429,48 @@ class TestJobOutput(AnsibleZuulTestCase): log_output = output.getvalue() self.assertIn('Final playbook failed', log_output) self.assertIn('Failure test', log_output) + + +class TestPlugins(AnsibleZuulTestCase): + tenant_config_file = 'config/speculative-plugins/main.yaml' + + def _run_job(self, job_name, project='org/project', roles=''): + # Output extra ansible info so we might see errors. + self.executor_server.verbose = True + conf = textwrap.dedent( + """ + - job: + name: {job_name} + run: playbooks/{job_name}/test.yaml + nodeset: + nodes: + - name: controller + label: whatever + {roles} + - project: + check: + jobs: + - {job_name} + """.format(job_name=job_name, roles=roles)) + + file_dict = {'zuul.yaml': conf} + A = self.fake_gerrit.addFakeChange(project, 'master', 'A', + files=file_dict) + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + message = A.messages[0] + self.assertIn('ERROR Ansible plugin dir', message) + self.assertIn('found adjacent to playbook', message) + self.assertIn('in non-trusted repo', message) + + def test_filter_plugin(self): + self._run_job('filter-plugin-playbook') + self._run_job('filter-plugin-playbook-symlink') + self._run_job('filter-plugin-bare-role') + self._run_job('filter-plugin-role') + self._run_job('filter-plugin-repo-role', project='org/projectrole') + self._run_job('filter-plugin-shared-role', + roles="roles: [{zuul: 'org/project2'}]") + self._run_job('filter-plugin-shared-bare-role', + roles="roles: [{zuul: 'org/project3', name: 'shared'}]") diff --git a/zuul/executor/server.py b/zuul/executor/server.py index 2009f26f47..68c35b4051 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -1028,6 +1028,7 @@ class AnsibleJob(object): ''' for entry in os.listdir(path): + entry = os.path.join(path, entry) if os.path.isdir(entry) and entry.endswith('_plugins'): raise ExecutorError( "Ansible plugin dir %s found adjacent to playbook %s in " @@ -1036,8 +1037,40 @@ class AnsibleJob(object): def findPlaybook(self, path, trusted=False): if os.path.exists(path): if not trusted: + # Plugins can be defined in multiple locations within the + # playbook's subtree. + # + # 1. directly within the playbook: + # block playbook_dir/*_plugins + # + # 2. within a role defined in playbook_dir/: + # block playbook_dir/*/*_plugins + # + # 3. within a role defined in playbook_dir/roles/: + # block playbook_dir/roles/*/*_plugins + playbook_dir = os.path.dirname(os.path.abspath(path)) - self._blockPluginDirs(playbook_dir) + paths_to_check = [] + + def addPathsToCheck(root_dir): + if os.path.isdir(root_dir): + for entry in os.listdir(root_dir): + entry = os.path.join(root_dir, entry) + if os.path.isdir(entry): + paths_to_check.append(entry) + + # handle case 1 + paths_to_check.append(playbook_dir) + + # handle case 2 + addPathsToCheck(playbook_dir) + + # handle case 3 + addPathsToCheck(os.path.join(playbook_dir, 'roles')) + + for path_to_check in paths_to_check: + self._blockPluginDirs(path_to_check) + return path raise ExecutorError("Unable to find playbook %s" % path)