Repository: opencontainers/runc
Pull request #2850 (2021-03-13 16:13:40)

libcontainer/system: move userns utilities, remove `GetParentNSeuid`, `UIDMapInUserNS`
### libcontainer/system: remove unused `GetParentNSeuid()`

This function was added in https://github.com/opencontainers/runc/commit/f103de57ec89137d86b77584f676b760ec7c7ba4 (https://github.com/opencontainers/runc/pull/1688), but no longer used since https://github.com/opencontainers/runc/commit/06f789cf26774dd64cb2a9cc0b3c6a6ff832733b (https://github.com/opencontainers/runc/pull/1862) (v1.0.0-rc6)

It is no longer used by runc itself, and only used by a  [single external project][3]

[3]: https://grep.app/search?q=.GetParentNSeuid


### libcontainer/system: un-export UIDMapInUserNS()

`UIDMapInUserNS()` is not used anywhere, only internally. so un-export it.


### libcontainer/system: move userns utilities to separate package

Moving these utilities to a separate package, so that consumers of this package don't have to pull in the whole "system" package.

Looking at uses of these utilities (outside of runc itself);

`RunningInUserNS()` is used by [various external consumers][1],
so adding a "Deprecated" alias for this.

[1]: https://grep.app/search?current=2&q=.RunningInUserNS



thaJeztah: Opening as draft; I'll create a PR in Moby to see if it has the desired result, then move it out of draft
thaJeztah: I removed those functions that we don't use; reordered the commits to first remove these parts before moving the code. Moved this out of draft.

I also have a branch to isolate the userns package from the user package, as we only use the `user` package for a single function, and we likely don't need that; https://github.com/opencontainers/runc/pull/2868
thaJeztah: @kolyshkin ptal 🤗 
thaJeztah: @kolyshkin PTAL (I'll rebase https://github.com/opencontainers/runc/pull/2868 after this)
kolyshkin: close/reopen to kick ci
thaJeztah: looks like ci/centos7 doesn't start; let me try another rebase to force CI to run
thaJeztah: close/reopen to trigger fedora CI

Patch:
diff --git a/checkpoint.go b/checkpoint.go
index e80dcc4f..3fe4541d 100644
--- a/checkpoint.go
+++ b/checkpoint.go
@@ -11,7 +11,7 @@ import (
 
 	criu "github.com/checkpoint-restore/go-criu/v5/rpc"
 	"github.com/opencontainers/runc/libcontainer"
-	"github.com/opencontainers/runc/libcontainer/system"
+	"github.com/opencontainers/runc/libcontainer/userns"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
@@ -48,7 +48,7 @@ checkpointed.`,
 			return err
 		}
 		// XXX: Currently this is untested with rootless containers.
-		if os.Geteuid() != 0 || system.RunningInUserNS() {
+		if os.Geteuid() != 0 || userns.RunningInUserNS() {
 			logrus.Warn("runc checkpoint is untested with rootless containers")
 		}
 
diff --git a/libcontainer/cgroups/fs/devices.go b/libcontainer/cgroups/fs/devices.go
index 9a37e68e..a098e3b7 100644
--- a/libcontainer/cgroups/fs/devices.go
+++ b/libcontainer/cgroups/fs/devices.go
@@ -12,7 +12,7 @@ import (
 	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/devices"
-	"github.com/opencontainers/runc/libcontainer/system"
+	"github.com/opencontainers/runc/libcontainer/userns"
 )
 
 type DevicesGroup struct {
@@ -55,7 +55,7 @@ func buildEmulator(rules []*devices.Rule) (*cgroupdevices.Emulator, error) {
 }
 
 func (s *DevicesGroup) Set(path string, cgroup *configs.Cgroup) error {
-	if system.RunningInUserNS() || cgroup.SkipDevices {
+	if userns.RunningInUserNS() || cgroup.SkipDevices {
 		return nil
 	}
 
diff --git a/libcontainer/cgroups/fs2/devices.go b/libcontainer/cgroups/fs2/devices.go
index 99559fec..61c00a96 100644
--- a/libcontainer/cgroups/fs2/devices.go
+++ b/libcontainer/cgroups/fs2/devices.go
@@ -7,7 +7,7 @@ import (
 	"github.com/opencontainers/runc/libcontainer/cgroups/ebpf/devicefilter"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/devices"
-	"github.com/opencontainers/runc/libcontainer/system"
+	"github.com/opencontainers/runc/libcontainer/userns"
 
 	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
@@ -36,7 +36,7 @@ func canSkipEBPFError(cgroup *configs.Cgroup) bool {
 	// have the necessary privileges to mknod(2) device inodes or access
 	// host-level instances (though ideally we would be blocking device access
 	// for rootless containers anyway).
-	if system.RunningInUserNS() {
+	if userns.RunningInUserNS() {
 		return true
 	}
 
diff --git a/libcontainer/cgroups/systemd/user.go b/libcontainer/cgroups/systemd/user.go
index 8fe91688..450a2469 100644
--- a/libcontainer/cgroups/systemd/user.go
+++ b/libcontainer/cgroups/systemd/user.go
@@ -13,7 +13,7 @@ import (
 
 	systemdDbus "github.com/coreos/go-systemd/v22/dbus"
 	dbus "github.com/godbus/dbus/v5"
-	"github.com/opencontainers/runc/libcontainer/system"
+	"github.com/opencontainers/runc/libcontainer/userns"
 	"github.com/pkg/errors"
 )
 
@@ -52,7 +52,7 @@ func NewUserSystemdDbus() (*systemdDbus.Conn, error) {
 //
 // Otherwise returns os.Getuid() .
 func DetectUID() (int, error) {
-	if !system.RunningInUserNS() {
+	if !userns.RunningInUserNS() {
 		return os.Getuid(), nil
 	}
 	b, err := exec.Command("busctl", "--user", "--no-pager", "status").CombinedOutput()
diff --git a/libcontainer/cgroups/utils.go b/libcontainer/cgroups/utils.go
index f6d04d58..35ce2c1c 100644
--- a/libcontainer/cgroups/utils.go
+++ b/libcontainer/cgroups/utils.go
@@ -16,7 +16,7 @@ import (
 	"time"
 
 	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
-	"github.com/opencontainers/runc/libcontainer/system"
+	"github.com/opencontainers/runc/libcontainer/userns"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
@@ -37,7 +37,7 @@ func IsCgroup2UnifiedMode() bool {
 		var st unix.Statfs_t
 		err := unix.Statfs(unifiedMountpoint, &st)
 		if err != nil {
-			if os.IsNotExist(err) && system.RunningInUserNS() {
+			if os.IsNotExist(err) && userns.RunningInUserNS() {
 				// ignore the "not found" error if running in userns
 				logrus.WithError(err).Debugf("%s missing, assuming cgroup v1", unifiedMountpoint)
 				isUnified = false
diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go
index 411496ab..ed38e772 100644
--- a/libcontainer/rootfs_linux.go
+++ b/libcontainer/rootfs_linux.go
@@ -19,7 +19,7 @@ import (
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/devices"
-	"github.com/opencontainers/runc/libcontainer/system"
+	"github.com/opencontainers/runc/libcontainer/userns"
 	"github.com/opencontainers/runc/libcontainer/utils"
 	libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils"
 	"github.com/opencontainers/runtime-spec/specs-go"
@@ -603,7 +603,7 @@ func reOpenDevNull() error {
 
 // Create the device nodes in the container.
 func createDevices(config *configs.Config) error {
-	useBindMount := system.RunningInUserNS() || config.Namespaces.Contains(configs.NEWUSER)
+	useBindMount := userns.RunningInUserNS() || config.Namespaces.Contains(configs.NEWUSER)
 	oldMask := unix.Umask(0000)
 	for _, node := range config.Devices {
 
diff --git a/libcontainer/system/linux.go b/libcontainer/system/linux.go
index 49471960..4379a207 100644
--- a/libcontainer/system/linux.go
+++ b/libcontainer/system/linux.go
@@ -3,12 +3,9 @@
 package system
 
 import (
-	"os"
 	"os/exec"
-	"sync"
 	"unsafe"
 
-	"github.com/opencontainers/runc/libcontainer/user"
 	"golang.org/x/sys/unix"
 )
 
@@ -87,52 +84,6 @@ func Setctty() error {
 	return nil
 }
 
-var (
-	inUserNS bool
-	nsOnce   sync.Once
-)
-
-// RunningInUserNS detects whether we are currently running in a user namespace.
-// Originally copied from github.com/lxc/lxd/shared/util.go
-func RunningInUserNS() bool {
-	nsOnce.Do(func() {
-		uidmap, err := user.CurrentProcessUIDMap()
-		if err != nil {
-			// This kernel-provided file only exists if user namespaces are supported
-			return
-		}
-		inUserNS = UIDMapInUserNS(uidmap)
-	})
-	return inUserNS
-}
-
-func UIDMapInUserNS(uidmap []user.IDMap) bool {
-	/*
-	 * We assume we are in the initial user namespace if we have a full
-	 * range - 4294967295 uids starting at uid 0.
-	 */
-	if len(uidmap) == 1 && uidmap[0].ID == 0 && uidmap[0].ParentID == 0 && uidmap[0].Count == 4294967295 {
-		return false
-	}
-	return true
-}
-
-// GetParentNSeuid returns the euid within the parent user namespace
-func GetParentNSeuid() int64 {
-	euid := int64(os.Geteuid())
-	uidmap, err := user.CurrentProcessUIDMap()
-	if err != nil {
-		// This kernel-provided file only exists if user namespaces are supported
-		return euid
-	}
-	for _, um := range uidmap {
-		if um.ID <= euid && euid <= um.ID+um.Count-1 {
-			return um.ParentID + euid - um.ID
-		}
-	}
-	return euid
-}
-
 // SetSubreaper sets the value i as the subreaper setting for the calling process
 func SetSubreaper(i int) error {
 	return unix.Prctl(unix.PR_SET_CHILD_SUBREAPER, uintptr(i), 0, 0, 0)
diff --git a/libcontainer/system/unsupported.go b/libcontainer/system/unsupported.go
deleted file mode 100644
index b94be74a..00000000
--- a/libcontainer/system/unsupported.go
+++ /dev/null
@@ -1,27 +0,0 @@
-// +build !linux
-
-package system
-
-import (
-	"os"
-
-	"github.com/opencontainers/runc/libcontainer/user"
-)
-
-// RunningInUserNS is a stub for non-Linux systems
-// Always returns false
-func RunningInUserNS() bool {
-	return false
-}
-
-// UIDMapInUserNS is a stub for non-Linux systems
-// Always returns false
-func UIDMapInUserNS(uidmap []user.IDMap) bool {
-	return false
-}
-
-// GetParentNSeuid returns the euid within the parent user namespace
-// Always returns os.Geteuid on non-linux
-func GetParentNSeuid() int {
-	return os.Geteuid()
-}
diff --git a/libcontainer/system/userns_deprecated.go b/libcontainer/system/userns_deprecated.go
new file mode 100644
index 00000000..2de3462a
--- /dev/null
+++ b/libcontainer/system/userns_deprecated.go
@@ -0,0 +1,5 @@
+package system
+
+import "github.com/opencontainers/runc/libcontainer/userns"
+
+var RunningInUserNS = userns.RunningInUserNS
diff --git a/libcontainer/userns/userns.go b/libcontainer/userns/userns.go
new file mode 100644
index 00000000..f6cb98e5
--- /dev/null
+++ b/libcontainer/userns/userns.go
@@ -0,0 +1,5 @@
+package userns
+
+// RunningInUserNS detects whether we are currently running in a user namespace.
+// Originally copied from github.com/lxc/lxd/shared/util.go
+var RunningInUserNS = runningInUserNS
diff --git a/libcontainer/system/system_fuzzer.go b/libcontainer/userns/userns_fuzzer.go
similarity index 82%
rename from libcontainer/system/system_fuzzer.go
rename to libcontainer/userns/userns_fuzzer.go
index c1d7630f..529f8eae 100644
--- a/libcontainer/system/system_fuzzer.go
+++ b/libcontainer/userns/userns_fuzzer.go
@@ -1,6 +1,6 @@
 // +build gofuzz
 
-package system
+package userns
 
 import (
 	"strings"
@@ -10,6 +10,6 @@ import (
 
 func FuzzUIDMap(data []byte) int {
 	uidmap, _ := user.ParseIDMap(strings.NewReader(string(data)))
-	_ = UIDMapInUserNS(uidmap)
+	_ = uidMapInUserNS(uidmap)
 	return 1
 }
diff --git a/libcontainer/userns/userns_linux.go b/libcontainer/userns/userns_linux.go
new file mode 100644
index 00000000..724e6df0
--- /dev/null
+++ b/libcontainer/userns/userns_linux.go
@@ -0,0 +1,37 @@
+package userns
+
+import (
+	"sync"
+
+	"github.com/opencontainers/runc/libcontainer/user"
+)
+
+var (
+	inUserNS bool
+	nsOnce   sync.Once
+)
+
+// runningInUserNS detects whether we are currently running in a user namespace.
+// Originally copied from github.com/lxc/lxd/shared/util.go
+func runningInUserNS() bool {
+	nsOnce.Do(func() {
+		uidmap, err := user.CurrentProcessUIDMap()
+		if err != nil {
+			// This kernel-provided file only exists if user namespaces are supported
+			return
+		}
+		inUserNS = uidMapInUserNS(uidmap)
+	})
+	return inUserNS
+}
+
+func uidMapInUserNS(uidmap []user.IDMap) bool {
+	/*
+	 * We assume we are in the initial user namespace if we have a full
+	 * range - 4294967295 uids starting at uid 0.
+	 */
+	if len(uidmap) == 1 && uidmap[0].ID == 0 && uidmap[0].ParentID == 0 && uidmap[0].Count == 4294967295 {
+		return false
+	}
+	return true
+}
diff --git a/libcontainer/userns/userns_unsupported.go b/libcontainer/userns/userns_unsupported.go
new file mode 100644
index 00000000..f45bb0c3
--- /dev/null
+++ b/libcontainer/userns/userns_unsupported.go
@@ -0,0 +1,17 @@
+// +build !linux
+
+package userns
+
+import "github.com/opencontainers/runc/libcontainer/user"
+
+// runningInUserNS is a stub for non-Linux systems
+// Always returns false
+func runningInUserNS() bool {
+	return false
+}
+
+// uidMapInUserNS is a stub for non-Linux systems
+// Always returns false
+func uidMapInUserNS(uidmap []user.IDMap) bool {
+	return false
+}
diff --git a/restore.go b/restore.go
index ec730822..ed69d15e 100644
--- a/restore.go
+++ b/restore.go
@@ -6,7 +6,7 @@ import (
 	"os"
 
 	"github.com/opencontainers/runc/libcontainer"
-	"github.com/opencontainers/runc/libcontainer/system"
+	"github.com/opencontainers/runc/libcontainer/userns"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
 )
@@ -97,7 +97,7 @@ using the runc checkpoint command.`,
 			return err
 		}
 		// XXX: Currently this is untested with rootless containers.
-		if os.Geteuid() != 0 || system.RunningInUserNS() {
+		if os.Geteuid() != 0 || userns.RunningInUserNS() {
 			logrus.Warn("runc checkpoint is untested with rootless containers")
 		}
 
diff --git a/rootless_linux.go b/rootless_linux.go
index 348f7c4f..3c210538 100644
--- a/rootless_linux.go
+++ b/rootless_linux.go
@@ -6,7 +6,7 @@ import (
 	"os"
 
 	"github.com/opencontainers/runc/libcontainer/cgroups/systemd"
-	"github.com/opencontainers/runc/libcontainer/system"
+	"github.com/opencontainers/runc/libcontainer/userns"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
 )
@@ -25,7 +25,7 @@ func shouldUseRootlessCgroupManager(context *cli.Context) (bool, error) {
 	if os.Geteuid() != 0 {
 		return true, nil
 	}
-	if !system.RunningInUserNS() {
+	if !userns.RunningInUserNS() {
 		// euid == 0 , in the initial ns (i.e. the real root)
 		return false, nil
 	}
@@ -60,7 +60,7 @@ func shouldHonorXDGRuntimeDir() bool {
 	if os.Geteuid() != 0 {
 		return true
 	}
-	if !system.RunningInUserNS() {
+	if !userns.RunningInUserNS() {
 		// euid == 0 , in the initial ns (i.e. the real root)
 		// in this case, we should use /run/runc and ignore
 		// $XDG_RUNTIME_DIR (e.g. /run/user/0) for backward


Test patch:
diff --git a/libcontainer/system/linux_test.go b/libcontainer/userns/userns_linux_test.go
similarity index 94%
rename from libcontainer/system/linux_test.go
rename to libcontainer/userns/userns_linux_test.go
index 4d613d84..2ddd3ff9 100644
--- a/libcontainer/system/linux_test.go
+++ b/libcontainer/userns/userns_linux_test.go
@@ -1,6 +1,6 @@
 // +build linux
 
-package system
+package userns
 
 import (
 	"strings"
@@ -37,7 +37,7 @@ func TestUIDMapInUserNS(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		actual := UIDMapInUserNS(uidmap)
+		actual := uidMapInUserNS(uidmap)
 		if c.expected != actual {
 			t.Fatalf("expected %v, got %v for %q", c.expected, actual, c.s)
 		}