nixos-rebuild-ng: fix too long TMPDIR detection

This code tried to compute a `TMPDIR` short enough to guarantee OpenSSH could create a unix socket underneath it. However, that computation was flawed, as the logic predates our recent change from `%n` to `%C`: https://github.com/NixOS/nixpkgs/commit/f23bab46664a3a511fa10d3dcdfb8a9322b14502 I ended up refactoring things here to put the ssh logic close to the tmpdir logic. I think that's appropriate as they really need to stay in sync, and this will hopefully help future developers keep them in sync. (I briefly explored trying to write the code in a way that forces them to stay in sync, but I abandoned that, as it made the code too obtuse.) I've documented all the implementation details we depend on. I also removed our assumptions on the behavior of Python's `tempfile` module: we now just ask it for a tempdir and check the resulting length. Now nothing will have to change in the future if Python starts using 9 characters instead of 8 for avoiding name conflicts. For the record, we ran into this over in ngipkgs: https://github.com/ngi-nix/ngipkgs/issues/2300, which is what triggered this investigation.
2026-07-01 15:26:54 -07:00
parent 0f6198ead0
commit b484df8760
3 changed files with 126 additions and 22 deletions
@@ -25,7 +25,7 @@ SSH_DEFAULT_OPTS: Final = [
    "-o",
    "ControlMaster=auto",
    "-o",
-    f"ControlPath={tmpdir.TMPDIR_PATH / 'ssh-%C'}",
+    f"ControlPath={tmpdir.SSH_CONTROL_PATH}",
    "-o",
    "ControlPersist=60",
 ]
@@ -1,38 +1,86 @@
 import logging
+import os
 from pathlib import Path
-from tempfile import TemporaryDirectory, gettempdir
+from tempfile import TemporaryDirectory
 from typing import Final

 logger: Final = logging.getLogger(__name__)

+# The Linux kernel hardcodes a limit of 108 bytes for Unix sockets [0],
+# but that includes one NULL byte at the very end, so the logical max
+# length is 107 bytes.
+# [0]: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/uapi/linux/un.h?h=v6.18.37#n7
+LINUX_MAX_SOCKET_LENGTH: Final = 107

-# Very long tmp dirs lead to "too long for Unix domain socket"
-# SSH ControlPath errors. Especially macOS sets long TMPDIR paths.
-# This is also required for Linux, if the user tries to build
-# from inside a shell using `--target-host`, which will cause
-# ssh to fail with "ControlPath too long"
+# OpenSSH expands %C to `conn_hash_hex` [0],
+# which is the result of calling `ssh_connection_hash` [1],
+# which computes a sha1 digest [2],
+# which is 20 bytes long [3].
+# which gets hex encoded [4] to double that length [5].
 #
-# The constant is based on a worst case example FQDN, e.g.:
-# `ec2-123-123-123-123.ap-southeast-2.compute.amazonaws.com` (56 bytes).
-# The `ControlPath` can maximum be 108 bytes. Given the prefix
-# that is used for the tempdir, ie. `nixos-rebuild.47i6dz8c` (22 bytes),
-# we have 30 bytes left to work with.
-# This should be fine for the usual temp folders:
-# /tmp/tmp.7hBqN2Fm5H (19)
-# /var/tmp/tmp.7hBqN2Fm5H (23)
-# /run/user/1000/tmp.7hBqN2Fm5H (29)
+# [0]: https://github.com/openssh/openssh-portable/blob/V_10_3_P1/sshconnect.h#L70
+# [1]: https://github.com/openssh/openssh-portable/blob/V_10_3_P1/ssh.c#L1464-L1465
+# [2]: https://github.com/openssh/openssh-portable/blob/V_10_3_P1/readconf.c#L345
+# [3]: https://github.com/openssh/openssh-portable/blob/V_10_3_P1/openbsd-compat/sha1.h#L15
+# [4]: https://github.com/openssh/openssh-portable/blob/V_10_3_P1/readconf.c#L360
+# [5]: https://github.com/openssh/openssh-portable/blob/V_10_3_P1/misc.c#L1627
+OPENSSH_PERCENT_C_LENGTH: Final = 40
+
+# OpenSSH adds a suffix to the given control path [0].
+# There's 1 character for a `.` separator, followed by 16 random characters [1].
+# [0]: https://github.com/openssh/openssh-portable/blob/V_10_3_P1/mux.c#L1348
+# [1]: https://github.com/openssh/openssh-portable/blob/V_10_3_P1/mux.c#L1324
+OPENSSH_CONTROL_PATH_SUFFIX_LENGTH: Final = 1 + 16
+
+# Carefully compute a maximum allowed length for a tmpdir, otherwise ssh crashes with errors like this:
+# > unix_listener: path "/home/runner/work/_temp/nixos-rebuild.aw8hzmq7/ssh-ea7c10de83787b1dec3f06ef20ee26b38c6bb0a5.x9MDykk4gmASsl3R"
+# > too long for Unix domain socket
+#
+# The full path to our tmpdir must be short enough for the resulting Unix domain
+# sockets that OpenSSH creates to fit within `LINUX_MAX_SOCKET_LENGTH`.
+# It's common for system configured temp dirs to be more
+# than a few characters:
+# - macOS sets long TMPDIR paths.
+# - Nix dev shells set a longer TMPDIR
+# - GitHub actions set TMPDIR to something like `/home/runner/work/_temp`.
+#
+# Breaking down the socket path into its component pieces:
+#
+# /home/runner/work/_temp/nixos-rebuild.aw8hzmq7/ssh-ea7c10de83787b1dec3f06ef20ee26b38c6bb0a5.x9MDykk4gmASsl3R
+# |-------------- TMPDIR ----------------------|^|----------------- ssh-%C -----------------||---------------|
+#                                               |                                                   |
+#                                Note the path separator character.                   OPENSSH_CONTROL_PATH_SUFFIX_LENGTH
+#
+MAX_TMPDIR_LENGTH: Final = (
+    LINUX_MAX_SOCKET_LENGTH
+    - 1  # Path separator between tmpdir and the socket name.
+    # Keep the following in sync with `SSH_CONTROL_PATH`.
+    - len("ssh-")
+    - OPENSSH_PERCENT_C_LENGTH
+    - OPENSSH_CONTROL_PATH_SUFFIX_LENGTH
+)
+
+
 def make_tmpdir() -> TemporaryDirectory[str]:
-    tmp = gettempdir()
-    if len(tmp) >= 30:
+    prefix = "nixos-rebuild."
+    tmpdir = TemporaryDirectory(prefix=prefix)
+    if len(os.fsencode(tmpdir.name)) > MAX_TMPDIR_LENGTH:
+        short_tmpdir = TemporaryDirectory(prefix=prefix, dir="/tmp")
        logger.debug(
-            "tempdir '%s' exceeds 30 bytes limit, defaulting to /tmp instead",
-            tmp,
+            "tempdir '%s' exceeds %s bytes limit, defaulting to '%s' instead",
+            tmpdir,
+            MAX_TMPDIR_LENGTH,
+            short_tmpdir,
        )

-        return TemporaryDirectory(prefix="nixos-rebuild.", dir="/tmp")
+        tmpdir.cleanup()
+        return short_tmpdir

-    return TemporaryDirectory(prefix="nixos-rebuild.")
+    return tmpdir


 TMPDIR: Final = make_tmpdir()
 TMPDIR_PATH: Final = Path(TMPDIR.name)
+
+# Keep this in sync with `MAX_TMPDIR_LENGTH`!
+SSH_CONTROL_PATH: Final = str(TMPDIR_PATH / "ssh-%C")
@@ -0,0 +1,56 @@
+import contextlib
+import os
+import tempfile
+import typing
+from pathlib import Path
+
+from nixos_rebuild.tmpdir import MAX_TMPDIR_LENGTH, make_tmpdir
+
+
+@contextlib.contextmanager
+def system_tempdir(path: Path) -> typing.Generator[None]:
+    path.mkdir(exist_ok=True)
+
+    # `tempfile` caches the tempdir, you must clear it for it to recompute.
+    tempfile.tempdir = None
+
+    og_tmpdir = os.environ.get("TMPDIR")
+
+    try:
+        os.environ["TMPDIR"] = str(path)
+        assert Path(tempfile.gettempdir()) == path
+        yield
+    finally:
+        if og_tmpdir is None:
+            del os.environ["TMPDIR"]
+        else:
+            os.environ["TMPDIR"] = og_tmpdir
+
+
+def test_make_tmpdir() -> None:
+    # Basic test: whatever the default system temp dir happens to be.
+    tmpdir = make_tmpdir()
+    tmp = Path(tmpdir.name)
+    assert tmp.exists()
+    assert tmp.is_dir()
+    assert len(os.fsencode(str(tmp))) <= MAX_TMPDIR_LENGTH
+
+    # Test with a short system temp dir. We should use it unmodified.
+    with system_tempdir(Path("/tmp/not-too-long")):
+        tmpdir = make_tmpdir()
+        tmp = Path(tmpdir.name)
+
+        assert tmp.exists()
+        assert tmp.is_dir()
+        assert len(os.fsencode(str(tmp))) <= MAX_TMPDIR_LENGTH
+
+    # Test with a long system temp dir. We should ignore
+    # it and fall back to something short enough for OpenSSH to
+    # create sockets in.
+    with system_tempdir(Path("/tmp/long" + ("g" * MAX_TMPDIR_LENGTH))):
+        tmpdir = make_tmpdir()
+        tmp = Path(tmpdir.name)
+
+        assert tmp.exists()
+        assert tmp.is_dir()
+        assert len(os.fsencode(str(tmp))) <= MAX_TMPDIR_LENGTH