From d2fa9d35b6cfd6dd0919ab7fac397e449e811bbb Mon Sep 17 00:00:00 2001
From: Abubakr-Sadik Nii Nai Davis <dwa2pac@gmail.com>
Date: Sat, 12 Aug 2017 12:29:21 +0000
Subject: [PATCH] Rewrite audit commands in the check definition that contain
 shell builtins and modify text to command function to support this.

Shell builtins fail the binary command lookup test which result in a
WARN. Audit commands which include shell builtins must use the form:

   "/bin/sh -c 'sh-builtin arg'"

So they are executed properly. Additionally Go will fail to execute
commands involving shell builtins if they are not in the above format.
---
 cfg/master.yaml | 54 ++++++++++++++++++++++++++++++++++++++++---------
 cfg/node.yaml   | 24 +++++++++++++---------
 check/check.go  | 52 +++++++++++++++++++++++++++++++++++++++++------
 check/test.go   | 15 +++++++++++---
 4 files changed, 116 insertions(+), 29 deletions(-)

diff --git a/cfg/master.yaml b/cfg/master.yaml
index 46aeb36..6f2cc33 100644
--- a/cfg/master.yaml
+++ b/cfg/master.yaml
@@ -595,10 +595,14 @@ groups:
   checks:
     - id: 1.4.1
       text: "Ensure that the apiserver file permissions are set to 644 or more restrictive (Scored)"
-      audit: "if test -e $apiserverconf; then stat -c %a $apiserverconf; fi"
+      # audit: "/bin/bash -c 'if test -e $apiserverconf; then stat -c %a $apiserverconf; fi'"
+      audit: "/bin/sh -c 'if test -e $apiserverconf; then stat -c %a $apiserverconf; fi'"
       tests:
         test_items:
         - flag: "644"
+          compare:
+            op: eq
+            value: "644"
           set: true
       remediation: "Run the below command (based on the file location on your system) on the master node. 
               \nFor example, chmod 644 $apiserverconf"
@@ -606,10 +610,13 @@ groups:
 
     - id: 1.4.2
       text: "Ensure that the apiserver file ownership is set to root:root (Scored)"
-      audit: "if test -e $apiserverconf; then stat -c %U:%G $apiserverconf; fi"
+      audit: "/bin/sh -c 'if test -e $apiserverconf; then stat -c %U:%G $apiserverconf; fi'"
       tests:
         test_items:
         - flag: "root:root"
+          compare:
+            op: eq
+            value: "root:root"
           set: true
       remediation: "Run the below command (based on the file location on your system) on the master node. 
               \nFor example, chown root:root $apiserverconf"
@@ -617,10 +624,13 @@ groups:
 
     - id: 1.4.3
       text: "Ensure that the config file permissions are set to 644 or more restrictive (Scored)"
-      audit: "if test -e $config; then stat -c %a $config; fi"
+      audit: "/bin/sh -c 'if test -e $config; then stat -c %a $config; fi'"
       tests:
         test_items:
         - flag: "644"
+          compare:
+            op: eq
+            value: "644"
           set: true
       remediation: "Run the below command (based on the file location on your system) on the master node. 
               \nFor example, chmod 644 $config"
@@ -628,10 +638,13 @@ groups:
 
     - id: 1.4.4
       text: "Ensure that the config file ownership is set to root:root (Scored)"
-      audit: "if test -e $config; then stat -c %U:%G $config; fi"
+      audit: "/bin/sh -c 'if test -e $config; then stat -c %U:%G $config; fi'"
       tests:
         test_items:
         - flag: "root:root"
+          compare:
+            op: eq
+            value: "root:root"
           set: true
       remediation: "Run the below command (based on the file location on your system) on the master node. 
               \nFor example, chown root:root $config"
@@ -639,10 +652,13 @@ groups:
 
     - id: 1.4.5
       text: "Ensure that the scheduler file permissions are set to 644 or more restrictive (Scored)"
-      audit: "if test -e $schedulerconf; then stat -c %a $schedulerconf; fi"
+      audit: "/bin/sh -c 'if test -e $schedulerconf; then stat -c %a $schedulerconf; fi'"
       tests:
         test_items:
           - flag: "644"
+            compare:
+              op: eq
+              value: "644"
             set: true
       remediation: "Run the below command (based on the file location on your system) on the master node. 
               \nFor example, chmod 644 $schedulerconf"
@@ -650,10 +666,13 @@ groups:
 
     - id: 1.4.6
       text: "Ensure that the scheduler file ownership is set to root:root (Scored)"
-      audit: "if test -e $schedulerconf; then stat -c %U:%G $schedulerconf; fi"
+      audit: "/bin/sh -c 'if test -e $schedulerconf; then stat -c %U:%G $schedulerconf; fi'"
       tests:
         test_items:
           - flag: "root:root"
+            compare:
+              op: eq
+              value: "root:root"
             set: true
       remediation: "Run the below command (based on the file location on your system) on the master node. 
               \nFor example, chown root:root $schedulerconf"
@@ -661,10 +680,13 @@ groups:
 
     - id: 1.4.7
       text: "Ensure that the etcd.conf file permissions are set to 644 or more restrictive (Scored)"
-      audit: "if test -e $etcdconf; then stat -c %a $etcdconf; fi"
+      audit: "/bin/sh -c 'if test -e $etcdconf; then stat -c %a $etcdconf; fi'"
       tests:
         test_items:
           - flag: "644"
+            compare:
+              op: eq
+              value: "644"
             set: true
       remediation: "Run the below command (based on the file location on your system) on the master node. 
               \nFor example, chmod 644 $etcdconf"
@@ -672,10 +694,13 @@ groups:
 
     - id: 1.4.8
       text: "Ensure that the etcd.conf file ownership is set to root:root (Scored)"
-      audit: "if test -e $etcdconf; then stat -c %U:%G $etcdconf; fi"
+      audit: "/bin/sh -c 'if test -e $etcdconf; then stat -c %U:%G $etcdconf; fi'"
       tests:
         test_items:
         - flag: "root:root"
+          compare:
+            op: eq
+            value: "root:root"
           set: true
       remediation: "Run the below command (based on the file location on your system) on the master node. 
               \nFor example, chown root:root $etcdconf"
@@ -683,10 +708,13 @@ groups:
 
     - id: 1.4.9
       text: "Ensure that the flanneld file permissions are set to 644 or more restrictive (Scored)"
-      audit: "if test -e $flanneldconf; then stat -c %a $flanneldconf; fi"
+      audit: "/bin/sh -c 'if test -e $flanneldconf; then stat -c %a $flanneldconf; fi'"
       tests:
         test_items:
         - flag: "644"
+          compare:
+            op: eq
+            value: "644"
           set: true
       remediation: "Run the below command (based on the file location on your system) on the master node. 
               \nFor example, chmod 644 $flanneldconf"
@@ -694,10 +722,13 @@ groups:
 
     - id: 1.4.10
       text: "Ensure that the flanneld file ownership is set to root:root (Scored)"
-      audit: "if test -e $flanneldconf; then stat -c %U:%G $flanneldconf; fi"
+      audit: "/bin/sh -c 'if test -e $flanneldconf; then stat -c %U:%G $flanneldconf; fi'"
       tests:
         test_items:
         - flag: "root:root"
+          compare:
+            op: eq
+            value: "root:root"
           set: true
       remediation: "Run the below command (based on the file location on your system) on the master node. 
               \nFor example, chown root:root $flanneldconf"
@@ -709,6 +740,9 @@ groups:
       tests:
         test_items:
         - flag: "700"
+          compare:
+            op: eq
+            value: "700"
           set: true
       remediation: "On the etcd server node, get the etcd data directory, passed as an argument --data-dir ,
               from the below command:\n
diff --git a/cfg/node.yaml b/cfg/node.yaml
index 94e48f7..0b5b90b 100644
--- a/cfg/node.yaml
+++ b/cfg/node.yaml
@@ -221,7 +221,7 @@ groups:
   checks:
     - id: 2.2.1
       text: "Ensure that the config file permissions are set to 644 or more restrictive (Scored)"
-      audit: "if test -e $config; then stat -c %a $config; fi"
+      audit: "/bin/sh -c 'if test -e $config; then stat -c %a $config; fi'"
       tests:
         test_items:
           - flag: "644"
@@ -232,10 +232,13 @@ groups:
 
     - id: 2.2.2
       text: "Ensure that the config file ownership is set to root:root (Scored)"
-      audit: "if test -e $config; then stat -c %U:%G $config; fi"
+      audit: "/bin/sh -c 'if test -e $config; then stat -c %U:%G $config; fi'"
       tests:
         test_items:
           - flag: "root:root"
+            compare:
+              op: eq
+              value: root:root
             set: true
       remediation: "Run the below command (based on the file location on your system) on the each worker node. 
               \nFor example, chown root:root $config"
@@ -243,10 +246,13 @@ groups:
 
     - id: 2.2.3
       text: "Ensure that the kubelet file permissions are set to 644 or more restrictive (Scored)"
-      audit: "if test -e $kubeletconf; then stat -c %a $kubeletconf; fi"
+      audit: "/bin/sh -c 'if test -e $kubeletconf; then stat -c %a $kubeletconf; fi'"
       tests:
         test_items:
           - flag: "644"
+            compare:
+              op: eq
+              value: 644
             set: true
       remediation: "Run the below command (based on the file location on your system) on the each worker node. 
               \nFor example, chmod 644 $kubeletconf"
@@ -254,7 +260,7 @@ groups:
 
     - id: 2.2.4
       text: "Ensure that the kubelet file ownership is set to root:root (Scored)"
-      audit: "if test -e $kubeletconf; then stat -c %U:%G $kubeletconf; fi"
+      audit: "/bin/sh -c 'if test -e $kubeletconf; then stat -c %U:%G $kubeletconf; fi'"
       tests:
         test_items:
           - flag: "root:root"
@@ -265,7 +271,7 @@ groups:
 
     - id: 2.2.5
       text: "Ensure that the proxy file permissions are set to 644 or more restrictive (Scored)"
-      audit: "if test -e $proxyconf; then stat -c %a $proxyconf; fi"
+      audit: "/bin/sh -c 'if test -e $proxyconf; then stat -c %a $proxyconf; fi'"
       tests:
         test_items:
           - flag: "644"
@@ -276,7 +282,7 @@ groups:
 
     - id: 2.2.6
       text: "Ensure that the proxy file ownership is set to root:root (Scored)"
-      audit: "if test -e $proxyconf; then stat -c %U:%G $proxyconf; fi"
+      audit: "/bin/sh -c 'if test -e $proxyconf; then stat -c %U:%G $proxyconf; fi'"
       tests:
         test_items:
           - flag: "root:root"
@@ -285,11 +291,10 @@ groups:
               \nFor example, chown root:root $proxyconf"
       scored: true
 
-# TODO: provide flag to WARN about manual checks.
     - id: 2.2.7
       text: "Ensure that the certificate authorities file permissions are set to
               644 or more restrictive (Scored)"
-      audit: "if test -e $ca-file; then stat -c %a $ca-file; fi"
+      audit: "/bin/sh -c 'if test -e $ca-file; then stat -c %a $ca-file; fi'"
       tests:
         test_items:
           - flag: "644"
@@ -298,10 +303,9 @@ groups:
               \nchmod 644 <filename>"
       scored: true
 
-# TODO: provide flag to WARN about manual checks.
     - id: 2.2.8
       text: "Ensure that the client certificate authorities file ownership is set to root:root"
-      audit: "if test -e $ca-file; then stat -c %U:%G $ca-file; fi"
+      audit: "/bin/sh -c 'if test -e $ca-file; then stat -c %U:%G $ca-file; fi'"
       tests:
         test_items:
           - flag: "notexist:notexist"
diff --git a/check/check.go b/check/check.go
index e73c58f..86f3939 100644
--- a/check/check.go
+++ b/check/check.go
@@ -20,6 +20,7 @@ import (
 	"io"
 	"os"
 	"os/exec"
+	"regexp"
 	"strings"
 )
 
@@ -75,8 +76,7 @@ func (c *Check) Run(verbose bool) {
 
 	// Check if command exists or exit with WARN.
 	for _, cmd := range c.Commands {
-		_, err := exec.LookPath(cmd.Path)
-		if err != nil {
+		if !isShellCommand(cmd.Path) {
 			c.State = WARN
 			return
 		}
@@ -111,7 +111,6 @@ func (c *Check) Run(verbose bool) {
 				cs[i].Args,
 			),
 		)
-
 		i++
 	}
 
@@ -160,18 +159,44 @@ func (c *Check) Run(verbose bool) {
 	}
 }
 
-// textToCommand transforms a text representation of commands to be
+// textToCommand transforms an input text representation of commands to be
 // run into a slice of commands.
 // TODO: Make this more robust.
 func textToCommand(s string) []*exec.Cmd {
 	cmds := []*exec.Cmd{}
 
 	cp := strings.Split(s, "|")
-	// fmt.Println("check.toCommand:", cp)
 
 	for _, v := range cp {
 		v = strings.Trim(v, " ")
-		cs := strings.Split(v, " ")
+
+		// TODO:
+		// GOAL: To split input text into arguments for exec.Cmd.
+		//
+		// CHALLENGE: The input text may contain quoted strings that
+		// must be passed as a unit to exec.Cmd.
+		// eg. bash -c 'foo bar'
+		// 'foo bar' must be passed as unit to exec.Cmd if not the command
+		// will fail when it is executed.
+		// eg. exec.Cmd("bash", "-c", "foo bar")
+		//
+		// PROBLEM: Current solution assumes the grouped string will always
+		// be at the end of the input text.
+		re := regexp.MustCompile(`^(.*)(['"].*['"])$`)
+		grps := re.FindStringSubmatch(v)
+
+		var cs []string
+		if len(grps) > 0 {
+			s := strings.Trim(grps[1], " ")
+			cs = strings.Split(s, " ")
+
+			s1 := grps[len(grps)-1]
+			s1 = strings.Trim(s1, "'\"")
+
+			cs = append(cs, s1)
+		} else {
+			cs = strings.Split(v, " ")
+		}
 
 		cmd := exec.Command(cs[0], cs[1:]...)
 		cmds = append(cmds, cmd)
@@ -179,3 +204,18 @@ func textToCommand(s string) []*exec.Cmd {
 
 	return cmds
 }
+
+func isShellCommand(s string) bool {
+	cmd := exec.Command("/bin/sh", "-c", "command -v "+s)
+
+	out, err := cmd.Output()
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "%s\n", err)
+		os.Exit(1)
+	}
+
+	if strings.Contains(string(out), s) {
+		return true
+	}
+	return false
+}
diff --git a/check/test.go b/check/test.go
index f89e989..06c1b93 100644
--- a/check/test.go
+++ b/check/test.go
@@ -38,6 +38,7 @@ const (
 
 type testItem struct {
 	Flag    string
+	Output  string
 	Value   string
 	Set     bool
 	Compare compare
@@ -57,14 +58,22 @@ func (t *testItem) execute(s string) (result bool) {
 		isset := match
 
 		if isset && t.Compare.Op != "" {
-			pttn := t.Flag + `=([^\s,]*) *`
+			// Expects flags in the form;
+			// --flag=somevalue
+			// --flag
+			// somevalue
+			pttn := `(` + t.Flag + `)(=)*([^\s,]*) *`
 			flagRe := regexp.MustCompile(pttn)
 			vals := flagRe.FindStringSubmatch(s)
 
 			if len(vals) > 0 {
-				flagVal = vals[1]
+				if vals[3] != "" {
+					flagVal = vals[3]
+				} else {
+					flagVal = vals[1]
+				}
 			} else {
-				fmt.Fprintf(os.Stderr, "expected value for %s but none found\n", t.Flag)
+				fmt.Fprintf(os.Stderr, "invalid flag in testitem definition")
 				os.Exit(1)
 			}
 
-- 
GitLab