Repository: opencontainers/runc (go)

Issue:
'/' is allowed as a separator in sysctl name
[Runtime spec](https://github.com/opencontainers/runtime-spec/blob/master/config-linux.md#sysctl) says:

> sysctl (object, OPTIONAL) allows kernel parameters to be modified at
runtime for the container. For more information, see the sysctl(8)
man page.


and linux sysctl says:

```
variable
The name of a key to read from. An example is
kernel.ostype. The '/' separator is also accepted in place of a '.'.
```
```
       Note that either "/" or "."  may be used as separators within
       sysctl variable names. If the first separator is a slash,
       remaining slashes and dots are left intact. If the first
       separator is a dot, dots and slashes are interchanged.
       "kernel.domainname=foo" and "kernel/domainname=foo" are
       equivalent and will cause "foo" to be written to
       /proc/sys/kernel/domainname. Either
       "net.ipv4.conf.enp3s0/200.forwarding" or
       "net/ipv4/conf/enp3s0.200/forwarding" may be used to refer to
       /proc/sys/net/ipv4/conf/enp3s0.200/forwarding. A glob glob(7)
       pattern may be used to write the same value to all matching keys.
       Keys for which an explicit pattern exists will be excluded from
       any glob matching. In addition, a key may be explicitly excluded
       from being set by any matching glob patterns by specifying the
       key name prefixed with a "-" character and not followed by "=",
       see SYNOPSIS.
```

https://man7.org/linux/man-pages/man8/sysctl.8.html
https://man7.org/linux/man-pages/man5/sysctl.d.5.html

However, in fact, the sysctl name in runc does not support a slash as a separator.

This PR  https://github.com/opencontainers/runc/pull/3254  wants to solve this problem, but I think the conversion method does not match the linux sysctl definition.

see https://github.com/opencontainers/runc/pull/3254/files#r739936786 for more details.

So I opened this issue to track the fix of this issue andI will open a new PR to fix this.

Related to: 
- kubernetes/kubernetes#102393
- https://github.com/opencontainers/runc/pull/3254

/assign

Pull request:
Fix the conversion of sysctl variable dots and slashes
Fixes #3256 

see  
- https://github.com/opencontainers/runc/pull/3254/files#r739936786 
- https://github.com/kubernetes/kubernetes/pull/102393#discussion_r661550379

Fix the sysctl conversion function according to the linux ssyctl conversion definition.
```
       Note that either "/" or "."  may be used as separators within
       sysctl variable names. If the first separator is a slash,
       remaining slashes and dots are left intact. If the first
       separator is a dot, dots and slashes are interchanged.
       "kernel.domainname=foo" and "kernel/domainname=foo" are
       equivalent and will cause "foo" to be written to
       /proc/sys/kernel/domainname. Either
       "net.ipv4.conf.enp3s0/200.forwarding" or
       "net/ipv4/conf/enp3s0.200/forwarding" may be used to refer to
       /proc/sys/net/ipv4/conf/enp3s0.200/forwarding. A glob glob(7)
       pattern may be used to write the same value to all matching keys.
       Keys for which an explicit pattern exists will be excluded from
       any glob matching. In addition, a key may be explicitly excluded
       from being set by any matching glob patterns by specifying the
       key name prefixed with a "-" character and not followed by "=",
       see SYNOPSIS.
```
see following webpages for more details:
https://man7.org/linux/man-pages/man8/sysctl.8.html
https://man7.org/linux/man-pages/man5/sysctl.d.5.html 

Related to: 
- kubernetes/kubernetes#102393
- https://github.com/opencontainers/runc/pull/3254




Patch:
diff --git a/libcontainer/configs/validate/validator.go b/libcontainer/configs/validate/validator.go
index a2a79c29..6493124a 100644
--- a/libcontainer/configs/validate/validator.go
+++ b/libcontainer/configs/validate/validator.go
@@ -128,6 +128,36 @@ func (v *ConfigValidator) cgroupnamespace(config *configs.Config) error {
 	return nil
 }
 
+// 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
+// For example:
+// Input sysctl variable "net/ipv4/conf/eno2.100.rp_filter"
+// will return the converted value "net.ipv4.conf.eno2/100.rp_filter"
+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)
+}
+
 // sysctl validates that the specified sysctl keys are valid or not.
 // /proc/sys isn't completely namespaced and depending on which namespaces
 // are specified, a subset of sysctls are permitted.
@@ -150,7 +180,7 @@ func (v *ConfigValidator) sysctl(config *configs.Config) error {
 	)
 
 	for s := range config.Sysctl {
-		s := strings.Replace(s, "/", ".", -1)
+		s := convertSysctlVariableToDotsSeparator(s)
 		if validSysctlMap[s] || strings.HasPrefix(s, "fs.mqueue.") {
 			if config.Namespaces.Contains(configs.NEWIPC) {
 				continue


Test patch:
diff --git a/libcontainer/configs/validate/validator_test.go b/libcontainer/configs/validate/validator_test.go
index 169c2751..5181333f 100644
--- a/libcontainer/configs/validate/validator_test.go
+++ b/libcontainer/configs/validate/validator_test.go
@@ -1,4 +1,4 @@
-package validate_test
+package validate
 
 import (
 	"os"
@@ -6,7 +6,6 @@ import (
 	"testing"
 
 	"github.com/opencontainers/runc/libcontainer/configs"
-	"github.com/opencontainers/runc/libcontainer/configs/validate"
 	"golang.org/x/sys/unix"
 )
 
@@ -15,7 +14,7 @@ func TestValidate(t *testing.T) {
 		Rootfs: "/var",
 	}
 
-	validator := validate.New()
+	validator := New()
 	err := validator.Validate(config)
 	if err != nil {
 		t.Errorf("Expected error to not occur: %+v", err)
@@ -33,7 +32,7 @@ func TestValidateWithInvalidRootfs(t *testing.T) {
 		Rootfs: dir,
 	}
 
-	validator := validate.New()
+	validator := New()
 	err := validator.Validate(config)
 	if err == nil {
 		t.Error("Expected error to occur but it was nil")
@@ -48,7 +47,7 @@ func TestValidateNetworkWithoutNETNamespace(t *testing.T) {
 		Networks:   []*configs.Network{network},
 	}
 
-	validator := validate.New()
+	validator := New()
 	err := validator.Validate(config)
 	if err == nil {
 		t.Error("Expected error to occur but it was nil")
@@ -63,7 +62,7 @@ func TestValidateNetworkRoutesWithoutNETNamespace(t *testing.T) {
 		Routes:     []*configs.Route{route},
 	}
 
-	validator := validate.New()
+	validator := New()
 	err := validator.Validate(config)
 	if err == nil {
 		t.Error("Expected error to occur but it was nil")
@@ -81,7 +80,7 @@ func TestValidateHostname(t *testing.T) {
 		),
 	}
 
-	validator := validate.New()
+	validator := New()
 	err := validator.Validate(config)
 	if err != nil {
 		t.Errorf("Expected error to not occur: %+v", err)
@@ -94,7 +93,7 @@ func TestValidateHostnameWithoutUTSNamespace(t *testing.T) {
 		Hostname: "runc",
 	}
 
-	validator := validate.New()
+	validator := New()
 	err := validator.Validate(config)
 	if err == nil {
 		t.Error("Expected error to occur but it was nil")
@@ -112,7 +111,7 @@ func TestValidateSecurityWithMaskPaths(t *testing.T) {
 		),
 	}
 
-	validator := validate.New()
+	validator := New()
 	err := validator.Validate(config)
 	if err != nil {
 		t.Errorf("Expected error to not occur: %+v", err)
@@ -130,7 +129,7 @@ func TestValidateSecurityWithROPaths(t *testing.T) {
 		),
 	}
 
-	validator := validate.New()
+	validator := New()
 	err := validator.Validate(config)
 	if err != nil {
 		t.Errorf("Expected error to not occur: %+v", err)
@@ -144,7 +143,7 @@ func TestValidateSecurityWithoutNEWNS(t *testing.T) {
 		ReadonlyPaths: []string{"/proc/sys"},
 	}
 
-	validator := validate.New()
+	validator := New()
 	err := validator.Validate(config)
 	if err == nil {
 		t.Error("Expected error to occur but it was nil")
@@ -164,7 +163,7 @@ func TestValidateUsernamespace(t *testing.T) {
 		),
 	}
 
-	validator := validate.New()
+	validator := New()
 	err := validator.Validate(config)
 	if err != nil {
 		t.Errorf("expected error to not occur %+v", err)
@@ -178,21 +177,47 @@ func TestValidateUsernamespaceWithoutUserNS(t *testing.T) {
 		UidMappings: []configs.IDMap{uidMap},
 	}
 
-	validator := validate.New()
+	validator := New()
 	err := validator.Validate(config)
 	if err == nil {
 		t.Error("Expected error to occur but it was nil")
 	}
 }
 
+// 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)
+		if convertSysctlVal != test.out {
+			t.Errorf("The sysctl variable was not converted correctly. got: %s, want: %s", convertSysctlVal, test.out)
+		}
+	}
+}
+
 func TestValidateSysctl(t *testing.T) {
 	sysctl := map[string]string{
-		"fs.mqueue.ctl": "ctl",
-		"fs/mqueue/ctl": "ctl",
-		"net.ctl":       "ctl",
-		"net/ctl":       "ctl",
-		"kernel.ctl":    "ctl",
-		"kernel/ctl":    "ctl",
+		"fs.mqueue.ctl":                    "ctl",
+		"fs/mqueue/ctl":                    "ctl",
+		"net.ctl":                          "ctl",
+		"net/ctl":                          "ctl",
+		"net.ipv4.conf.eno2/100.rp_filter": "ctl",
+		"kernel.ctl":                       "ctl",
+		"kernel/ctl":                       "ctl",
 	}
 
 	for k, v := range sysctl {
@@ -201,7 +226,7 @@ func TestValidateSysctl(t *testing.T) {
 			Sysctl: map[string]string{k: v},
 		}
 
-		validator := validate.New()
+		validator := New()
 		err := validator.Validate(config)
 		if err == nil {
 			t.Error("Expected error to occur but it was nil")
@@ -211,12 +236,13 @@ func TestValidateSysctl(t *testing.T) {
 
 func TestValidateValidSysctl(t *testing.T) {
 	sysctl := map[string]string{
-		"fs.mqueue.ctl": "ctl",
-		"fs/mqueue/ctl": "ctl",
-		"net.ctl":       "ctl",
-		"net/ctl":       "ctl",
-		"kernel.msgmax": "ctl",
-		"kernel/msgmax": "ctl",
+		"fs.mqueue.ctl":                    "ctl",
+		"fs/mqueue/ctl":                    "ctl",
+		"net.ctl":                          "ctl",
+		"net/ctl":                          "ctl",
+		"net.ipv4.conf.eno2/100.rp_filter": "ctl",
+		"kernel.msgmax":                    "ctl",
+		"kernel/msgmax":                    "ctl",
 	}
 
 	for k, v := range sysctl {
@@ -233,7 +259,7 @@ func TestValidateValidSysctl(t *testing.T) {
 			},
 		}
 
-		validator := validate.New()
+		validator := New()
 		err := validator.Validate(config)
 		if err != nil {
 			t.Errorf("Expected error to not occur with {%s=%s} but got: %q", k, v, err)
@@ -255,7 +281,7 @@ func TestValidateSysctlWithSameNs(t *testing.T) {
 		),
 	}
 
-	validator := validate.New()
+	validator := New()
 	err := validator.Validate(config)
 	if err == nil {
 		t.Error("Expected error to occur but it was nil")
@@ -297,7 +323,7 @@ func TestValidateSysctlWithBindHostNetNS(t *testing.T) {
 		),
 	}
 
-	validator := validate.New()
+	validator := New()
 	if err := validator.Validate(config); err == nil {
 		t.Error("Expected error to occur but it was nil")
 	}
@@ -310,7 +336,7 @@ func TestValidateSysctlWithoutNETNamespace(t *testing.T) {
 		Namespaces: []configs.Namespace{},
 	}
 
-	validator := validate.New()
+	validator := New()
 	err := validator.Validate(config)
 	if err == nil {
 		t.Error("Expected error to occur but it was nil")
@@ -332,7 +358,7 @@ func TestValidateMounts(t *testing.T) {
 		{isErr: false, dest: "/abs/but/../unclean"},
 	}
 
-	validator := validate.New()
+	validator := New()
 
 	for _, tc := range testCases {
 		config := &configs.Config{