summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorToshio Kuratomi <a.badger@gmail.com>2018-08-13 18:16:24 -0700
committerMatt Davis <nitzmahone@users.noreply.github.com>2018-08-13 18:16:24 -0700
commit8d2c1299447ab7423f0c51dfafc3b7e6478ad883 (patch)
tree011073f956a0729b809068fa1c96c1b8d77256bf
parente22bff5b76fd30c1d2f14e3e9a1f12a021ef89b6 (diff)
downloadansible-8d2c1299447ab7423f0c51dfafc3b7e6478ad883.tar.gz
[stable-2.5] Only print warning when ansible.cfg is actually skipped (#43583) (#43649)
Only print warning when ansible.cfg is actually skipped * Also add unittests for the find_ini_config_file function * Add documentation on world writable current working directory config files can no longer be loaded from a world writable current working directory but the end user is allowed to specify that explicitly. Give appropriate warnings and information on how. Fixes #42388 (cherry picked from commit 30662bedadda1cc00efb1946e8f75c5b9fb42d66) Co-authored-by: Toshio Kuratomi <a.badger@gmail.com>
-rw-r--r--changelogs/fragments/fix-world-readable-skip-message.yaml9
-rw-r--r--docs/templates/config.rst.j234
-rw-r--r--lib/ansible/config/manager.py63
-rw-r--r--test/units/config/manager/__init__.py0
-rw-r--r--test/units/config/manager/test_find_ini_config_file.py221
-rw-r--r--test/units/config/test_manager.py7
6 files changed, 311 insertions, 23 deletions
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 0000000000..1164599ecb
--- /dev/null
+++ b/changelogs/fragments/fix-world-readable-skip-message.yaml
@@ -0,0 +1,9 @@
+---
+bugfixes:
+- The fix for `CVE-2018-10875 <https://access.redhat.com/security/cve/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 cf0cb91ef4..bf5637b7c4 100644
--- a/docs/templates/config.rst.j2
+++ b/docs/templates/config.rst.j2
@@ -39,6 +39,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 <http://jeremykendall.net/2013/08/09/vagrant-synced-folders-permissions/>`_ covers synced folder permissions.
+* for WSL, the `WSL docs <https://docs.microsoft.com/en-us/windows/wsl/wsl-config#set-wsl-launch-settings>`_
+ and this `Microsoft blog post <https://blogs.msdn.microsoft.com/commandline/2018/01/12/chmod-chown-wsl-improvements/>`_ 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 ae1bae5a00..f0587b162c 100644
--- a/lib/ansible/config/manager.py
+++ b/lib/ansible/config/manager.py
@@ -5,6 +5,7 @@ from __future__ import (absolute_import, division, print_function)
__metaclass__ = type
import os
+import os.path
import sys
import stat
import tempfile
@@ -144,31 +145,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 0000000000..e69de29bb2
--- /dev/null
+++ b/test/units/config/manager/__init__.py
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 0000000000..f8f3d72c23
--- /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 c4a9db2df0..316d6a433a 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))