Support idmap mounts on volumes without userns.

Commit fda12ab ("Support idmap mounts on volumes") introduces support for idmap mount on volumes. At this time, it required userns to be used and idmap mount must have the same UID/GID mappings than userns. This commit removes this requirement, so it is possible to use idmap mount without userns. You can, of course, use id map mount with userns, but the UID/GID mappings can be different. Signed-off-by: Francis Laniel <[email protected]>
opencontainers · Jul 21, 2023 · 65d54df · 65d54df
1 parent 74895d4
commit 65d54df
Show file tree

Hide file tree

Showing 7 changed files with 203 additions and 71 deletions.
diff --git a/libcontainer/configs/validate/validator.go b/libcontainer/configs/validate/validator.go
@@ -95,7 +95,7 @@ func namespaces(config *configs.Config) error {
 			return errors.New("USER namespaces aren't enabled in the kernel")
 		}
 	} else {
-		if config.UIDMappings != nil || config.GIDMappings != nil {
+		if len(config.UIDMappings) != 0 || len(config.GIDMappings) != 0 {
 			return errors.New("User namespace mappings specified, but USER namespace isn't enabled in the config")
 		}
 	}
@@ -264,14 +264,8 @@ func checkIDMapMounts(config *configs.Config, m *configs.Mount) error {
 	if config.RootlessEUID {
 		return fmt.Errorf("gidMappings/uidMappings is not supported when runc is being launched with EUID != 0, needs CAP_SYS_ADMIN on the runc parent's user namespace")
 	}
-	if len(config.UIDMappings) == 0 || len(config.GIDMappings) == 0 {
-		return fmt.Errorf("not yet supported to use gidMappings/uidMappings in a mount without also using a user namespace")
-	}
-	if !sameMapping(config.UIDMappings, m.UIDMappings) {
-		return fmt.Errorf("not yet supported for the mount uidMappings to be different than user namespace uidMapping")
-	}
-	if !sameMapping(config.GIDMappings, m.GIDMappings) {
-		return fmt.Errorf("not yet supported for the mount gidMappings to be different than user namespace gidMapping")
+	if len(m.UIDMappings) == 0 || len(m.GIDMappings) == 0 {
+		return fmt.Errorf("id map mount without UID/GID mappings")
 	}
 	if !filepath.IsAbs(m.Source) {
 		return fmt.Errorf("mount source not absolute")
@@ -298,27 +292,6 @@ func mounts(config *configs.Config) error {
 	return nil
 }
 
-// sameMapping checks if the mappings are the same. If the mappings are the same
-// but in different order, it returns false.
-func sameMapping(a, b []configs.IDMap) bool {
-	if len(a) != len(b) {
-		return false
-	}
-
-	for i := range a {
-		if a[i].ContainerID != b[i].ContainerID {
-			return false
-		}
-		if a[i].HostID != b[i].HostID {
-			return false
-		}
-		if a[i].Size != b[i].Size {
-			return false
-		}
-	}
-	return true
-}
-
 func isHostNetNS(path string) (bool, error) {
 	const currentProcessNetns = "/proc/self/ns/net"
 

diff --git a/libcontainer/configs/validate/validator_test.go b/libcontainer/configs/validate/validator_test.go
@@ -436,8 +436,7 @@ func TestValidateIDMapMounts(t *testing.T) {
 			},
 		},
 		{
-			name:  "idmap mount without userns mappings",
-			isErr: true,
+			name: "idmap mount without userns mappings",
 			config: &configs.Config{
 				Mounts: []*configs.Mount{
 					{
@@ -451,8 +450,7 @@ func TestValidateIDMapMounts(t *testing.T) {
 			},
 		},
 		{
-			name:  "idmap mounts with different userns and mount mappings",
-			isErr: true,
+			name: "idmap mounts with different userns and mount mappings",
 			config: &configs.Config{
 				UIDMappings: mapping,
 				GIDMappings: mapping,
@@ -474,8 +472,7 @@ func TestValidateIDMapMounts(t *testing.T) {
 			},
 		},
 		{
-			name:  "idmap mounts with different userns and mount mappings",
-			isErr: true,
+			name: "idmap mounts with different userns and mount mappings",
 			config: &configs.Config{
 				UIDMappings: mapping,
 				GIDMappings: mapping,

diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go
@@ -558,11 +558,6 @@ func (c *Container) shouldSendIdmapSources() bool {
 		return false
 	}
 
-	// For the time being we require userns to be in use.
-	if !c.config.Namespaces.Contains(configs.NEWUSER) {
-		return false
-	}
-
 	// We need to send sources if there are idmap bind-mounts.
 	for _, m := range c.config.Mounts {
 		if m.IsBind() && m.IsIDMapped() {
@@ -2301,6 +2296,9 @@ func (c *Container) bootstrapData(cloneFlags uintptr, nsMaps map[configs.Namespa
 	// Idmap mount sources to open.
 	if it == initStandard && c.shouldSendIdmapSources() {
 		var mounts []byte
+		var uidmaps []byte
+		var gidmaps []byte
+
 		for _, m := range c.config.Mounts {
 			if m.IsBind() && m.IsIDMapped() {
 				// While other parts of the code check this too (like
@@ -2310,11 +2308,35 @@ func (c *Container) bootstrapData(cloneFlags uintptr, nsMaps map[configs.Namespa
 					return nil, fmt.Errorf("mount source string contains null byte: %q", m.Source)
 				}
 
+				uidBytes, err := encodeIDMapping(m.UIDMappings)
+				if err != nil {
+					return nil, err
+				}
+				gidBytes, err := encodeIDMapping(m.GIDMappings)
+				if err != nil {
+					return nil, err
+				}
+
+				uidmaps = append(uidmaps, uidBytes...)
+				gidmaps = append(gidmaps, gidBytes...)
 				mounts = append(mounts, []byte(m.Source)...)
 			}
+
+			uidmaps = append(uidmaps, byte(0))
+			gidmaps = append(gidmaps, byte(0))
 			mounts = append(mounts, byte(0))
 		}
 
+		r.AddData(&Bytemsg{
+			Type:  IdmapUidmapAttr,
+			Value: uidmaps,
+		})
+
+		r.AddData(&Bytemsg{
+			Type:  IdmapGidmapAttr,
+			Value: gidmaps,
+		})
+
 		r.AddData(&Bytemsg{
 			Type:  IdmapSourcesAttr,
 			Value: mounts,

diff --git a/libcontainer/message_linux.go b/libcontainer/message_linux.go
@@ -23,6 +23,8 @@ const (
 	GidmapPathAttr   uint16 = 27289
 	MountSourcesAttr uint16 = 27290
 	IdmapSourcesAttr uint16 = 27291
+	IdmapUidmapAttr  uint16 = 27292
+	IdmapGidmapAttr  uint16 = 27293
 )
 
 type Int32msg struct {