From acbb1523c1dc28043d6dab729db696485938f969 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?=
 <theophane.hufschmitt@tweag.io>
Date: Fri, 12 Apr 2024 15:57:53 +0200
Subject: [PATCH] Fix the access of symlinks to host files in the sandbox

https://github.com/NixOS/nix/pull/10456 fixed the addition of symlink
store paths to the sandbox, but also made it so that the hardcoded
sandbox paths (like `/etc/hosts`) were now bind-mounted without
following the possible symlinks. This made these files unreadable if
there were symlinks (because the sandbox would now contain a symlink to
an unreachable file rather than the underlying file).
In particular, this broke FOD derivations on NixOS as `/etc/hosts` is a
symlink there.

Fix that by canonicalizing all these hardcoded sandbox paths before
adding them to the sandbox.
---
 src/libstore/build/local-derivation-goal.cc | 13 +++--
 tests/functional/linux-sandbox.sh           | 14 +++++-
 tests/functional/symlink-derivation.nix     | 55 +++++++++++++++------
 3 files changed, 62 insertions(+), 20 deletions(-)

diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc
index db12af810..c2af0b270 100644
--- a/src/libstore/build/local-derivation-goal.cc
+++ b/src/libstore/build/local-derivation-goal.cc
@@ -1823,11 +1823,18 @@ void LocalDerivationGoal::runChild()
                     if (pathExists(path))
                         ss.push_back(path);
 
-                if (settings.caFile != "")
-                    pathsInChroot.try_emplace("/etc/ssl/certs/ca-certificates.crt", settings.caFile, true);
+                if (settings.caFile != "" && pathExists(settings.caFile)) {
+                    Path caFile = settings.caFile;
+                    pathsInChroot.try_emplace("/etc/ssl/certs/ca-certificates.crt", canonPath(caFile, true), true);
+                }
             }
 
-            for (auto & i : ss) pathsInChroot.emplace(i, i);
+            for (auto & i : ss) {
+                // For backwards-compatibiliy, resolve all the symlinks in the
+                // chroot paths
+                auto canonicalPath = canonPath(i, true);
+                pathsInChroot.emplace(i, canonicalPath);
+            }
 
             /* Bind-mount all the directories from the "host"
                filesystem that we want in the chroot
diff --git a/tests/functional/linux-sandbox.sh b/tests/functional/linux-sandbox.sh
index 04209277b..880d56fca 100644
--- a/tests/functional/linux-sandbox.sh
+++ b/tests/functional/linux-sandbox.sh
@@ -60,7 +60,11 @@ testCert () {
 
 nocert=$TEST_ROOT/no-cert-file.pem
 cert=$TEST_ROOT/some-cert-file.pem
+symlinkcert=$TEST_ROOT/symlink-cert-file.pem
+symlinkDir=$TEST_ROOT/symlink-dir
 echo -n "CERT_CONTENT" > $cert
+ln -s $cert $symlinkcert
+ln -s $TEST_ROOT $symlinkDir
 
 # No cert in sandbox when not a fixed-output derivation
 testCert missing normal       "$cert"
@@ -74,5 +78,13 @@ testCert missing fixed-output "$nocert"
 # Cert in sandbox when ssl-cert-file is set to an existing file
 testCert present fixed-output "$cert"
 
+# Cert in sandbox when ssl-cert-file is set to a symlink to an existing file
+testCert present fixed-output "$symlinkcert"
+
 # Symlinks should be added in the sandbox directly and not followed
-nix-sandbox-build symlink-derivation.nix
+nix-sandbox-build symlink-derivation.nix -A depends_on_symlink
+nix-sandbox-build symlink-derivation.nix -A test_sandbox_paths \
+    --option extra-sandbox-paths "/file=$cert" \
+    --option extra-sandbox-paths "/dir=$TEST_ROOT" \
+    --option extra-sandbox-paths "/symlinkDir=$symlinkDir" \
+    --option extra-sandbox-paths "/symlink=$symlinkcert"
diff --git a/tests/functional/symlink-derivation.nix b/tests/functional/symlink-derivation.nix
index 17ba37424..e9a74cdce 100644
--- a/tests/functional/symlink-derivation.nix
+++ b/tests/functional/symlink-derivation.nix
@@ -15,22 +15,45 @@ let
     '';
   };
 in
-mkDerivation {
-  name = "depends-on-symlink";
-  buildCommand = ''
-    (
-      set -x
+{
+  depends_on_symlink = mkDerivation {
+    name = "depends-on-symlink";
+    buildCommand = ''
+      (
+        set -x
 
-      # `foo_symlink` should be a symlink pointing to `foo_in_store`
-      [[ -L ${foo_symlink} ]]
-      [[ $(readlink ${foo_symlink}) == ${foo_in_store} ]]
+        # `foo_symlink` should be a symlink pointing to `foo_in_store`
+        [[ -L ${foo_symlink} ]]
+        [[ $(readlink ${foo_symlink}) == ${foo_in_store} ]]
 
-      # `symlink_to_not_in_store` should be a symlink pointing to `./.`, which
-      # is not available in the sandbox
-      [[ -L ${symlink_to_not_in_store} ]]
-      [[ $(readlink ${symlink_to_not_in_store}) == ${builtins.toString ./.} ]]
-      (! ls ${symlink_to_not_in_store}/)
-    )
-    echo "Success!" > $out
-  '';
+        # `symlink_to_not_in_store` should be a symlink pointing to `./.`, which
+        # is not available in the sandbox
+        [[ -L ${symlink_to_not_in_store} ]]
+        [[ $(readlink ${symlink_to_not_in_store}) == ${builtins.toString ./.} ]]
+        (! ls ${symlink_to_not_in_store}/)
+
+        # Native paths
+      )
+      echo "Success!" > $out
+    '';
+  };
+
+  test_sandbox_paths = mkDerivation {
+    # Depends on the caller to set a bunch of `--sandbox-path` arguments
+    name = "test-sandbox-paths";
+    buildCommand = ''
+      (
+        set -x
+        [[ -f /file ]]
+        [[ -d /dir ]]
+
+        # /symlink and /symlinkDir should be available as raw symlinks
+        # (pointing to files outside of the sandbox)
+        [[ -L /symlink ]] && [[ ! -e $(readlink /symlink) ]]
+        [[ -L /symlinkDir ]] && [[ ! -e $(readlink /symlinkDir) ]]
+      )
+
+      touch $out
+    '';
+  };
 }