parent: exclude the own UID/GID from the subid ranges

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{

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)
+}