Merge pull request #1862 from AkihiroSuda/decompose-rootless-pr

Disable rootless mode except RootlessCgMgr when executed as the root in userns (fix Docker-in-LXD regression)

Author: Mrunal Patel

checkpoint.go

@@ -4,11 +4,14 @@ package main
 
 import (
 	"fmt"
+	"os"
 	"strconv"
 	"strings"
 
 	"github.com/opencontainers/runc/libcontainer"
+	"github.com/opencontainers/runc/libcontainer/system"
 	"github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
 
 	"golang.org/x/sys/unix"
@@ -44,12 +47,8 @@ checkpointed.`,
 			return err
 		}
 		// XXX: Currently this is untested with rootless containers.
-		rootless, err := isRootless(context)
-		if err != nil {
-			return err
-		}
-		if rootless {
-			return fmt.Errorf("runc checkpoint requires root")
+		if os.Geteuid() != 0 || system.RunningInUserNS() {
+			logrus.Warn("runc checkpoint is untested with rootless containers")
 		}
 
 		container, err := getContainer(context)

libcontainer/cgroups/fs/apply_raw.go

@@ -65,7 +65,7 @@ type subsystem interface {
 type Manager struct {
 	mu       sync.Mutex
 	Cgroups  *configs.Cgroup
-	Rootless bool
+	Rootless bool // ignore permission-related errors
 	Paths    map[string]string
 }
 
@@ -174,7 +174,7 @@ func (m *Manager) Apply(pid int) (err error) {
 		m.Paths[sys.Name()] = p
 
 		if err := sys.Apply(d); err != nil {
-			// In the case of rootless, where an explicit cgroup path hasn't
+			// In the case of rootless (including euid=0 in userns), where an explicit cgroup path hasn't
 			// been set, we don't bail on error in case of permission problems.
 			// Cases where limits have been set (and we couldn't create our own
 			// cgroup) are handled by Set.
@@ -236,6 +236,12 @@ func (m *Manager) Set(container *configs.Config) error {
 	for _, sys := range subsystems {
 		path := paths[sys.Name()]
 		if err := sys.Set(path, container.Cgroups); err != nil {
+			if m.Rootless && sys.Name() == "devices" {
+				continue
+			}
+			// When m.Rootless is true, errors from the device subsystem are ignored because it is really not expected to work.
+			// However, errors from other subsystems are not ignored.
+			// see @test "runc create (rootless + limits + no cgrouppath + no permission) fails with informative error"
 			if path == "" {
 				// We never created a path for this cgroup, so we cannot set
 				// limits for it (though we have already tried at this point).

libcontainer/configs/config.go

@@ -186,12 +186,19 @@ type Config struct {
 	// callers keyring in this case.
 	NoNewKeyring bool `json:"no_new_keyring"`
 
-	// Rootless specifies whether the container is a rootless container.
-	Rootless bool `json:"rootless"`
-
 	// IntelRdt specifies settings for Intel RDT/CAT group that the container is placed into
 	// to limit the resources (e.g., L3 cache) the container has available
 	IntelRdt *IntelRdt `json:"intel_rdt,omitempty"`
+
+	// RootlessEUID is set when the runc was launched with non-zero EUID.
+	// Note that RootlessEUID is set to false when launched with EUID=0 in userns.
+	// When RootlessEUID is set, runc creates a new userns for the container.
+	// (config.json needs to contain userns settings)
+	RootlessEUID bool `json:"rootless_euid,omitempty"`
+
+	// RootlessCgroups is set when unlikely to have the full access to cgroups.
+	// When RootlessCgroups is set, cgroups errors are ignored.
+	RootlessCgroups bool `json:"rootless_cgroups,omitempty"`
 }
 
 type Hooks struct {

libcontainer/configs/validate/rootless.go

@@ -2,23 +2,18 @@ package validate
 
 import (
 	"fmt"
-	"os"
-	"reflect"
 	"strings"
 
 	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
-var (
-	geteuid = os.Geteuid
-	getegid = os.Getegid
-)
-
-func (v *ConfigValidator) rootless(config *configs.Config) error {
-	if err := rootlessMappings(config); err != nil {
+// rootlessEUID makes sure that the config can be applied when runc
+// is being executed as a non-root user (euid != 0) in the current user namespace.
+func (v *ConfigValidator) rootlessEUID(config *configs.Config) error {
+	if err := rootlessEUIDMappings(config); err != nil {
 		return err
 	}
-	if err := rootlessMount(config); err != nil {
+	if err := rootlessEUIDMount(config); err != nil {
 		return err
 	}
 
@@ -38,46 +33,24 @@ func hasIDMapping(id int, mappings []configs.IDMap) bool {
 	return false
 }
 
-func rootlessMappings(config *configs.Config) error {
-	if euid := geteuid(); euid != 0 {
-		if !config.Namespaces.Contains(configs.NEWUSER) {
-			return fmt.Errorf("rootless containers require user namespaces")
-		}
-		if len(config.UidMappings) == 0 {
-			return fmt.Errorf("rootless containers requires at least one UID mapping")
-		}
-		if len(config.GidMappings) == 0 {
-			return fmt.Errorf("rootless containers requires at least one GID mapping")
-		}
+func rootlessEUIDMappings(config *configs.Config) error {
+	if !config.Namespaces.Contains(configs.NEWUSER) {
+		return fmt.Errorf("rootless container requires user namespaces")
 	}
 
-	return nil
-}
-
-// cgroup verifies that the user isn't trying to set any cgroup limits or paths.
-func rootlessCgroup(config *configs.Config) error {
-	// Nothing set at all.
-	if config.Cgroups == nil || config.Cgroups.Resources == nil {
-		return nil
+	if len(config.UidMappings) == 0 {
+		return fmt.Errorf("rootless containers requires at least one UID mapping")
 	}
-
-	// Used for comparing to the zero value.
-	left := reflect.ValueOf(*config.Cgroups.Resources)
-	right := reflect.Zero(left.Type())
-
-	// This is all we need to do, since specconv won't add cgroup options in
-	// rootless mode.
-	if !reflect.DeepEqual(left.Interface(), right.Interface()) {
-		return fmt.Errorf("cannot specify resource limits in rootless container")
+	if len(config.GidMappings) == 0 {
+		return fmt.Errorf("rootless containers requires at least one GID mapping")
 	}
-
 	return nil
 }
 
 // mount verifies that the user isn't trying to set up any mounts they don't have
 // the rights to do. In addition, it makes sure that no mount has a `uid=` or
 // `gid=` option that doesn't resolve to root.
-func rootlessMount(config *configs.Config) error {
+func rootlessEUIDMount(config *configs.Config) error {
 	// XXX: We could whitelist allowed devices at this point, but I'm not
 	//      convinced that's a good idea. The kernel is the best arbiter of
 	//      access control.

libcontainer/configs/validate/rootless_test.go

@@ -6,82 +6,78 @@ import (
 	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
-func init() {
-	geteuid = func() int { return 1337 }
-	getegid = func() int { return 7331 }
-}
-
-func rootlessConfig() *configs.Config {
+func rootlessEUIDConfig() *configs.Config {
 	return &configs.Config{
-		Rootfs:   "/var",
-		Rootless: true,
+		Rootfs:          "/var",
+		RootlessEUID:    true,
+		RootlessCgroups: true,
 		Namespaces: configs.Namespaces(
 			[]configs.Namespace{
 				{Type: configs.NEWUSER},
 			},
 		),
 		UidMappings: []configs.IDMap{
 			{
-				HostID:      geteuid(),
+				HostID:      1337,
 				ContainerID: 0,
 				Size:        1,
 			},
 		},
 		GidMappings: []configs.IDMap{
 			{
-				HostID:      getegid(),
+				HostID:      7331,
 				ContainerID: 0,
 				Size:        1,
 			},
 		},
 	}
 }
 
-func TestValidateRootless(t *testing.T) {
+func TestValidateRootlessEUID(t *testing.T) {
 	validator := New()
 
-	config := rootlessConfig()
+	config := rootlessEUIDConfig()
 	if err := validator.Validate(config); err != nil {
 		t.Errorf("Expected error to not occur: %+v", err)
 	}
 }
 
-/* rootlessMappings() */
+/* rootlessEUIDMappings */
 
-func TestValidateRootlessUserns(t *testing.T) {
+func TestValidateRootlessEUIDUserns(t *testing.T) {
 	validator := New()
 
-	config := rootlessConfig()
+	config := rootlessEUIDConfig()
 	config.Namespaces = nil
 	if err := validator.Validate(config); err == nil {
 		t.Errorf("Expected error to occur if user namespaces not set")
 	}
 }
 
-func TestValidateRootlessMappingUid(t *testing.T) {
+func TestValidateRootlessEUIDMappingUid(t *testing.T) {
 	validator := New()
 
-	config := rootlessConfig()
+	config := rootlessEUIDConfig()
 	config.UidMappings = nil
 	if err := validator.Validate(config); err == nil {
 		t.Errorf("Expected error to occur if no uid mappings provided")
 	}
 }
 
-func TestValidateRootlessMappingGid(t *testing.T) {
+func TestValidateNonZeroEUIDMappingGid(t *testing.T) {
 	validator := New()
 
-	config := rootlessConfig()
+	config := rootlessEUIDConfig()
 	config.GidMappings = nil
 	if err := validator.Validate(config); err == nil {
 		t.Errorf("Expected error to occur if no gid mappings provided")
 	}
 }
 
-/* rootlessMount() */
+/* rootlessEUIDMount() */
 
-func TestValidateRootlessMountUid(t *testing.T) {
-	config := rootlessConfig()
+func TestValidateRootlessEUIDMountUid(t *testing.T) {
+	config := rootlessEUIDConfig()
 	validator := New()
 
 	config.Mounts = []*configs.Mount{
@@ -119,8 +115,8 @@ func TestValidateRootlessMountUid(t *testing.T) {
 	}
 }
 
-func TestValidateRootlessMountGid(t *testing.T) {
-	config := rootlessConfig()
+func TestValidateRootlessEUIDMountGid(t *testing.T) {
+	config := rootlessEUIDConfig()
 	validator := New()
 
 	config.Mounts = []*configs.Mount{

libcontainer/configs/validate/validator.go

@@ -44,8 +44,8 @@ func (v *ConfigValidator) Validate(config *configs.Config) error {
 	if err := v.intelrdt(config); err != nil {
 		return err
 	}
-	if config.Rootless {
-		if err := v.rootless(config); err != nil {
+	if config.RootlessEUID {
+		if err := v.rootlessEUID(config); err != nil {
 			return err
 		}
 	}