Only print warning when ansible.cfg is actually skipped (#43583)

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

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.

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 <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
 ==============
 

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 warnings is None:
-    if path0 is not None:
+        # Note: In this case, warnings does nothing
-        path0 = unfrackpath(path0, follow=False)
+        warnings = set()
-        if os.path.isdir(path0):
-            path0 += "/ansible.cfg"
-    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
-        else:
-            path1 += "/ansible.cfg"
-    except OSError:
-        path1 = None
-    path2 = unfrackpath("~/.ansible.cfg", follow=False)
-    path3 = "/etc/ansible/ansible.cfg"
 
-    for path in [path0, path1, path2, path3]:
+    # A value that can never be a valid path so that we can tell if ANSIBLE_CONFIG was set later
-        if path is not None and os.path.exists(path):
+    # 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:
+        cwd = os.getcwd()
+        perms = os.stat(cwd)
+        if perms.st_mode & stat.S_IWOTH:
+            warn_cmd_public = True
+        else:
+            potential_paths.append(os.path.join(cwd, "ansible.cfg"))
+    except OSError:
+        # 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))
+
+    # 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
 
 

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

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))