From 630b35aba7cbad23b0b28f4b80b2a0feb6b011a4 Mon Sep 17 00:00:00 2001
From: Martin Vrachev <mvrachev@vmware.com>
Date: Thu, 29 Jul 2021 17:08:17 +0300
Subject: [PATCH] Implement glob-like pattern matching

According to the recently updated version of the specification the shell
style wildcard matching is glob-like (see https://github.com/theupdateframework/specification/pull/174),
and therefore a path separator in a path should not be matched by a
wildcard in the PATHPATTERN.

That's not what happens with `fnmatch.fnmatch()` which doesn't
see "/" separator as a special symbol.
For example: fnmatch.fnmatch("targets/foo.tgz", "*.tgz") will return
True which is not what glob-like implementation will do.

That's why before using `fnmatch.fnmatch()` we should manually check
that "targetname" and "pathpattern" are pointing to the same directory.

Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
---
 tests/test_updater_ng.py | 27 +++++++++++++++++++++++++++
 tuf/ngclient/updater.py  | 33 +++++++++++++++++++++++++++++++--
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/tests/test_updater_ng.py b/tests/test_updater_ng.py
index eec10d73cb..38d01bb287 100644
--- a/tests/test_updater_ng.py
+++ b/tests/test_updater_ng.py
@@ -16,6 +16,7 @@
 
 from tests import utils
 from tuf import ngclient
+from tuf.ngclient.updater import _is_target_in_pathpattern
 
 logger = logging.getLogger(__name__)
 
@@ -148,6 +149,32 @@ def test_refresh_with_only_local_root(self):
     # Get targetinfo for 'file3.txt' listed in the delegated role1
     targetinfo3= self.repository_updater.get_one_valid_targetinfo('file3.txt')
 
+
+  def test_is_target_in_pathpattern(self):
+    supported_use_cases = [
+      ("foo.tgz", "foo.tgz"),
+      ("foo.tgz", "*"),
+      ("foo.tgz", "*.tgz"),
+      ("foo-version-a.tgz", "foo-version-?.tgz"),
+      ("targets/foo.tgz", "targets/*.tgz"),
+      ("foo/bar/zoo/k.tgz", "foo/bar/zoo/*"),
+      ("foo/bar/zoo/k.tgz", "foo/*/zoo/*"),
+      ("foo/bar/zoo/k.tgz", "*/*/*/*")
+    ]
+    for targetname, pathpattern in supported_use_cases:
+      self.assertTrue(_is_target_in_pathpattern(targetname, pathpattern))
+
+    invalid_use_cases = [
+      ("targets/foo.tgz", "*.tgz"),
+      ("/foo.tgz", "*.tgz",),
+      ("targets/foo.tgz", "*"),
+      ("foo/bar/k.tgz", "foo/bar/zoo/*"),
+      ("foo/bar/zoo/k.tgz", "foo/bar/*"),
+      ("foo-version-alpha.tgz", "foo-version-?.tgz")
+    ]
+    for pathpattern, targetname in invalid_use_cases:
+      self.assertFalse(_is_target_in_pathpattern(pathpattern, targetname))
+
 if __name__ == '__main__':
   utils.configure_test_logging(sys.argv)
   unittest.main()
diff --git a/tuf/ngclient/updater.py b/tuf/ngclient/updater.py
index 850f46b9cf..41e9ded730 100644
--- a/tuf/ngclient/updater.py
+++ b/tuf/ngclient/updater.py
@@ -463,6 +463,34 @@ def _preorder_depth_first_walk(self, target_filepath) -> Dict:
         return {"filepath": target_filepath, "fileinfo": target}
 
 
+def _is_target_in_pathpattern(targetname: str, pathpattern: str) -> bool:
+    """Determines whether "targetname" matches the "pathpattern"."""
+    # We need to make sure that targetname and pathpattern are pointing to the
+    # same directory as fnmatch doesn't threat "/" as a special symbol. Example:
+    # fnmatch.fnmatch("targets/foo.tgz", "*.tgz") will return True.
+    target_dirs = os.path.dirname(targetname).split("/")
+    pattern_dirs = os.path.dirname(pathpattern).split("/")
+    if len(target_dirs) != len(pattern_dirs):
+        # Difference in the number of nested dirs means that target_dir and
+        # pathpattern_dir point to different places.
+        return False
+
+    in_the_same_dir = True
+    for index, pattern_dir in enumerate(pattern_dirs):
+        if target_dirs[index] == pattern_dir:
+            in_the_same_dir = True
+        elif pattern_dir == "*":
+            in_the_same_dir = True
+        else:
+            in_the_same_dir = False
+            break
+
+    if in_the_same_dir:
+        return fnmatch.fnmatch(targetname, pathpattern)
+
+    return False
+
+
 def _visit_child_role(child_role: Dict, target_filepath: str) -> str:
     """
     <Purpose>
@@ -527,8 +555,9 @@ def _visit_child_role(child_role: Dict, target_filepath: str) -> str:
             # target without a leading path separator - make sure to strip any
             # leading path separators so that a match is made.
             # Example: "foo.tgz" should match with "/*.tgz".
-            if fnmatch.fnmatch(
-                target_filepath.lstrip(os.sep), child_role_path.lstrip(os.sep)
+            if _is_target_in_pathpattern(
+                target_filepath.lstrip(os.sep),
+                child_role_path.lstrip(os.sep),
             ):
                 logger.debug(
                     "Child role %s is allowed to sign for %s",