diff --git a/changelogs/fragments/fix-world-readable-skip-message.yaml b/changelogs/fragments/fix-world-readable-skip-message.yaml new file mode 100644 index 00000000000..1164599ecb3 --- /dev/null +++ b/changelogs/fragments/fix-world-readable-skip-message.yaml @@ -0,0 +1,9 @@ +--- +bugfixes: +- The fix for `CVE-2018-10875 `_ + prints out a warning message about skipping a config file from a world + writable current working directory. However, if the user explicitly + specifies that the config file should be used via the ANSIBLE_CONFIG + environment variable then Ansible would honor that but still print out the + warning message. This has been fixed so that Ansible honors the user's + explicit wishes and does not print a warning message in that circumstance. diff --git a/docs/templates/config.rst.j2 b/docs/templates/config.rst.j2 index cf5baaf4eec..bc6c71713fd 100644 --- a/docs/templates/config.rst.j2 +++ b/docs/templates/config.rst.j2 @@ -40,6 +40,40 @@ Ansible will process the above list and use the first file found, all others are inventory = /etc/ansible/hosts ; This points to the file that lists your hosts +.. _cfg_in_world_writable_dir: + +Avoiding security risks with ``ansible.cfg`` in the current directory +--------------------------------------------------------------------- + + +If Ansible were to load :file:ansible.cfg from a world-writable current working +directory, it would create a serious security risk. Another user could place +their own config file there, designed to make Ansible run malicious code both +locally and remotely, possibly with elevated privileges. For this reason, +Ansible will not automatically load a config file from the current working +directory if the directory is world-writable. + +If you depend on using Ansible with a config file in the current working +directory, the best way to avoid this problem is to restrict access to your +Ansible directories to particular user(s) and/or group(s). If your Ansible +directories live on a filesystem which has to emulate Unix permissions, like +Vagrant or Windows Subsystem for Linux (WSL), you may, at first, not know how +you can fix this as ``chmod``, ``chown``, and ``chgrp`` might not work there. +In most of those cases, the correct fix is to modify the mount options of the +filesystem so the files and directories are readable and writable by the users +and groups running Ansible but closed to others. For more details on the +correct settings, see: + +* for Vagrant, Jeremy Kendall's `blog post `_ covers synced folder permissions. +* for WSL, the `WSL docs `_ + and this `Microsoft blog post `_ cover mount options. + +If you absolutely depend on having the config live in a world-writable current +working directory, you can explicitly specify the config file via the +:envvar:`ANSIBLE_CONFIG` environment variable. Please take +appropriate steps to mitigate the security concerns above before doing so. + + Common Options ============== diff --git a/lib/ansible/config/manager.py b/lib/ansible/config/manager.py index 0afb41f7124..3fd31ac88ff 100644 --- a/lib/ansible/config/manager.py +++ b/lib/ansible/config/manager.py @@ -6,6 +6,7 @@ __metaclass__ = type import io import os +import os.path import sys import stat import tempfile @@ -152,31 +153,59 @@ def find_ini_config_file(warnings=None): ''' Load INI Config File order(first found is used): ENV, CWD, HOME, /etc/ansible ''' # FIXME: eventually deprecate ini configs - path0 = os.getenv("ANSIBLE_CONFIG", None) - if path0 is not None: - path0 = unfrackpath(path0, follow=False) - if os.path.isdir(path0): - path0 += "/ansible.cfg" + if warnings is None: + # Note: In this case, warnings does nothing + warnings = set() + + # A value that can never be a valid path so that we can tell if ANSIBLE_CONFIG was set later + # We can't use None because we could set path to None. + SENTINEL = object + + potential_paths = [] + + # Environment setting + path_from_env = os.getenv("ANSIBLE_CONFIG", SENTINEL) + if path_from_env is not SENTINEL: + path_from_env = unfrackpath(path_from_env, follow=False) + if os.path.isdir(path_from_env): + path_from_env = os.path.join(path_from_env, "ansible.cfg") + potential_paths.append(path_from_env) + + # Current working directory + warn_cmd_public = False try: - path1 = os.getcwd() - perms1 = os.stat(path1) - if perms1.st_mode & stat.S_IWOTH: - if warnings is not None: - warnings.add("Ansible is in a world writable directory (%s), ignoring it as an ansible.cfg source." % to_text(path1)) - path1 = None + cwd = os.getcwd() + perms = os.stat(cwd) + if perms.st_mode & stat.S_IWOTH: + warn_cmd_public = True else: - path1 += "/ansible.cfg" + potential_paths.append(os.path.join(cwd, "ansible.cfg")) except OSError: - path1 = None - path2 = unfrackpath("~/.ansible.cfg", follow=False) - path3 = "/etc/ansible/ansible.cfg" + # If we can't access cwd, we'll simply skip it as a possible config source + pass + + # Per user location + potential_paths.append(unfrackpath("~/.ansible.cfg", follow=False)) - for path in [path0, path1, path2, path3]: - if path is not None and os.path.exists(path): + # System location + potential_paths.append("/etc/ansible/ansible.cfg") + + for path in potential_paths: + if os.path.exists(path): break else: path = None + # Emit a warning if all the following are true: + # * We did not use a config from ANSIBLE_CONFIG + # * There's an ansible.cfg in the current working directory that we skipped + if path_from_env != path and warn_cmd_public: + warnings.add(u"Ansible is being run in a world writable directory (%s)," + u" ignoring it as an ansible.cfg source." + u" For more information see" + u" https://docs.ansible.com/ansible/devel/reference_appendices/config.html#cfg-in-world-writable-dir" + % to_text(cwd)) + return path diff --git a/test/units/config/manager/__init__.py b/test/units/config/manager/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/units/config/manager/test_find_ini_config_file.py b/test/units/config/manager/test_find_ini_config_file.py new file mode 100644 index 00000000000..f8f3d72c239 --- /dev/null +++ b/test/units/config/manager/test_find_ini_config_file.py @@ -0,0 +1,221 @@ +# -*- coding: utf-8 -*- +# Copyright: (c) 2017, Ansible Project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +# Make coding more python3-ish +from __future__ import (absolute_import, division) +__metaclass__ = type + +import os +import os.path +import stat + +import pytest + +from ansible.config.manager import find_ini_config_file + + +real_exists = os.path.exists +real_isdir = os.path.isdir + +working_dir = os.path.dirname(__file__) +cfg_in_cwd = os.path.join(working_dir, 'ansible.cfg') + +cfg_dir = os.path.join(working_dir, 'data') +cfg_file = os.path.join(cfg_dir, 'ansible.cfg') +alt_cfg_file = os.path.join(cfg_dir, 'test.cfg') +cfg_in_homedir = os.path.expanduser('~/.ansible.cfg') + + +@pytest.fixture +def setup_env(request): + cur_config = os.environ.get('ANSIBLE_CONFIG', None) + cfg_path = request.param[0] + + if cfg_path is None and cur_config: + del os.environ['ANSIBLE_CONFIG'] + else: + os.environ['ANSIBLE_CONFIG'] = request.param[0] + + yield + + if cur_config is None and cfg_path: + del os.environ['ANSIBLE_CONFIG'] + else: + os.environ['ANSIBLE_CONFIG'] = cur_config + + +@pytest.fixture +def setup_existing_files(request, monkeypatch): + def _os_path_exists(path): + if path in (request.param[0]): + return True + else: + return False + + # Enable user and system dirs so that we know cwd takes precedence + monkeypatch.setattr("os.path.exists", _os_path_exists) + monkeypatch.setattr("os.getcwd", lambda: os.path.dirname(cfg_dir)) + monkeypatch.setattr("os.path.isdir", lambda path: True if path == cfg_dir else real_isdir(path)) + + +class TestFindIniFile: + # This tells us to run twice, once with a file specified and once with a directory + @pytest.mark.parametrize('setup_env, expected', (([alt_cfg_file], alt_cfg_file), ([cfg_dir], cfg_file)), indirect=['setup_env']) + # This just passes the list of files that exist to the fixture + @pytest.mark.parametrize('setup_existing_files', + [[('/etc/ansible/ansible.cfg', cfg_in_homedir, cfg_in_cwd, alt_cfg_file, cfg_file)]], + indirect=['setup_existing_files']) + def test_env_has_cfg_file(self, setup_env, setup_existing_files, expected): + """ANSIBLE_CONFIG is specified, use it""" + warnings = set() + assert find_ini_config_file(warnings) == expected + assert warnings == set() + + @pytest.mark.parametrize('setup_env', ([alt_cfg_file], [cfg_dir]), indirect=['setup_env']) + @pytest.mark.parametrize('setup_existing_files', + [[('/etc/ansible/ansible.cfg', cfg_in_homedir, cfg_in_cwd)]], + indirect=['setup_existing_files']) + def test_env_has_no_cfg_file(self, setup_env, setup_existing_files): + """ANSIBLE_CONFIG is specified but the file does not exist""" + + warnings = set() + # since the cfg file specified by ANSIBLE_CONFIG doesn't exist, the one at cwd that does + # exist should be returned + assert find_ini_config_file(warnings) == cfg_in_cwd + assert warnings == set() + + # ANSIBLE_CONFIG not specified + @pytest.mark.parametrize('setup_env', [[None]], indirect=['setup_env']) + # All config files are present + @pytest.mark.parametrize('setup_existing_files', + [[('/etc/ansible/ansible.cfg', cfg_in_homedir, cfg_in_cwd, cfg_file, alt_cfg_file)]], + indirect=['setup_existing_files']) + def test_ini_in_cwd(self, setup_env, setup_existing_files): + """ANSIBLE_CONFIG not specified. Use the cwd cfg""" + warnings = set() + assert find_ini_config_file(warnings) == cfg_in_cwd + assert warnings == set() + + # ANSIBLE_CONFIG not specified + @pytest.mark.parametrize('setup_env', [[None]], indirect=['setup_env']) + # No config in cwd + @pytest.mark.parametrize('setup_existing_files', + [[('/etc/ansible/ansible.cfg', cfg_in_homedir, cfg_file, alt_cfg_file)]], + indirect=['setup_existing_files']) + def test_ini_in_homedir(self, setup_env, setup_existing_files): + """First config found is in the homedir""" + warnings = set() + assert find_ini_config_file(warnings) == cfg_in_homedir + assert warnings == set() + + # ANSIBLE_CONFIG not specified + @pytest.mark.parametrize('setup_env', [[None]], indirect=['setup_env']) + # No config in cwd + @pytest.mark.parametrize('setup_existing_files', [[('/etc/ansible/ansible.cfg', cfg_file, alt_cfg_file)]], indirect=['setup_existing_files']) + def test_ini_in_systemdir(self, setup_env, setup_existing_files): + """First config found is the system config""" + warnings = set() + assert find_ini_config_file(warnings) == '/etc/ansible/ansible.cfg' + assert warnings == set() + + # ANSIBLE_CONFIG not specified + @pytest.mark.parametrize('setup_env', [[None]], indirect=['setup_env']) + # No config in cwd + @pytest.mark.parametrize('setup_existing_files', + [[('/etc/ansible/ansible.cfg', cfg_in_homedir, cfg_file, alt_cfg_file)]], + indirect=['setup_existing_files']) + def test_cwd_does_not_exist(self, setup_env, setup_existing_files, monkeypatch): + """Smoketest current working directory doesn't exist""" + def _os_stat(path): + raise OSError('%s does not exist' % path) + monkeypatch.setattr('os.stat', _os_stat) + + warnings = set() + assert find_ini_config_file(warnings) == cfg_in_homedir + assert warnings == set() + + @pytest.mark.parametrize('setup_env', [[None]], indirect=['setup_env']) + # No config in cwd + @pytest.mark.parametrize('setup_existing_files', [[list()]], indirect=['setup_existing_files']) + def test_no_config(self, setup_env, setup_existing_files): + """No config present, no config found""" + warnings = set() + assert find_ini_config_file(warnings) is None + assert warnings == set() + + # ANSIBLE_CONFIG not specified + @pytest.mark.parametrize('setup_env', [[None]], indirect=['setup_env']) + # All config files are present + @pytest.mark.parametrize('setup_existing_files', + [[('/etc/ansible/ansible.cfg', cfg_in_homedir, cfg_in_cwd, cfg_file, alt_cfg_file)]], + indirect=['setup_existing_files']) + def test_cwd_warning_on_writable(self, setup_env, setup_existing_files, monkeypatch): + """If the cwd is writable, warn and skip it """ + real_stat = os.stat + + def _os_stat(path): + if path == working_dir: + from posix import stat_result + stat_info = list(real_stat(path)) + stat_info[stat.ST_MODE] |= stat.S_IWOTH + return stat_result(stat_info) + else: + return real_stat(path) + + monkeypatch.setattr('os.stat', _os_stat) + + warnings = set() + assert find_ini_config_file(warnings) == cfg_in_homedir + assert len(warnings) == 1 + warning = warnings.pop() + assert u'Ansible is being run in a world writable directory' in warning + assert u'ignoring it as an ansible.cfg source' in warning + + # ANSIBLE_CONFIG is sepcified + @pytest.mark.parametrize('setup_env, expected', (([alt_cfg_file], alt_cfg_file), ([cfg_in_cwd], cfg_in_cwd)), indirect=['setup_env']) + # All config files are present + @pytest.mark.parametrize('setup_existing_files', + [[('/etc/ansible/ansible.cfg', cfg_in_homedir, cfg_in_cwd, cfg_file, alt_cfg_file)]], + indirect=['setup_existing_files']) + def test_no_warning_on_writable_if_env_used(self, setup_env, setup_existing_files, monkeypatch, expected): + """If the cwd is writable but ANSIBLE_CONFIG was used, no warning should be issued""" + real_stat = os.stat + + def _os_stat(path): + if path == working_dir: + from posix import stat_result + stat_info = list(real_stat(path)) + stat_info[stat.ST_MODE] |= stat.S_IWOTH + return stat_result(stat_info) + else: + return real_stat(path) + + monkeypatch.setattr('os.stat', _os_stat) + + warnings = set() + assert find_ini_config_file(warnings) == expected + assert warnings == set() + + # ANSIBLE_CONFIG not specified + @pytest.mark.parametrize('setup_env', [[None]], indirect=['setup_env']) + # All config files are present + @pytest.mark.parametrize('setup_existing_files', + [[('/etc/ansible/ansible.cfg', cfg_in_homedir, cfg_in_cwd, cfg_file, alt_cfg_file)]], + indirect=['setup_existing_files']) + def test_cwd_warning_on_writable_no_warning_set(self, setup_env, setup_existing_files, monkeypatch): + """Smoketest that the function succeeds even though no warning set was passed in""" + real_stat = os.stat + + def _os_stat(path): + if path == working_dir: + from posix import stat_result + stat_info = list(real_stat(path)) + stat_info[stat.ST_MODE] |= stat.S_IWOTH + return stat_result(stat_info) + else: + return real_stat(path) + + monkeypatch.setattr('os.stat', _os_stat) + + assert find_ini_config_file() == cfg_in_homedir diff --git a/test/units/config/test_manager.py b/test/units/config/test_manager.py index c4a9db2df0e..316d6a433a5 100644 --- a/test/units/config/test_manager.py +++ b/test/units/config/test_manager.py @@ -3,6 +3,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type import os +import os.path from ansible.compat.tests import unittest @@ -50,12 +51,6 @@ class TestConfigData(unittest.TestCase): self.assertIsInstance(ensure_type('0.10', 'float'), float) self.assertIsInstance(ensure_type(0.2, 'float'), float) - def test_find_ini_file(self): - cur_config = os.environ['ANSIBLE_CONFIG'] - os.environ['ANSIBLE_CONFIG'] = cfg_file - self.assertEquals(cfg_file, find_ini_config_file()) - os.environ['ANSIBLE_CONFIG'] = cur_config - def test_resolve_path(self): self.assertEquals(os.path.join(curdir, 'test.yml'), resolve_path('./test.yml', cfg_file))