Prohibit invalid uri usages on localhost

The uri module could potentially be used to expose files within the
bubblewrap context either through its src or dest parameters or its url
parameter. In the case of src and dest we use zuuls utilility functions
to filter out invalid srcs and dests. In the case of url we have been
relying on an ansible bug that prevents request responses without a
status code from completing successfully due to an unchecked type
coercion.

This change adds our own check to url schemes and restricts it to http,
https, and ftp so that if ansible fixes their bugs zuul will continue to
do the right thing.

Then we add testing for all of the cases talked about above.

Change-Id: I527a4082c1ec5556e4c8347ff08b2e89ce0edaaa
Task: #40940
This commit is contained in:
Clark Boylan 2020-09-22 14:07:06 -07:00 committed by Jeremy Stanley
parent 79ae9e2f6f
commit 1fb7bd33e7
6 changed files with 61 additions and 0 deletions

View File

@ -0,0 +1,6 @@
- hosts: localhost
tasks:
- name: Request with bad src
uri:
url: https://zuul.opendev.org
dest: /etc/zuul-uri-output-testing

View File

@ -0,0 +1,7 @@
- hosts: localhost
tasks:
- name: Request with bad src
uri:
url: https://zuul.opendev.org
method: POST
src: /etc/resolv.conf

View File

@ -0,0 +1,14 @@
- hosts: localhost
tasks:
- name: Request with bad url scheme
uri:
url: "file:///etc/resolv.conf"
dest: "{{ zuul.executor.log_root }}/resolv.conf"
- name: stat file that shouldnt exist
stat:
path: "{{ zuul.executor.log_root }}/resolv.conf"
register: test_stat
- name: Debug the stat
debug:
msg: "resolv.conf exists when it shouldn't"
when: test_stat.stat.exists

View File

@ -0,0 +1,16 @@
- hosts: localhost
tasks:
- name: Safe uri request from localhost
uri:
# We don't seem to have working ssl cert chains in
# the test bwrap context. Use http to workaround that
# and don't follow the redirect to https.
url: http://zuul.opendev.org
follow_redirects: none
return_content: yes
status_code:
- 301
- 302
- 303
- 307
- 308

View File

@ -22,6 +22,7 @@ ERROR_LOCAL_CODE = "Executing local code is prohibited"
ERROR_SYNC_TO_OUTSIDE = "Syncing files to outside the working dir"
ERROR_SYNC_FROM_OUTSIDE = "Syncing files from outside the working dir"
ERROR_SYNC_RSH = "Using custom synchronize rsh is prohibited"
ERROR_SCHEME_INVALID = "file urls are not allowed from localhost."
class FunctionalActionModulesMixIn:
@ -221,6 +222,12 @@ class FunctionalActionModulesMixIn:
self._run_job('known-hosts-bad', 'FAILURE', ERROR_ACCESS_OUTSIDE)
def test_uri_module(self):
self._run_job('uri-good', 'SUCCESS')
self._run_job('uri-bad-src', 'FAILURE', ERROR_ACCESS_OUTSIDE)
self._run_job('uri-bad-dest', 'FAILURE', ERROR_ACCESS_OUTSIDE)
self._run_job('uri-bad-url', 'FAILURE', ERROR_SCHEME_INVALID)
class TestActionModules28(AnsibleZuulTestCase, FunctionalActionModulesMixIn):
ansible_version = '2.8'

View File

@ -13,10 +13,14 @@
# 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 ansible.errors import AnsibleError
from ansible.module_utils.six.moves.urllib.parse import urlparse
from zuul.ansible import paths
uri = paths._import_ansible_action_plugin("uri")
ALLOWED_URL_SCHEMES = ('https', 'http', 'ftp')
class ActionModule(uri.ActionModule):
@ -34,5 +38,12 @@ class ActionModule(uri.ActionModule):
dest = self._task.args.get(arg)
if dest:
paths._fail_if_unsafe(dest)
scheme = urlparse(self._task.args['url']).scheme
if scheme not in ALLOWED_URL_SCHEMES:
raise AnsibleError(
"{scheme} urls are not allowed from localhost."
" Only {allowed_schemes} are allowed".format(
scheme=scheme,
allowed_schemes=ALLOWED_URL_SCHEMES))
return super(ActionModule, self).run(tmp, task_vars)