config/sysctl: fail if there is a + in the value

Signed-off-by: Peter Hunt <[email protected]>

Author: haircommander

internal/config/nsmgr/nsmgr.go

@@ -88,7 +88,11 @@ func (mgr *NamespaceManager) NewPodNamespaces(cfg *PodNamespacesConfig) ([]Names
 	}
 
 	if len(cfg.Sysctls) != 0 {
-		pinnsArgs = append(pinnsArgs, "-s", getSysctlForPinns(cfg.Sysctls))
+		pinnsSysctls, err := getSysctlForPinns(cfg.Sysctls)
+		if err != nil {
+			return nil, errors.Wrapf(err, "invalid sysctl")
+		}
+		pinnsArgs = append(pinnsArgs, "-s", pinnsSysctls)
 	}
 
 	var rootPair idtools.IDPair
@@ -171,14 +175,18 @@ func getMappingsForPinns(mappings []idtools.IDMap) string {
 	return g.String()
 }
 
-func getSysctlForPinns(sysctls map[string]string) string {
-	// this assumes there's no sysctl with a `+` in it
+func getSysctlForPinns(sysctls map[string]string) (string, error) {
+	// This assumes there's no valid sysctl value with a `+` in it
+	// and as such errors if one is found.
 	const pinnsSysctlDelim = "+"
 	g := new(bytes.Buffer)
 	for key, value := range sysctls {
+		if strings.Contains(key, pinnsSysctlDelim) || strings.Contains(value, pinnsSysctlDelim) {
+			return "", errors.Errorf("'%s=%s' is invalid: %s found yet should not be present", key, value, pinnsSysctlDelim)
+		}
 		fmt.Fprintf(g, "'%s=%s'%s", key, value, pinnsSysctlDelim)
 	}
-	return strings.TrimSuffix(g.String(), pinnsSysctlDelim)
+	return strings.TrimSuffix(g.String(), pinnsSysctlDelim), nil
 }
 
 // NamespaceFromProcEntry creates a new namespace object from a bind mount from a processes proc entry.

test/pod.bats

@@ -156,11 +156,32 @@ function teardown() {
 	[[ "$output" == *"net.ipv4.ip_forward = 1"* ]]
 }
 
-@test "fail to pass pod sysctls to runtime if invalid" {
+@test "fail to pass pod sysctls to runtime if invalid spaces" {
 	CONTAINER_DEFAULT_SYSCTLS="net.ipv4.ip_forward = 1" crio &
 	! wait_until_reachable
 }
 
+@test "fail to pass pod sysctl to runtime if invalid value" {
+	if test -n "$CONTAINER_UID_MAPPINGS"; then
+		skip "userNS enabled"
+	fi
+	start_crio
+
+	jq --arg sysctl "1024 65000'+'net.ipv4.ip_forward=0'" \
+	'	  .linux.sysctls = {
+			"net.ipv4.ip_local_port_range": $sysctl,
+		}' "$TESTDATA"/sandbox_config.json > "$TESTDIR"/sandbox.json
+
+	! crictl runp "$TESTDIR"/sandbox.json
+
+	jq --arg sysctl "net.ipv4.ip_local_port_range=1024 65000'+'net.ipv4.ip_forward" \
+	'	  .linux.sysctls = {
+			($sysctl): "0",
+		}' "$TESTDATA"/sandbox_config.json > "$TESTDIR"/sandbox.json
+
+	! crictl runp "$TESTDIR"/sandbox.json
+}
+
 @test "skip pod sysctls to runtime if host" {
 	if test -n "$CONTAINER_UID_MAPPINGS"; then
 		skip "userNS enabled"