Skip to content

Commit

Permalink
idmap: allow arbitrary idmap mounts regardless of userns configuration
Browse files Browse the repository at this point in the history 
With the rework of nsexec.c to handle MOUNT_ATTR_IDMAP in our Go code we
can now handle arbitrary mappings without issue, so remove the primary
artificial limit of mappings (must use the same mapping as the
container's userns) and add some tests.

We still only support idmap mounts for bind-mounts because configuring
mappings for other filesystems would require switching our entire mount
machinery to the new mount API. The current design would easily allow
for this but we would need to convert new mount options entirely to the
fsopen/fsconfig/fsmount API. This can be done in the future.

Signed-off-by: Aleksa Sarai <[email protected]>
  • Loading branch information
@cyphar
cyphar committed Aug 8, 2023
1 parent acaf423 commit 18ce9de
Show file tree
Hide file tree
Showing 3 changed files with 287 additions and 185 deletions.
38 changes: 2 additions & 36 deletions libcontainer/configs/validate/validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,24 +259,11 @@ func checkIDMapMounts(config *configs.Config, m *configs.Mount) error {
}

if !m.IsBind() {
return fmt.Errorf("gidMappings/uidMappings is supported only for mounts with the option 'bind'")
return fmt.Errorf("id-mapped mounts are only supported for bind-mounts")
}
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")
return fmt.Errorf("id-mapped mounts are not supported for rootless containers")
}
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 !filepath.IsAbs(m.Source) {
return fmt.Errorf("mount source not absolute")
}

return nil
}

Expand All @@ -298,27 +285,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"

Expand Down
81 changes: 30 additions & 51 deletions libcontainer/configs/validate/validator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,129 +436,108 @@ func TestValidateIDMapMounts(t *testing.T) {
},
},
{
name: "idmap mount without userns mappings",
isErr: true,
config: &configs.Config{
Mounts: []*configs.Mount{
{
Source: "/abs/path/",
Destination: "/abs/path/",
Flags: unix.MS_BIND,
UIDMappings: mapping,
GIDMappings: mapping,
},
},
},
},
{
name: "idmap mounts with different userns and mount mappings",
name: "idmap mounts without abs dest path",
isErr: true,
config: &configs.Config{
UIDMappings: mapping,
GIDMappings: mapping,
Mounts: []*configs.Mount{
{
Source: "/abs/path/",
Destination: "/abs/path/",
Destination: "./rel/path/",
Flags: unix.MS_BIND,
UIDMappings: []configs.IDMap{
{
ContainerID: 10,
HostID: 10,
Size: 1,
},
},
UIDMappings: mapping,
GIDMappings: mapping,
},
},
},
},
{
name: "idmap mounts with different userns and mount mappings",
isErr: true,
name: "simple idmap mount",
config: &configs.Config{
UIDMappings: mapping,
GIDMappings: mapping,
Mounts: []*configs.Mount{
{
Source: "/abs/path/",
Source: "/another-abs/path/",
Destination: "/abs/path/",
Flags: unix.MS_BIND,
UIDMappings: mapping,
GIDMappings: []configs.IDMap{
{
ContainerID: 10,
HostID: 10,
Size: 1,
},
},
GIDMappings: mapping,
},
},
},
},
{
name: "idmap mounts without abs source path",
isErr: true,
name: "idmap mount with more flags",
config: &configs.Config{
UIDMappings: mapping,
GIDMappings: mapping,
Mounts: []*configs.Mount{
{
Source: "./rel/path/",
Source: "/another-abs/path/",
Destination: "/abs/path/",
Flags: unix.MS_BIND,
Flags: unix.MS_BIND | unix.MS_RDONLY,
UIDMappings: mapping,
GIDMappings: mapping,
},
},
},
},
{
name: "idmap mounts without abs dest path",
isErr: true,
name: "idmap mount without userns mappings",
config: &configs.Config{
UIDMappings: mapping,
GIDMappings: mapping,
Mounts: []*configs.Mount{
{
Source: "/abs/path/",
Destination: "./rel/path/",
Destination: "/abs/path/",
Flags: unix.MS_BIND,
UIDMappings: mapping,
GIDMappings: mapping,
},
},
},
},

{
name: "simple idmap mount",
name: "idmap mounts with different userns and mount mappings",
config: &configs.Config{
UIDMappings: mapping,
GIDMappings: mapping,
Mounts: []*configs.Mount{
{
Source: "/another-abs/path/",
Source: "/abs/path/",
Destination: "/abs/path/",
Flags: unix.MS_BIND,
UIDMappings: mapping,
UIDMappings: []configs.IDMap{
{
ContainerID: 10,
HostID: 10,
Size: 1,
},
},
GIDMappings: mapping,
},
},
},
},
{
name: "idmap mount with more flags",
name: "idmap mounts with different userns and mount mappings",
config: &configs.Config{
UIDMappings: mapping,
GIDMappings: mapping,
Mounts: []*configs.Mount{
{
Source: "/another-abs/path/",
Source: "/abs/path/",
Destination: "/abs/path/",
Flags: unix.MS_BIND | unix.MS_RDONLY,
Flags: unix.MS_BIND,
UIDMappings: mapping,
GIDMappings: mapping,
GIDMappings: []configs.IDMap{
{
ContainerID: 10,
HostID: 10,
Size: 1,
},
},
},
},
},
Expand Down
Loading

0 comments on commit 18ce9de

Please sign in to comment.