From d21903586606e112f20b4b3c9441bc6c55038ec6 Mon Sep 17 00:00:00 2001 From: Russell Martin Date: Wed, 5 Jul 2023 13:23:47 -0400 Subject: [PATCH] Revise user mapping inspection implementation - Perform check in constructor so the setting is always available - Perform write test in `build` directory to help prevent project pollution --- src/briefcase/integrations/docker.py | 70 +++++++++++++++++------ src/briefcase/platforms/linux/appimage.py | 8 +-- src/briefcase/platforms/linux/system.py | 7 +-- 3 files changed, 57 insertions(+), 28 deletions(-) diff --git a/src/briefcase/integrations/docker.py b/src/briefcase/integrations/docker.py index d2c4dbd52..c28cd36b8 100644 --- a/src/briefcase/integrations/docker.py +++ b/src/briefcase/integrations/docker.py @@ -111,6 +111,18 @@ class Docker(Tool): "Linux": "https://docs.docker.com/engine/install/#server", } + def __init__(self, tools: ToolCache, image_tag: str | None = None): + """A wrapper for the user-installed Docker. + + :param tools: ToolCache of available tools + :param image_tag: An optional image used to access attributes of the Docker + environment, such as how user permissions are managed in bind mounts. A + lightweight image will be used if one is not specified but this image is not + at all bound to the instance. + """ + super().__init__(tools=tools) + self.is_users_mapped = self._is_user_mapping_enabled(image_tag) + @classmethod def verify_install( cls, @@ -121,7 +133,9 @@ def verify_install( """Verify Docker is installed and operational. :param tools: ToolCache of available tools - :param image_tag: Specific image to use to assess Docker's operating mode + :param image_tag: An optional image used during verification to access + attributes of the local Docker environment. This image is not bound to the + instance and only used during instantiation. """ # short circuit since already verified and available if hasattr(tools, "docker"): @@ -131,9 +145,7 @@ def verify_install( cls._user_access(tools=tools) cls._buildx_installed(tools=tools) - tools.docker = Docker(tools=tools) - tools.docker._determine_docker_mode(image_tag=image_tag) - + tools.docker = Docker(tools=tools, image_tag=image_tag) return tools.docker @classmethod @@ -200,8 +212,8 @@ def _buildx_installed(cls, tools: ToolCache): except subprocess.CalledProcessError: raise BriefcaseCommandError(cls.BUILDX_PLUGIN_MISSING) - def _determine_docker_mode(self, image_tag: str | None = None): - """Determine Docker's operating mode from how it interacts with bind mounts. + def _is_user_mapping_enabled(self, image_tag: str | None = None) -> bool: + """Determine whether Docker is mapping users between the container and the host. Docker can be installed in different ways on Linux that significantly impact how containers interact with the host system. Of particular note is ownership of @@ -215,9 +227,10 @@ def _determine_docker_mode(self, image_tag: str | None = None): that matches the host user running Docker. Other installation methods of Docker, though, are not compatible with using such - a step-down user. This includes Docker Desktop and rootless Docker. In these - modes, Docker maps the host user to the root user inside the container; this - mapping is transparent and would require changes to the host environment to + a step-down user. This includes Docker Desktop and rootless Docker (although, + even a traditional installation of Docker Engine can be configured similarly). + In these modes, Docker maps the host user to the root user inside the container; + this mapping is transparent and would require changes to the host environment to disable, if it can be disabled at all. This allows files created in bind mounts inside the container to be owned on the host file system by the user running Docker. Additionally, though, because the host user is mapped to root inside the @@ -229,9 +242,28 @@ def _determine_docker_mode(self, image_tag: str | None = None): created inside a bind mount in the container. If the owning user of that file on the host file system is root, then a step-down user is necessary inside containers. If the owning user is the host user, root should be used. + + On macOS, Docker Desktop is the only option to use Docker and user mapping + happens differently such that any user in the container is mapped to the host + user. Instead of leveraging user namespaces as on Linux, this user mapping + manifests as a consequence of bind mounts being implemented as NFS shares + between macOS and the Linux VM that Docker Desktop runs containers in. So, + using a step-down user on macOS is effectively inconsequential. + + On Windows WSL 2, Docker Desktop operates similarly to how it does on Linux. + However, user namespace mapping is not possible because the Docker Desktop VM + and the WSL distro are already running in different user namespaces...and + therefore, Docker cannot even see the users in the distro to map them in to the + container. So, a step-down user is always used. + + ref: https://docs.docker.com/engine/security/userns-remap/ + + :param image_tag: The image:tag to use to create the container for the test; if + one is not specified, then `alpine:latest` will be used. + :returns: True if users are being mapped; False otherwise """ write_test_filename = "container_write_test" - host_write_test_dir_path = Path.cwd() + host_write_test_dir_path = Path.cwd() / "build" host_write_test_file_path = Path(host_write_test_dir_path, write_test_filename) container_mount_host_dir = "/host_write_test" container_write_test_file_path = PurePosixPath( @@ -247,13 +279,15 @@ def _determine_docker_mode(self, image_tag: str | None = None): "alpine" if image_tag is None else image_tag, ] + host_write_test_dir_path.mkdir(exist_ok=True) + try: host_write_test_file_path.unlink(missing_ok=True) except OSError as e: raise BriefcaseCommandError( - f""" -The file path used to determine Docker's operating mode already exists and -cannot be automatically deleted. + f"""\ +The file path used to determine how Docker is mapping users between the host +and Docker containers already exists and cannot be automatically deleted. {host_write_test_file_path} @@ -269,11 +303,11 @@ def _determine_docker_mode(self, image_tag: str | None = None): ) except subprocess.CalledProcessError as e: raise BriefcaseCommandError( - "Unable to determine Docker's operating mode" + "Unable to determine if Docker is mapping users" ) from e # if the file is not owned by `root`, then Docker is mapping usernames - self.is_userns_remap = 0 != self.tools.os.stat(host_write_test_file_path).st_uid + is_users_mapped = 0 != self.tools.os.stat(host_write_test_file_path).st_uid try: self.tools.subprocess.run( @@ -283,9 +317,11 @@ def _determine_docker_mode(self, image_tag: str | None = None): ) except subprocess.CalledProcessError as e: raise BriefcaseCommandError( - "Unable to clean up from determining Docker's operating mode" + "Unable to clean up from determining if Docker is mapping users" ) from e + return is_users_mapped + def cache_image(self, image_tag: str): """Ensures an image is available and cached locally. @@ -305,7 +341,7 @@ def cache_image(self, image_tag: str): if not image_id: try: - self.tools.subprocess.run(["docker", "pull", image_tag]) + self.tools.subprocess.run(["docker", "pull", image_tag], check=True) except subprocess.CalledProcessError as e: raise BriefcaseCommandError( f"Unable to obtain the Docker image for {image_tag}. " diff --git a/src/briefcase/platforms/linux/appimage.py b/src/briefcase/platforms/linux/appimage.py index 8fc392f29..8843aca37 100644 --- a/src/briefcase/platforms/linux/appimage.py +++ b/src/briefcase/platforms/linux/appimage.py @@ -168,12 +168,8 @@ def output_format_template_context(self, app: AppConfig): except AttributeError: pass - # Use the non-root brutus user if Docker is mapping usernames - # (only relevant if Docker is being used for the target platform) - try: - context["use_non_root_user"] = not self.tools.docker.is_userns_remap - except AttributeError: - pass + # Use the non-root brutus user if Docker is not mapping usernames + context["use_non_root_user"] = not self.tools.docker.is_users_mapped return context diff --git a/src/briefcase/platforms/linux/system.py b/src/briefcase/platforms/linux/system.py index 3593225cf..2086f794a 100644 --- a/src/briefcase/platforms/linux/system.py +++ b/src/briefcase/platforms/linux/system.py @@ -3,7 +3,6 @@ import re import subprocess import sys -from contextlib import suppress from pathlib import Path from typing import List @@ -559,10 +558,8 @@ def output_format_template_context(self, app: AppConfig): # Add the vendor base context["vendor_base"] = app.target_vendor_base - # Use the non-root brutus user if Docker is mapping usernames - # (only relevant if Docker is being used for the target platform) - with suppress(AttributeError): - context["use_non_root_user"] = not self.tools.docker.is_userns_remap + # Use the non-root brutus user if Docker is not mapping usernames + context["use_non_root_user"] = not self.tools.docker.is_users_mapped return context