Merge pull request #102393 from mengjiao-liu/fix-sysctl-regex

Upgrade preparation to verify sysctl values containing forward slashes by regex

pkg/kubelet/kuberuntime/kuberuntime_manager.go

@@ -52,6 +52,7 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/metrics"
 	proberesults "k8s.io/kubernetes/pkg/kubelet/prober/results"
 	"k8s.io/kubernetes/pkg/kubelet/runtimeclass"
+	"k8s.io/kubernetes/pkg/kubelet/sysctl"
 	"k8s.io/kubernetes/pkg/kubelet/types"
 	"k8s.io/kubernetes/pkg/kubelet/util/cache"
 	"k8s.io/kubernetes/pkg/kubelet/util/format"
@@ -802,6 +803,17 @@ func (m *kubeGenericRuntimeManager) SyncPod(pod *v1.Pod, podStatus *kubecontaine
 		metrics.StartedPodsTotal.Inc()
 		createSandboxResult := kubecontainer.NewSyncResult(kubecontainer.CreatePodSandbox, format.Pod(pod))
 		result.AddSyncResult(createSandboxResult)
+
+		// ConvertPodSysctlsVariableToDotsSeparator converts sysctl variable
+		// in the Pod.Spec.SecurityContext.Sysctls slice into a dot as a separator.
+		// runc uses the dot as the separator to verify whether the sysctl variable
+		// is correct in a separate namespace, so when using the slash as the sysctl
+		// variable separator, runc returns an error: "sysctl is not in a separate kernel namespace"
+		// and the podSandBox cannot be successfully created. Therefore, before calling runc,
+		// we need to convert the sysctl variable, the dot is used as a separator to separate the kernel namespace.
+		// When runc supports slash as sysctl separator, this function can no longer be used.
+		sysctl.ConvertPodSysctlsVariableToDotsSeparator(pod.Spec.SecurityContext)
+
 		podSandboxID, msg, err = m.createPodSandbox(pod, podContainerChanges.Attempt)
 		if err != nil {
 			// createPodSandbox can return an error from CNI, CSI,

pkg/kubelet/kuberuntime/kuberuntime_manager_test.go

@@ -533,6 +533,64 @@ func TestSyncPod(t *testing.T) {
 	}
 }
 
+func TestSyncPodWithConvertedPodSysctls(t *testing.T) {
+	fakeRuntime, _, m, err := createTestRuntimeManager()
+	assert.NoError(t, err)
+
+	containers := []v1.Container{
+		{
+			Name:            "foo",
+			Image:           "busybox",
+			ImagePullPolicy: v1.PullIfNotPresent,
+		},
+	}
+
+	securityContext := &v1.PodSecurityContext{
+		Sysctls: []v1.Sysctl{
+			{
+				Name:  "kernel/shm_rmid_forced",
+				Value: "1",
+			},
+			{
+				Name:  "net/ipv4/ip_local_port_range",
+				Value: "1024 65535",
+			},
+		},
+	}
+	exceptSysctls := []v1.Sysctl{
+		{
+			Name:  "kernel.shm_rmid_forced",
+			Value: "1",
+		},
+		{
+			Name:  "net.ipv4.ip_local_port_range",
+			Value: "1024 65535",
+		},
+	}
+	pod := &v1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			UID:       "12345678",
+			Name:      "foo",
+			Namespace: "new",
+		},
+		Spec: v1.PodSpec{
+			Containers:      containers,
+			SecurityContext: securityContext,
+		},
+	}
+
+	backOff := flowcontrol.NewBackOff(time.Second, time.Minute)
+	result := m.SyncPod(pod, &kubecontainer.PodStatus{}, []v1.Secret{}, backOff)
+	assert.NoError(t, result.Error())
+	assert.Equal(t, exceptSysctls, pod.Spec.SecurityContext.Sysctls)
+	for _, sandbox := range fakeRuntime.Sandboxes {
+		assert.Equal(t, runtimeapi.PodSandboxState_SANDBOX_READY, sandbox.State)
+	}
+	for _, c := range fakeRuntime.Containers {
+		assert.Equal(t, runtimeapi.ContainerState_CONTAINER_RUNNING, c.State)
+	}
+}
+
 func TestPruneInitContainers(t *testing.T) {
 	fakeRuntime, _, m, err := createTestRuntimeManager()
 	assert.NoError(t, err)

pkg/kubelet/sysctl/allowlist.go

@@ -48,13 +48,14 @@ func NewAllowlist(patterns []string) (*patternAllowlist, error) {
 	}
 
 	for _, s := range patterns {
-		if !policyvalidation.IsValidSysctlPattern(s) {
+		if !policyvalidation.IsValidSysctlPattern(s, true) {
 			return nil, fmt.Errorf("sysctl %q must have at most %d characters and match regex %s",
 				s,
 				validation.SysctlMaxLength,
-				policyvalidation.SysctlPatternFmt,
+				policyvalidation.SysctlContainSlashPatternFmt,
 			)
 		}
+		s = convertSysctlVariableToDotsSeparator(s)
 		if strings.HasSuffix(s, "*") {
 			prefix := s[:len(s)-1]
 			ns := NamespacedBy(prefix)
@@ -81,6 +82,7 @@ func NewAllowlist(patterns []string) (*patternAllowlist, error) {
 // respective namespaces with the host. This check is only possible for sysctls on
 // the static default allowlist, not those on the custom allowlist provided by the admin.
 func (w *patternAllowlist) validateSysctl(sysctl string, hostNet, hostIPC bool) error {
+	sysctl = convertSysctlVariableToDotsSeparator(sysctl)
 	nsErrorFmt := "%q not allowed with host %s enabled"
 	if ns, found := w.sysctls[sysctl]; found {
 		if ns == ipcNamespace && hostIPC {

pkg/kubelet/sysctl/allowlist_test.go

@@ -29,10 +29,12 @@ func TestNewAllowlist(t *testing.T) {
 	}
 	for _, test := range []Test{
 		{sysctls: []string{"kernel.msg*", "kernel.sem"}},
+		{sysctls: []string{"kernel/msg*", "kernel/sem"}},
 		{sysctls: []string{" kernel.msg*"}, err: true},
 		{sysctls: []string{"kernel.msg* "}, err: true},
 		{sysctls: []string{"net.-"}, err: true},
 		{sysctls: []string{"net.*.foo"}, err: true},
+		{sysctls: []string{"net.*/foo"}, err: true},
 		{sysctls: []string{"foo"}, err: true},
 	} {
 		_, err := NewAllowlist(append(sysctl.SafeSysctlAllowlist(), test.sysctls...))
@@ -51,9 +53,11 @@ func TestAllowlist(t *testing.T) {
 	}
 	valid := []Test{
 		{sysctl: "kernel.shm_rmid_forced"},
+		{sysctl: "kernel/shm_rmid_forced"},
 		{sysctl: "net.ipv4.ip_local_port_range"},
 		{sysctl: "kernel.msgmax"},
 		{sysctl: "kernel.sem"},
+		{sysctl: "kernel/sem"},
 	}
 	invalid := []Test{
 		{sysctl: "kernel.shm_rmid_forced", hostIPC: true},

pkg/kubelet/sysctl/util.go

@@ -0,0 +1,63 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package sysctl
+
+import (
+	"strings"
+
+	"k8s.io/api/core/v1"
+)
+
+// convertSysctlVariableToDotsSeparator can return sysctl variables in dots separator format.
+// The '/' separator is also accepted in place of a '.'.
+// Convert the sysctl variables to dots separator format for validation.
+// More info:
+//   https://man7.org/linux/man-pages/man8/sysctl.8.html
+//   https://man7.org/linux/man-pages/man5/sysctl.d.5.html
+func convertSysctlVariableToDotsSeparator(val string) string {
+	if val == "" {
+		return val
+	}
+	firstSepIndex := strings.IndexAny(val, "./")
+	if firstSepIndex == -1 || val[firstSepIndex] == '.' {
+		return val
+	}
+
+	f := func(r rune) rune {
+		switch r {
+		case '.':
+			return '/'
+		case '/':
+			return '.'
+		}
+		return r
+	}
+	return strings.Map(f, val)
+}
+
+// ConvertPodSysctlsVariableToDotsSeparator converts sysctls variable in the Pod.Spec.SecurityContext.Sysctls slice into a dot as a separator
+// according to the linux sysctl conversion rules.
+// see https://man7.org/linux/man-pages/man5/sysctl.d.5.html for more details.
+func ConvertPodSysctlsVariableToDotsSeparator(securityContext *v1.PodSecurityContext) {
+	if securityContext == nil {
+		return
+	}
+	for i, sysctl := range securityContext.Sysctls {
+		securityContext.Sysctls[i].Name = convertSysctlVariableToDotsSeparator(sysctl.Name)
+	}
+	return
+}

pkg/kubelet/sysctl/util_test.go

@@ -0,0 +1,97 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package sysctl
+
+import (
+	"testing"
+
+	v1 "k8s.io/api/core/v1"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestConvertSysctlVariableToDotsSeparator tests whether the sysctl variable
+// can be correctly converted to a dot as a separator.
+func TestConvertSysctlVariableToDotsSeparator(t *testing.T) {
+	type testCase struct {
+		in  string
+		out string
+	}
+	valid := []testCase{
+		{in: "kernel.shm_rmid_forced", out: "kernel.shm_rmid_forced"},
+		{in: "kernel/shm_rmid_forced", out: "kernel.shm_rmid_forced"},
+		{in: "net.ipv4.conf.eno2/100.rp_filter", out: "net.ipv4.conf.eno2/100.rp_filter"},
+		{in: "net/ipv4/conf/eno2.100/rp_filter", out: "net.ipv4.conf.eno2/100.rp_filter"},
+		{in: "net/ipv4/ip_local_port_range", out: "net.ipv4.ip_local_port_range"},
+		{in: "kernel/msgmax", out: "kernel.msgmax"},
+		{in: "kernel/sem", out: "kernel.sem"},
+	}
+
+	for _, test := range valid {
+		convertSysctlVal := convertSysctlVariableToDotsSeparator(test.in)
+		assert.Equalf(t, test.out, convertSysctlVal, "The sysctl variable was not converted correctly. got: %s, want: %s", convertSysctlVal, test.out)
+	}
+}
+
+// TestConvertPodSysctlsVariableToDotsSeparator tests whether the sysctls variable
+// can be correctly converted to a dot as a separator.
+func TestConvertPodSysctlsVariableToDotsSeparator(t *testing.T) {
+
+	sysctls := []v1.Sysctl{
+		{
+			Name:  "kernel.msgmax",
+			Value: "8192",
+		},
+		{
+			Name:  "kernel.shm_rmid_forced",
+			Value: "1",
+		},
+		{
+			Name:  "net.ipv4.conf.eno2/100.rp_filter",
+			Value: "1",
+		},
+		{
+			Name:  "net/ipv4/ip_local_port_range",
+			Value: "1024 65535",
+		},
+	}
+	exceptSysctls := []v1.Sysctl{
+		{
+			Name:  "kernel.msgmax",
+			Value: "8192",
+		},
+		{
+			Name:  "kernel.shm_rmid_forced",
+			Value: "1",
+		},
+		{
+			Name:  "net.ipv4.conf.eno2/100.rp_filter",
+			Value: "1",
+		},
+		{
+			Name:  "net.ipv4.ip_local_port_range",
+			Value: "1024 65535",
+		},
+	}
+	securityContext := &v1.PodSecurityContext{
+		Sysctls: sysctls,
+	}
+
+	ConvertPodSysctlsVariableToDotsSeparator(securityContext)
+	assert.Equalf(t, securityContext.Sysctls, exceptSysctls, "The sysctls name was not converted correctly. got: %s, want: %s", securityContext.Sysctls, exceptSysctls)
+
+}