Rework _check_relpath() method

sechkova · sechkova · commit 9eef0bd14370 · 2020-04-03T16:55:56.000+03:00
Adopt a stict behavor allowing only paths that match
the definition of a PATHPATTERN or aTARGETPATH (use the forward
slash (/) as directory separator and do not start with a
directory separator). Raise an exception otherwise.

Rename the method to a more general  _check_path().

Signed-off-by: Teodora Sechkova &lt;tsechkova@vmware.com&gt;
diff --git a/tuf/repository_tool.py b/tuf/repository_tool.py
@@ -1854,8 +1854,7 @@ def add_paths(self, paths, child_rolename):
       securesystemslib.exceptions.Error, if 'child_rolename' has not been
       delegated yet.
 
-      tuf.exceptions.InvalidNameError, if any path in 'paths' starts with
-      a directory separator.
+      tuf.exceptions.InvalidNameError, if 'pathname' does not match pattern.
 
     <Side Effects>
       Modifies this Targets' delegations field.
@@ -1882,11 +1881,11 @@ def add_paths(self, paths, child_rolename):
           ' not exist.')
 
     for path in paths:
-      # Check if the delegated paths or glob patterns are relative and
-      # normalize them. Paths' existence on the file system is not verified.
-      # If the path is incorrect, the targetfile won't be matched successfully
-      # during a client update.
-      path = self._check_relpath(path)
+      # Check if the delegated paths or glob patterns are relative and use
+      # forward slash as a separator or raise an exception. Paths' existence
+      # on the file system is not verified. If the path is incorrect,
+      # the targetfile won't be matched successfully during a client update.
+      self._check_path(path)
       relative_paths.append(path)
 
     # Get the current role's roleinfo, so that its delegations field can be
@@ -1945,8 +1944,7 @@ def add_target(self, filepath, custom=None, fileinfo=None):
       securesystemslib.exceptions.FormatError, if 'filepath' is improperly
       formatted.
 
-      tuf.exceptions.InvalidNameError, if 'filepath' is not relative (starts
-      with a directory separator).
+      tuf.exceptions.InvalidNameError, if 'pathname' does not match pattern.
 
     <Side Effects>
       Adds 'filepath' to this role's list of targets.  This role's
@@ -1977,28 +1975,29 @@ def add_target(self, filepath, custom=None, fileinfo=None):
     # Add 'filepath' (i.e., relative to the targets directory) to the role's
     # list of targets.  'filepath' will not be verified as an allowed path
     # according to some delegating role.  Not verifying 'filepath' here allows
-    # freedom to add targets and parent restrictions in any order, minimize the
-    # number of times these checks are performed, and allow any role to
+    # freedom to add targets and parent restrictions in any order, minimize
+    # the number of times these checks are performed, and allow any role to
     # delegate trust of packages to this Targets role.
 
-    # Check if the target path is relative and normalize it. File's existence
-    # on the file system is not verified. If the file does not exist relative
-    # to the targets directory, later calls to write() will fail.
-    relative_path = self._check_relpath(filepath)
+    # Check if the target is relative and uses forward slash as a separator
+    # or raise an exception. File's existence on the file system is not
+    # verified. If the file does not exist relative to the targets directory,
+    # later calls to write() will fail.
+    self._check_path(filepath)
 
     # Update the role's 'tuf.roledb.py' entry and avoid duplicates.
     roleinfo = tuf.roledb.get_roleinfo(self._rolename, self._repository_name)
 
-    if relative_path not in roleinfo['paths']:
-      logger.debug('Adding new target: ' + repr(relative_path))
+    if filepath not in roleinfo['paths']:
+      logger.debug('Adding new target: ' + repr(filepath))
 
     else:
-      logger.debug('Replacing target: ' + repr(relative_path))
+      logger.debug('Replacing target: ' + repr(filepath))
 
     if fileinfo:
-      roleinfo['paths'].update({relative_path: fileinfo})
+      roleinfo['paths'].update({filepath: fileinfo})
     else:
-      roleinfo['paths'].update({relative_path: {'custom': custom}})
+      roleinfo['paths'].update({filepath: {'custom': custom}})
 
     tuf.roledb.update_roleinfo(self._rolename, roleinfo,
         repository_name=self._repository_name)
@@ -2025,8 +2024,7 @@ def add_targets(self, list_of_targets):
       securesystemslib.exceptions.FormatError, if the arguments are improperly
       formatted.
 
-      tuf.exceptions.InvalidNameError, if any target in 'list_of_targets'
-      is not relative (starts with a directory separator).
+      tuf.exceptions.InvalidNameError, if 'pathname' does not match pattern.
 
     <Side Effects>
       This Targets' roleinfo is updated with the paths in 'list_of_targets'.
@@ -2044,14 +2042,15 @@ def add_targets(self, list_of_targets):
     # Update the tuf.roledb entry.
     relative_list_of_targets = []
 
-    # Ensure the paths in 'list_of_targets' are valid and are located in the
-    # repository's targets directory.  The paths of 'list_of_targets' will be
-    # verified as allowed paths according to this Targets parent role when
-    # write() or writeall() is called.  Not verifying filepaths here allows the
-    # freedom to add targets and parent restrictions in any order, and minimize
-    # the number of times these checks are performed.
+    # Ensure the paths in 'list_of_targets' are relative and use forward slash
+    # as a separator or raise an exception. The paths of 'list_of_targets'
+    # will be verified as existing and allowed paths according to this Targets
+    # parent role when write() or writeall() is called.  Not verifying
+    # filepaths here allows the freedom to add targets and parent restrictions
+    # in any order and minimize the number of times these checks are performed.
     for target in list_of_targets:
-      relative_list_of_targets.append(self._check_relpath(target))
+      self._check_path(target)
+      relative_list_of_targets.append(target)
 
     # Update this Targets 'tuf.roledb.py' entry.
     roleinfo = tuf.roledb.get_roleinfo(self._rolename, self._repository_name)
@@ -2241,9 +2240,7 @@ def delegate(self, rolename, public_keys, paths, threshold=1,
 
       securesystemslib.exceptions.Error, if the delegated role already exists.
 
-      tuf.exceptions.InvalidNameError, if any path in 'paths' or any
-      target in 'list_of_targets' is not relative (starts with a directory
-      separator).
+      tuf.exceptions.InvalidNameError, if 'pathname' does not match pattern.
 
     <Side Effects>
       A new Target object is created for 'rolename' that is accessible to the
@@ -2292,24 +2289,19 @@ def delegate(self, rolename, public_keys, paths, threshold=1,
 
     if list_of_targets:
       for target in list_of_targets:
-        # Check if the target path is relative and normalize it. File's
+        # Check if the target path is relative or raise an exception. File's
         # existence on the file system is not verified. If the file does not
         # exist relative to the targets directory, later calls to write()
         # will fail.
-        rel_targetpath = self._check_relpath(target)
-        relative_targetpaths.update({rel_targetpath: {}})
-
-    # A list of relative and verified paths or glob patterns to be added to the
-    # child role's entry in the parent's delegations field.
-    relative_paths = []
+        self._check_path(target)
+        relative_targetpaths.update({target: {}})
 
     for path in paths:
-      # Check if the delegated paths or glob patterns are relative and
-      # normalize them. Paths' existense on the file system is not verified.
-      # If the path is incorrect, the targetfile won't be matched successfully
-      # during a client update.
-      path = self._check_relpath(path)
-      relative_paths.append(path)
+      # Check if the delegated paths or glob patterns are relative or
+      # raise an exception. Paths' existence on the file system is not
+      # verified. If the path is incorrect, the targetfile won't be matched
+      # successfully during a client update.
+      self._check_path(path)
 
     # Create a new Targets object for the 'rolename' delegation.  An initial
     # expiration is set (3 months from the current time).
@@ -2340,8 +2332,8 @@ def delegate(self, rolename, public_keys, paths, threshold=1,
                 'terminating': terminating,
                 'paths': list(roleinfo['paths'].keys())}
 
-    if relative_paths:
-      roleinfo['paths'] = relative_paths
+    if paths:
+      roleinfo['paths'] = paths
 
     if path_hash_prefixes:
       roleinfo['path_hash_prefixes'] = path_hash_prefixes
@@ -2485,8 +2477,7 @@ def delegate_hashed_bins(self, list_of_targets, keys_of_hashed_bins,
       2, or one of the targets in 'list_of_targets' is not relative to the
       repository's targets directory.
 
-      tuf.exceptions.InvalidNameError, if any target in 'list_of_targets'
-      is not relative (starts with a directory separator).
+      tuf.exceptions.InvalidNameError, if 'pathname' does not match pattern.
 
     <Side Effects>
       Delegates multiple target roles from the current parent role.
@@ -2535,7 +2526,7 @@ def delegate_hashed_bins(self, list_of_targets, keys_of_hashed_bins,
       # Check if the target path is relative and normalize it. File's existence
       # on the file system is not verified. If the file does not exist relative
       # to the targets directory, later calls to write() will fail.
-      target_path = self._check_relpath(target_path)
+      self._check_path(target_path)
 
       # Determine the hash prefix of 'target_path' by computing the digest of
       # its path relative to the targets directory.
@@ -2706,7 +2697,7 @@ def delegations(self):
 
 
 
-  def _check_relpath(self, pathname):
+  def _check_path(self, pathname):
     """
     <Purpose>
       Check if a path matches the definition of a PATHPATTERN or a
@@ -2722,44 +2713,23 @@ def _check_relpath(self, pathname):
       securesystemslib.exceptions.FormatError, if 'pathname' is improperly
       formatted.
 
-      tuf.exceptions.InvalidNameError, if 'pathname' starts with a directory
-      separator.
-
-    <Side Effects>
-      Normalizes pathname.
+      tuf.exceptions.InvalidNameError, if 'pathname' does not match pattern.
 
     <Returns>
-        The normazlied 'pathname'.
+      None.
     """
 
     tuf.formats.RELPATH_SCHEMA.check_match(pathname)
 
-    if os.path.isabs(pathname):
-      raise tuf.exceptions.InvalidNameError(repr(pathname) + ' contains a leading'
-          ' path separator. All paths should be relative to the repository\'s'
-          ' targets directory.')
-
-    # Trigger a warning when the path contains '..' since directories
-    # upper than the current cannot be resolved by normpath.
-    dir_list = pathname.split(os.sep)
-    for dirname in dir_list:
-      if dirname == '..':
-        logger.warning('Path ' + repr(pathname) + ' contains \'..\'.'
-            ' Parent directory may not be resolved.')
-        break
-
-    pathname = os.path.normpath(pathname)
-
-    # Trigger a warning in case path name is not passed as a relative
-    # to the targets directory but starts with it instead. Ensure a trailing
-    # path separator with os.path.join(path, '').
-    targets_directory = os.path.join(self._targets_directory, '')
-    if pathname.startswith(targets_directory):
-      logger.warning(repr(pathname) + ' should not include the'
-          ' repository\'s targets'
-          ' directory: ' + repr(self._targets_directory))
-
-    return pathname
+    if '\\' in pathname:
+      raise tuf.exceptions.InvalidNameError('Path ' + repr(pathname)
+          + ' does not use the forward slash (/) as directory separator.')
+
+    if pathname.startswith(os.sep):
+      raise tuf.exceptions.InvalidNameError('Path ' + repr(pathname)
+          + ' starts with a directory separator. All paths should be relative'
+          '  to targets directory.')
+
 
 
 
