Merge pull request #37 from aquasecurity/multiple-words

Support executables with multiple words (e.g. hyperkube apiserver)

check/data

@@ -94,3 +94,25 @@ groups:
             op: eq
             value: "644"
           set: true
+
+    - id: 9
+      text: "test permissions"
+      audit: "/bin/sh -c 'if test -e $config; then stat -c %a $config; fi'"
+      tests:
+        bin_op: or
+        test_items:
+        - flag: "644"
+          compare:
+            op: eq
+            value: "644"
+          set: true
+        - flag: "640"
+          compare:
+            op: eq
+            value: "640"
+          set: true
+        - flag: "600"
+          compare:
+            op: eq
+            value: "600"
+          set: true

check/test_test.go

@@ -86,6 +86,14 @@ func TestTestExecute(t *testing.T) {
 			controls.Groups[0].Checks[8],
 			"644",
 		},
+		{
+			controls.Groups[0].Checks[9],
+			"640",
+		},
+		{
+			controls.Groups[0].Checks[9],
+			"600",
+		},
 	}
 
 	for _, c := range cases {

cmd/common.go

@@ -17,7 +17,7 @@ package cmd
 import (
 	"fmt"
 	"io/ioutil"
-	"strings"
+	"os"
 
 	"github.com/aquasecurity/kube-bench/check"
 	"github.com/spf13/viper"
@@ -96,26 +96,26 @@ func runChecks(t check.NodeType) {
 	}
 
 	// Variable substitutions. Replace all occurrences of variables in controls files.
-	s := strings.Replace(string(in), "$apiserverbin", apiserverBin, -1)
+	s := multiWordReplace(string(in), "$apiserverbin", apiserverBin)
-	s = strings.Replace(s, "$apiserverconf", apiserverConf, -1)
+	s = multiWordReplace(s, "$apiserverconf", apiserverConf)
-	s = strings.Replace(s, "$schedulerbin", schedulerBin, -1)
+	s = multiWordReplace(s, "$schedulerbin", schedulerBin)
-	s = strings.Replace(s, "$schedulerconf", schedulerConf, -1)
+	s = multiWordReplace(s, "$schedulerconf", schedulerConf)
-	s = strings.Replace(s, "$controllermanagerbin", controllerManagerBin, -1)
+	s = multiWordReplace(s, "$controllermanagerbin", controllerManagerBin)
-	s = strings.Replace(s, "$controllermanagerconf", controllerManagerConf, -1)
+	s = multiWordReplace(s, "$controllermanagerconf", controllerManagerConf)
-	s = strings.Replace(s, "$config", config, -1)
+	s = multiWordReplace(s, "$config", config)
 
-	s = strings.Replace(s, "$etcdbin", etcdBin, -1)
+	s = multiWordReplace(s, "$etcdbin", etcdBin)
-	s = strings.Replace(s, "$etcdconf", etcdConf, -1)
+	s = multiWordReplace(s, "$etcdconf", etcdConf)
-	s = strings.Replace(s, "$flanneldbin", flanneldBin, -1)
+	s = multiWordReplace(s, "$flanneldbin", flanneldBin)
-	s = strings.Replace(s, "$flanneldconf", flanneldConf, -1)
+	s = multiWordReplace(s, "$flanneldconf", flanneldConf)
 
-	s = strings.Replace(s, "$kubeletbin", kubeletBin, -1)
+	s = multiWordReplace(s, "$kubeletbin", kubeletBin)
-	s = strings.Replace(s, "$kubeletconf", kubeletConf, -1)
+	s = multiWordReplace(s, "$kubeletconf", kubeletConf)
-	s = strings.Replace(s, "$proxybin", proxyBin, -1)
+	s = multiWordReplace(s, "$proxybin", proxyBin)
-	s = strings.Replace(s, "$proxyconf", proxyConf, -1)
+	s = multiWordReplace(s, "$proxyconf", proxyConf)
 
-	s = strings.Replace(s, "$fedapiserverbin", fedApiserverBin, -1)
+	s = multiWordReplace(s, "$fedapiserverbin", fedApiserverBin)
-	s = strings.Replace(s, "$fedcontrollermanagerbin", fedControllerManagerBin, -1)
+	s = multiWordReplace(s, "$fedcontrollermanagerbin", fedControllerManagerBin)
 
 	controls, err := check.NewControls(t, []byte(s))
 	if err != nil {
@@ -150,15 +150,35 @@ func runChecks(t check.NodeType) {
 // verifyNodeType checks the executables and config files are as expected
 // for the specified tests (master, node or federated).
 func verifyNodeType(t check.NodeType) {
+	var bins []string
+	var confs []string
+
 	switch t {
 	case check.MASTER:
-		verifyBin(apiserverBin, schedulerBin, controllerManagerBin)
+		bins = []string{apiserverBin, schedulerBin, controllerManagerBin}
-		verifyConf(apiserverConf, schedulerConf, controllerManagerConf)
+		confs = []string{apiserverConf, schedulerConf, controllerManagerConf}
 	case check.NODE:
-		verifyBin(kubeletBin, proxyBin)
+		bins = []string{kubeletBin, proxyBin}
-		verifyConf(kubeletConf, proxyConf)
+		confs = []string{kubeletConf, proxyConf}
 	case check.FEDERATED:
-		verifyBin(fedApiserverBin, fedControllerManagerBin)
+		bins = []string{fedApiserverBin, fedControllerManagerBin}
+	}
+
+	for _, bin := range bins {
+		if !verifyBin(bin, ps) {
+			printlnWarn(fmt.Sprintf("%s is not running", bin))
+		}
+	}
+
+	for _, conf := range confs {
+		_, err := os.Stat(conf)
+		if err != nil {
+			if os.IsNotExist(err) {
+				printlnWarn(fmt.Sprintf("Missing kubernetes config file: %s", conf))
+			} else {
+				exitWithError(fmt.Errorf("error looking for file %s: %v", conf, err))
+			}
+		}
 	}
 }
 

cmd/util.go

@@ -64,66 +64,30 @@ func cleanIDs(list string) []string {
 	return ids
 }
 
-func verifyConf(confPath ...string) {
+// ps execs out to the ps command; it's separated into a function so we can write tests
-	var missing string
+func ps(proc string) string {
-
+	cmd := exec.Command("ps", "-C", proc, "-o", "cmd", "--no-headers")
-	for _, c := range confPath {
-		if _, err := os.Stat(c); err != nil && os.IsNotExist(err) {
-			e := fmt.Errorf("configuration file %s not found", c)
-			continueWithError(e, "")
-			missing += c + ", "
-		}
-	}
-
-	if len(missing) > 0 {
-		missing = strings.Trim(missing, ", ")
-		printlnWarn(fmt.Sprintf("Missing kubernetes config files: %s", missing))
-	}
-
-}
-
-func verifyBin(binPath ...string) {
-	var binSlice []string
-	var bin string
-	var missing string
-	var notRunning string
-
-	// Construct proc name for ps(1)
-	for _, b := range binPath {
-		_, err := exec.LookPath(b)
-		bin = bin + "," + b
-		binSlice = append(binSlice, b)
-		if err != nil {
-			e := fmt.Errorf("executable file %s not found", b)
-			continueWithError(e, "")
-			missing += b + ", "
-		}
-	}
-	bin = strings.Trim(bin, ",")
-
-	cmd := exec.Command("ps", "-C", bin, "-o", "cmd", "--no-headers")
 	out, err := cmd.Output()
 	if err != nil {
 		continueWithError(fmt.Errorf("%s: %s", cmd.Args, err), "")
 	}
 
-	for _, b := range binSlice {
+	return string(out)
-		matched := strings.Contains(string(out), b)
-
-		if !matched {
-			notRunning += b + ", "
-		}
 }
 
-	if len(missing) > 0 {
+// verifyBin checks that the binary specified is running
-		missing = strings.Trim(missing, ", ")
+func verifyBin(bin string, psFunc func(string) string) bool {
-		printlnWarn(fmt.Sprintf("Missing kubernetes binaries: %s", missing))
-	}
 
-	if len(notRunning) > 0 {
+	// Strip any quotes
-		notRunning = strings.Trim(notRunning, ", ")
+	bin = strings.Trim(bin, "'\"")
-		printlnWarn(fmt.Sprintf("Kubernetes binaries not running: %s", notRunning))
+
-	}
+	// bin could consist of more than one word
+	// We'll search for running processes with the first word, and then check the whole
+	// proc as supplied is included in the results
+	proc := strings.Fields(bin)[0]
+	out := psFunc(proc)
+
+	return strings.Contains(out, bin)
 }
 
 func verifyKubeVersion(major string, minor string) {
@@ -140,8 +104,10 @@ func verifyKubeVersion(major string, minor string) {
 	if err != nil {
 		s := fmt.Sprintf("Kubernetes version check skipped with error %v", err)
 		continueWithError(err, sprintlnWarn(s))
+		if len(out) == 0 {
 			return
 		}
+	}
 
 	msg := checkVersion("Client", string(out), major, minor)
 	if msg != "" {
@@ -184,3 +150,12 @@ func versionMatch(r *regexp.Regexp, s string) string {
 	}
 	return match[1]
 }
+
+func multiWordReplace(s string, subname string, sub string) string {
+	f := strings.Fields(sub)
+	if len(f) > 1 {
+		sub = "'" + sub + "'"
+	}
+
+	return strings.Replace(s, subname, sub, -1)
+}

cmd/util_test.go

@@ -16,6 +16,7 @@ package cmd
 
 import (
 	"regexp"
+	"strconv"
 	"testing"
 )
 
@@ -36,8 +37,8 @@ func TestCheckVersion(t *testing.T) {
 		{t: "Server", s: "something unexpected", major: "2", minor: "0", exp: "Couldn't find Server version from kubectl output 'something unexpected'"},
 	}
 
-	for _, c := range cases {
+	for id, c := range cases {
-		t.Run("", func(t *testing.T) {
+		t.Run(strconv.Itoa(id), func(t *testing.T) {
 			m := checkVersion(c.t, c.s, c.major, c.minor)
 			if m != c.exp {
 				t.Fatalf("Got: %s, expected: %s", m, c.exp)
@@ -66,8 +67,8 @@ func TestVersionMatch(t *testing.T) {
 		{r: minor}, // Checking that we don't fall over if the string is empty
 	}
 
-	for _, c := range cases {
+	for id, c := range cases {
-		t.Run("", func(t *testing.T) {
+		t.Run(strconv.Itoa(id), func(t *testing.T) {
 			m := versionMatch(c.r, c.s)
 			if m != c.exp {
 				t.Fatalf("Got %s expected %s", m, c.exp)
@@ -75,3 +76,56 @@ func TestVersionMatch(t *testing.T) {
 		})
 	}
 }
+
+var g string
+
+func fakeps(proc string) string {
+	return g
+}
+func TestVerifyBin(t *testing.T) {
+	cases := []struct {
+		proc  string
+		psOut string
+		exp   bool
+	}{
+		{proc: "single", psOut: "single", exp: true},
+		{proc: "single", psOut: "", exp: false},
+		{proc: "two words", psOut: "two words", exp: true},
+		{proc: "two words", psOut: "", exp: false},
+		{proc: "cmd", psOut: "cmd param1 param2", exp: true},
+		{proc: "cmd param", psOut: "cmd param1 param2", exp: true},
+		{proc: "cmd param", psOut: "cmd", exp: false},
+	}
+
+	for id, c := range cases {
+		t.Run(strconv.Itoa(id), func(t *testing.T) {
+			g = c.psOut
+			v := verifyBin(c.proc, fakeps)
+			if v != c.exp {
+				t.Fatalf("Expected %v got %v", c.exp, v)
+			}
+		})
+	}
+}
+
+func TestMultiWordReplace(t *testing.T) {
+	cases := []struct {
+		input   string
+		sub     string
+		subname string
+		output  string
+	}{
+		{input: "Here's a file with no substitutions", sub: "blah", subname: "blah", output: "Here's a file with no substitutions"},
+		{input: "Here's a file with a substitution", sub: "blah", subname: "substitution", output: "Here's a file with a blah"},
+		{input: "Here's a file with multi-word substitutions", sub: "multi word", subname: "multi-word", output: "Here's a file with 'multi word' substitutions"},
+		{input: "Here's a file with several several substitutions several", sub: "blah", subname: "several", output: "Here's a file with blah blah substitutions blah"},
+	}
+	for id, c := range cases {
+		t.Run(strconv.Itoa(id), func(t *testing.T) {
+			s := multiWordReplace(c.input, c.subname, c.sub)
+			if s != c.output {
+				t.Fatalf("Expected %s got %s", c.output, s)
+			}
+		})
+	}
+}