diff --git a/pkg/parent/parent.go b/pkg/parent/parent.go index 2fbed789..a9babe40 100644 --- a/pkg/parent/parent.go +++ b/pkg/parent/parent.go @@ -398,7 +398,45 @@ func newugidmapArgs(subidSource SubidSource) ([]string, []string, error) { return newugidmapArgsFromSubIDRanges(u, subuidRanges, subgidRanges) } +// withoutSelfID removes id from the ranges, and splits a range when necessary. +// The own ID is already mapped to 0, and newuidmap/newgidmap reject a map that +// refers to the same host ID twice. +// The second return value contains the original ranges that contained id. +func withoutSelfID(ranges []idtools.SubIDRange, id int) (res, removed []idtools.SubIDRange) { + for _, f := range ranges { + if id < f.Start || id >= f.Start+f.Length { + res = append(res, f) + continue + } + removed = append(removed, f) + if head := id - f.Start; head > 0 { + res = append(res, idtools.SubIDRange{Start: f.Start, Length: head}) + } + if tail := f.Start + f.Length - (id + 1); tail > 0 { + res = append(res, idtools.SubIDRange{Start: id + 1, Length: tail}) + } + } + return res, removed +} + +// warnSelfIDRanges prints a warning for each range that contained the own ID. +// kind is "UID" or "GID". file is the subid file that defines the ranges. +func warnSelfIDRanges(removed []idtools.SubIDRange, id int, kind, file string) { + for _, f := range removed { + logrus.Warnf("%s: the range %d:%d contains the own %s %d, which is already mapped to %s 0 in the user namespace. RootlessKit ignores the %s %d in this range. Remove the own %s from %s.", + file, f.Start, f.Length, kind, id, kind, kind, id, kind, file) + } +} + func newugidmapArgsFromSubIDRanges(u *user.User, subuidRanges, subgidRanges []idtools.SubIDRange) ([]string, []string, error) { + uid, err := strconv.Atoi(u.Uid) + if err != nil { + return nil, nil, err + } + gid, err := strconv.Atoi(u.Gid) + if err != nil { + return nil, nil, err + } uidMap := []string{ "0", u.Uid, @@ -410,6 +448,11 @@ func newugidmapArgsFromSubIDRanges(u *user.User, subuidRanges, subgidRanges []id "1", } + subuidRanges, removedSubuidRanges := withoutSelfID(subuidRanges, uid) + warnSelfIDRanges(removedSubuidRanges, uid, "UID", "/etc/subuid") + subgidRanges, removedSubgidRanges := withoutSelfID(subgidRanges, gid) + warnSelfIDRanges(removedSubgidRanges, gid, "GID", "/etc/subgid") + uidMapLast := 1 for _, f := range subuidRanges { uidMap = append(uidMap, []string{ diff --git a/pkg/parent/parent_test.go b/pkg/parent/parent_test.go index 05357b6b..668a3f82 100644 --- a/pkg/parent/parent_test.go +++ b/pkg/parent/parent_test.go @@ -6,6 +6,8 @@ import ( "testing" "github.com/rootless-containers/rootlesskit/v3/pkg/parent/idtools" + "github.com/sirupsen/logrus" + "github.com/sirupsen/logrus/hooks/test" "golang.org/x/sys/unix" "gotest.tools/v3/assert" ) @@ -51,3 +53,98 @@ func TestNewugidmapArgsFromSubIDRanges(t *testing.T) { assert.DeepEqual(t, expectedU, newuidmapArgs) assert.DeepEqual(t, expectedG, newgidmapArgs) } + +func TestNewugidmapArgsFromSubIDRangesWithSelfRange(t *testing.T) { + // The user's own ID is always mapped to 0, so a subid range that consists + // of the own ID has to be excluded, otherwise newuidmap fails with EINVAL. + u := &user.User{Uid: "1001", Gid: "1001"} + subuidRanges := []idtools.SubIDRange{ + {Start: 1001, Length: 1}, + {Start: 165536, Length: 65536}, + } + subgidRanges := []idtools.SubIDRange{ + {Start: 1001, Length: 1}, + {Start: 165536, Length: 65536}, + } + newuidmapArgs, newgidmapArgs, err := newugidmapArgsFromSubIDRanges(u, subuidRanges, subgidRanges) + assert.NilError(t, err) + expectedU := []string{ + "0", u.Uid, "1", "1", "165536", "65536", + } + expectedG := []string{ + "0", u.Gid, "1", "1", "165536", "65536", + } + assert.DeepEqual(t, expectedU, newuidmapArgs) + assert.DeepEqual(t, expectedG, newgidmapArgs) +} + +func TestNewugidmapArgsFromSubIDRangesWithSelfInsideRange(t *testing.T) { + // The own UID 1001 is in the middle of the range, the own GID 1001 is at + // the end of the range. Both have to be excluded from the ranges. + u := &user.User{Uid: "1001", Gid: "1001"} + subuidRanges := []idtools.SubIDRange{ + {Start: 1000, Length: 10}, + } + subgidRanges := []idtools.SubIDRange{ + {Start: 1000, Length: 2}, + {Start: 2000, Length: 3}, + } + newuidmapArgs, newgidmapArgs, err := newugidmapArgsFromSubIDRanges(u, subuidRanges, subgidRanges) + assert.NilError(t, err) + expectedU := []string{ + "0", u.Uid, "1", "1", "1000", "1", "2", "1002", "8", + } + expectedG := []string{ + "0", u.Gid, "1", "1", "1000", "1", "2", "2000", "3", + } + assert.DeepEqual(t, expectedU, newuidmapArgs) + assert.DeepEqual(t, expectedG, newgidmapArgs) +} + +func TestWithoutSelfID(t *testing.T) { + ranges := []idtools.SubIDRange{ + {Start: 1001, Length: 1}, + {Start: 2000, Length: 10}, + {Start: 3000, Length: 5}, + } + res, removed := withoutSelfID(ranges, 2005) + expectedRes := []idtools.SubIDRange{ + {Start: 1001, Length: 1}, + {Start: 2000, Length: 5}, + {Start: 2006, Length: 4}, + {Start: 3000, Length: 5}, + } + expectedRemoved := []idtools.SubIDRange{ + {Start: 2000, Length: 10}, + } + assert.DeepEqual(t, expectedRes, res) + assert.DeepEqual(t, expectedRemoved, removed) + + // An ID that is not in any range keeps the ranges unmodified. + res, removed = withoutSelfID(ranges, 4000) + assert.DeepEqual(t, ranges, res) + assert.Assert(t, removed == nil) +} + +func TestNewugidmapArgsFromSubIDRangesWarnsAboutSelfID(t *testing.T) { + // The warning tells the admin which range of /etc/subuid and /etc/subgid + // is misconfigured. It is printed once per range, not once per ID. + hook := test.NewGlobal() + defer hook.Reset() + u := &user.User{Uid: "1001", Gid: "1001"} + subuidRanges := []idtools.SubIDRange{ + {Start: 1000, Length: 10}, + {Start: 165536, Length: 65536}, + } + subgidRanges := []idtools.SubIDRange{ + {Start: 1001, Length: 1}, + } + _, _, err := newugidmapArgsFromSubIDRanges(u, subuidRanges, subgidRanges) + assert.NilError(t, err) + entries := hook.AllEntries() + assert.Equal(t, 2, len(entries)) + assert.Equal(t, logrus.WarnLevel, entries[0].Level) + assert.Equal(t, "/etc/subuid: the range 1000:10 contains the own UID 1001, which is already mapped to UID 0 in the user namespace. RootlessKit ignores the UID 1001 in this range. Remove the own UID from /etc/subuid.", entries[0].Message) + assert.Equal(t, logrus.WarnLevel, entries[1].Level) + assert.Equal(t, "/etc/subgid: the range 1001:1 contains the own GID 1001, which is already mapped to GID 0 in the user namespace. RootlessKit ignores the GID 1001 in this range. Remove the own GID from /etc/subgid.", entries[1].Message) +}