*: fix several issues with namespace path handling

.gitignore

@@ -7,6 +7,7 @@ vendor/pkg
 /contrib/cmd/fs-idmap/fs-idmap
 /contrib/cmd/memfd-bind/memfd-bind
 /contrib/cmd/pidfd-kill/pidfd-kill
+/contrib/cmd/remap-rootfs/remap-rootfs
 man/man8
 release
 Vagrantfile

Makefile

@@ -71,10 +71,10 @@ runc-bin: runc-dmz
 	$(GO_BUILD) -o runc .
 
 .PHONY: all
-all: runc recvtty sd-helper seccompagent fs-idmap memfd-bind pidfd-kill
+all: runc recvtty sd-helper seccompagent fs-idmap memfd-bind pidfd-kill remap-rootfs
 
-.PHONY: recvtty sd-helper seccompagent fs-idmap memfd-bind pidfd-kill
-recvtty sd-helper seccompagent fs-idmap memfd-bind pidfd-kill:
+.PHONY: recvtty sd-helper seccompagent fs-idmap memfd-bind pidfd-kill remap-rootfs
+recvtty sd-helper seccompagent fs-idmap memfd-bind pidfd-kill remap-rootfs:
 	$(GO_BUILD) -o contrib/cmd/$@/$@ ./contrib/cmd/$@
 
 .PHONY: static

contrib/cmd/remap-rootfs/remap-rootfs.go

@@ -0,0 +1,143 @@
+package main
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"syscall"
+
+	"github.com/urfave/cli"
+
+	"github.com/opencontainers/runtime-spec/specs-go"
+)
+
+const usage = `contrib/cmd/remap-rootfs
+
+remap-rootfs is a helper tool to remap the root filesystem of a Open Container
+Initiative bundle using user namespaces such that the file owners are remapped
+from "host" mappings to the user namespace's mappings.
+
+Effectively, this is a slightly more complicated 'chown -R', and is primarily
+used within runc's integration tests to remap the test filesystem to match the
+test user namespace. Note that calling remap-rootfs multiple times, or changing
+the mapping and then calling remap-rootfs will likely produce incorrect results
+because we do not "un-map" any pre-applied mappings from previous remap-rootfs
+calls.
+
+Note that the bundle is assumed to be produced by a trusted source, and thus
+malicious configuration files will likely not be handled safely.
+
+To use remap-rootfs, simply pass it the path to an OCI bundle (a directory
+containing a config.json):
+
+    $ sudo remap-rootfs ./bundle
+`
+
+func toHostID(mappings []specs.LinuxIDMapping, id uint32) (int, bool) {
+	for _, m := range mappings {
+		if m.ContainerID <= id && id < m.ContainerID+m.Size {
+			return int(m.HostID + id), true
+		}
+	}
+	return -1, false
+}
+
+type inodeID struct {
+	Dev, Ino uint64
+}
+
+func toInodeID(st *syscall.Stat_t) inodeID {
+	return inodeID{Dev: st.Dev, Ino: st.Ino}
+}
+
+func remapRootfs(root string, uidMap, gidMap []specs.LinuxIDMapping) error {
+	seenInodes := make(map[inodeID]struct{})
+	return filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		mode := info.Mode()
+		st := info.Sys().(*syscall.Stat_t)
+
+		// Skip symlinks.
+		if mode.Type() == os.ModeSymlink {
+			return nil
+		}
+		// Skip hard-links to files we've already remapped.
+		id := toInodeID(st)
+		if _, seen := seenInodes[id]; seen {
+			return nil
+		}
+		seenInodes[id] = struct{}{}
+
+		// Calculate the new uid:gid.
+		uid := st.Uid
+		newUID, ok1 := toHostID(uidMap, uid)
+		gid := st.Gid
+		newGID, ok2 := toHostID(gidMap, gid)
+
+		// Skip files that cannot be mapped.
+		if !ok1 || !ok2 {
+			niceName := path
+			if relName, err := filepath.Rel(root, path); err == nil {
+				niceName = "/" + relName
+			}
+			fmt.Printf("skipping file %s: cannot remap user %d:%d -> %d:%d\n", niceName, uid, gid, newUID, newGID)
+			return nil
+		}
+		if err := os.Lchown(path, newUID, newGID); err != nil {
+			return err
+		}
+		// Re-apply any setid bits that would be cleared due to chown(2).
+		return os.Chmod(path, mode)
+	})
+}
+
+func main() {
+	app := cli.NewApp()
+	app.Name = "remap-rootfs"
+	app.Usage = usage
+
+	app.Action = func(ctx *cli.Context) error {
+		args := ctx.Args()
+		if len(args) != 1 {
+			return errors.New("exactly one bundle argument must be provided")
+		}
+		bundle := args[0]
+
+		configFile, err := os.Open(filepath.Join(bundle, "config.json"))
+		if err != nil {
+			return err
+		}
+		defer configFile.Close()
+
+		var spec specs.Spec
+		if err := json.NewDecoder(configFile).Decode(&spec); err != nil {
+			return fmt.Errorf("parsing config.json: %w", err)
+		}
+
+		if spec.Root == nil {
+			return errors.New("invalid config.json: root section is null")
+		}
+		rootfs := filepath.Join(bundle, spec.Root.Path)
+
+		if spec.Linux == nil {
+			return errors.New("invalid config.json: linux section is null")
+		}
+		uidMap := spec.Linux.UIDMappings
+		gidMap := spec.Linux.GIDMappings
+		if len(uidMap) == 0 && len(gidMap) == 0 {
+			fmt.Println("skipping remapping -- no userns mappings specified")
+			return nil
+		}
+
+		return remapRootfs(rootfs, uidMap, gidMap)
+	}
+	if err := app.Run(os.Args); err != nil {
+		fmt.Fprintln(os.Stderr, "error:", err)
+		os.Exit(1)
+	}
+}

libcontainer/configs/config_linux.go

@@ -1,12 +1,13 @@
 package configs
 
-import "errors"
+import (
+	"errors"
+	"fmt"
+)
 
 var (
-	errNoUIDMap   = errors.New("User namespaces enabled, but no uid mappings found.")
-	errNoUserMap  = errors.New("User namespaces enabled, but no user mapping found.")
-	errNoGIDMap   = errors.New("User namespaces enabled, but no gid mappings found.")
-	errNoGroupMap = errors.New("User namespaces enabled, but no group mapping found.")
+	errNoUIDMap = errors.New("user namespaces enabled, but no uid mappings found")
+	errNoGIDMap = errors.New("user namespaces enabled, but no gid mappings found")
 )
 
 // Please check https://man7.org/linux/man-pages/man2/personality.2.html for const details.
@@ -31,7 +32,7 @@ func (c Config) HostUID(containerId int) (int, error) {
 		}
 		id, found := c.hostIDFromMapping(containerId, c.UIDMappings)
 		if !found {
-			return -1, errNoUserMap
+			return -1, fmt.Errorf("user namespaces enabled, but no mapping found for uid %d", containerId)
 		}
 		return id, nil
 	}
@@ -54,7 +55,7 @@ func (c Config) HostGID(containerId int) (int, error) {
 		}
 		id, found := c.hostIDFromMapping(containerId, c.GIDMappings)
 		if !found {
-			return -1, errNoGroupMap
+			return -1, fmt.Errorf("user namespaces enabled, but no mapping found for gid %d", containerId)
 		}
 		return id, nil
 	}

libcontainer/configs/validate/rootless.go

@@ -2,6 +2,7 @@ package validate
 
 import (
 	"errors"
+	"fmt"
 	"strconv"
 	"strings"
 
@@ -28,25 +29,18 @@ func rootlessEUIDCheck(config *configs.Config) error {
 	return nil
 }
 
-func hasIDMapping(id int, mappings []configs.IDMap) bool {
-	for _, m := range mappings {
-		if id >= m.ContainerID && id < m.ContainerID+m.Size {
-			return true
-		}
-	}
-	return false
-}
-
 func rootlessEUIDMappings(config *configs.Config) error {
 	if !config.Namespaces.Contains(configs.NEWUSER) {
 		return errors.New("rootless container requires user namespaces")
 	}
-
-	if len(config.UIDMappings) == 0 {
-		return errors.New("rootless containers requires at least one UID mapping")
-	}
-	if len(config.GIDMappings) == 0 {
-		return errors.New("rootless containers requires at least one GID mapping")
+	// We only require mappings if we are not joining another userns.
+	if path := config.Namespaces.PathOf(configs.NEWUSER); path == "" {
+		if len(config.UIDMappings) == 0 {
+			return errors.New("rootless containers requires at least one UID mapping")
+		}
+		if len(config.GIDMappings) == 0 {
+			return errors.New("rootless containers requires at least one GID mapping")
+		}
 	}
 	return nil
 }
@@ -68,8 +62,8 @@ func rootlessEUIDMount(config *configs.Config) error {
 					// Ignore unknown mount options.
 					continue
 				}
-				if !hasIDMapping(uid, config.UIDMappings) {
-					return errors.New("cannot specify uid= mount options for unmapped uid in rootless containers")
+				if _, err := config.HostUID(uid); err != nil {
+					return fmt.Errorf("cannot specify uid=%d mount option for rootless container: %w", uid, err)
 				}
 			}
 
@@ -79,8 +73,8 @@ func rootlessEUIDMount(config *configs.Config) error {
 					// Ignore unknown mount options.
 					continue
 				}
-				if !hasIDMapping(gid, config.GIDMappings) {
-					return errors.New("cannot specify gid= mount options for unmapped gid in rootless containers")
+				if _, err := config.HostGID(gid); err != nil {
+					return fmt.Errorf("cannot specify gid=%d mount option for rootless container: %w", gid, err)
 				}
 			}
 		}

libcontainer/configs/validate/validator.go

@@ -104,11 +104,19 @@ func security(config *configs.Config) error {
 func namespaces(config *configs.Config) error {
 	if config.Namespaces.Contains(configs.NEWUSER) {
 		if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) {
-			return errors.New("USER namespaces aren't enabled in the kernel")
+			return errors.New("user namespaces aren't enabled in the kernel")
 		}
+		hasPath := config.Namespaces.PathOf(configs.NEWUSER) != ""
+		hasMappings := config.UIDMappings != nil || config.GIDMappings != nil
+		if !hasPath && !hasMappings {
+			return errors.New("user namespaces enabled, but no namespace path to join nor mappings to apply specified")
+		}
+		// The hasPath && hasMappings validation case is handled in specconv --
+		// we cache the mappings in Config during specconv in the hasPath case,
+		// so we cannot do that validation here.
 	} else {
 		if config.UIDMappings != nil || config.GIDMappings != nil {
-			return errors.New("User namespace mappings specified, but USER namespace isn't enabled in the config")
+			return errors.New("user namespace mappings specified, but user namespace isn't enabled in the config")
 		}
 	}
 
@@ -122,6 +130,11 @@ func namespaces(config *configs.Config) error {
 		if _, err := os.Stat("/proc/self/timens_offsets"); os.IsNotExist(err) {
 			return errors.New("time namespaces aren't enabled in the kernel")
 		}
+		hasPath := config.Namespaces.PathOf(configs.NEWTIME) != ""
+		hasOffsets := config.TimeOffsets != nil
+		if hasPath && hasOffsets {
+			return errors.New("time namespace enabled, but both namespace path and time offsets specified -- you may only provide one")
+		}
 	} else {
 		if config.TimeOffsets != nil {
 			return errors.New("time namespace offsets specified, but time namespace isn't enabled in the config")

libcontainer/configs/validate/validator_test.go

@@ -170,7 +170,7 @@ func TestValidateSecurityWithoutNEWNS(t *testing.T) {
 	}
 }
 
-func TestValidateUsernamespace(t *testing.T) {
+func TestValidateUserNamespace(t *testing.T) {
 	if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) {
 		t.Skip("Test requires userns.")
 	}
@@ -181,6 +181,8 @@ func TestValidateUsernamespace(t *testing.T) {
 				{Type: configs.NEWUSER},
 			},
 		),
+		UIDMappings: []configs.IDMap{{HostID: 0, ContainerID: 123, Size: 100}},
+		GIDMappings: []configs.IDMap{{HostID: 0, ContainerID: 123, Size: 100}},
 	}
 
 	err := Validate(config)
@@ -189,11 +191,11 @@ func TestValidateUsernamespace(t *testing.T) {
 	}
 }
 
-func TestValidateUsernamespaceWithoutUserNS(t *testing.T) {
-	uidMap := configs.IDMap{ContainerID: 123}
+func TestValidateUsernsMappingWithoutNamespace(t *testing.T) {
 	config := &configs.Config{
 		Rootfs:      "/var",
-		UIDMappings: []configs.IDMap{uidMap},
+		UIDMappings: []configs.IDMap{{HostID: 0, ContainerID: 123, Size: 100}},
+		GIDMappings: []configs.IDMap{{HostID: 0, ContainerID: 123, Size: 100}},
 	}
 
 	err := Validate(config)
@@ -221,6 +223,29 @@ func TestValidateTimeNamespace(t *testing.T) {
 	}
 }
 
+func TestValidateTimeNamespaceWithBothPathAndTimeOffset(t *testing.T) {
+	if _, err := os.Stat("/proc/self/ns/time"); os.IsNotExist(err) {
+		t.Skip("Test requires timens.")
+	}
+	config := &configs.Config{
+		Rootfs: "/var",
+		Namespaces: configs.Namespaces(
+			[]configs.Namespace{
+				{Type: configs.NEWTIME, Path: "/proc/1/ns/time"},
+			},
+		),
+		TimeOffsets: map[string]specs.LinuxTimeOffset{
+			"boottime":  {Secs: 150, Nanosecs: 314159},
+			"monotonic": {Secs: 512, Nanosecs: 271818},
+		},
+	}
+
+	err := Validate(config)
+	if err == nil {
+		t.Error("Expected error to occur but it was nil")
+	}
+}
+
 func TestValidateTimeOffsetsWithoutTimeNamespace(t *testing.T) {
 	config := &configs.Config{
 		Rootfs: "/var",

libcontainer/init_linux.go

@@ -494,15 +494,6 @@ func setupUser(config *initConfig) error {
 		}
 	}
 
-	// Rather than just erroring out later in setuid(2) and setgid(2), check
-	// that the user is mapped here.
-	if _, err := config.Config.HostUID(execUser.Uid); err != nil {
-		return errors.New("cannot set uid to unmapped user in user namespace")
-	}
-	if _, err := config.Config.HostGID(execUser.Gid); err != nil {
-		return errors.New("cannot set gid to unmapped user in user namespace")
-	}
-
 	if config.RootlessEUID {
 		// We cannot set any additional groups in a rootless container and thus
 		// we bail if the user asked us to do so. TODO: We currently can't do
@@ -538,9 +529,15 @@ func setupUser(config *initConfig) error {
 	}
 
 	if err := unix.Setgid(execUser.Gid); err != nil {
+		if err == unix.EINVAL {
+			return fmt.Errorf("cannot setgid to unmapped gid %d in user namespace", execUser.Gid)
+		}
 		return err
 	}
 	if err := unix.Setuid(execUser.Uid); err != nil {
+		if err == unix.EINVAL {
+			return fmt.Errorf("cannot setuid to unmapped uid %d in user namespace", execUser.Uid)
+		}
 		return err
 	}