From: John Mulligan <jmulligan@redhat.com>
Date: Mon, 4 Dec 2023 18:20:02 +0000 (-0500)
Subject: cephadm: simplify loading jinja2 templates from zipapp
X-Git-Tag: v19.3.0~420^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F54773%2Fhead;p=ceph.git

cephadm: simplify loading jinja2 templates from zipapp

The saga of failures in teuthology brought on by the combination of
zipapp and jinja2 template loading continues in this episode. Remove
some of the path handling that was taken from the original jinja2
implementation and try to replace it with something inherently simpler
such that we always construct paths relative to the zip archive and pass
that to the zipimporter. The hope here is that by doing fewer
manipulations of the "path" we are more likely to match the items
tracked inside the zipimporter class.

We do not worry about some of the concerns that regular jinja2 has as we
never expect cephadm to be run on something other than linux and we have
control over what sources we expect to load templates from.  We do take
one precaution and that is to reject any paths that contain "." and ".."
as components. Less to avoid malicious attempts to read files it should
not (but that would be nice) and more to nudge toward simple template
references recorded in our Templates enum.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
---

diff --git a/src/cephadm/cephadmlib/templating.py b/src/cephadm/cephadmlib/templating.py
index ceef32ff9fed..3e852b2d397d 100644
--- a/src/cephadm/cephadmlib/templating.py
+++ b/src/cephadm/cephadmlib/templating.py
@@ -34,27 +34,18 @@ class TemplateNotFoundInZipApp(jinja2.TemplateNotFound):
         self,
         template: str,
         *,
-        path: str = '',
         relative_path: str = '',
-        archive_norm_path: str = '',
-        archive_path: str = ''
+        archive_path: str = '',
     ) -> None:
         super().__init__(template)
-        self.path = path
         self.relative_path = relative_path
-        self.archive_norm_path = archive_norm_path
         self.archive_path = archive_path
 
     def __str__(self) -> str:
-        msg = self.message
-        msg2 = ''
-        if self.path or self.relative_path:
-            msg2 += f' path [{self.path!r}, rel={self.relative_path!r}] not found'
-        if self.archive_norm_path or self.archive_path:
-            msg2 += f' in [{self.archive_norm_path!r}, orig={self.archive_path!r}]'
-        if msg2:
-            msg2 = ':' + msg2
-        return f'{msg}{msg2}'
+        return (
+            f'{self.message}: path {self.relative_path!r}'
+            f' not found in {self.archive_path!r}'
+        )
 
 
 class _PackageLoader(jinja2.PackageLoader):
@@ -86,27 +77,23 @@ class _PackageLoader(jinja2.PackageLoader):
 
     def _get_archive_source(self, template: str) -> Tuple[str, str, None]:
         assert isinstance(self._loader, zipimport.zipimporter)
-        path = arelpath = os.path.normpath(
-            posixpath.join(
-                self._template_root,
-                *jinja2.loaders.split_template_path(template)
-            )
+        arelpath = posixpath.join(
+            self.package_name, self.package_path, template
         )
-        archive_path = os.path.normpath(self._loader.archive)
-        if arelpath.startswith(archive_path + '/'):
-            plen = len(archive_path) + 1
-            arelpath = arelpath[plen:]
+        if any(p == '.' or p == '..' for p in arelpath.split(posixpath.sep)):
+            raise ValueError('template path contains invalid components')
         try:
             source = cast(bytes, self._loader.get_data(arelpath))
         except OSError as e:
             not_found = TemplateNotFoundInZipApp(
                 template,
-                path=path,
                 relative_path=arelpath,
-                archive_norm_path=archive_path,
                 archive_path=self._loader.archive,
             )
             raise not_found from e
+        path = os.path.normpath(
+            posixpath.join(self._loader.archive, arelpath)
+        )
         return source.decode(self.encoding), path, None