Skip to content

Commit

Permalink
[python] Fix measuring absolute paths in chroots
Browse files Browse the repository at this point in the history
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 <bartlomiej.sulich@intel.com>
Signed-off-by: Wojtek Porczyk <woju@invisiblethingslab.com>
  • Loading branch information
woju committed Dec 13, 2023
1 parent cdca1e3 commit 02bce3f
Show file tree
Hide file tree
Showing 2 changed files with 178 additions and 4 deletions.
83 changes: 79 additions & 4 deletions python/graminelibos/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@
Gramine manifest management and rendering
"""

import errno
import hashlib
import os
import pathlib
import posixpath

import tomli
import tomli_w
Expand All @@ -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.
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
99 changes: 99 additions & 0 deletions tests/test_chroot.py
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit 02bce3f

Please sign in to comment.