Better protection against symlink traversal in flatpak. Other hardening
This commit is contained in:
parent
903125976d
commit
f5b85d29d3
GPG key ID:
03906B4110AAD3B8
9 changed files with 429 additions and 53 deletions
|
|
@ -3,11 +3,18 @@ from __future__ import annotations
|
|||
import configparser
|
||||
import os
|
||||
import re
|
||||
import stat
|
||||
import shutil
|
||||
import subprocess # nosec
|
||||
from dataclasses import dataclass, field
|
||||
from typing import Dict, List, Optional, Set, Tuple
|
||||
|
||||
from .fsutil import (
|
||||
is_dir_no_symlink_components,
|
||||
open_no_follow_path,
|
||||
path_has_symlink_component,
|
||||
)
|
||||
|
||||
|
||||
@dataclass
|
||||
class FlatpakInstall:
|
||||
|
|
@ -161,15 +168,40 @@ def find_user_ssh_files(home: str) -> List[str]:
|
|||
return sorted(set(out))
|
||||
|
||||
|
||||
def _read_first_existing_text(paths: List[str]) -> Optional[str]:
|
||||
def _read_first_existing_text(
|
||||
paths: List[str], *, max_bytes: int = 8192
|
||||
) -> Optional[str]:
|
||||
"""Read the first small regular text file without following symlinks.
|
||||
|
||||
Per-user Flatpak metadata lives under user-controlled home directories.
|
||||
When Enroll is run as root, plain ``open()`` would let a user replace
|
||||
``active/origin`` or ``repo/config`` with a symlink to a privileged file and
|
||||
have its contents copied into state.json. Use the same no-symlink component
|
||||
invariant as the normal harvester, require a regular file, and cap reads to
|
||||
avoid device/large-file DoS.
|
||||
"""
|
||||
|
||||
for path in paths:
|
||||
fd: Optional[int] = None
|
||||
try:
|
||||
with open(path, "r", encoding="utf-8", errors="replace") as f:
|
||||
value = f.read().strip()
|
||||
if value:
|
||||
return value
|
||||
fd = open_no_follow_path(path)
|
||||
st = os.fstat(fd)
|
||||
if not stat.S_ISREG(st.st_mode) or st.st_size > max_bytes:
|
||||
continue
|
||||
data = os.read(fd, max_bytes + 1)
|
||||
if len(data) > max_bytes:
|
||||
continue
|
||||
value = data.decode("utf-8", errors="replace").strip()
|
||||
if value:
|
||||
return value
|
||||
except OSError:
|
||||
continue
|
||||
finally:
|
||||
if fd is not None:
|
||||
try:
|
||||
os.close(fd)
|
||||
except OSError:
|
||||
pass
|
||||
return None
|
||||
|
||||
|
||||
|
|
@ -457,7 +489,7 @@ def _flatpak_remote_from_ref(
|
|||
arch,
|
||||
branch,
|
||||
)
|
||||
if os.path.exists(ref):
|
||||
if not path_has_symlink_component(ref) and os.path.exists(ref):
|
||||
return remote_name
|
||||
return None
|
||||
|
||||
|
|
@ -473,11 +505,11 @@ def _parse_flatpak_deploy_origin(branch_dir: str) -> Optional[str]:
|
|||
if origin:
|
||||
return origin
|
||||
|
||||
metadata = candidates[1]
|
||||
if os.path.isfile(metadata):
|
||||
metadata = _read_first_existing_text([candidates[1]])
|
||||
if metadata:
|
||||
parser = configparser.ConfigParser(interpolation=None)
|
||||
try:
|
||||
parser.read(metadata, encoding="utf-8")
|
||||
parser.read_string(metadata)
|
||||
except Exception:
|
||||
return None
|
||||
for section in ("Application", "Runtime"):
|
||||
|
|
@ -496,7 +528,7 @@ def _find_flatpaks_in_root(
|
|||
home: Optional[str] = None,
|
||||
) -> List[FlatpakInstall]:
|
||||
apps_dir = os.path.join(flatpak_root, "app")
|
||||
if not os.path.isdir(apps_dir):
|
||||
if not is_dir_no_symlink_components(apps_dir):
|
||||
return []
|
||||
|
||||
remote_names = [
|
||||
|
|
@ -513,7 +545,7 @@ def _find_flatpaks_in_root(
|
|||
seen: Set[Tuple[str, Optional[str], Optional[str], Optional[str]]] = set()
|
||||
for app_id in app_ids:
|
||||
app_path = os.path.join(apps_dir, app_id)
|
||||
if not os.path.isdir(app_path):
|
||||
if not is_dir_no_symlink_components(app_path):
|
||||
continue
|
||||
try:
|
||||
arches = sorted(os.listdir(app_path))
|
||||
|
|
@ -521,7 +553,7 @@ def _find_flatpaks_in_root(
|
|||
continue
|
||||
for arch in arches:
|
||||
arch_path = os.path.join(app_path, arch)
|
||||
if not os.path.isdir(arch_path):
|
||||
if not is_dir_no_symlink_components(arch_path):
|
||||
continue
|
||||
try:
|
||||
branches = sorted(os.listdir(arch_path))
|
||||
|
|
@ -529,10 +561,10 @@ def _find_flatpaks_in_root(
|
|||
continue
|
||||
for branch in branches:
|
||||
branch_path = os.path.join(arch_path, branch)
|
||||
if not os.path.isdir(branch_path):
|
||||
if not is_dir_no_symlink_components(branch_path):
|
||||
continue
|
||||
active_dir = os.path.join(branch_path, "active")
|
||||
if not os.path.exists(active_dir):
|
||||
if not is_dir_no_symlink_components(active_dir):
|
||||
continue
|
||||
remote = _parse_flatpak_deploy_origin(branch_path)
|
||||
if not remote:
|
||||
|
|
@ -576,12 +608,13 @@ def find_flatpak_remotes(
|
|||
.flatpakref/.flatpakrepo URL that was used during installation.
|
||||
"""
|
||||
config_path = os.path.join(flatpak_root, "repo", "config")
|
||||
if not os.path.isfile(config_path):
|
||||
config_text = _read_first_existing_text([config_path])
|
||||
if not config_text:
|
||||
return []
|
||||
|
||||
parser = configparser.ConfigParser(interpolation=None, strict=False)
|
||||
try:
|
||||
parser.read(config_path, encoding="utf-8")
|
||||
parser.read_string(config_text)
|
||||
except Exception:
|
||||
return []
|
||||
|
||||
|
|
|
|||
|
|
@ -126,6 +126,57 @@ def open_no_follow_path(path: str, *, write: bool = False, mode: int = 0o600) ->
|
|||
os.close(dir_fd)
|
||||
|
||||
|
||||
def path_has_symlink_component(path: str) -> bool:
|
||||
"""Return True if any existing component of *path* is a symlink.
|
||||
|
||||
This is a lightweight discovery-time companion to ``open_no_follow_path``.
|
||||
It is intended for directory-walking code paths that must decide whether a
|
||||
candidate root is safe to enumerate before opening individual files. Missing
|
||||
trailing components are treated as non-symlinks; ``..`` is treated as unsafe
|
||||
and therefore reported as a symlink-like component.
|
||||
"""
|
||||
|
||||
norm = os.path.normpath(path)
|
||||
if norm in ("", "."):
|
||||
return False
|
||||
|
||||
if os.path.isabs(norm):
|
||||
cur = os.sep
|
||||
parts = [p for p in norm.split(os.sep) if p]
|
||||
else:
|
||||
cur = os.getcwd()
|
||||
parts = [p for p in norm.split(os.sep) if p]
|
||||
|
||||
for part in parts:
|
||||
if part in ("", "."):
|
||||
continue
|
||||
if part == "..":
|
||||
return True
|
||||
cur = os.path.join(cur, part)
|
||||
try:
|
||||
st = os.lstat(cur)
|
||||
except FileNotFoundError:
|
||||
return False
|
||||
except OSError:
|
||||
# Fail closed for unreadable/racy paths used as discovery roots.
|
||||
return True
|
||||
if stat.S_ISLNK(st.st_mode):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def is_dir_no_symlink_components(path: str) -> bool:
|
||||
"""Return True only for directories reached without symlink components."""
|
||||
|
||||
if path_has_symlink_component(path):
|
||||
return False
|
||||
try:
|
||||
st = os.stat(path, follow_symlinks=False)
|
||||
except OSError:
|
||||
return False
|
||||
return stat.S_ISDIR(st.st_mode)
|
||||
|
||||
|
||||
def stat_triplet_from_stat(st: os.stat_result) -> Tuple[str, str, str]:
|
||||
"""Return (owner, group, mode) for an existing stat result."""
|
||||
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ from ..harvest_types import (
|
|||
)
|
||||
from ..system_paths import MAX_FILES_CAP
|
||||
from ..pathfilter import expand_includes
|
||||
from ..fsutil import is_dir_no_symlink_components, path_has_symlink_component
|
||||
from .context import HarvestCollector, HarvestContext
|
||||
|
||||
|
||||
|
|
@ -192,13 +193,13 @@ class ExtraPathsCollector(HarvestCollector):
|
|||
path = pat.value
|
||||
if os.path.islink(path):
|
||||
self._capture_included_link(path, role_seen)
|
||||
elif os.path.isdir(path):
|
||||
elif is_dir_no_symlink_components(path):
|
||||
self._walk_and_capture_dirs(path, role_seen)
|
||||
elif pat.kind == "glob":
|
||||
for hit in glob.glob(pat.value, recursive=True):
|
||||
if os.path.islink(hit):
|
||||
self._capture_included_link(hit, role_seen)
|
||||
elif os.path.isdir(hit):
|
||||
elif is_dir_no_symlink_components(hit):
|
||||
self._walk_and_capture_dirs(hit, role_seen)
|
||||
|
||||
def _capture_included_link(self, path: str, role_seen: Set[str]) -> None:
|
||||
|
|
@ -224,7 +225,7 @@ class ExtraPathsCollector(HarvestCollector):
|
|||
root = os.path.normpath(root)
|
||||
if not root.startswith("/"):
|
||||
root = "/" + root
|
||||
if not os.path.isdir(root) or os.path.islink(root):
|
||||
if not is_dir_no_symlink_components(root):
|
||||
return
|
||||
for dirpath, dirnames, filenames in os.walk(root, followlinks=False):
|
||||
if len(self.managed_dirs) >= MAX_FILES_CAP:
|
||||
|
|
@ -238,7 +239,7 @@ class ExtraPathsCollector(HarvestCollector):
|
|||
if self.context.path_filter.is_excluded(dirpath):
|
||||
dirnames[:] = []
|
||||
continue
|
||||
if os.path.islink(dirpath) or not os.path.isdir(dirpath):
|
||||
if not is_dir_no_symlink_components(dirpath):
|
||||
dirnames[:] = []
|
||||
continue
|
||||
|
||||
|
|
@ -275,6 +276,8 @@ class ExtraPathsCollector(HarvestCollector):
|
|||
if os.path.islink(path):
|
||||
self._capture_included_link(path, role_seen)
|
||||
continue
|
||||
if path_has_symlink_component(path):
|
||||
continue
|
||||
pruned.append(dirname)
|
||||
dirnames[:] = pruned
|
||||
|
||||
|
|
|
|||
|
|
@ -69,6 +69,33 @@ def _merge_mappings_overwrite(
|
|||
return merged
|
||||
|
||||
|
||||
_RESERVED_ROLE_VAR_SUFFIXES = {
|
||||
"managed_files",
|
||||
"managed_dirs",
|
||||
"managed_links",
|
||||
"packages",
|
||||
"restart_units",
|
||||
"system_flatpaks",
|
||||
"remotes",
|
||||
"user_flatpaks",
|
||||
"user_flatpak_remotes",
|
||||
}
|
||||
|
||||
|
||||
def reserved_role_var_names(role_name: str) -> Set[str]:
|
||||
"""Return Enroll-owned variable names for a generated role.
|
||||
|
||||
JinjaTurtle variables come from harvested, attacker-influenceable config
|
||||
content. They must never overwrite variables that drive Enroll's renderer
|
||||
tasks, such as ``<role>_managed_files``.
|
||||
"""
|
||||
|
||||
role = re.sub(r"[^A-Za-z0-9_]+", "_", role_name.strip().lower()).strip("_")
|
||||
if not role:
|
||||
return set()
|
||||
return {f"{role}_{suffix}" for suffix in _RESERVED_ROLE_VAR_SUFFIXES}
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class JinjifiedArtifact:
|
||||
template_rel: str
|
||||
|
|
@ -131,6 +158,7 @@ def jinjify_artifact(
|
|||
jt_enabled: bool,
|
||||
overwrite_templates: bool = True,
|
||||
role_name: Optional[str] = None,
|
||||
reserved_context_keys: Optional[Set[str]] = None,
|
||||
) -> Optional[JinjifiedArtifact]:
|
||||
"""Best-effort conversion of one harvested artifact into a Jinja2 template."""
|
||||
if not (jt_enabled and jt_exe and can_jinjify_path(dest_path)):
|
||||
|
|
@ -155,6 +183,9 @@ def jinjify_artifact(
|
|||
template_dst = Path(template_root) / template_rel
|
||||
|
||||
context = yaml_load_mapping(result.vars_text)
|
||||
if reserved_context_keys and (set(context) & set(reserved_context_keys)):
|
||||
return None
|
||||
|
||||
missing = missing_jinja_template_vars(result.template_text, context)
|
||||
if missing:
|
||||
# If this role was generated into an existing output directory, avoid
|
||||
|
|
@ -181,10 +212,12 @@ def managed_file_var_prefix(role_name: str, src_rel: str) -> str:
|
|||
JinjaTurtle's ``--role-name`` is a variable prefix. Enroll can place many
|
||||
unrelated managed files in one generated role, so using only the role name
|
||||
can collide for common keys such as ``enabled``, ``ignore``, or ``name``.
|
||||
Include the relative artifact path when a role templates multiple files.
|
||||
Always include a ``jt`` namespace and the relative artifact path so harvested
|
||||
config keys cannot produce Enroll-owned variables such as
|
||||
``<role>_managed_files``.
|
||||
"""
|
||||
|
||||
raw = f"{role_name}_{src_rel}"
|
||||
raw = f"{role_name}_jt_{src_rel}"
|
||||
safe = re.sub(r"[^A-Za-z0-9_]+", "_", raw).strip("_").lower()
|
||||
safe = re.sub(r"_+", "_", safe)
|
||||
if not safe:
|
||||
|
|
@ -215,14 +248,7 @@ def jinjify_managed_files(
|
|||
templated: Set[str] = set()
|
||||
vars_map: Dict[str, Any] = {}
|
||||
base_role_name = role_name or artifact_role
|
||||
candidates = [
|
||||
mf
|
||||
for mf in managed_files
|
||||
if str(mf.get("path") or "")
|
||||
and str(mf.get("src_rel") or "")
|
||||
and can_jinjify_path(str(mf.get("path") or ""))
|
||||
]
|
||||
namespace_by_file = len(candidates) > 1
|
||||
reserved_context_keys = reserved_role_var_names(base_role_name)
|
||||
|
||||
for mf in managed_files:
|
||||
dest_path = str(mf.get("path") or "")
|
||||
|
|
@ -239,11 +265,8 @@ def jinjify_managed_files(
|
|||
jt_exe=jt_exe,
|
||||
jt_enabled=jt_enabled,
|
||||
overwrite_templates=overwrite_templates,
|
||||
role_name=(
|
||||
managed_file_var_prefix(base_role_name, src_rel)
|
||||
if namespace_by_file
|
||||
else base_role_name
|
||||
),
|
||||
role_name=managed_file_var_prefix(base_role_name, src_rel),
|
||||
reserved_context_keys=reserved_context_keys,
|
||||
)
|
||||
if converted is None:
|
||||
continue
|
||||
|
|
|
|||
|
|
@ -7,10 +7,42 @@ from dataclasses import dataclass
|
|||
from pathlib import PurePosixPath
|
||||
from typing import List, Optional, Sequence, Set, Tuple
|
||||
|
||||
|
||||
_REGEX_PREFIXES = ("re:", "regex:")
|
||||
|
||||
|
||||
def _path_has_symlink_component_for_discovery(path: str) -> bool:
|
||||
"""Return True if any path component is visibly a symlink.
|
||||
|
||||
``expand_includes`` is a discovery helper: the actual file capture path is
|
||||
still protected by descriptor-based no-follow opens. Keep this check
|
||||
intentionally based on ``os.path.islink`` so tests can mock a synthetic
|
||||
filesystem with ``os.path``/``os.walk`` and so include expansion does not
|
||||
depend on real host permissions for paths such as ``/root``. Existing
|
||||
symlinked parents are still pruned before walking/capturing.
|
||||
"""
|
||||
|
||||
norm = os.path.normpath(path)
|
||||
if norm in ("", "."):
|
||||
return False
|
||||
|
||||
if os.path.isabs(norm):
|
||||
cur = os.sep
|
||||
parts = [p for p in norm.split(os.sep) if p]
|
||||
else:
|
||||
cur = os.getcwd()
|
||||
parts = [p for p in norm.split(os.sep) if p]
|
||||
|
||||
for part in parts:
|
||||
if part in ("", "."):
|
||||
continue
|
||||
if part == "..":
|
||||
return True
|
||||
cur = os.path.join(cur, part)
|
||||
if os.path.islink(cur):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _has_glob_chars(s: str) -> bool:
|
||||
return any(ch in s for ch in "*?[")
|
||||
|
||||
|
|
@ -191,6 +223,37 @@ def expand_includes(
|
|||
notes: List[str] = []
|
||||
seen: Set[str] = set()
|
||||
|
||||
def _is_file_no_symlink_components(p: str) -> bool:
|
||||
return not _path_has_symlink_component_for_discovery(p) and os.path.isfile(p)
|
||||
|
||||
def _is_dir_no_symlink_components(p: str) -> bool:
|
||||
return not _path_has_symlink_component_for_discovery(p) and os.path.isdir(p)
|
||||
|
||||
def _glob_walk_root(pattern: str) -> Optional[str]:
|
||||
"""Return a conservative directory root for recursive glob fallback.
|
||||
|
||||
This keeps existing tests that mock ``os.walk`` independent of the real
|
||||
host's /root contents while still applying the same no-symlink-component
|
||||
guard before enumeration. Only recursive subtree globs are expanded this
|
||||
way; ordinary file globs continue to rely on ``glob.glob`` hits.
|
||||
"""
|
||||
|
||||
parts = pattern.split(os.sep)
|
||||
literal_parts: List[str] = []
|
||||
absolute = pattern.startswith(os.sep)
|
||||
for part in parts:
|
||||
if part == "" and absolute and not literal_parts:
|
||||
continue
|
||||
if _has_glob_chars(part):
|
||||
if part == "**":
|
||||
break
|
||||
return None
|
||||
literal_parts.append(part)
|
||||
if not literal_parts:
|
||||
return os.sep if absolute else None
|
||||
root = os.path.join(os.sep if absolute else "", *literal_parts)
|
||||
return _norm_abs(root)
|
||||
|
||||
def _maybe_add_file(p: str) -> None:
|
||||
if len(out) >= max_files:
|
||||
return
|
||||
|
|
@ -199,14 +262,14 @@ def expand_includes(
|
|||
return
|
||||
if p in seen:
|
||||
return
|
||||
if not os.path.isfile(p) or os.path.islink(p):
|
||||
if not _is_file_no_symlink_components(p):
|
||||
return
|
||||
seen.add(p)
|
||||
out.append(p)
|
||||
|
||||
def _walk_dir(root: str, match: Optional[CompiledPathPattern] = None) -> None:
|
||||
root = _norm_abs(root)
|
||||
if not os.path.isdir(root) or os.path.islink(root):
|
||||
if not _is_dir_no_symlink_components(root):
|
||||
return
|
||||
for dirpath, dirnames, filenames in os.walk(root, followlinks=False):
|
||||
# Prune excluded directories early.
|
||||
|
|
@ -215,13 +278,13 @@ def expand_includes(
|
|||
d
|
||||
for d in dirnames
|
||||
if not exclude.is_excluded(os.path.join(dirpath, d))
|
||||
and not os.path.islink(os.path.join(dirpath, d))
|
||||
and _is_dir_no_symlink_components(os.path.join(dirpath, d))
|
||||
]
|
||||
for fn in filenames:
|
||||
if len(out) >= max_files:
|
||||
return
|
||||
p = os.path.join(dirpath, fn)
|
||||
if os.path.islink(p) or not os.path.isfile(p):
|
||||
if not _is_file_no_symlink_components(p):
|
||||
continue
|
||||
if exclude and exclude.is_excluded(p):
|
||||
continue
|
||||
|
|
@ -243,10 +306,10 @@ def expand_includes(
|
|||
|
||||
if pat.kind == "prefix":
|
||||
p = pat.value
|
||||
if os.path.isfile(p) and not os.path.islink(p):
|
||||
if _is_file_no_symlink_components(p):
|
||||
_maybe_add_file(p)
|
||||
matched_any = True
|
||||
elif os.path.isdir(p) and not os.path.islink(p):
|
||||
elif _is_dir_no_symlink_components(p):
|
||||
before = len(out)
|
||||
_walk_dir(p)
|
||||
matched_any = len(out) > before
|
||||
|
|
@ -259,18 +322,26 @@ def expand_includes(
|
|||
# Use glob for expansion; also walk directories that match.
|
||||
gpat = pat.value
|
||||
hits = glob.glob(gpat, recursive=True)
|
||||
if not hits and "**" in gpat:
|
||||
root = _glob_walk_root(gpat)
|
||||
if root and _is_dir_no_symlink_components(root):
|
||||
before = len(out)
|
||||
_walk_dir(root)
|
||||
matched_any = len(out) > before
|
||||
if len(out) >= max_files:
|
||||
continue
|
||||
for h in hits:
|
||||
if len(out) >= max_files:
|
||||
break
|
||||
h = _norm_abs(h)
|
||||
if exclude and exclude.is_excluded(h):
|
||||
continue
|
||||
if os.path.isdir(h) and not os.path.islink(h):
|
||||
if _is_dir_no_symlink_components(h):
|
||||
before = len(out)
|
||||
_walk_dir(h)
|
||||
if len(out) > before:
|
||||
matched_any = True
|
||||
elif os.path.isfile(h) and not os.path.islink(h):
|
||||
elif _is_file_no_symlink_components(h):
|
||||
_maybe_add_file(h)
|
||||
matched_any = True
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue