From b95af5c672c3a9e8299ee6dc99a5ab5b3d14560f Mon Sep 17 00:00:00 2001 From: Noam Yorav-Raphael Date: Fri, 30 Aug 2024 13:35:56 +0300 Subject: [PATCH 1/5] Make build root read-only, so `mkdir /homeless-shelter` would fail with permission error. --- src/libstore/unix/build/local-derivation-goal.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 01a133766d9..c53bbab7581 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -771,6 +771,9 @@ void LocalDerivationGoal::startBuilder() pathsInChroot.erase(worker.store.printStorePath(*i.second.second)); } + // Make build root read-only, so `mkdir /homeless-shelter` would fail. + chmod_(chrootRootDir, 01555); + if (cgroup) { if (mkdir(cgroup->c_str(), 0755) != 0) throw SysError("creating cgroup '%s'", *cgroup); From a1337f5a0cfd5587b05373f0c87ac361f20e9b76 Mon Sep 17 00:00:00 2001 From: Noam Yorav-Raphael Date: Fri, 30 Aug 2024 18:27:19 +0300 Subject: [PATCH 2/5] Add test that `mkdir $HOME` fails with permission error --- tests/functional/build.sh | 5 +++++ tests/functional/mkdir-home-failing.nix | 8 ++++++++ 2 files changed, 13 insertions(+) create mode 100644 tests/functional/mkdir-home-failing.nix diff --git a/tests/functional/build.sh b/tests/functional/build.sh index 5396a465fe3..9906803c9b6 100755 --- a/tests/functional/build.sh +++ b/tests/functional/build.sh @@ -152,6 +152,11 @@ nix build --impure -f multiple-outputs.nix --json e --no-link \ (.outputs | keys == ["a_a", "b"])) ' +# Make sure that `mkdir $HOME fails with a "Permission denied" error` +out="$(nix build -f mkdir-home-failing.nix -L 2>&1)" && status=0 || status=$? +test "$status" = 1 +<<<"$out" grepQuiet -E "Permission denied" + # Make sure that `--stdin` works and does not apply any defaults printf "" | nix build --no-link --stdin --json | jq --exit-status '. == []' printf "%s\n" "$drv^*" | nix build --no-link --stdin --json | jq --exit-status '.[0]|has("drvPath")' diff --git a/tests/functional/mkdir-home-failing.nix b/tests/functional/mkdir-home-failing.nix new file mode 100644 index 00000000000..8f0ac39ba10 --- /dev/null +++ b/tests/functional/mkdir-home-failing.nix @@ -0,0 +1,8 @@ +with import ./config.nix; +mkDerivation { + name = "mkdir-home-no-permission"; + builder = builtins.toFile "builder.sh" + '' + mkdir $HOME + ''; +} From 89ace85071ea5672dadcdd1fa549d55f13186ae9 Mon Sep 17 00:00:00 2001 From: Noam Yorav-Raphael Date: Sun, 1 Sep 2024 11:48:01 +0300 Subject: [PATCH 3/5] Also allow "Operation not permitted" for `mkdir $HOME` --- tests/functional/build.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/build.sh b/tests/functional/build.sh index 9906803c9b6..5206940af9b 100755 --- a/tests/functional/build.sh +++ b/tests/functional/build.sh @@ -152,10 +152,10 @@ nix build --impure -f multiple-outputs.nix --json e --no-link \ (.outputs | keys == ["a_a", "b"])) ' -# Make sure that `mkdir $HOME fails with a "Permission denied" error` +# Make sure that `mkdir $HOME` fails with a "Permission denied" or "Operation not permitted" error out="$(nix build -f mkdir-home-failing.nix -L 2>&1)" && status=0 || status=$? test "$status" = 1 -<<<"$out" grepQuiet -E "Permission denied" +<<<"$out" grepQuiet -E "Permission denied" || <<<"$out" grepQuiet -E "Operation not permitted" # Make sure that `--stdin` works and does not apply any defaults printf "" | nix build --no-link --stdin --json | jq --exit-status '. == []' From d6fd7eee563f6d7ac4e8c8d5aaecc9dba653eac7 Mon Sep 17 00:00:00 2001 From: Noam Yorav-Raphael Date: Mon, 9 Sep 2024 19:08:11 +0300 Subject: [PATCH 4/5] chmod later --- src/libstore/unix/build/local-derivation-goal.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index c53bbab7581..4ca0cdfb7e4 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -771,9 +771,6 @@ void LocalDerivationGoal::startBuilder() pathsInChroot.erase(worker.store.printStorePath(*i.second.second)); } - // Make build root read-only, so `mkdir /homeless-shelter` would fail. - chmod_(chrootRootDir, 01555); - if (cgroup) { if (mkdir(cgroup->c_str(), 0755) != 0) throw SysError("creating cgroup '%s'", *cgroup); @@ -1973,6 +1970,9 @@ void LocalDerivationGoal::runChild() if (rmdir("real-root") == -1) throw SysError("cannot remove real-root directory"); + // Make build root read-only, so `mkdir /homeless-shelter` would fail. + chmod_(chrootRootDir, 01555); + /* Switch to the sandbox uid/gid in the user namespace, which corresponds to the build user or calling user in the parent namespace. */ From fdc5d1e8947bf60643c696443ad5d42e46305d24 Mon Sep 17 00:00:00 2001 From: Noam Yorav-Raphael Date: Mon, 9 Sep 2024 19:11:31 +0300 Subject: [PATCH 5/5] Oops - after chroot, the root is `/`. --- src/libstore/unix/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 4ca0cdfb7e4..168bf45319b 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -1971,7 +1971,7 @@ void LocalDerivationGoal::runChild() throw SysError("cannot remove real-root directory"); // Make build root read-only, so `mkdir /homeless-shelter` would fail. - chmod_(chrootRootDir, 01555); + chmod_("/", 0555); /* Switch to the sandbox uid/gid in the user namespace, which corresponds to the build user or calling user in