diff --git a/libcontainer/configs/validate/validator.go b/libcontainer/configs/validate/validator.go index 0df7242b3..2594d05c7 100644 --- a/libcontainer/configs/validate/validator.go +++ b/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") diff --git a/libcontainer/configs/validate/validator_test.go b/libcontainer/configs/validate/validator_test.go index a882fa64c..755d2a07b 100644 --- a/libcontainer/configs/validate/validator_test.go +++ b/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", diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 9795964ca..1bc840116 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" @@ -1550,6 +1551,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 2529e1e7e..90275043e 100644 --- a/libcontainer/specconv/spec_linux.go +++ b/libcontainer/specconv/spec_linux.go @@ -516,6 +516,9 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) { } func toConfigIDMap(specMaps []specs.LinuxIDMapping) []configs.IDMap { + if specMaps == nil { + return nil + } idmaps := make([]configs.IDMap, len(specMaps)) for i, id := range specMaps { idmaps[i] = configs.IDMap{ @@ -970,6 +973,11 @@ func setupUserNamespace(spec *specs.Spec, config *configs.Config) error { config.GIDMappings = toConfigIDMap(spec.Linux.GIDMappings) } 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 d84853bc7..9fd64de76 100644 --- a/libcontainer/specconv/spec_linux_test.go +++ b/libcontainer/specconv/spec_linux_test.go @@ -608,6 +608,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.")