[python] Fix measuring absolute paths in chroots

This fixes, among others, symlinks to `/etc/alternatives` in Debian- and
Ubuntu-based images. While most symlinks in well-behaved chroots are
relative, some of them might be absolute and this case needs to be
handled.

Reported-by: Bartłomiej Sulich <[email protected]>
Signed-off-by: Wojtek Porczyk <[email protected]>

python/graminelibos/manifest.py

@@ -10,9 +10,11 @@
 Gramine manifest management and rendering
 """
 
+import errno
 import hashlib
 import os
 import pathlib
+import posixpath
 
 import tomli
 import tomli_w
@@ -35,6 +37,82 @@ def uri2path(uri):
     return pathlib.Path(uri[len('file:'):])
 
 
+# loosely based on posixpath._joinrealpath
+def resolve_symlinks(path, *, chroot, seen=None):
+    """Resolve symlink inside chroot
+
+    Args:
+        path (pathlib.Path or str): the path to resolve
+        chroot (pathlib.Path): path to chroot
+
+    Raises:
+        OSError: When resolution fails. The following variants can be raised: ``ENOTDIR`` aka
+            :py:class:`NotADirectoryError` for paths like ``a/b/file/c``; ``ELOOP`` for loops.
+    """
+    path = pathlib.Path(path)
+    if not path.is_absolute():
+        raise ManifestError('only absolute paths can be measured in chroot')
+
+    if seen is None:
+        # a mapping of linksrc -> linkdest (all within chroot), but linkdest values can be None
+        # while recursing, and if None is encountered, then we'll know we have a loop
+        seen = {}
+
+    # Current state (what we already resolved). This is a path that is:
+    # - an instance of pathlib.Path;
+    # - absolute (starts with '/');
+    # - already resolved path (contains no symlinks);
+    # - inside chroot (outer_current_path is this path as seen from outside).
+    # Therefore it's safe to traverse '..' in inner_current_path by just taking .parent attribute.
+    inner_current_path = pathlib.Path('/')
+    outer_current_path = chroot / inner_current_path.relative_to('/')
+
+    for part in path.relative_to('/').parts:
+        if not outer_current_path.is_dir():
+            raise NotADirectoryError(errno.ENOTDIR, os.strerror(errno.ENOTDIR), inner_current_path)
+
+        if part == posixpath.curdir: # '.'
+            continue
+
+        if part == posixpath.pardir: # '..'
+            inner_current_path = inner_current_path.parent # this works also for /, just returns /
+            outer_current_path = chroot / inner_current_path.relative_to('/')
+            continue
+
+        inner_current_path /= part
+        outer_current_path = chroot / inner_current_path.relative_to('/')
+
+        if not outer_current_path.is_symlink():
+            continue
+
+        # else: here's the hard part, symlink resolution
+
+        if inner_current_path not in seen:
+            seen[inner_current_path] = None
+            # TODO after python >= 3.9: use Path.readlink()
+            next_path = pathlib.Path(os.readlink(outer_current_path))
+
+            # XXX(woju 12.12.2023): The following path concatenation is suboptimal, it will cause
+            # the recurring function to traverse and stat() all parts of inner_current_path again,
+            # so it's easy to construct exploding O(n²) tree. However, to write this optimally, it
+            # would require to complicate already convoluted logic. Trees that would trigger
+            # suboptimal complexity are uncommon, so I think it's a reasonable tradeoff.
+            if not next_path.is_absolute():
+                next_path = inner_current_path.parent / next_path
+
+            seen[inner_current_path] = resolve_symlinks(next_path, chroot=chroot, seen=seen)
+
+        if seen[inner_current_path] is None:
+            # we have a loop in symlinks
+            raise OSError(errno.ELOOP, os.strerror(errno.ELOOP), inner_current_path)
+
+        inner_current_path = seen[inner_current_path]
+        outer_current_path = chroot / inner_current_path.relative_to('/')
+        continue
+
+    return inner_current_path
+
+
 class TrustedFile:
     """Represents a single entry in sgx.trusted_files.
 
@@ -63,9 +141,7 @@ def __init__(self, uri, sha256=None, *, chroot=None):
         if self.chroot is None:
             self.realpath = pathlib.Path(path)
         else:
-            if not path.is_absolute():
-                raise ManifestError('only absolute paths can be measured in chroot')
-            self.realpath = self.chroot / path.relative_to('/')
+            self.realpath = chroot / resolve_symlinks(path, chroot=self.chroot).relative_to('/')
 
     @classmethod
     def from_manifest(cls, data, *, chroot=None):
@@ -116,7 +192,6 @@ def from_realpath(cls, realpath, *, chroot=None):
             # path.relative_to(chroot) will throw ValueError if the path is not relative to chroot
             path = '/' / path.relative_to(chroot)
         self = cls(f'file:{path}{"/" if realpath.is_dir() else ""}', chroot=chroot)
-        assert self.realpath == realpath
         return self
 
     def __repr__(self):

tests/test_chroot.py

@@ -0,0 +1,99 @@
+import pytest
+from graminelibos import manifest
+
+
+# TODO: use tmp_path after deprecating *EL8
+if tuple(int(i) for i in pytest.__version__.split('.')[:2]) < (3, 9):
+    import pathlib
+    @pytest.fixture
+    def tmp_path(tmpdir):
+        return pathlib.Path(tmpdir)
+
+@pytest.fixture
+def read_resolved_path(tmp_path):
+    def read_resolved_path(path):
+        inner_path = manifest.resolve_symlinks(path, chroot=tmp_path)
+        outer_path = (tmp_path / inner_path.relative_to('/'))
+        return outer_path.read_text()
+    return read_resolved_path
+
+
+def test_file_relative_symlink_1(tmp_path, read_resolved_path):
+    (tmp_path / 'target').write_text('pass')
+    (tmp_path / 'symlink').symlink_to('target')
+    assert read_resolved_path('/symlink') == 'pass'
+
+def test_file_relative_symlink_2(tmp_path, read_resolved_path):
+    (tmp_path / 'subdir').mkdir()
+    (tmp_path / 'subdir/target').write_text('pass')
+    (tmp_path / 'subdir/symlink').symlink_to('target')
+    assert read_resolved_path('/subdir/symlink') == 'pass'
+
+def test_file_relative_symlink_3(tmp_path, read_resolved_path):
+    (tmp_path / 'subdir').mkdir()
+    (tmp_path / 'target').write_text('pass')
+    (tmp_path / 'subdir/symlink').symlink_to('../target')
+    assert read_resolved_path('/subdir/symlink') == 'pass'
+
+
+def test_file_absolute_symlink_1(tmp_path, read_resolved_path):
+    (tmp_path / 'target').write_text('pass')
+    (tmp_path / 'symlink').symlink_to('/target')
+    assert read_resolved_path('/symlink') == 'pass'
+
+def test_file_absolute_symlink_2(tmp_path, read_resolved_path):
+    (tmp_path / 'subdir').mkdir()
+    (tmp_path / 'subdir/target').write_text('pass')
+    (tmp_path / 'subdir/symlink').symlink_to('/subdir/target')
+    assert read_resolved_path('/subdir/symlink') == 'pass'
+
+def test_file_absolute_symlink_3(tmp_path, read_resolved_path):
+    (tmp_path / 'subdir').mkdir()
+    (tmp_path / 'target').write_text('pass')
+    (tmp_path / 'subdir/symlink').symlink_to('/target')
+    assert read_resolved_path('/subdir/symlink') == 'pass'
+
+
+def test_directory_relative_symlink_1(tmp_path, read_resolved_path):
+    (tmp_path / 'subdir').mkdir()
+    (tmp_path / 'subdir/target').write_text('pass')
+    (tmp_path / 'symlink').symlink_to('subdir')
+    assert read_resolved_path('/symlink/target') == 'pass'
+
+
+def test_bump_parent_against_root(tmp_path, read_resolved_path):
+    (tmp_path / 'target').write_text('pass')
+    (tmp_path / 'symlink').symlink_to('../../../target')
+
+    assert read_resolved_path('/symlink') == 'pass'
+
+
+def test_eloop_1(tmp_path):
+    (tmp_path / 'symlink').symlink_to('symlink')
+    with pytest.raises(OSError, match=r'\[Errno 40\] Too many levels of symbolic links'):
+        manifest.resolve_symlinks('/symlink', chroot=tmp_path)
+
+def test_eloop_2(tmp_path):
+    (tmp_path / 'symlink').symlink_to('symlink')
+    with pytest.raises(OSError, match=r'\[Errno 40\] Too many levels of symbolic links'):
+        manifest.resolve_symlinks('/symlink/target', chroot=tmp_path)
+
+
+def test_enotdir_1(tmp_path):
+    (tmp_path / 'target').write_text('pass')
+    with pytest.raises(NotADirectoryError):
+        manifest.resolve_symlinks('/target/subdir', chroot=tmp_path)
+
+@pytest.mark.xfail(
+    reason='pathlib silently truncates trailing "/.", so "/target/." is equivalent to "/target"',
+    strict=True,
+)
+def test_enotdir_2(tmp_path):
+    (tmp_path / 'target').write_text('pass')
+    with pytest.raises(NotADirectoryError):
+        manifest.resolve_symlinks('/target/.', chroot=tmp_path)
+
+def test_enotdir_3(tmp_path):
+    (tmp_path / 'target').write_text('pass')
+    with pytest.raises(NotADirectoryError):
+        manifest.resolve_symlinks('/target/../target', chroot=tmp_path)