]> git.ipfire.org Git - thirdparty/mkosi.git/commitdiff
Remove setuid/setgid bits from build and workspace directory
authorDaan De Meyer <daan.j.demeyer@gmail.com>
Thu, 21 Mar 2024 18:59:24 +0000 (19:59 +0100)
committerDaan De Meyer <daan.j.demeyer@gmail.com>
Fri, 22 Mar 2024 13:04:27 +0000 (14:04 +0100)
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.

mkosi/__init__.py
mkosi/user.py

index 08a6d95015501e53009e2bace8e8b2786660c16f..3046e5adc79ec7c5d7ffbd334a0e3eee7f79e9d6 100644 (file)
@@ -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),
index 4437fd5e71b6be4a2a5fad581e7b482af5878855..faa2c729a474e9f6e01b5789bc1f653589b67ce9 100644 (file)
@@ -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():