fixup! mount: add support for ridmap and idmap

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

libcontainer/configs/mount_linux.go

@@ -6,6 +6,11 @@ type MountMapping struct {
 	// Recursive indicates if the mapping needs to be recursive.
 	Recursive bool `json:"recursive"`
 
+	// NamespacePath is a path to a user namespace that indicates the necessary
+	// id-mappings for MOUNT_ATTR_IDMAP. If set to non-"", UIDMappings and
+	// GIDMappings must be set to nil.
+	NamespacePath string `json:"userns_path,omitempty"`
+
 	// UIDMappings is the uid mapping set for this mount, to be used with
 	// MOUNT_ATTR_IDMAP.
 	UIDMappings []IDMap `json:"uid_mappings,omitempty"`

libcontainer/configs/validate/validator.go

@@ -317,8 +317,15 @@ func checkIDMapMounts(config *configs.Config, m *configs.Mount) error {
 	if config.RootlessEUID {
 		return errors.New("id-mapped mounts are not supported for rootless containers")
 	}
-	if len(m.Mapping.UIDMappings) == 0 || len(m.Mapping.GIDMappings) == 0 {
-		return errors.New("id-mapped mounts must have both uid and gid mappings specified")
+	if m.Mapping.NamespacePath == "" {
+		if len(m.Mapping.UIDMappings) == 0 || len(m.Mapping.GIDMappings) == 0 {
+			return errors.New("id-mapped mounts must have both uid and gid mappings specified")
+		}
+	} else {
+		if m.Mapping.UIDMappings != nil || m.Mapping.GIDMappings != nil {
+			// should never happen
+			return errors.New("[internal error] id-mapped mounts cannot have both userns_path and uid and gid mappings specified")
+		}
 	}
 	return nil
 }

libcontainer/mount_linux.go

@@ -223,12 +223,20 @@ func mountFd(nsHandles *userns.Handles, m *configs.Mount) (*mountSource, error)
 			sourceType = mountSourceOpenTree
 
 			// Configure the id mapping.
-			usernsFile, err := nsHandles.Get(userns.Mapping{
-				UIDMappings: m.Mapping.UIDMappings,
-				GIDMappings: m.Mapping.GIDMappings,
-			})
-			if err != nil {
-				return nil, fmt.Errorf("failed to create userns for %s id-mapping: %w", m.Source, err)
+			var usernsFile *os.File
+			if m.Mapping.NamespacePath == "" {
+				usernsFile, err = nsHandles.Get(userns.Mapping{
+					UIDMappings: m.Mapping.UIDMappings,
+					GIDMappings: m.Mapping.GIDMappings,
+				})
+				if err != nil {
+					return nil, fmt.Errorf("failed to create userns for %s id-mapping: %w", m.Source, err)
+				}
+			} else {
+				usernsFile, err = os.Open(m.Mapping.NamespacePath)
+				if err != nil {
+					return nil, fmt.Errorf("failed to open existing userns for %s id-mapping: %w", m.Source, err)
+				}
 			}
 			defer usernsFile.Close()
 

libcontainer/specconv/spec_linux.go

@@ -426,11 +426,16 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
 				return nil, err
 			}
 			// For idmap and ridmap mounts without explicit mappings, use the
-			// ones from the container's userns.
+			// ones from the container's userns. If we are joining another
+			// userns, stash the path.
 			for _, m := range config.Mounts {
 				if m.Mapping != nil && m.Mapping.UIDMappings == nil && m.Mapping.GIDMappings == nil {
-					m.Mapping.UIDMappings = config.UIDMappings
-					m.Mapping.GIDMappings = config.GIDMappings
+					if path := config.Namespaces.PathOf(configs.NEWUSER); path != "" {
+						m.Mapping.NamespacePath = path
+					} else {
+						m.Mapping.UIDMappings = config.UIDMappings
+						m.Mapping.GIDMappings = config.GIDMappings
+					}
 				}
 			}
 		}

tests/integration/idmap.bats

@@ -636,3 +636,45 @@ function setup_idmap_basic_mount() {
 	[[ "$output" == *"=/tmp/mount-tree/multi2/bar.txt:201=222="* ]]
 	[[ "$output" == *"=/tmp/mount-tree/multi2/baz.txt:202=233="* ]]
 }
+
+@test "idmap mount (idmap flag, implied mapping, userns join) [userns]" {
+	# Create a detached container with the id-mapping we want.
+	cp config.json config.json.bak
+	update_config '.linux.namespaces += [{"type": "user"}]
+		| .linux.uidMappings += [{"containerID": 0, "hostID": 100000, "size": 65536}]
+		| .linux.gidMappings += [{"containerID": 0, "hostID": 100000, "size": 65536}]'
+	update_config '.process.args = ["sleep", "infinity"]'
+
+	runc run -d --console-socket "$CONSOLE_SOCKET" target_userns
+	[ "$status" -eq 0 ]
+
+	# Configure our container to attach to the first container's userns.
+	target_pid="$(runc state target_userns | jq .pid)"
+	target_userns="/proc/$target_pid/ns/user"
+	update_config '.linux.namespaces += [{"type": "user", "path": "'"$target_userns"'"}]'
+
+	setup_host_bind_mount "source-multi1/" "mnt-subtree/multi1"
+	setup_host_bind_mount "source-multi2/" "mnt-subtree/multi2"
+
+	update_config '.mounts += [
+			{
+				"source": "mnt-subtree/",
+				"destination": "/tmp/mount-tree",
+				"options": ["rbind", "idmap"],
+			}
+		]'
+
+	update_config '.process.args = ["bash", "-c", "stat -c =%n:%u=%g= /tmp/mount-tree{,/multi1,/multi2}/{foo,bar,baz}.txt"]'
+	runc run test_debian
+	[ "$status" -eq 0 ]
+	[[ "$output" == *"=/tmp/mount-tree/foo.txt:100=211="* ]]
+	[[ "$output" == *"=/tmp/mount-tree/bar.txt:200=222="* ]]
+	[[ "$output" == *"=/tmp/mount-tree/baz.txt:300=233="* ]]
+	# Because we used "idmap", the child mounts were not remapped recursively.
+	[[ "$output" == *"=/tmp/mount-tree/multi1/foo.txt:$OVERFLOW_UID=$OVERFLOW_GID="* ]]
+	[[ "$output" == *"=/tmp/mount-tree/multi1/bar.txt:$OVERFLOW_UID=$OVERFLOW_GID="* ]]
+	[[ "$output" == *"=/tmp/mount-tree/multi1/baz.txt:$OVERFLOW_UID=$OVERFLOW_GID="* ]]
+	[[ "$output" == *"=/tmp/mount-tree/multi2/foo.txt:$OVERFLOW_UID=$OVERFLOW_GID="* ]]
+	[[ "$output" == *"=/tmp/mount-tree/multi2/bar.txt:$OVERFLOW_UID=$OVERFLOW_GID="* ]]
+	[[ "$output" == *"=/tmp/mount-tree/multi2/baz.txt:$OVERFLOW_UID=$OVERFLOW_GID="* ]]
+}