Merge pull request #44518 from twangboy/win_fix_verify_env

Pass root_dir to the win_verify_env function

salt/cli/daemons.py

@@ -160,6 +160,7 @@ class Master(parsers.MasterOptionParser, DaemonsMixin):  # pylint: disable=no-in
                     self.config['user'],
                     permissive=self.config['permissive_pki_access'],
                     pki_dir=self.config['pki_dir'],
+                    root_dir=self.config['root_dir'],
                 )
                 # Clear out syndics from cachedir
                 for syndic_file in os.listdir(self.config['syndic_dir']):
@@ -280,6 +281,7 @@ class Minion(parsers.MinionOptionParser, DaemonsMixin):  # pylint: disable=no-in
                     self.config['user'],
                     permissive=self.config['permissive_pki_access'],
                     pki_dir=self.config['pki_dir'],
+                    root_dir=self.config['root_dir'],
                 )
         except OSError as error:
             self.environment_failure(error)
@@ -467,6 +469,7 @@ class ProxyMinion(parsers.ProxyMinionOptionParser, DaemonsMixin):  # pylint: dis
                     self.config['user'],
                     permissive=self.config['permissive_pki_access'],
                     pki_dir=self.config['pki_dir'],
+                    root_dir=self.config['root_dir'],
                 )
         except OSError as error:
             self.environment_failure(error)
@@ -575,6 +578,7 @@ class Syndic(parsers.SyndicOptionParser, DaemonsMixin):  # pylint: disable=no-in
                     self.config['user'],
                     permissive=self.config['permissive_pki_access'],
                     pki_dir=self.config['pki_dir'],
+                    root_dir=self.config['root_dir'],
                 )
         except OSError as error:
             self.environment_failure(error)

salt/cli/spm.py

@@ -32,7 +32,10 @@ class SPM(parsers.SPMParser):
         v_dirs = [
             self.config['cachedir'],
         ]
-        verify_env(v_dirs, self.config['user'],)
+        verify_env(v_dirs,
+                   self.config['user'],
+                   root_dir=self.config['root_dir'],
+                   )
         verify_log(self.config)
         client = salt.spm.SPMClient(ui, self.config)
         client.run(self.args)

salt/cloud/cli.py

@@ -66,7 +66,8 @@ class SaltCloud(parsers.SaltCloudParser):
             if self.config['verify_env']:
                 verify_env(
                     [os.path.dirname(self.config['conf_file'])],
-                    salt_master_user
+                    salt_master_user,
+                    root_dir=self.config['root_dir'],
                 )
                 logfile = self.config['log_file']
                 if logfile is not None and not logfile.startswith('tcp://') \

salt/modules/win_pkg.py

@@ -52,6 +52,7 @@ from salt.exceptions import (CommandExecutionError,
                              SaltRenderError)
 import salt.utils
 import salt.utils.pkg
+import salt.utils.path
 import salt.syspaths
 import salt.payload
 from salt.exceptions import MinionError
@@ -641,33 +642,10 @@ def _get_repo_details(saltenv):
         # Do some safety checks on the repo_path as its contents can be removed,
         # this includes check for bad coding
         system_root = os.environ.get('SystemRoot', r'C:\Windows')
-        deny_paths = (
-            r'[a-z]\:\\$',  # C:\, D:\, etc
-            r'\\$',  # \
-            re.escape(system_root)  # C:\Windows
-        )
+        if not salt.utils.path.safe_path(
+                path=local_dest,
+                allow_path='\\'.join([system_root, 'TEMP'])):
 
-        # Since the above checks anything in C:\Windows, there are some
-        # directories we may want to make exceptions for
-        allow_paths = (
-            re.escape('\\'.join([system_root, 'TEMP'])),  # C:\Windows\TEMP
-        )
-
-        # Check the local_dest to make sure it's not one of the bad paths
-        good_path = True
-        for d_path in deny_paths:
-            if re.match(d_path, local_dest, flags=re.IGNORECASE) is not None:
-                # Found deny path
-                good_path = False
-
-        # If local_dest is one of the bad paths, check for exceptions
-        if not good_path:
-            for a_path in allow_paths:
-                if re.match(a_path, local_dest, flags=re.IGNORECASE) is not None:
-                    # Found exception
-                    good_path = True
-
-        if not good_path:
             raise CommandExecutionError(
                 'Attempting to delete files from a possibly unsafe location: '
                 '{0}'.format(local_dest)

salt/utils/path.py

@@ -174,3 +174,60 @@ def _get_reparse_data(path):
             win32file.CloseHandle(fileHandle)
 
     return reparseData
+
+
+def safe_path(path, allow_path=None):
+    r'''
+    .. versionadded:: 2017.7.3
+
+    Checks that the path is safe for modification by Salt. For example, you
+    wouldn't want to have salt delete the contents of ``C:\Windows``. The
+    following directories are considered unsafe:
+
+    - C:\, D:\, E:\, etc.
+    - \
+    - C:\Windows
+
+    Args:
+
+        path (str): The path to check
+
+        allow_paths (str, list): A directory or list of directories inside of
+            path that may be safe. For example: ``C:\Windows\TEMP``
+
+    Returns:
+        bool: True if safe, otherwise False
+    '''
+    # Create regex definitions for directories that may be unsafe to modify
+    system_root = os.environ.get('SystemRoot', 'C:\\Windows')
+    deny_paths = (
+        r'[a-z]\:\\$',  # C:\, D:\, etc
+        r'\\$',  # \
+        re.escape(system_root)  # C:\Windows
+    )
+
+    # Make allow_path a list
+    if allow_path and not isinstance(allow_path, list):
+        allow_path = [allow_path]
+
+    # Create regex definition for directories we may want to make exceptions for
+    allow_paths = list()
+    if allow_path:
+        for item in allow_path:
+            allow_paths.append(re.escape(item))
+
+    # Check the path to make sure it's not one of the bad paths
+    good_path = True
+    for d_path in deny_paths:
+        if re.match(d_path, path, flags=re.IGNORECASE) is not None:
+            # Found deny path
+            good_path = False
+
+    # If local_dest is one of the bad paths, check for exceptions
+    if not good_path:
+        for a_path in allow_paths:
+            if re.match(a_path, path, flags=re.IGNORECASE) is not None:
+                # Found exception
+                good_path = True
+
+    return good_path

salt/utils/verify.py

@@ -31,6 +31,8 @@ import salt.utils
 
 log = logging.getLogger(__name__)
 
+ROOT_DIR = 'c:\\salt' if salt.utils.is_windows() else '/'
+
 
 def zmq_version():
     '''
@@ -192,13 +194,13 @@ def verify_files(files, user):
     return True
 
 
-def verify_env(dirs, user, permissive=False, pki_dir='', skip_extra=False):
+def verify_env(dirs, user, permissive=False, pki_dir='', skip_extra=False, root_dir=ROOT_DIR):
     '''
     Verify that the named directories are in place and that the environment
     can shake the salt
     '''
     if salt.utils.is_windows():
-        return win_verify_env(dirs, permissive, pki_dir, skip_extra)
+        return win_verify_env(root_dir, dirs, permissive, pki_dir, skip_extra)
     import pwd  # after confirming not running Windows
     try:
         pwnam = pwd.getpwnam(user)
@@ -523,18 +525,21 @@ def verify_log(opts):
         log.warning('Insecure logging configuration detected! Sensitive data may be logged.')
 
 
-def win_verify_env(dirs, permissive=False, pki_dir='', skip_extra=False):
+def win_verify_env(path, dirs, permissive=False, pki_dir='', skip_extra=False):
     '''
     Verify that the named directories are in place and that the environment
     can shake the salt
     '''
     import salt.utils.win_functions
     import salt.utils.win_dacl
+    import salt.utils.path
 
-    # Get the root path directory where salt is installed
-    path = dirs[0]
-    while os.path.basename(path) not in ['salt', 'salt-tests-tmpdir']:
-        path, base = os.path.split(path)
+    # Make sure the file_roots is not set to something unsafe since permissions
+    # on that directory are reset
+    if not salt.utils.path.safe_path(path=path):
+        raise CommandExecutionError(
+            '`file_roots` set to a possibly unsafe location: {0}'.format(path)
+        )
 
     # Create the root path directory if missing
     if not os.path.isdir(path):

tests/integration/__init__.py

@@ -983,7 +983,9 @@ class TestDaemon(object):
                     RUNTIME_VARS.TMP_PRODENV_STATE_TREE,
                     TMP,
                     ],
-                   RUNTIME_VARS.RUNNING_TESTS_USER)
+                   RUNTIME_VARS.RUNNING_TESTS_USER,
+                   root_dir=master_opts['root_dir'],
+                   )
 
         cls.master_opts = master_opts
         cls.minion_opts = minion_opts

tests/support/mixins.py

@@ -107,7 +107,9 @@ class AdaptedConfigurationTestCaseMixin(object):
                     rdict['sock_dir'],
                     conf_dir
                    ],
-                   RUNTIME_VARS.RUNNING_TESTS_USER)
+                   RUNTIME_VARS.RUNNING_TESTS_USER,
+                   root_dir=rdict['root_dir'],
+                   )
 
         rdict['config_dir'] = conf_dir
         rdict['conf_file'] = os.path.join(conf_dir, config_for)

tests/unit/utils/test_verify.py

@@ -111,7 +111,7 @@ class TestVerify(TestCase):
     def test_verify_env(self):
         root_dir = tempfile.mkdtemp(dir=TMP)
         var_dir = os.path.join(root_dir, 'var', 'log', 'salt')
-        verify_env([var_dir], getpass.getuser())
+        verify_env([var_dir], getpass.getuser(), root_dir=root_dir)
         self.assertTrue(os.path.exists(var_dir))
         dir_stat = os.stat(var_dir)
         self.assertEqual(dir_stat.st_uid, os.getuid())