diff --git a/libcontainer/configs/validate/validator.go b/libcontainer/configs/validate/validator.go index 4fbd308dadd..ece70a45d31 100644 --- a/libcontainer/configs/validate/validator.go +++ b/libcontainer/configs/validate/validator.go @@ -109,11 +109,19 @@ func (v *ConfigValidator) security(config *configs.Config) error { func (v *ConfigValidator) usernamespace(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") } } return nil diff --git a/libcontainer/configs/validate/validator_test.go b/libcontainer/configs/validate/validator_test.go index 5181333fb12..e89a60b08c5 100644 --- a/libcontainer/configs/validate/validator_test.go +++ b/libcontainer/configs/validate/validator_test.go @@ -150,7 +150,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.") } @@ -161,6 +161,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}}, } validator := New() @@ -170,11 +172,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}}, } validator := New() diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 5c6272e5d42..670b33fcbb3 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -18,6 +18,7 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/cgroups/systemd" "github.com/opencontainers/runc/libcontainer/configs" + "github.com/opencontainers/runc/libcontainer/userns" "github.com/opencontainers/runtime-spec/specs-go" "golang.org/x/sys/unix" @@ -1588,6 +1589,11 @@ func TestInitJoinNetworkAndUser(t *testing.T) { config2 := newTemplateConfig(t, &tParam{userns: true}) config2.Namespaces.Add(configs.NEWNET, netns1) config2.Namespaces.Add(configs.NEWUSER, userns1) + // Emulate specconv.setupUserNamespace(). + uidMap, gidMap, err := userns.GetUserNamespaceMappings(userns1) + ok(t, err) + config2.UidMappings = uidMap + config2.GidMappings = gidMap config2.Cgroups.Path = "integration/test2" container2, err := newContainer(t, config2) ok(t, err) diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go index bef6a51df42..0cbf578112d 100644 --- a/libcontainer/specconv/spec_linux.go +++ b/libcontainer/specconv/spec_linux.go @@ -941,6 +941,11 @@ func setupUserNamespace(spec *specs.Spec, config *configs.Config) error { } } if path := config.Namespaces.PathOf(configs.NEWUSER); path != "" { + // We cannot allow uid or gid mappings to be set if we are also asked + // to join a userns. + if config.UidMappings != nil || config.GidMappings != nil { + return errors.New("user namespaces enabled, but both namespace path and mapping specified -- you may only provide one") + } // Cache the current userns mappings in our configuration, so that we // can calculate uid and gid mappings within runc. These mappings are // never used for configuring the container if the path is set. diff --git a/libcontainer/specconv/spec_linux_test.go b/libcontainer/specconv/spec_linux_test.go index 79768e31861..68e57a08308 100644 --- a/libcontainer/specconv/spec_linux_test.go +++ b/libcontainer/specconv/spec_linux_test.go @@ -610,6 +610,40 @@ func TestDupNamespaces(t *testing.T) { } } +func TestUserNamespaceMappingAndPath(t *testing.T) { + if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) { + t.Skip("Test requires userns.") + } + + spec := &specs.Spec{ + Root: &specs.Root{ + Path: "rootfs", + }, + Linux: &specs.Linux{ + UIDMappings: []specs.LinuxIDMapping{ + {ContainerID: 0, HostID: 1000, Size: 1000}, + }, + GIDMappings: []specs.LinuxIDMapping{ + {ContainerID: 0, HostID: 2000, Size: 1000}, + }, + Namespaces: []specs.LinuxNamespace{ + { + Type: "user", + Path: "/proc/1/ns/user", + }, + }, + }, + } + + _, err := CreateLibcontainerConfig(&CreateOpts{ + Spec: spec, + }) + + if !strings.Contains(err.Error(), "user namespaces enabled, but both namespace path and mapping specified") { + t.Errorf("user namespace with mapping and namespace path should be forbidden") + } +} + func TestNonZeroEUIDCompatibleSpecconvValidate(t *testing.T) { if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) { t.Skip("Test requires userns.")