Better handling of parameters and config audits (#674)

* read-only-port defaults are correct

* Tests that should catch good read-only-port

* Rework checks & tests

* Linting on issue template YAML

* More explicit test for 4.2.4

.github/ISSUE_TEMPLATE/config.yml

@@ -1,3 +1,4 @@
+---
 blank_issues_enabled: false
 contact_links:
   - name: Feature request

cfg/cis-1.5/node.yaml

@@ -236,13 +236,17 @@ groups:
         audit: "/bin/ps -fC $kubeletbin"
         audit_config: "/bin/cat $kubeletconf"
         tests:
+          bin_op: or
           test_items:
             - flag: "--read-only-port"
               path: '{.readOnlyPort}'
               set: true
               compare:
                 op: eq
                 value: 0
+            - flag: "--read-only-port"
+              path: '{.readOnlyPort}'
+              set: false
         remediation: |
           If using a Kubelet config file, edit the file to set readOnlyPort to 0.
           If using command line arguments, edit the kubelet service file

check/check.go

@@ -62,21 +62,23 @@ const (
 // Check contains information about a recommendation in the
 // CIS Kubernetes document.
 type Check struct {
-	ID             string   `yaml:"id" json:"test_number"`
-	Text           string   `json:"test_desc"`
-	Audit          string   `json:"audit"`
-	AuditConfig    string   `yaml:"audit_config"`
-	Type           string   `json:"type"`
-	Tests          *tests   `json:"omit"`
-	Set            bool     `json:"omit"`
-	Remediation    string   `json:"remediation"`
-	TestInfo       []string `json:"test_info"`
-	State          `json:"status"`
-	ActualValue    string `json:"actual_value"`
-	Scored         bool   `json:"scored"`
-	IsMultiple     bool   `yaml:"use_multiple_values"`
-	ExpectedResult string `json:"expected_result"`
-	Reason         string `json:"reason,omitempty"`
+	ID                string   `yaml:"id" json:"test_number"`
+	Text              string   `json:"test_desc"`
+	Audit             string   `json:"audit"`
+	AuditConfig       string   `yaml:"audit_config"`
+	Type              string   `json:"type"`
+	Tests             *tests   `json:"omit"`
+	Set               bool     `json:"omit"`
+	Remediation       string   `json:"remediation"`
+	TestInfo          []string `json:"test_info"`
+	State             `json:"status"`
+	ActualValue       string `json:"actual_value"`
+	Scored            bool   `json:"scored"`
+	IsMultiple        bool   `yaml:"use_multiple_values"`
+	ExpectedResult    string `json:"expected_result"`
+	Reason            string `json:"reason,omitempty"`
+	AuditOutput       string `json:"omit"`
+	AuditConfigOutput string `json:"omit"`
 }
 
 // Runner wraps the basic Run method.
@@ -123,63 +125,46 @@ func (c *Check) run() State {
 		return c.State
 	}
 
-	lastCommand := c.Audit
-	hasAuditConfig := c.AuditConfig != ""
-
-	state, finalOutput, retErrmsgs := performTest(c.Audit, c.Tests, c.IsMultiple)
-	if len(state) > 0 {
-		c.Reason = retErrmsgs
-		c.State = state
+	// If there aren't any tests defined this is a FAIL or WARN
+	if c.Tests == nil || len(c.Tests.TestItems) == 0 {
+		c.Reason = "No tests defined"
+		if c.Scored {
+			c.State = FAIL
+		} else {
+			c.State = WARN
+		}
 		return c.State
 	}
-	errmsgs := retErrmsgs
-
-	// If something went wrong with the 'Audit' command
-	// and an 'AuditConfig' command was provided, use it to
-	// execute tests
-	if (finalOutput == nil || !finalOutput.testResult) && hasAuditConfig {
-		lastCommand = c.AuditConfig
-
-		nItems := len(c.Tests.TestItems)
-		// The reason we're creating a copy of the "tests"
-		// is so that tests can executed
-		// with the AuditConfig command
-		// against the Path only
-		currentTests := &tests{
-			BinOp:     c.Tests.BinOp,
-			TestItems: make([]*testItem, nItems),
-		}
 
-		for i := 0; i < nItems; i++ {
-			ti := c.Tests.TestItems[i]
-			nti := &testItem{
-				// Path is used to test Command Param values
-				// AuditConfig ==> Path
-				Path:    ti.Path,
-				Set:     ti.Set,
-				Compare: ti.Compare,
-			}
-			currentTests.TestItems[i] = nti
-		}
+	// Command line parameters override the setting in the config file, so if we get a good result from the Audit command that's all we need to run
+	var finalOutput *testOutput
+	var lastCommand string
 
-		state, finalOutput, retErrmsgs = performTest(c.AuditConfig, currentTests, c.IsMultiple)
-		if len(state) > 0 {
-			c.Reason = retErrmsgs
-			c.State = state
-			return c.State
-		}
-		errmsgs += retErrmsgs
+	lastCommand, err := c.runAuditCommands()
+	if err == nil {
+		finalOutput, err = c.execute()
 	}
 
-	if finalOutput != nil && finalOutput.testResult {
-		c.State = PASS
+	if finalOutput != nil {
+		if finalOutput.testResult {
+			c.State = PASS
+		} else {
+			if c.Scored {
+				c.State = FAIL
+			} else {
+				c.State = WARN
+			}
+		}
+
 		c.ActualValue = finalOutput.actualResult
 		c.ExpectedResult = finalOutput.ExpectedResult
-	} else {
+	}
+
+	if err != nil {
+		c.Reason = err.Error()
 		if c.Scored {
 			c.State = FAIL
 		} else {
-			c.Reason = errmsgs
 			c.State = WARN
 		}
 	}
@@ -190,39 +175,97 @@ func (c *Check) run() State {
 		glog.V(3).Infof("Check.ID: %s Command: %q TestResult: <<EMPTY>> \n", c.ID, lastCommand)
 	}
 
-	if errmsgs != "" {
-		glog.V(2).Info(errmsgs)
+	if c.Reason != "" {
+		glog.V(2).Info(c.Reason)
 	}
 	return c.State
 }
 
-func performTest(audit string, tests *tests, isMultipleOutput bool) (State, *testOutput, string) {
-	if len(strings.TrimSpace(audit)) == 0 {
-		return "", failTestItem("missing command"), "missing audit command"
+func (c *Check) runAuditCommands() (lastCommand string, err error) {
+	// Run the audit command and auditConfig commands, if present
+	c.AuditOutput, err = runAudit(c.Audit)
+	if err != nil {
+		return c.Audit, err
 	}
 
-	var out bytes.Buffer
-	errmsgs := runAudit(audit, &out)
+	c.AuditConfigOutput, err = runAudit(c.AuditConfig)
+	return c.AuditConfig, err
+}
+
+func (c *Check) execute() (finalOutput *testOutput, err error) {
+	finalOutput = &testOutput{}
+
+	ts := c.Tests
+	res := make([]testOutput, len(ts.TestItems))
+	expectedResultArr := make([]string, len(res))
+
+	glog.V(3).Infof("%d tests", len(ts.TestItems))
+	for i, t := range ts.TestItems {
+
+		t.isMultipleOutput = c.IsMultiple
 
-	finalOutput := tests.execute(out.String(), isMultipleOutput)
-	if finalOutput == nil {
-		errmsgs += fmt.Sprintf("Final output is <<EMPTY>>. Failed to run: %s\n", audit)
+		// Try with the auditOutput first, and if that's not found, try the auditConfigOutput
+		t.isConfigSetting = false
+		result := *(t.execute(c.AuditOutput))
+		if !result.flagFound {
+			t.isConfigSetting = true
+			result = *(t.execute(c.AuditConfigOutput))
+		}
+		res[i] = result
+		expectedResultArr[i] = res[i].ExpectedResult
 	}
 
-	return "", finalOutput, errmsgs
+	var result bool
+	// If no binary operation is specified, default to AND
+	switch ts.BinOp {
+	default:
+		glog.V(2).Info(fmt.Sprintf("unknown binary operator for tests %s\n", ts.BinOp))
+		finalOutput.actualResult = fmt.Sprintf("unknown binary operator for tests %s\n", ts.BinOp)
+		return finalOutput, fmt.Errorf("unknown binary operator for tests %s", ts.BinOp)
+	case and, "":
+		result = true
+		for i := range res {
+			result = result && res[i].testResult
+		}
+		// Generate an AND expected result
+		finalOutput.ExpectedResult = strings.Join(expectedResultArr, " AND ")
+
+	case or:
+		result = false
+		for i := range res {
+			result = result || res[i].testResult
+		}
+		// Generate an OR expected result
+		finalOutput.ExpectedResult = strings.Join(expectedResultArr, " OR ")
+	}
+
+	finalOutput.testResult = result
+	finalOutput.actualResult = res[0].actualResult
+
+	glog.V(3).Infof("Returning from execute on tests: finalOutput %#v", finalOutput)
+	return finalOutput, nil
 }
 
-func runAudit(audit string, out *bytes.Buffer) string {
-	errmsgs := ""
+func runAudit(audit string) (output string, err error) {
+	var out bytes.Buffer
+
+	audit = strings.TrimSpace(audit)
+	if len(audit) == 0 {
+		return output, err
+	}
 
 	cmd := exec.Command("/bin/sh")
 	cmd.Stdin = strings.NewReader(audit)
-	cmd.Stdout = out
-	cmd.Stderr = out
-	if err := cmd.Run(); err != nil {
-		errmsgs += fmt.Sprintf("failed to run: %q, output: %q, error: %s\n", audit, out.String(), err)
-	}
+	cmd.Stdout = &out
+	cmd.Stderr = &out
+	err = cmd.Run()
+	output = out.String()
+
+	if err != nil {
+		err = fmt.Errorf("failed to run: %q, output: %q, error: %s", audit, output, err)
+	} else {
+		glog.V(3).Infof("Command %q\n - Output:\n %q", audit, output)
 
-	glog.V(3).Infof("Command %q - Output:\n\n %q\n - Error Messages:%q \n", audit, out.String(), errmsgs)
-	return errmsgs
+	}
+	return output, err
 }