From e2dae5ea97df05a914e90e7be189a2b78e07d0a4 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 27 Apr 2022 16:45:39 +0200 Subject: [PATCH 1/3] graphtest: use unique names for the file system layers Signed-off-by: Giuseppe Scrivano --- drivers/graphtest/graphtest_unix.go | 54 ++++++++++++++--------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/drivers/graphtest/graphtest_unix.go b/drivers/graphtest/graphtest_unix.go index cdd6271acc..2147b5203b 100644 --- a/drivers/graphtest/graphtest_unix.go +++ b/drivers/graphtest/graphtest_unix.go @@ -125,11 +125,11 @@ func DriverTestCreateBase(t testing.TB, drivername string, driverOptions ...stri driver := GetDriver(t, drivername, driverOptions...) defer PutDriver(t) - createBase(t, driver, "Base") + createBase(t, driver, "Base1") defer func() { - require.NoError(t, driver.Remove("Base")) + require.NoError(t, driver.Remove("Base1")) }() - verifyBase(t, driver, "Base", defaultPerms) + verifyBase(t, driver, "Base1", defaultPerms) } // DriverTestCreateSnap Create a driver and snap and verify. @@ -137,26 +137,26 @@ func DriverTestCreateSnap(t testing.TB, drivername string, driverOptions ...stri driver := GetDriver(t, drivername, driverOptions...) defer PutDriver(t) - createBase(t, driver, "Base") + createBase(t, driver, "Base2") defer func() { - require.NoError(t, driver.Remove("Base")) + require.NoError(t, driver.Remove("Base2")) }() - err := driver.Create("Snap", "Base", nil) + err := driver.Create("Snap2", "Base2", nil) require.NoError(t, err) defer func() { - require.NoError(t, driver.Remove("Snap")) + require.NoError(t, driver.Remove("Snap2")) }() - verifyBase(t, driver, "Snap", defaultPerms) + verifyBase(t, driver, "Snap2", defaultPerms) - root, err := driver.Get("Snap", graphdriver.MountOpts{}) + root, err := driver.Get("Snap2", graphdriver.MountOpts{}) assert.NoError(t, err) err = os.Chmod(root, modifiedPerms) require.NoError(t, err) - driver.Put("Snap") + driver.Put("Snap2") - err = driver.Create("SecondSnap", "Snap", nil) + err = driver.Create("SecondSnap", "Snap2", nil) require.NoError(t, err) defer func() { require.NoError(t, driver.Remove("SecondSnap")) @@ -171,29 +171,29 @@ func DriverTestCreateFromTemplate(t testing.TB, drivername string, driverOptions driver := GetDriver(t, drivername, driverOptions...) defer PutDriver(t) - createBase(t, driver, "Base") + createBase(t, driver, "Base3") defer func() { - require.NoError(t, driver.Remove("Base")) + require.NoError(t, driver.Remove("Base3")) }() - err := driver.Create("Snap", "Base", nil) + err := driver.Create("Snap3", "Base3", nil) require.NoError(t, err) defer func() { - require.NoError(t, driver.Remove("Snap")) + require.NoError(t, driver.Remove("Snap3")) }() content := []byte("test content") - if err := addFile(driver, "Snap", "testfile.txt", content); err != nil { + if err := addFile(driver, "Snap3", "testfile.txt", content); err != nil { t.Fatal(err) } - err = driver.CreateFromTemplate("FromTemplate", "Snap", nil, "Base", nil, nil, true) + err = driver.CreateFromTemplate("FromTemplate", "Snap3", nil, "Base3", nil, nil, true) require.NoError(t, err) defer func() { require.NoError(t, driver.Remove("FromTemplate")) }() - err = driver.CreateFromTemplate("ROFromTemplate", "Snap", nil, "Base", nil, nil, false) + err = driver.CreateFromTemplate("ROFromTemplate", "Snap3", nil, "Base3", nil, nil, false) require.NoError(t, err) defer func() { require.NoError(t, driver.Remove("ROFromTemplate")) @@ -201,7 +201,7 @@ func DriverTestCreateFromTemplate(t testing.TB, drivername string, driverOptions noChanges := []archive.Change{} - changes, err := driver.Changes("FromTemplate", nil, "Snap", nil, "") + changes, err := driver.Changes("FromTemplate", nil, "Snap3", nil, "") if err != nil { t.Fatal(err) } @@ -209,7 +209,7 @@ func DriverTestCreateFromTemplate(t testing.TB, drivername string, driverOptions t.Fatal(err) } - changes, err = driver.Changes("ROFromTemplate", nil, "Snap", nil, "") + changes, err = driver.Changes("ROFromTemplate", nil, "Snap3", nil, "") if err != nil { t.Fatal(err) } @@ -223,7 +223,7 @@ func DriverTestCreateFromTemplate(t testing.TB, drivername string, driverOptions if err := checkFile(driver, "ROFromTemplate", "testfile.txt", content); err != nil { t.Fatal(err) } - if err := checkFile(driver, "Snap", "testfile.txt", content); err != nil { + if err := checkFile(driver, "Snap3", "testfile.txt", content); err != nil { t.Fatal(err) } @@ -232,7 +232,7 @@ func DriverTestCreateFromTemplate(t testing.TB, drivername string, driverOptions Kind: archive.ChangeAdd, }} - changes, err = driver.Changes("Snap", nil, "Base", nil, "") + changes, err = driver.Changes("Snap3", nil, "Base3", nil, "") if err != nil { t.Fatal(err) } @@ -240,7 +240,7 @@ func DriverTestCreateFromTemplate(t testing.TB, drivername string, driverOptions t.Fatal(err) } - changes, err = driver.Changes("FromTemplate", nil, "Base", nil, "") + changes, err = driver.Changes("FromTemplate", nil, "Base3", nil, "") if err != nil { t.Fatal(err) } @@ -248,7 +248,7 @@ func DriverTestCreateFromTemplate(t testing.TB, drivername string, driverOptions t.Fatal(err) } - changes, err = driver.Changes("ROFromTemplate", nil, "Base", nil, "") + changes, err = driver.Changes("ROFromTemplate", nil, "Base3", nil, "") if err != nil { t.Fatal(err) } @@ -256,7 +256,7 @@ func DriverTestCreateFromTemplate(t testing.TB, drivername string, driverOptions t.Fatal(err) } - verifyBase(t, driver, "Base", defaultPerms) + verifyBase(t, driver, "Base3", defaultPerms) } // DriverTestDeepLayerRead reads a file from a lower layer under a given number of layers @@ -429,11 +429,11 @@ func DriverTestSetQuota(t *testing.T, drivername string) { driver := GetDriver(t, drivername) defer PutDriver(t) - createBase(t, driver, "Base") + createBase(t, driver, "Base4") createOpts := &graphdriver.CreateOpts{} createOpts.StorageOpt = make(map[string]string, 1) createOpts.StorageOpt["size"] = "50M" - if err := driver.Create("zfsTest", "Base", createOpts); err != nil { + if err := driver.Create("zfsTest", "Base4", createOpts); err != nil { t.Fatal(err) } From ed447c3df8264279c408f20e228a9629f33c3aca Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 27 Apr 2022 15:37:11 +0200 Subject: [PATCH 2/3] idtools: add new function to fallback to overflow id add a new function ToHostOverflow() that instead of raising an error when the mapping is not possible in the target user namespace, fall back to using the overflow ID. Signed-off-by: Giuseppe Scrivano --- pkg/idtools/idtools.go | 64 +++++++++++++++++++++++++++++++++++++ pkg/idtools/idtools_test.go | 42 ++++++++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/pkg/idtools/idtools.go b/pkg/idtools/idtools.go index 7c8f4d10c2..7a8fec0ce5 100644 --- a/pkg/idtools/idtools.go +++ b/pkg/idtools/idtools.go @@ -3,15 +3,18 @@ package idtools import ( "bufio" "fmt" + "io/ioutil" "os" "os/user" "sort" "strconv" "strings" + "sync" "syscall" "github.com/containers/storage/pkg/system" "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) // IDMap contains a single entry for user namespace range remapping. An array @@ -203,6 +206,67 @@ func (i *IDMappings) ToHost(pair IDPair) (IDPair, error) { return target, err } +var ( + overflowUIDOnce sync.Once + overflowGIDOnce sync.Once + overflowUID int + overflowGID int +) + +// getOverflowUID returns the UID mapped to the overflow user +func getOverflowUID() int { + overflowUIDOnce.Do(func() { + // 65534 is the value on older kernels where /proc/sys/kernel/overflowuid is not present + overflowUID = 65534 + if content, err := ioutil.ReadFile("/proc/sys/kernel/overflowuid"); err == nil { + if tmp, err := strconv.Atoi(string(content)); err == nil { + overflowUID = tmp + } + } + }) + return overflowUID +} + +// getOverflowUID returns the GID mapped to the overflow user +func getOverflowGID() int { + overflowGIDOnce.Do(func() { + // 65534 is the value on older kernels where /proc/sys/kernel/overflowgid is not present + overflowGID = 65534 + if content, err := ioutil.ReadFile("/proc/sys/kernel/overflowgid"); err == nil { + if tmp, err := strconv.Atoi(string(content)); err == nil { + overflowGID = tmp + } + } + }) + return overflowGID +} + +// ToHost returns the host UID and GID for the container uid, gid. +// Remapping is only performed if the ids aren't already the remapped root ids +// If the mapping is not possible because the target ID is not mapped into +// the namespace, then the overflow ID is used. +func (i *IDMappings) ToHostOverflow(pair IDPair) (IDPair, error) { + var err error + target := i.RootPair() + + if pair.UID != target.UID { + target.UID, err = RawToHost(pair.UID, i.uids) + if err != nil { + target.UID = getOverflowUID() + logrus.Debugf("Failed to map UID %v to the target mapping, using the overflow ID %v", pair.UID, target.UID) + } + } + + if pair.GID != target.GID { + target.GID, err = RawToHost(pair.GID, i.gids) + if err != nil { + target.GID = getOverflowGID() + logrus.Debugf("Failed to map GID %v to the target mapping, using the overflow ID %v", pair.GID, target.GID) + } + } + return target, nil +} + // ToContainer returns the container UID and GID for the host uid and gid func (i *IDMappings) ToContainer(pair IDPair) (int, int, error) { uid, err := RawToContainer(pair.UID, i.uids) diff --git a/pkg/idtools/idtools_test.go b/pkg/idtools/idtools_test.go index c22b87d99b..1315e5d89e 100644 --- a/pkg/idtools/idtools_test.go +++ b/pkg/idtools/idtools_test.go @@ -46,6 +46,48 @@ func TestToHost(t *testing.T) { } } +func TestToHostOverflow(t *testing.T) { + idMappings := []IDMap{ + { + ContainerID: 0, + HostID: 1000, + Size: 1, + }, + { + ContainerID: 1, + HostID: 100000, + Size: 65536, + }, + } + + mappings := IDMappings{ + uids: idMappings, + gids: idMappings, + } + + pair, err := mappings.ToHostOverflow(IDPair{UID: 65538, GID: 0}) + if err != nil { + t.Fatal(err) + } + if pair.UID != getOverflowUID() { + t.Fatalf("Converted to the wrong UID") + } + if pair.GID != 1000 { + t.Fatalf("Converted to the wrong GID") + } + + pair, err = mappings.ToHostOverflow(IDPair{UID: 10, GID: 65539}) + if err != nil { + t.Fatal(err) + } + if pair.UID != 100009 { + t.Fatalf("Converted to the wrong UID") + } + if pair.GID != getOverflowGID() { + t.Fatalf("Converted to the wrong GID") + } +} + func TestGetRootUIDGID(t *testing.T) { mappingsUIDs := []IDMap{ { From 28c166fa18ee3ad9286cc5016d01a38c5baac10c Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 27 Apr 2022 15:38:31 +0200 Subject: [PATCH 3/3] chown: use ToHostOverflow when chowning when chowning an image, fall back to the overflow ID when a UID or GID cannot be mapped to the target user namespace. This ensures the chown driver works similar to what we do with idmapped mounts when it is supported for overlay. It is needed for CRI-O to support user namespaces in Kubernetes since the Kubelet picks a static size for the user namespace and it might break some images using IDs outside the picked range. Signed-off-by: Giuseppe Scrivano --- drivers/chown_unix.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/chown_unix.go b/drivers/chown_unix.go index 3c508b66b1..c598b936d6 100644 --- a/drivers/chown_unix.go +++ b/drivers/chown_unix.go @@ -76,7 +76,7 @@ func (c *platformChowner) LChown(path string, info os.FileInfo, toHost, toContai UID: uid, GID: gid, } - mappedPair, err := toHost.ToHost(pair) + mappedPair, err := toHost.ToHostOverflow(pair) if err != nil { return fmt.Errorf("error mapping container ID pair %#v for %q to host: %v", pair, path, err) }