zuul
/
zuul
Code Issues Proposed changes

Merge "Add missing localhost delegation checks to some modules"

This commit is contained in:
Zuul 2018-03-22 20:20:37 +00:00 committed by Gerrit Code Review
parent 93e63e7c0f 5763b8e4d7
commit 9141ae5a45
53 changed files with 516 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
tests/fixtures/bwrap-mounts/patch-dest/readme.txt vendored Normal file
View File

@ -0,0 +1 @@
This is a readme

3
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/assemble-delegate.yaml vendored Normal file
View File

@ -0,0 +1,3 @@
- hosts: all
roles:
- assemble-test-delegate

11
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/assemble-localhost.yaml vendored Normal file
View File

@ -0,0 +1,11 @@
- hosts: localhost
roles:
- assemble-test-localhost
- hosts: 127.0.0.1
roles:
- assemble-test-localhost
- hosts: "::1"
roles:
- assemble-test-localhost

3
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/copy-delegate.yaml vendored Normal file
View File

@ -0,0 +1,3 @@
- hosts: all
roles:
- copy-test-delegate

11
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/copy-localhost.yaml vendored Normal file
View File

@ -0,0 +1,11 @@
- hosts: localhost
roles:
- copy-test-localhost
- hosts: 127.0.0.1
roles:
- copy-test-localhost
- hosts: "::1"
roles:
- copy-test-localhost

3
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/patch-delegate.yaml vendored Normal file
View File

@ -0,0 +1,3 @@
- hosts: all
roles:
- patch-test-delegate

11
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/patch-localhost.yaml vendored Normal file
View File

@ -0,0 +1,11 @@
- hosts: localhost
roles:
- patch-test-localhost
- hosts: 127.0.0.1
roles:
- patch-test-localhost
- hosts: "::1"
roles:
- patch-test-localhost

1
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/assemble-test-delegate/files/dir/resolv.conf vendored Normal file
View File

@ -0,0 +1 @@
one

14
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/assemble-test-delegate/tasks/assemble-delegate.yaml vendored Normal file
View File

@ -0,0 +1,14 @@
- name: Assemble
assemble:
src: dir
dest: /opt/assemble-dest
remote_src: no
delegate_to: "{{ item }}"
ignore_errors: true
register: result
- assert:
that:
- "result.failed == true"
- "'Accessing files from outside the working dir' in result.msg"
msg: Assemble must fail due to accessing files outside the working dir

22
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/assemble-test-delegate/tasks/main.yaml vendored Normal file
View File

@ -0,0 +1,22 @@
- include: assemble-delegate.yaml
with_items:
- ::1
- 127.0.0.1
- localhost
- name: Define target dir
set_fact:
targetdir: "{{ zuul.executor.work_root }}/assemble-target"
- name: Create target dir
file:
state: directory
path: "{{ targetdir }}"
delegate_to: localhost
- name: Assemble to safe local path
assemble:
src: dir
dest: "{{ targetdir }}/assemble-dest.conf"
remote_src: no
delegate_to: localhost

1
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/assemble-test-localhost/files/dir/resolv.conf vendored Normal file
View File

@ -0,0 +1 @@
one

13
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/assemble-test-localhost/tasks/main.yaml vendored Normal file
View File

@ -0,0 +1,13 @@
- name: Assemble
assemble:
src: dir
dest: /opt/assemble-dest
remote_src: no
ignore_errors: true
register: result
- assert:
that:
- "result.failed == true"
- "'Accessing files from outside the working dir' in result.msg"
msg: Assemble must fail due to accessing files outside the working dir

0
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/copy-test-delegate/files/file vendored Normal file
View File

13
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/copy-test-delegate/tasks/copy-delegate.yaml vendored Normal file
View File

@ -0,0 +1,13 @@
- name: Copy
copy:
src: file
dest: /opt/copy-dest
delegate_to: "{{ item }}"
register: result
ignore_errors: true
- assert:
that:
- "result.failed == true"
- "'Accessing files from outside the working dir' in result.msg"
msg: Copy must fail due to accessing files outside the working dir

27
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/copy-test-delegate/tasks/main.yaml vendored Normal file
View File

@ -0,0 +1,27 @@
- include: copy-delegate.yaml
with_items:
- ::1
- 127.0.0.1
- localhost
- name: Define target dir
set_fact:
targetdir: "{{ zuul.executor.work_root }}/copy-target"
- name: Create target dir
file:
state: directory
path: "{{ targetdir }}"
delegate_to: localhost
- name: Copy file into safe path
copy:
src: file
dest: "{{ targetdir }}/dest-file"
delegate_to: localhost
- name: Copy file into safe directory
copy:
src: file
dest: "{{ targetdir }}/dest-dir/"
delegate_to: localhost

0
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/copy-test-localhost/files/file vendored Normal file
View File

12
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/copy-test-localhost/tasks/main.yaml vendored Normal file
View File

@ -0,0 +1,12 @@
- name: Copy
copy:
src: file
dest: /opt/copy-dest
register: result
ignore_errors: true
- assert:
that:
- "result.failed == true"
- "'Accessing files from outside the working dir' in result.msg"
msg: Copy must fail due to accessing files outside the working dir

7
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/patch-test-delegate/files/patch vendored Normal file
View File

@ -0,0 +1,7 @@
diff --git a/readme.txt b/readme.txt
index 24308cb..b07f0ed 100644
--- a/readme.txt
+++ b/readme.txt
@@ -1 +1 @@
-This is a readme
+This is a README

1
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/patch-test-delegate/files/readme.txt vendored Normal file
View File

@ -0,0 +1 @@
This is a readme

41
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/patch-test-delegate/tasks/main.yaml vendored Normal file
View File

@ -0,0 +1,41 @@
- include: patch-delegate.yaml
with_items:
- ::1
- 127.0.0.1
- localhost
- name: Define target dir
set_fact:
targetdir: "{{ zuul.executor.work_root }}/patch-target"
- name: Create target dir
file:
state: directory
path: "{{ targetdir }}"
delegate_to: localhost
- name: Copy readme
copy:
src: readme.txt
dest: "{{ targetdir }}/readme.txt"
delegate_to: localhost
- name: Patch in safe path using basedir
patch:
src: "patch"
basedir: "{{ targetdir }}"
strip: 1
delegate_to: localhost
- name: Copy readme again
copy:
src: readme.txt
dest: "{{ targetdir }}/readme.txt"
delegate_to: localhost
- name: Patch in safe path using dest
patch:
src: "patch"
dest: "{{ targetdir }}/readme.txt"
strip: 1
delegate_to: localhost

29
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/patch-test-delegate/tasks/patch-delegate.yaml vendored Normal file
View File

@ -0,0 +1,29 @@
- name: Patch with basedir
patch:
src: patch
basedir: "/opt/patch-dest"
strip: 1
delegate_to: "{{ item }}"
register: result
ignore_errors: true
- assert:
that:
- "result.failed == true"
- "'Accessing files from outside the working dir' in result.msg"
msg: Patch must fail due to accessing files outside the working dir
- name: Patch with dest
patch:
src: patch
dest: "/opt/patch-dest/readme"
strip: 1
delegate_to: "{{ item }}"
register: result
ignore_errors: true
- assert:
that:
- "result.failed == true"
- "'Accessing files from outside the working dir' in result.msg"
msg: Patch must fail due to accessing files outside the working dir

7
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/patch-test-localhost/files/patch vendored Normal file
View File

@ -0,0 +1,7 @@
diff --git a/readme.txt b/readme.txt
index 24308cb..b07f0ed 100644
--- a/readme.txt
+++ b/readme.txt
@@ -1 +1 @@
-This is a readme
+This is a README

1
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/patch-test-localhost/files/readme.txt vendored Normal file
View File

@ -0,0 +1 @@
This is a readme

27
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/patch-test-localhost/tasks/main.yaml vendored Normal file
View File

@ -0,0 +1,27 @@
- name: Patch with basedir
patch:
src: patch
basedir: "/opt/patch-dest"
strip: 1
register: result
ignore_errors: true
- assert:
that:
- "result.failed == true"
- "'Accessing files from outside the working dir' in result.msg"
msg: Patch must fail due to accessing files outside the working dir
- name: Patch with dest
patch:
src: patch
dest: "/opt/patch-dest/readme"
strip: 1
register: result
ignore_errors: true
- assert:
that:
- "result.failed == true"
- "'Accessing files from outside the working dir' in result.msg"
msg: Patch must fail due to accessing files outside the working dir

3
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/script-test-delegate/files/script.sh vendored Normal file
View File

@ -0,0 +1,3 @@
#!/bin/sh
echo one

5
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/script-test-delegate/tasks/main.yaml vendored Normal file
View File

@ -0,0 +1,5 @@
- include: script-delegate.yaml
with_items:
- ::1
- 127.0.0.1
- localhost

11
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/script-test-delegate/tasks/script-delegate.yaml vendored Normal file
View File

@ -0,0 +1,11 @@
- name: Script
script: script.sh
delegate_to: "{{ item }}"
register: result
ignore_errors: true
- assert:
that:
- "result.failed == true"
- "'Executing local code is prohibited' in result.msg"
msg: Script must fail due to local code execution restriction

3
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/script-test-localhost/files/script.sh vendored Normal file
View File

@ -0,0 +1,3 @@
#!/bin/sh
echo one

10
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/script-test-localhost/tasks/main.yaml vendored Normal file
View File

@ -0,0 +1,10 @@
- name: Script
script: script.sh
register: result
ignore_errors: true
- assert:
that:
- "result.failed == true"
- "'Executing local code is prohibited' in result.msg"
msg: Script must fail due to local code execution restriction

21
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/template-test-delegate/tasks/main.yaml vendored Normal file
View File

@ -0,0 +1,21 @@
- include: template-delegate.yaml
with_items:
- ::1
- 127.0.0.1
- localhost
- name: Define target dir
set_fact:
targetdir: "{{ zuul.executor.work_root }}/template-target"
- name: Create target dir
file:
state: directory
path: "{{ targetdir }}"
delegate_to: localhost
- name: Template into safe path
template:
src: template
dest: "{{ targetdir }}/dest-file"
delegate_to: localhost

13
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/template-test-delegate/tasks/template-delegate.yaml vendored Normal file
View File

@ -0,0 +1,13 @@
- name: Template
copy:
src: template
dest: /opt/copy-dest
delegate_to: "{{ item }}"
register: result
ignore_errors: true
- assert:
that:
- "result.failed == true"
- "'Accessing files from outside the working dir' in result.msg"
msg: Template must fail due to accessing files outside the working dir

0
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/template-test-delegate/templates/template vendored Normal file
View File

12
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/template-test-localhost/tasks/main.yaml vendored Normal file
View File

@ -0,0 +1,12 @@
- name: Template
copy:
src: template
dest: /opt/copy-dest
register: result
ignore_errors: true
- assert:
that:
- "result.failed == true"
- "'Accessing files from outside the working dir' in result.msg"
msg: Template must fail due to accessing files outside the working dir

0
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/template-test-localhost/templates/template vendored Normal file
View File

BIN
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/unarchive-test-delegate/files/archive.tar vendored Normal file
View File

Binary file not shown.

21
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/unarchive-test-delegate/tasks/main.yaml vendored Normal file
View File

@ -0,0 +1,21 @@
- include: unarchive-delegate.yaml
with_items:
- ::1
- 127.0.0.1
- localhost
- name: Define target dir
set_fact:
targetdir: "{{ zuul.executor.work_root }}/unarchive-target"
- name: Create target dir
file:
state: directory
path: "{{ targetdir }}"
delegate_to: localhost
- name: Unarchive
copy:
src: archive.tar
dest: "{{ targetdir }}"
delegate_to: localhost

13
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/unarchive-test-delegate/tasks/unarchive-delegate.yaml vendored Normal file
View File

@ -0,0 +1,13 @@
- name: Unarchive
copy:
src: archive.tar
dest: /opt/unarchive-dest
delegate_to: "{{ item }}"
register: result
ignore_errors: true
- assert:
that:
- "result.failed == true"
- "'Accessing files from outside the working dir' in result.msg"
msg: Unarchive must fail due to accessing files outside the working dir

BIN
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/unarchive-test-localhost/files/archive.tar vendored Normal file
View File

Binary file not shown.

12
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/unarchive-test-localhost/tasks/main.yaml vendored Normal file
View File

@ -0,0 +1,12 @@
- name: Unarchive
copy:
src: archive.tar
dest: /opt/unarchive-dest
register: result
ignore_errors: true
- assert:
that:
- "result.failed == true"
- "'Accessing files from outside the working dir' in result.msg"
msg: Unarchive must fail due to accessing files outside the working dir

3
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/script-delegate.yaml vendored Normal file
View File

@ -0,0 +1,3 @@
- hosts: all
roles:
- script-test-delegate

11
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/script-localhost.yaml vendored Normal file
View File

@ -0,0 +1,11 @@
- hosts: localhost
roles:
- script-test-localhost
- hosts: 127.0.0.1
roles:
- script-test-localhost
- hosts: "::1"
roles:
- script-test-localhost

3
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/template-delegate.yaml vendored Normal file
View File

@ -0,0 +1,3 @@
- hosts: all
roles:
- template-test-delegate

11
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/template-localhost.yaml vendored Normal file
View File

@ -0,0 +1,11 @@
- hosts: localhost
roles:
- template-test-localhost
- hosts: 127.0.0.1
roles:
- template-test-localhost
- hosts: "::1"
roles:
- template-test-localhost

3
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/unarchive-delegate.yaml vendored Normal file
View File

@ -0,0 +1,3 @@
- hosts: all
roles:
- unarchive-test-delegate

11
tests/fixtures/config/remote-action-modules/git/org_project/playbooks/unarchive-localhost.yaml vendored Normal file
View File

@ -0,0 +1,11 @@
- hosts: localhost
roles:
- unarchive-test-localhost
- hosts: 127.0.0.1
roles:
- unarchive-test-localhost
- hosts: "::1"
roles:
- unarchive-test-localhost

36
tests/remote/test_remote_action_modules.py
View File

@ -81,6 +81,12 @@ class TestActionModules(AnsibleZuulTestCase):
def test_assemble_module(self):
self._run_job('assemble-good', 'SUCCESS')
# assemble-delegate does multiple tests with various delegates and
# safe and non-safe paths. It asserts by itself within ansible so we
# expect SUCCESS here.
self._run_job('assemble-delegate', 'SUCCESS')
self._run_job('assemble-localhost', 'SUCCESS')
self._run_job('assemble-bad', 'FAILURE', ERROR_ACCESS_OUTSIDE)
self._run_job('assemble-bad-symlink', 'FAILURE', ERROR_ACCESS_OUTSIDE)
self._run_job('assemble-bad-dir-with-symlink', 'FAILURE',
@ -89,6 +95,12 @@ class TestActionModules(AnsibleZuulTestCase):
def test_copy_module(self):
self._run_job('copy-good', 'SUCCESS')
# copy-delegate does multiple tests with various delegates and
# safe and non-safe paths. It asserts by itself within ansible so we
# expect SUCCESS here.
self._run_job('copy-delegate', 'SUCCESS')
self._run_job('copy-localhost', 'SUCCESS')
self._run_job('copy-bad', 'FAILURE', ERROR_ACCESS_OUTSIDE)
self._run_job('copy-bad-symlink', 'FAILURE', ERROR_ACCESS_OUTSIDE)
self._run_job('copy-bad-dir-with-symlink', 'FAILURE',
@ -112,23 +124,47 @@ class TestActionModules(AnsibleZuulTestCase):
def test_patch_module(self):
self._run_job('patch-good', 'SUCCESS')
# patch-delegate does multiple tests with various delegates and
# safe and non-safe paths. It asserts by itself within ansible so we
# expect SUCCESS here.
self._run_job('patch-delegate', 'SUCCESS')
self._run_job('patch-localhost', 'SUCCESS')
self._run_job('patch-bad', 'FAILURE', ERROR_ACCESS_OUTSIDE)
self._run_job('patch-bad-symlink', 'FAILURE', ERROR_ACCESS_OUTSIDE)
def test_script_module(self):
self._run_job('script-good', 'SUCCESS')
# script-delegate does multiple tests with various delegates. It
# asserts by itself within ansible so we
# expect SUCCESS here.
self._run_job('script-delegate', 'SUCCESS')
self._run_job('script-localhost', 'SUCCESS')
self._run_job('script-bad', 'FAILURE', ERROR_ACCESS_OUTSIDE)
self._run_job('script-bad-symlink', 'FAILURE', ERROR_ACCESS_OUTSIDE)
def test_template_module(self):
self._run_job('template-good', 'SUCCESS')
# template-delegate does multiple tests with various delegates and
# safe and non-safe paths. It asserts by itself within ansible so we
# expect SUCCESS here.
self._run_job('template-delegate', 'SUCCESS')
self._run_job('template-localhost', 'SUCCESS')
self._run_job('template-bad', 'FAILURE', ERROR_ACCESS_OUTSIDE)
self._run_job('template-bad-symlink', 'FAILURE', ERROR_ACCESS_OUTSIDE)
def test_unarchive_module(self):
self._run_job('unarchive-good', 'SUCCESS')
# template-delegate does multiple tests with various delegates and
# safe and non-safe paths. It asserts by itself within ansible so we
# expect SUCCESS here.
self._run_job('unarchive-delegate', 'SUCCESS')
self._run_job('unarchive-localhost', 'SUCCESS')
self._run_job('unarchive-bad', 'FAILURE', ERROR_ACCESS_OUTSIDE)
self._run_job('unarchive-bad-symlink', 'FAILURE', ERROR_ACCESS_OUTSIDE)

3
zuul/ansible/action/assemble.py
View File

@ -29,4 +29,7 @@ class ActionModule(assemble.ActionModule):
if not paths._is_official_module(self):
return paths._fail_module_dict(self._task.action)
if paths._is_localhost_task(self):
paths._fail_if_unsafe(self._task.args['dest'])
return super(ActionModule, self).run(tmp, task_vars)

3
zuul/ansible/action/copy.py
View File

@ -29,4 +29,7 @@ class ActionModule(copy.ActionModule):
if not paths._is_official_module(self):
return paths._fail_module_dict(self._task.action)
if paths._is_localhost_task(self):
paths._fail_if_unsafe(self._task.args['dest'])
return super(ActionModule, self).run(tmp, task_vars)

7
zuul/ansible/action/normal.py
View File

@ -35,12 +35,7 @@ class ActionModule(normal.ActionModule):
def run(self, tmp=None, task_vars=None):
'''Overridden primary method from the base class.'''
if (self._play_context.connection == 'local'
or self._play_context.remote_addr == 'localhost'
or self._play_context.remote_addr.startswith('127.')
or self._task.delegate_to == 'localhost'
or (self._task.delegate_to
and self._task.delegate_to.startswith('127.'))):
if paths._is_localhost_task(self):
if not self.dispatch_handler():
raise AnsibleError("Executing local code is prohibited")
return super(ActionModule, self).run(tmp, task_vars)

14
zuul/ansible/action/patch.py
View File

@ -13,7 +13,6 @@
# You should have received a copy of the GNU General Public License
# along with this software. If not, see <http://www.gnu.org/licenses/>.
from zuul.ansible import paths
patch = paths._import_ansible_action_plugin("patch")
@ -28,4 +27,17 @@ class ActionModule(patch.ActionModule):
if not paths._is_official_module(self):
return paths._fail_module_dict(self._task.action)
if paths._is_localhost_task(self):
# The patch module has two possibilities of describing where to
# operate, basedir and dest. We need to perform the safe path check
# for both.
dirs_to_check = [
self._task.args.get('basedir'),
self._task.args.get('dest'),
]
for directory in dirs_to_check:
if directory is not None:
paths._fail_if_unsafe(directory)
return super(ActionModule, self).run(tmp, task_vars)

4
zuul/ansible/action/script.py
View File

@ -14,6 +14,7 @@
# along with this software. If not, see <http://www.gnu.org/licenses/>.
from ansible.errors import AnsibleError
from zuul.ansible import paths
script = paths._import_ansible_action_plugin("script")
@ -29,4 +30,7 @@ class ActionModule(script.ActionModule):
if not paths._is_official_module(self):
return paths._fail_module_dict(self._task.action)
if paths._is_localhost_task(self):
raise AnsibleError("Executing local code is prohibited")
return super(ActionModule, self).run(tmp, task_vars)

5
zuul/ansible/action/unarchive.py
View File

@ -28,4 +28,9 @@ class ActionModule(unarchive.ActionModule):
if not paths._is_official_module(self):
return paths._fail_module_dict(self._task.action)
# Note: The unarchive module reuses the copy module to copy the archive
# to the remote. Thus we don't need to check the dest here if we run
# against localhost. We also have tests that would break if this
# changes in the future.
return super(ActionModule, self).run(tmp, task_vars)

26
zuul/ansible/paths.py
View File

@ -151,3 +151,29 @@ def _fail_if_local_module(module):
if not _is_official_module(module):
msg_dict = _fail_module_dict(module._task.action)
raise AnsibleError(msg_dict['msg'])
def _is_localhost_task(task):
# remote_addr is what's in the value of ansible_host and/or the opposite
# side of a mapping. So if you had an inventory with:
#
# all:
# hosts:
# ubuntu-xenial:
# ansible_connection: ssh
# ansible_host: 23.253.109.74
# remote_addr would be 23.253.109.74.
#
# localhost is special, since it's not in the inventory but instead is
# added directly by ansible.
#
# The only way a user could supply a remote_addr with arbitrary ipv6
# values is if they used add_host - which we don't let unprivileged code
# do.
if (task._play_context.connection == 'local'
or task._play_context.remote_addr == 'localhost'
or task._task.delegate_to == 'localhost'):
return True
return False