From 4a77b0f8eca7ca6dbc1e9b13d73d197dd210f26b Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Tue, 28 Jan 2020 15:23:35 +0000 Subject: [PATCH] rootless: use libcontainer API to detect rootless libcontainer already has an API to detect if the runtime is running rootless. Use libcontainer API instead of reinventing the wheel. fixes #2415 Signed-off-by: Julio Montes --- cli/main_test.go | 4 + pkg/katautils/oci_test.go | 5 ++ pkg/rootless/rootless.go | 85 ++++-------------- pkg/rootless/rootless_test.go | 162 ---------------------------------- virtcontainers/api_test.go | 5 ++ 5 files changed, 31 insertions(+), 230 deletions(-) diff --git a/cli/main_test.go b/cli/main_test.go index 796d426479..e6b92c334d 100644 --- a/cli/main_test.go +++ b/cli/main_test.go @@ -24,6 +24,7 @@ import ( "github.com/dlespiau/covertool/pkg/cover" ktu "github.com/kata-containers/runtime/pkg/katatestutils" "github.com/kata-containers/runtime/pkg/katautils" + "github.com/kata-containers/runtime/pkg/rootless" vc "github.com/kata-containers/runtime/virtcontainers" "github.com/kata-containers/runtime/virtcontainers/pkg/compatoci" "github.com/kata-containers/runtime/virtcontainers/pkg/oci" @@ -1068,6 +1069,9 @@ func TestMainResetCLIGlobals(t *testing.T) { } func createTempContainerIDMapping(containerID, sandboxID string) (string, error) { + // Mocking rootless + rootless.IsRootless = func() bool { return false } + tmpDir, err := ioutil.TempDir("", "containers-mapping") if err != nil { return "", err diff --git a/pkg/katautils/oci_test.go b/pkg/katautils/oci_test.go index 1d19485d27..f6588e7cce 100644 --- a/pkg/katautils/oci_test.go +++ b/pkg/katautils/oci_test.go @@ -13,9 +13,14 @@ import ( "path/filepath" "testing" + "github.com/kata-containers/runtime/pkg/rootless" "github.com/stretchr/testify/assert" ) +func init() { + rootless.IsRootless = func() bool { return false } +} + func createTempContainerIDMapping(containerID, sandboxID string) (string, error) { tmpDir, err := ioutil.TempDir("", "containers-mapping") if err != nil { diff --git a/pkg/rootless/rootless.go b/pkg/rootless/rootless.go index 2bb702ab18..8366bb4b44 100644 --- a/pkg/rootless/rootless.go +++ b/pkg/rootless/rootless.go @@ -20,20 +20,17 @@ package rootless import ( - "bufio" "context" "crypto/rand" "fmt" - "io" "os" "path/filepath" "runtime" - "strconv" "strings" "sync" "github.com/containernetworking/plugins/pkg/ns" - "github.com/pkg/errors" + "github.com/opencontainers/runc/libcontainer/system" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) @@ -53,13 +50,12 @@ var ( // which user-specific non-essential runtime files are stored. rootlessDir = os.Getenv("XDG_RUNTIME_DIR") - // uidMapPath defines the location of the uid_map file to - // determine whether a user is root or not - uidMapPath = "/proc/self/uid_map" - rootlessLog = logrus.WithFields(logrus.Fields{ "source": "rootless", }) + + //The function is declared this way for mocking in unit tests + IsRootless = isRootlessFunc ) // SetLogger sets up a logger for the rootless pkg @@ -68,71 +64,24 @@ func SetLogger(ctx context.Context, logger *logrus.Entry) { rootlessLog = logger.WithFields(fields) } -// setRootless reads a uid_map file, compares the UID of the -// user inside the container vs on the host. If the host UID -// is not root, but the container ID is, it can be determined -// the user is running rootlessly. -func setRootless() error { - initRootless = true - file, err := os.Open(uidMapPath) - if err != nil { - return err - } - defer file.Close() - - buf := bufio.NewReader(file) - for { - line, _, err := buf.ReadLine() - if err != nil { - if err == io.EOF { - return nil - } - return err - } - if line == nil { - return nil - } - - var parseError = errors.Errorf("Failed to parse uid map file %s", uidMapPath) - // if the container id (id[0]) is 0 (root inside the container) - // has a mapping to the host id (id[1]) that is not root, then - // it can be determined that the host user is running rootless - ids := strings.Fields(string(line)) - // do some sanity checks - if len(ids) != 3 { - return parseError - } - userNSUid, err := strconv.ParseUint(ids[0], 10, 0) - if err != nil { - return parseError - } - hostUID, err := strconv.ParseUint(ids[1], 10, 0) - if err != nil { - return parseError - } - rangeUID, err := strconv.ParseUint(ids[2], 10, 0) - if err != nil || rangeUID == 0 { - return parseError - } - - if userNSUid == 0 && hostUID != 0 { - rootlessLog.Info("Running as rootless") - isRootless = true - return nil - } - } -} - // IsRootless states whether kata is being ran with root or not -func IsRootless() bool { +func isRootlessFunc() bool { rLock.Lock() + defer rLock.Unlock() if !initRootless { - err := setRootless() - if err != nil { - rootlessLog.WithError(err).Error("Unable to determine if running rootless") + initRootless = true + isRootless = true + // --rootless and --systemd-cgroup options must honoured + // but with the current implementation this is not possible + // https://github.com/kata-containers/runtime/issues/2412 + if os.Geteuid() != 0 { + return true + } + if system.RunningInUserNS() { + return true } + isRootless = false } - rLock.Unlock() return isRootless } diff --git a/pkg/rootless/rootless_test.go b/pkg/rootless/rootless_test.go index 1a5289477d..46f387d7eb 100644 --- a/pkg/rootless/rootless_test.go +++ b/pkg/rootless/rootless_test.go @@ -4,165 +4,3 @@ // package rootless - -import ( - "fmt" - "io/ioutil" - "os" - "path/filepath" - "testing" - - "github.com/stretchr/testify/assert" -) - -type uidMapping struct { - userNSUID int - hostUID int - rangeUID int -} - -type testScenario struct { - isRootless bool - uidMap []uidMapping -} - -var uidMapPathStore = uidMapPath - -func createTestUIDMapFile(input string) error { - f, err := os.Create(uidMapPath) - if err != nil { - return err - } - defer f.Close() - - _, err = f.WriteString(input) - if err != nil { - return err - } - - return nil -} - -func uidMapString(userNSUID, hostUID, rangeUID int) string { - return fmt.Sprintf("\t%d\t%d\t%d", userNSUID, hostUID, rangeUID) -} - -func testWithUIDMapContent(content string, expectedRootless bool, t *testing.T) { - assert := assert.New(t) - // Create a test-specific message that is added to each assert - // call. It will be displayed if any assert test fails. - msg := fmt.Sprintf("isRootless[%t]: %s", expectedRootless, content) - - tmpDir, err := ioutil.TempDir("", "") - assert.NoError(err) - - uidMapPath = filepath.Join(tmpDir, "testUIDMapFile") - defer func() { - uidMapPath = uidMapPathStore - os.RemoveAll(uidMapPath) - os.RemoveAll(tmpDir) - isRootless = false - initRootless = false - }() - - err = createTestUIDMapFile(content) - assert.NoError(err, msg) - - // make call to IsRootless, this should also call - // SetRootless - assert.Equal(expectedRootless, IsRootless(), msg) -} - -func TestIsRootless(t *testing.T) { - assert := assert.New(t) - - // by default isRootless should be set to false initially - assert.False(isRootless) - - allScenarios := []testScenario{ - //"User NS UID is not root UID" - { - isRootless: false, - uidMap: []uidMapping{ - {1, 0, 1}, - {1, 0, 1000}, - - {1, 1000, 1}, - {1, 1000, 1000}, - - {1000, 1000, 1}, - {1000, 1000, 1000}, - - {1000, 1000, 5555}, - }, - }, - - //"Host NS UID is root UID" - { - isRootless: false, - uidMap: []uidMapping{ - {0, 0, 1}, - {0, 0, 1000}, - - {1, 0, 1}, - {1, 0, 1000}, - - {1000, 0, 0}, - {1000, 0, 1}, - {1000, 0, 1000}, - }, - }, - - //"UID range is zero" - { - isRootless: false, - uidMap: []uidMapping{ - {0, 0, 0}, - {1, 0, 0}, - {0, 1, 0}, - {1, 1000, 0}, - {1000, 1000, 0}, - }, - }, - - //"Negative UIDs" - { - isRootless: false, - uidMap: []uidMapping{ - {-1, 0, 0}, - {-1, 0, 1}, - {-1, 0, 1000}, - - {0, -1, 0}, - {0, -1, 1}, - {0, -1, 1000}, - - {1000, 1000, -1}, - {1000, 1000, -1}, - {1000, 1000, -1000}, - }, - }, - - //"User NS UID is root UID, host UID is not root UID" - { - isRootless: true, - uidMap: []uidMapping{ - {0, 1, 1}, - {0, 1000, 1}, - {0, 1000, 5555}, - }, - }, - } - - // Run the tests - for _, scenario := range allScenarios { - for _, uidMap := range scenario.uidMap { - mapping := uidMapString(uidMap.userNSUID, uidMap.hostUID, uidMap.rangeUID) - testWithUIDMapContent(mapping, scenario.isRootless, t) - } - } - - testWithUIDMapContent("", false, t) - - testWithUIDMapContent("This is not a mapping", false, t) -} diff --git a/virtcontainers/api_test.go b/virtcontainers/api_test.go index 70ecfb928b..999999696e 100644 --- a/virtcontainers/api_test.go +++ b/virtcontainers/api_test.go @@ -16,6 +16,7 @@ import ( "testing" ktu "github.com/kata-containers/runtime/pkg/katatestutils" + "github.com/kata-containers/runtime/pkg/rootless" "github.com/kata-containers/runtime/virtcontainers/persist" "github.com/kata-containers/runtime/virtcontainers/persist/fs" "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" @@ -40,6 +41,10 @@ var containerAnnotations = map[string]string{ "container.hello": "container.world", } +func init() { + rootless.IsRootless = func() bool { return false } +} + func newEmptySpec() *specs.Spec { return &specs.Spec{ Linux: &specs.Linux{