Merge pull request #693 from mmirecki/POC_sysctl_whitelist

Add sysctl allowList

plugins/meta/tuning/tuning.go

@@ -19,12 +19,14 @@ package main
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net"
 	"os"
 	"path"
 	"path/filepath"
+	"regexp"
 	"strings"
 
 	"github.com/vishvananda/netlink"
@@ -40,6 +42,8 @@ import (
 )
 
 const defaultDataDir = "/run/cni/tuning"
+const defaultAllowlistDir = "/etc/cni/tuning/"
+const defaultAllowlistFile = "allowlist.conf"
 
 // TuningConf represents the network tuning configuration.
 type TuningConf struct {
@@ -305,6 +309,10 @@ func cmdAdd(args *skel.CmdArgs) error {
 		return err
 	}
 
+	if err = validateSysctlConf(tuningConf); err != nil {
+		return err
+	}
+
 	// Parse previous result.
 	if tuningConf.RawPrevResult == nil {
 		return fmt.Errorf("Required prevResult missing")
@@ -477,3 +485,60 @@ func cmdCheck(args *skel.CmdArgs) error {
 
 	return nil
 }
+
+// Validate the sysctls in the tuning config are on the sysctl allowlist file.
+// Note that if the allowlist file is missing no validation takes place.
+func validateSysctlConf(tuningConf *TuningConf) error {
+	isPresent, allowlist, err := readAllowlist()
+	if err != nil {
+		return err
+	}
+	if !isPresent {
+		return nil
+	}
+	for sysctl, _ := range tuningConf.SysCtl {
+		match, err := contains(sysctl, allowlist)
+		if err != nil {
+			return err
+		}
+		if !match {
+			return errors.New(fmt.Sprintf("Sysctl %s is not allowed. Only the following sysctls are allowed: %+v", sysctl, allowlist))
+		}
+	}
+	return nil
+}
+
+// Validate the allowList contains the given sysctl
+func contains(sysctl string, allowList []string) (bool, error) {
+	for _, allowListElement := range allowList {
+		match, err := regexp.MatchString(allowListElement, sysctl)
+		if err != nil {
+			return false, err
+		}
+		if match {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
+// Read the systctl allowlist from file. Return info if the file is present and the read allowList if it is
+func readAllowlist() (bool, []string, error) {
+	if _, err := os.Stat(filepath.Join(defaultAllowlistDir, defaultAllowlistFile)); os.IsNotExist(err) {
+		return false, nil, nil
+	}
+	dat, err := os.ReadFile(filepath.Join(defaultAllowlistDir, defaultAllowlistFile))
+	if err != nil {
+		return false, nil, err
+	}
+
+	lines := strings.Split(string(dat), "\n")
+	allowList := []string{}
+	for _, line := range lines {
+		line = strings.TrimSpace(line)
+		if len(line) > 0 {
+			allowList = append(allowList, line)
+		}
+	}
+	return true, allowList, nil
+}

plugins/meta/tuning/tuning_test.go

@@ -18,6 +18,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"net"
+	"os"
+	"path/filepath"
 
 	"github.com/containernetworking/cni/pkg/skel"
 	"github.com/containernetworking/cni/pkg/types"
@@ -75,6 +77,24 @@ func buildOneConfig(name, cniVersion string, orig *TuningConf, prevResult types.
 
 }
 
+func createSysctlAllowFile(sysctls []string) error {
+	err := os.MkdirAll(defaultAllowlistDir, 0755)
+	if err != nil {
+		return err
+	}
+	f, err := os.Create(filepath.Join(defaultAllowlistDir, defaultAllowlistFile))
+	if err != nil {
+		return err
+	}
+	for _, sysctl := range sysctls {
+		_, err = f.WriteString(fmt.Sprintf("%s\n", sysctl))
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 var _ = Describe("tuning plugin", func() {
 	var originalNS, targetNS ns.NetNS
 	const IFNAME string = "dummy0"
@@ -117,6 +137,7 @@ var _ = Describe("tuning plugin", func() {
 		Expect(testutils.UnmountNS(originalNS)).To(Succeed())
 		Expect(targetNS.Close()).To(Succeed())
 		Expect(testutils.UnmountNS(targetNS)).To(Succeed())
+		os.RemoveAll(defaultAllowlistDir)
 	})
 
 	for _, ver := range []string{"0.3.0", "0.3.1", "0.4.0", "1.0.0"} {
@@ -989,5 +1010,117 @@ var _ = Describe("tuning plugin", func() {
 			})
 			Expect(err).NotTo(HaveOccurred())
 		})
+
+		It(fmt.Sprintf("[%s] passes prevResult through unchanged", ver), func() {
+			conf := []byte(fmt.Sprintf(`{
+				"name": "test",
+				"type": "tuning",
+				"cniVersion": "%s",
+				"sysctl": {
+					"net.ipv4.conf.all.log_martians": "1"
+				},
+				"prevResult": {
+					"interfaces": [
+						{"name": "dummy0", "sandbox":"netns"}
+					],
+					"ips": [
+						{
+							"version": "4",
+							"address": "10.0.0.2/24",
+							"gateway": "10.0.0.1",
+							"interface": 0
+						}
+					]
+				}
+			}`, ver))
+
+			args := &skel.CmdArgs{
+				ContainerID: "dummy",
+				Netns:       targetNS.Path(),
+				IfName:      IFNAME,
+				StdinData:   conf,
+			}
+
+			beforeConf = configToRestore{}
+
+			err := createSysctlAllowFile([]string{"^net\\.ipv4\\.conf\\.other\\.[a-z_]*$"})
+			Expect(err).NotTo(HaveOccurred())
+
+			err = originalNS.Do(func(ns.NetNS) error {
+				defer GinkgoRecover()
+
+				_, _, err := testutils.CmdAddWithArgs(args, func() error {
+					return cmdAdd(args)
+				})
+				Expect(err).To(HaveOccurred())
+
+				err = testutils.CmdDel(originalNS.Path(),
+					args.ContainerID, "", func() error { return cmdDel(args) })
+				Expect(err).NotTo(HaveOccurred())
+
+				return nil
+			})
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It(fmt.Sprintf("[%s] passes prevResult through unchanged", ver), func() {
+			conf := []byte(fmt.Sprintf(`{
+				"name": "test",
+				"type": "tuning",
+				"cniVersion": "%s",
+				"sysctl": {
+					"net.ipv4.conf.all.log_martians": "1"
+				},
+				"prevResult": {
+					"interfaces": [
+						{"name": "dummy0", "sandbox":"netns"}
+					],
+					"ips": [
+						{
+							"version": "4",
+							"address": "10.0.0.2/24",
+							"gateway": "10.0.0.1",
+							"interface": 0
+						}
+					]
+				}
+			}`, ver))
+
+			args := &skel.CmdArgs{
+				ContainerID: "dummy",
+				Netns:       targetNS.Path(),
+				IfName:      IFNAME,
+				StdinData:   conf,
+			}
+
+			err := createSysctlAllowFile([]string{"^net\\.ipv4\\.conf\\.all\\.[a-z_]*$"})
+			Expect(err).NotTo(HaveOccurred())
+
+			beforeConf = configToRestore{}
+
+			err = originalNS.Do(func(ns.NetNS) error {
+				defer GinkgoRecover()
+
+				r, _, err := testutils.CmdAddWithArgs(args, func() error {
+					return cmdAdd(args)
+				})
+				Expect(err).NotTo(HaveOccurred())
+
+				result, err := types100.GetResult(r)
+				Expect(err).NotTo(HaveOccurred())
+
+				Expect(len(result.Interfaces)).To(Equal(1))
+				Expect(result.Interfaces[0].Name).To(Equal(IFNAME))
+				Expect(len(result.IPs)).To(Equal(1))
+				Expect(result.IPs[0].Address.String()).To(Equal("10.0.0.2/24"))
+
+				err = testutils.CmdDel(originalNS.Path(),
+					args.ContainerID, "", func() error { return cmdDel(args) })
+				Expect(err).NotTo(HaveOccurred())
+
+				return nil
+			})
+			Expect(err).NotTo(HaveOccurred())
+		})
 	}
 })