podman wrapper: uses userns option when possible (#211)

* podman wrapper: uses userns option when possible On podman >= 4.3, the --userns keep-id:uid=X,gid=Y option allows to map the current user to a specific container user. Previous code: we created an entire UID/GID mapping to make this happen In this change: if podman >= 4.3, we simply use the userns option, otherwise we fallback to the previous workaround. Also: - in the workaround: only do the mapping when subUID/subGID are configured. - Comments improved (hopefully) - Updated README troubleshooting * added pytest * Exception if subuid is not correctly enabled
RedHatProductSecurity · Sep 26, 2024 · 0fccfb5 · 0fccfb5
1 parent a9786d0
commit 0fccfb5
Show file tree

Hide file tree

Showing 3 changed files with 143 additions and 21 deletions.
diff --git a/README.md b/README.md
@@ -619,6 +619,23 @@ Solutions:
 * Selenium, used to control Firefox, uses shared memory (`/dev/shm/`). When using the RapiDAST image or the ZAP image, the user needs to make sure that sufficient space is available in `/dev/shm/` (in podman, by default, its size is 64MB). A size of 2G is the recommended value by the Selenium community. In podman for example, the option would be `--shm-size=2g`.
 * Zap and Firefox can create a huge numbers of threads. Some container engines will default to 2048 concurrent pids, which is not sufficient for the Ajax Spider. Whenever possible, RapiDAST will check if that limit was reached, after the scan is finished, and prints a warning if this happened. In podman, increasing the maximum number of concurrent pids is done via the `--pids-limit=-1` option to prevent any limits.
 
+## Podman errors
+
+### subuid/subgid are not enabled
+
+If you see one of those errors:
+
+```
+Error: copying system image from manifest list: writing blob: adding layer with blob "sha256:82aabceedc2fbf89030cbb4ff98215b70d9ae35c780ade6c784d9b447b1109ed": processing tar file(potentially insufficient UIDs or GIDs available in user namespace (requested 0:42 for /etc/gshadow): Check /etc/subuid and /etc/subgid if configured locally and run "podman system migrate": lchown /etc/gshadow: invalid argument): exit status 1
+```
+ -or-
+```
+Error: parsing id map value "-1000": strconv.ParseUint: parsing "-1000": invalid syntax
+```
+
+Podman, in rootless mode (running as a regular user), needs subuid/subgit to be enabled: [rootless mode](https://docs.podman.io/en/latest/markdown/podman.1.html#rootless-mode)
+
+
 ## Caveats
 
 * Currently, RapiDAST does not clean up the temporary data when there is an error. The data may include:

diff --git a/scanners/podman_wrapper.py b/scanners/podman_wrapper.py
@@ -92,13 +92,50 @@ def add_volume_map(self, mapping):
         self.add_option("--volume", mapping)
 
     def change_user_id(self, runas_uid, runas_gid):
-        """Adds a specific user mapping between host user and user in the podman container.
-        Some containers, such as Zap, focused on docker require this to prevent UID mismatch.
-        This function aims as preparing a specific UID/GID mapping so that a particular UID/GID maps to the host user
+        """
+        Specify a container user ID to which the current user should be mapped to.
+        This is sometimes required because rootless podman uses Linux' subUIDs.
+
+        If podman version >= 4.3, use the `--userns keep-id:uid=X,gid=Y`
+        otherwise, call `change_user_id_workaround()` to manually create a user mapping
+        """
+        logging.info(f"Current user mapped to container user {runas_uid}:{runas_gid}")
+        try:
+            vers = json.loads(
+                subprocess.run(
+                    ["podman", "version", "--format", "json"],
+                    stdout=subprocess.PIPE,
+                    check=True,
+                ).stdout.decode("utf-8")
+            )
+            major, minor = map(int, vers["Client"]["Version"].split(".")[:2])
+            logging.debug(f"podman version: {vers}. Major: {major}, minor: {minor}")
+            if major < 4 or (major == 4 and minor < 3):
+                # podman < 4.3.0 : the option `keep-id:uid=1000,gid=1000` is not present, we need a workaround
+                self.change_user_id_workaround(runas_uid, runas_gid)
+            else:
+                # use option: keep-id:uid=1000,gid=1000
+                self.add_option("--userns", f"keep-id:uid={runas_uid},gid={runas_gid}")
+
+        except json.JSONDecodeError as exc:
+            raise RuntimeError(
+                f"Unable to parse `podman version` output: {exc}"
+            ) from exc
+        except (KeyError, AttributeError) as exc:
+            raise RuntimeError(
+                f"Unexpected podman version output: Version not found: {exc}"
+            ) from exc
+        except ValueError as exc:
+            raise RuntimeError(
+                f"Unexpected podman version output: unable to decode major/minor version: {exc}"
+            ) from exc
+
+    def change_user_id_workaround(self, runas_uid, runas_gid):
+        """This function aims as preparing a specific UID/GID mapping so that a particular UID/GID maps to the host user
+        Should be called only for podman < 4.3
         source of the hack :
         https://github.com/containers/podman/blob/main/troubleshooting.md#39-podman-run-fails-with-error-unrecognized-namespace-mode-keep-iduid1000gid1000-passed
         """
-
         subuid_size = self.DEFAULT_ID_MAPPING_MAP_SIZE - 1
         subgid_size = self.DEFAULT_ID_MAPPING_MAP_SIZE - 1
 
@@ -144,17 +181,33 @@ def change_user_id(self, runas_uid, runas_gid):
             raise RuntimeError(f"Unable to retrieve podman UID mapping: {exc}") from exc
 
         # UID mapping
-        self.add_option("--uidmap", f"0:1:{runas_uid}")
-        self.add_option("--uidmap", f"{runas_uid}:0:1")
-        self.add_option(
-            "--uidmap", f"{runas_uid+1}:{runas_uid+1}:{subuid_size-runas_uid}"
-        )
+        if subuid_size >= runas_uid:
+            self.add_option("--uidmap", f"0:1:{runas_uid}")
+            self.add_option("--uidmap", f"{runas_uid}:0:1")
+            self.add_option(
+                "--uidmap", f"{runas_uid+1}:{runas_uid+1}:{subuid_size-runas_uid}"
+            )
+            logging.debug(
+                "podman enabled UID mapping arguments (using uidmap workaround)"
+            )
+        else:
+            raise RuntimeError(
+                "subUIDs seem to be disabled/misconfigured for the current user. \
+                Rootless podman can not run without subUIDs"
+            )
 
         # GID mapping
-        self.add_option("--gidmap", f"0:1:{runas_gid}")
-        self.add_option("--gidmap", f"{runas_gid}:0:1")
-        self.add_option(
-            "--gidmap", f"{runas_gid+1}:{runas_gid+1}:{subgid_size-runas_gid}"
-        )
-
-        logging.debug("podman enabled UID/GID mapping arguments")
+        if subgid_size >= runas_gid:
+            self.add_option("--gidmap", f"0:1:{runas_gid}")
+            self.add_option("--gidmap", f"{runas_gid}:0:1")
+            self.add_option(
+                "--gidmap", f"{runas_gid+1}:{runas_gid+1}:{subgid_size-runas_gid}"
+            )
+            logging.debug(
+                "podman enabled GID mapping arguments (using uidmap workaround)"
+            )
+        else:
+            raise RuntimeError(
+                "subGIDs seem to be disabled/misconfigured for the current user. \
+                Rootless podman can not run without subGIDs"
+            )
diff --git a/tests/scanners/test_podman_wrapper.py b/tests/scanners/test_podman_wrapper.py
@@ -1,18 +1,70 @@
 import shutil
-
 import pytest
+import subprocess
+
+from unittest.mock import patch
 
 from scanners.podman_wrapper import PodmanWrapper
 
 
-@pytest.mark.skipif(
-    shutil.which("podman") == False, reason="podman is required for this test"
-)
-def test_podman_mappings():
+@patch("scanners.podman_wrapper.subprocess.run")
+def test_change_user_id(mock_subprocess):
     wrap = PodmanWrapper(app_name="pytest", scan_name="pytest", image="nothing")
 
+    version = '{"Client":{"APIVersion":"5.2.2","Version":"5.2.2","GoVersion":"go1.22.6","GitCommit":"","BuiltTime":"Wed Aug 21 02:00:00 2024","Built":1724198400,"OsArch":"linux/amd64","Os":"linux"}}'
+    run = subprocess.CompletedProcess(args=None, returncode=0, stdout=version.encode('utf-8'))
+
+    mock_subprocess.return_value = run
+
     wrap.change_user_id(1000, 1000)
 
+    i = wrap.opts.index("--userns")
+    assert wrap.opts[i + 1] == "keep-id:uid=1000,gid=1000"
+
+@patch("scanners.podman_wrapper.subprocess.run")
+def test_change_user_id_workaround(mock_subprocess):
+    wrap = PodmanWrapper(app_name="pytest", scan_name="pytest", image="nothing")
+
+    info = """
+{
+  "host": {
+    "idMappings": {
+      "gidmap": [
+        {
+          "container_id": 0,
+          "host_id": 1000,
+          "size": 1
+        },
+        {
+          "container_id": 1,
+          "host_id": 524288,
+          "size": 65536
+        }
+      ],
+      "uidmap": [
+        {
+          "container_id": 0,
+          "host_id": 1000,
+          "size": 1
+        },
+        {
+          "container_id": 1,
+          "host_id": 524288,
+          "size": 65536
+        }
+      ]
+    }
+  }
+}
+"""
+
+
+    run = subprocess.CompletedProcess(args=None, returncode=0, stdout=info.encode('utf-8'))
+
+    mock_subprocess.return_value = run
+
+    wrap.change_user_id_workaround(1000, 1000)
+
     assert "--uidmap" in wrap.opts
     assert "0:1:1000" in wrap.opts
     assert "--gidmap" in wrap.opts