From: Daan De Meyer <daan.j.demeyer@gmail.com>
Date: Thu, 21 Mar 2024 18:59:24 +0000 (+0100)
Subject: Remove setuid/setgid bits from build and workspace directory
X-Git-Tag: v23~66
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=59101e7db0dda064b86bcc89e534e15c93ae1ff2;p=thirdparty%2Fmkosi.git

Remove setuid/setgid bits from build and workspace directory

Both of these can be inherited so remove them from both the workspace
and the build directory where inheriting these bits could end up
leaking stuff from the host into the image.

Also remove INVOKING_USER.mkdir() while we're at it as only one user
was remaining which we can do much more easily by doing the logic
before we go into the user namespace.
---

diff --git a/mkosi/__init__.py b/mkosi/__init__.py
index 08a6d9501..3046e5adc 100644
--- a/mkosi/__init__.py
+++ b/mkosi/__init__.py
@@ -3350,7 +3350,8 @@ def normalize_mtime(root: Path, mtime: Optional[int], directory: Optional[Path]
 def setup_workspace(args: Args, config: Config) -> Iterator[Path]:
     with contextlib.ExitStack() as stack:
         workspace = Path(tempfile.mkdtemp(dir=config.workspace_dir_or_default(), prefix="mkosi-workspace"))
-        os.chmod(workspace, 0o700)
+        # Discard setuid/setgid bits as these are inherited and can leak into the image.
+        workspace.chmod(stat.S_IMODE(workspace.stat().st_mode) & ~(stat.S_ISGID|stat.S_ISUID))
         stack.callback(lambda: rmtree(workspace, tools=config.tools(), sandbox=config.sandbox))
         (workspace / "tmp").mkdir(mode=0o1777)
 
@@ -4118,6 +4119,27 @@ def run_sync(args: Args, config: Config, *, resources: Path) -> None:
 def run_build(args: Args, config: Config, *, resources: Path) -> None:
     check_inputs(config)
 
+    for p in (
+        config.output_dir,
+        config.cache_dir,
+        config.package_cache_dir_or_default(),
+        config.build_dir,
+        config.workspace_dir,
+    ):
+        if not p or p.exists():
+            continue
+
+        p.mkdir()
+
+        # If we created the directory in a parent directory owned by the invoking user, make sure the directory itself
+        # is owned by the invoking user as well.
+        if INVOKING_USER.is_regular_user() and p.parent.stat().st_uid == INVOKING_USER.uid:
+            os.chown(p, INVOKING_USER.uid, INVOKING_USER.gid)
+
+    # Discard setuid/setgid bits as these are inherited and can leak into the image.
+    if config.build_dir:
+        config.build_dir.chmod(stat.S_IMODE(config.build_dir.stat().st_mode) & ~(stat.S_ISGID|stat.S_ISUID))
+
     if (uid := os.getuid()) != 0:
         become_root()
     unshare(CLONE_NEWNS)
@@ -4141,16 +4163,6 @@ def run_build(args: Args, config: Config, *, resources: Path) -> None:
     ):
         check_tools(config, Verb.build)
 
-        for p in (
-            config.output_dir,
-            config.cache_dir,
-            config.package_cache_dir_or_default(),
-            config.build_dir,
-            config.workspace_dir,
-        ):
-            if p and not p.exists():
-                INVOKING_USER.mkdir(p)
-
         with (
             acl_toggle_build(config, INVOKING_USER.uid),
             rchown_package_manager_dirs(config),
diff --git a/mkosi/user.py b/mkosi/user.py
index 4437fd5e7..faa2c729a 100644
--- a/mkosi/user.py
+++ b/mkosi/user.py
@@ -66,20 +66,6 @@ class INVOKING_USER:
 
         return cache / "mkosi"
 
-    @classmethod
-    def mkdir(cls, path: Path) -> Path:
-        cond = (
-            not cls.invoked_as_root or
-            (cls.is_regular_user() and any(p.exists() and p.stat().st_uid == cls.uid for p in path.parents))
-        )
-        run(
-            ["mkdir", "--parents", path],
-            user=cls.uid if cond else os.getuid(),
-            group=cls.gid if cond else os.getgid(),
-            extra_groups=cls.extra_groups() if cls.invoked_as_root and cond else None,
-        )
-        return path
-
     @classmethod
     def rchown(cls, path: Path) -> None:
         if cls.is_regular_user() and any(p.stat().st_uid == cls.uid for p in path.parents) and path.exists():