diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go index 0cbf578112d..e0bb528f39a 100644 --- a/libcontainer/specconv/spec_linux.go +++ b/libcontainer/specconv/spec_linux.go @@ -941,11 +941,6 @@ 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. @@ -953,6 +948,25 @@ func setupUserNamespace(spec *specs.Spec, config *configs.Config) error { if err != nil { return fmt.Errorf("failed to cache mappings for userns: %w", err) } + // 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 { + // FIXME: It turns out that containerd and CRIO pass both a userns + // path and the mappings of the namespace in the same config.json. + // Such a configuration is technically not valid, but we used to + // require mappings be specified, and thus users worked around our + // bug -- so we can't regress it at the moment. But we also don't + // want to produce broken behaviour if the mapping doesn't match + // the userns. So (for now) we output a warning if the actual + // userns mappings match the configuration, otherwise we return an + // error. + if !userns.IsSameMapping(uidMap, config.UidMappings) || + !userns.IsSameMapping(gidMap, config.GidMappings) { + return errors.New("user namespaces enabled, but both namespace path and non-matching mapping specified -- you may only provide one") + } + logrus.Warnf("config.json has both a userns path to join and a matching userns mapping specified -- you may only provide one. Future versions of runc may return an error with this configuration, please report a bug on if you see this warning and cannot update your configuration.") + } + config.UidMappings = uidMap config.GidMappings = gidMap logrus.WithFields(logrus.Fields{ diff --git a/libcontainer/specconv/spec_linux_test.go b/libcontainer/specconv/spec_linux_test.go index 68e57a08308..f79ea0e3727 100644 --- a/libcontainer/specconv/spec_linux_test.go +++ b/libcontainer/specconv/spec_linux_test.go @@ -639,8 +639,8 @@ func TestUserNamespaceMappingAndPath(t *testing.T) { 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") + if !strings.Contains(err.Error(), "both namespace path and non-matching mapping specified") { + t.Errorf("user namespace with path and non-matching mapping should be forbidden, got error %v", err) } } diff --git a/libcontainer/userns/userns_maps_linux.go b/libcontainer/userns/userns_maps_linux.go index d81290de2cb..7a8c2b023b3 100644 --- a/libcontainer/userns/userns_maps_linux.go +++ b/libcontainer/userns/userns_maps_linux.go @@ -169,3 +169,18 @@ func GetUserNamespaceMappings(nsPath string) (uidMap, gidMap []configs.IDMap, er return uidMap, gidMap, nil } + +// IsSameMapping returns whether or not the two id mappings are the same. Note +// that if the order of the mappings is different, or a mapping has been split, +// the mappings will be considered different. +func IsSameMapping(a, b []configs.IDMap) bool { + if len(a) != len(b) { + return false + } + for idx := range a { + if a[idx] != b[idx] { + return false + } + } + return true +}