Merge pull request #4889 from tych0/allow-ucounts

libcontainer/validator: allow setting user.* sysctls inside userns

Author: rata

libcontainer/configs/validate/validator.go

@@ -274,6 +274,30 @@ func sysctl(config *configs.Config) error {
 				return fmt.Errorf("sysctl %q is not allowed as it conflicts with the OCI %q field", s, "hostname")
 			}
 		}
+
+		if strings.HasPrefix(s, "user.") {
+
+			// while it is technically true that a non-userns
+			// container can write to /proc/sys/user on behalf of
+			// the init_user_ns, it was not previously supported,
+			// and doesn't guarantee that someone else spawns a
+			// different container and writes there, changing the
+			// values. in particular, setting something like
+			// max_user_namespaces to non-zero could be a vector to
+			// use 0-days where the admin had previously disabled
+			// them.
+			//
+			// additionally, this setting affects other host
+			// processes that are not container related.
+			//
+			// so let's refuse this unless we know for sure it
+			// won't touch anything else.
+			if !config.Namespaces.Contains(configs.NEWUSER) {
+				return fmt.Errorf("setting ucounts without a user namespace not allowed: %v", s)
+			}
+			continue
+		}
+
 		return fmt.Errorf("sysctl %q is not in a separate kernel namespace", s)
 	}
 

libcontainer/configs/validate/validator_test.go

@@ -1019,6 +1019,40 @@ func TestValidateNetDevices(t *testing.T) {
 	}
 }
 
+func TestValidateUserSysctlWithUserNamespace(t *testing.T) {
+	if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) {
+		t.Skip("Test requires userns.")
+	}
+	config := &configs.Config{
+		Rootfs: "/var",
+		Sysctl: map[string]string{"user.max_inotify_watches": "8192"},
+		Namespaces: configs.Namespaces(
+			[]configs.Namespace{
+				{Type: configs.NEWUSER},
+			},
+		),
+		UIDMappings: []configs.IDMap{{HostID: 0, ContainerID: 123, Size: 100}},
+		GIDMappings: []configs.IDMap{{HostID: 0, ContainerID: 123, Size: 100}},
+	}
+
+	err := Validate(config)
+	if err != nil {
+		t.Errorf("Expected error to not occur with user.* sysctl and NEWUSER namespace: %+v", err)
+	}
+}
+
+func TestValidateUserSysctlWithoutUserNamespace(t *testing.T) {
+	config := &configs.Config{
+		Rootfs: "/var",
+		Sysctl: map[string]string{"user.max_inotify_watches": "8192"},
+	}
+
+	err := Validate(config)
+	if err == nil {
+		t.Error("Expected error to occur with user.* sysctl without NEWUSER namespace but it was nil")
+	}
+}
+
 func TestDevValidName(t *testing.T) {
 	testCases := []struct {
 		name  string