From 6d1aa523de8ece115eb0504b2313c12dc5302383 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 8 May 2023 14:41:47 -0400 Subject: [PATCH] Create escape hatch for supplementary group sandboxing woes There is no obvious good solution for this that has occured to anyone. --- src/libstore/build/local-derivation-goal.cc | 13 ++++--------- src/libstore/globals.hh | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 21cd6e7ee..6d2d458da 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -907,15 +907,10 @@ void LocalDerivationGoal::startBuilder() openSlave(); /* Drop additional groups here because we can't do it - after we've created the new user namespace. FIXME: - this means that if we're not root in the parent - namespace, we can't drop additional groups; they will - be mapped to nogroup in the child namespace. There does - not seem to be a workaround for this. (But who can tell - from reading user_namespaces(7)?) - See also https://lwn.net/Articles/621612/. */ - if (getuid() == 0 && setgroups(0, 0) == -1) - throw SysError("setgroups failed"); + after we've created the new user namespace. */ + if (settings.dropSupplementaryGroups) + if (setgroups(0, 0) == -1) + throw SysError("setgroups failed"); ProcessOptions options; options.cloneFlags = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | CLONE_NEWUTS | CLONE_PARENT | SIGCHLD; diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 609cf53b8..0b429cf98 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -515,6 +515,25 @@ public: Setting sandboxFallback{this, true, "sandbox-fallback", "Whether to disable sandboxing when the kernel doesn't allow it."}; + Setting dropSupplementaryGroups{this, getuid() == 0, "drop-supplementary-groups", + R"( + Whether to drop supplementary groups when building with sandboxing. + This is normally a good idea if we are root and have the capability to + do so. + + But if this "root" is mapped from a non-root user in a larger + namespace, we won't be able drop additional groups; they will be + mapped to nogroup in the child namespace. There does not seem to be a + workaround for this. + + (But who can tell from reading user_namespaces(7)? See also https://lwn.net/Articles/621612/.) + + TODO: It might be good to create a middle ground option that allows + `setgroups` to fail if all additional groups are "nogroup" / the value + of `/proc/sys/fs/overflowuid`. This would handle the common + nested-sandboxing case identified above. + )"}; + #if __linux__ Setting sandboxShmSize{ this, "50%", "sandbox-dev-shm-size",