Skip to content

Commit

Permalink
specconv: temporarily allow userns path and mapping if they match
Browse files Browse the repository at this point in the history
It turns out that the error added in commit 09822c3 ("configs:
disallow ambiguous userns and timens configurations") causes issues with
containerd and CRIO because they pass both userns mappings and a userns
path.

These configurations are broken, but to avoid the regression in this one
case, output a warning to tell the user that the configuration is
incorrect but we will continue to use it if and only if the configured
mappings are identical to the mappings of the provided namespace.

Fixes: 09822c3 ("configs: disallow ambiguous userns and timens configurations")
Signed-off-by: Aleksa Sarai <[email protected]>
  • Loading branch information
cyphar committed Dec 6, 2023
1 parent 4516c25 commit 92f64df
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 5 deletions.
23 changes: 18 additions & 5 deletions libcontainer/specconv/spec_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -973,18 +973,31 @@ 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.
uidMap, gidMap, err := userns.GetUserNamespaceMappings(path)
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 according to
// runtime-spec, but we previously ignored this behaviour and thus
// cannot regress it. 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. runc 1.3 and later will return an error with this configuration.")
}

config.UIDMappings = uidMap
config.GIDMappings = gidMap
logrus.WithFields(logrus.Fields{
Expand Down
15 changes: 15 additions & 0 deletions libcontainer/userns/userns_maps_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit 92f64df

Please sign in to comment.