From f88de572f6ff4799636b86bb6237b491613211c0 Mon Sep 17 00:00:00 2001 From: Abubakr-Sadik Nii Nai Davis Date: Tue, 25 Jul 2017 00:34:07 +0000 Subject: [PATCH 01/13] Improve error handling. --- cfg/config.yaml | 2 +- check/check.go | 9 ++-- check/controls.go | 8 ++-- cmd/common.go | 60 ++++++++--------------- cmd/root.go | 14 ++++-- cmd/util.go | 120 +++++++++++++++++++++++++++++----------------- 6 files changed, 117 insertions(+), 96 deletions(-) diff --git a/cfg/config.yaml b/cfg/config.yaml index c8c89c9..07548ac 100644 --- a/cfg/config.yaml +++ b/cfg/config.yaml @@ -29,7 +29,7 @@ installation: conf: apiserver: /etc/kubernetes/apiserver scheduler: /etc/kubernetes/scheduler - controller-manager: /etc/kubernetes/apiserver + controller-manager: /etc/kubernetes/controller-manager node: bin: kubelet: kubelet diff --git a/check/check.go b/check/check.go index e73c58f..5d8221a 100644 --- a/check/check.go +++ b/check/check.go @@ -18,9 +18,10 @@ import ( "bytes" "fmt" "io" - "os" "os/exec" "strings" + + "github.com/golang/glog" ) // NodeType indicates the type of node (master, node, federated). @@ -69,7 +70,7 @@ type Check struct { // Run executes the audit commands specified in a check and outputs // the results. -func (c *Check) Run(verbose bool) { +func (c *Check) Run() { var out bytes.Buffer var errmsgs string @@ -148,9 +149,7 @@ func (c *Check) Run(verbose bool) { i++ } - if verbose && errmsgs != "" { - fmt.Fprintf(os.Stderr, "%s\n", errmsgs) - } + glog.V(2).Info("%s\n", errmsgs) res := c.Tests.execute(out.String()) if res { diff --git a/check/controls.go b/check/controls.go index 8f7530c..dfea006 100644 --- a/check/controls.go +++ b/check/controls.go @@ -68,7 +68,7 @@ func NewControls(t NodeType, in []byte) (*Controls, error) { } // RunGroup runs all checks in a group. -func (controls *Controls) RunGroup(verbose bool, gids ...string) Summary { +func (controls *Controls) RunGroup(gids ...string) Summary { g := []*Group{} controls.Summary.Pass, controls.Summary.Fail, controls.Summary.Warn = 0, 0, 0 @@ -82,7 +82,7 @@ func (controls *Controls) RunGroup(verbose bool, gids ...string) Summary { for _, gid := range gids { if gid == group.ID { for _, check := range group.Checks { - check.Run(verbose) + check.Run() summarize(controls, check) } @@ -96,7 +96,7 @@ func (controls *Controls) RunGroup(verbose bool, gids ...string) Summary { } // RunChecks runs the checks with the supplied IDs. -func (controls *Controls) RunChecks(verbose bool, ids ...string) Summary { +func (controls *Controls) RunChecks(ids ...string) Summary { g := []*Group{} m := make(map[string]*Group) controls.Summary.Pass, controls.Summary.Fail, controls.Summary.Warn = 0, 0, 0 @@ -110,7 +110,7 @@ func (controls *Controls) RunChecks(verbose bool, ids ...string) Summary { for _, check := range group.Checks { for _, id := range ids { if id == check.ID { - check.Run(verbose) + check.Run() summarize(controls, check) // Check if we have already added this checks group. diff --git a/cmd/common.go b/cmd/common.go index c002945..50c9c41 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -17,7 +17,6 @@ package cmd import ( "fmt" "io/ioutil" - "os" "strings" "github.com/aquasecurity/kube-bench/check" @@ -59,7 +58,7 @@ func runChecks(t check.NodeType) { schedulerBin = viper.GetString("installation." + installation + ".master.bin.scheduler") schedulerConf = viper.GetString("installation." + installation + ".master.conf.scheduler") controllerManagerBin = viper.GetString("installation." + installation + ".master.bin.controller-manager") - controllerManagerConf = viper.GetString("installation." + installation + ".master.conf.controler-manager") + controllerManagerConf = viper.GetString("installation." + installation + ".master.conf.controller-manager") config = viper.GetString("installation." + installation + ".config") etcdBin = viper.GetString("etcd.bin") @@ -78,7 +77,7 @@ func runChecks(t check.NodeType) { fedControllerManagerBin = viper.GetString("installation." + installation + ".federated.bin.controller-manager") // Run kubernetes installation validation checks. - warns := verifyNodeType(t) + verifyNodeType(t) switch t { case check.MASTER: @@ -91,8 +90,7 @@ func runChecks(t check.NodeType) { in, err := ioutil.ReadFile(file) if err != nil { - fmt.Fprintf(os.Stderr, "error opening %s controls file: %v\n", t, err) - os.Exit(1) + exitWithError(fmt.Errorf("error opening %s controls file: %v", t, err)) } // Variable substitutions. Replace all occurrences of variables in controls files. @@ -102,7 +100,6 @@ func runChecks(t check.NodeType) { s = strings.Replace(s, "$schedulerconf", schedulerConf, -1) s = strings.Replace(s, "$controllermanagerbin", controllerManagerBin, -1) s = strings.Replace(s, "$controllermanagerconf", controllerManagerConf, -1) - s = strings.Replace(s, "$controllermanagerconf", controllerManagerConf, -1) s = strings.Replace(s, "$config", config, -1) s = strings.Replace(s, "$etcdbin", etcdBin, -1) @@ -120,63 +117,50 @@ func runChecks(t check.NodeType) { controls, err := check.NewControls(t, []byte(s)) if err != nil { - fmt.Fprintf(os.Stderr, "error setting up %s controls: %v\n", t, err) - os.Exit(1) + exitWithError(fmt.Errorf("error setting up %s controls: %v", t, err)) } if groupList != "" && checkList == "" { ids := cleanIDs(groupList) - summary = controls.RunGroup(verbose, ids...) + summary = controls.RunGroup(ids...) } else if checkList != "" && groupList == "" { ids := cleanIDs(checkList) - summary = controls.RunChecks(verbose, ids...) + summary = controls.RunChecks(ids...) } else if checkList != "" && groupList != "" { - fmt.Fprintf(os.Stderr, "group option and check option can't be used together\n") - os.Exit(1) + exitWithError(fmt.Errorf("group option and check option can't be used together")) } else { - summary = controls.RunGroup(verbose) + summary = controls.RunGroup() } // if we successfully ran some tests and it's json format, ignore the warnings if (summary.Fail > 0 || summary.Warn > 0 || summary.Pass > 0) && jsonFmt { out, err := controls.JSON() if err != nil { - fmt.Fprintf(os.Stderr, "failed to output in JSON format: %v\n", err) - os.Exit(1) + exitWithError(fmt.Errorf("failed to output in JSON format: %v", err)) } fmt.Println(string(out)) } else { - prettyPrint(warns, controls, summary) + prettyPrint(controls, summary) } } // verifyNodeType checks the executables and config files are as expected // for the specified tests (master, node or federated). -func verifyNodeType(t check.NodeType) []string { - var w []string - // Always clear out error messages. - errmsgs = "" - +func verifyNodeType(t check.NodeType) { switch t { case check.MASTER: - w = append(w, verifyBin(apiserverBin, schedulerBin, controllerManagerBin)...) - w = append(w, verifyConf(apiserverConf, schedulerConf, controllerManagerConf)...) - w = append(w, verifyKubeVersion(apiserverBin)...) + verifyKubeVersion(apiserverBin) + verifyBin(apiserverBin, schedulerBin, controllerManagerBin) + verifyConf(apiserverConf, schedulerConf, controllerManagerConf) case check.NODE: - w = append(w, verifyBin(kubeletBin, proxyBin)...) - w = append(w, verifyConf(kubeletConf, proxyConf)...) - w = append(w, verifyKubeVersion(kubeletBin)...) + verifyKubeVersion(kubeletBin) + verifyBin(kubeletBin, proxyBin) + verifyConf(kubeletConf, proxyConf) case check.FEDERATED: - w = append(w, verifyBin(fedApiserverBin, fedControllerManagerBin)...) - w = append(w, verifyKubeVersion(fedApiserverBin)...) + verifyKubeVersion(fedApiserverBin) + verifyBin(fedApiserverBin, fedControllerManagerBin) } - - if verbose { - fmt.Fprintf(os.Stderr, "%s\n", errmsgs) - } - - return w } // colorPrint outputs the state in a specific colour, along with a message string @@ -186,13 +170,9 @@ func colorPrint(state check.State, s string) { } // prettyPrint outputs the results to stdout in human-readable format -func prettyPrint(warnings []string, r *check.Controls, summary check.Summary) { +func prettyPrint(r *check.Controls, summary check.Summary) { colorPrint(check.INFO, fmt.Sprintf("Using config file: %s\n", viper.ConfigFileUsed())) - for _, w := range warnings { - colorPrint(check.WARN, w) - } - colorPrint(check.INFO, fmt.Sprintf("%s %s\n", r.ID, r.Text)) for _, g := range r.Groups { colorPrint(check.INFO, fmt.Sprintf("%s %s\n", g.ID, g.Text)) diff --git a/cmd/root.go b/cmd/root.go index 5429c2c..aed8b42 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -15,6 +15,7 @@ package cmd import ( + goflag "flag" "fmt" "os" @@ -34,11 +35,12 @@ var ( nodeFile string federatedFile string + loud bool + kubeConfDir string etcdConfDir string flanneldConfDir string - verbose bool installation string ) @@ -52,6 +54,9 @@ var RootCmd = &cobra.Command{ // Execute adds all child commands to the root command sets flags appropriately. // This is called by main.main(). It only needs to happen once to the rootCmd. func Execute() { + goflag.Set("logtostderr", "true") + goflag.CommandLine.Parse([]string{}) + if err := RootCmd.Execute(); err != nil { fmt.Println(err) os.Exit(-1) @@ -83,7 +88,11 @@ func init() { `Run all the checks under this comma-delimited list of groups. Example --group="1.1"`, ) RootCmd.PersistentFlags().StringVar(&cfgFile, "config", "", "config file (default is ./cfg/config.yaml)") - RootCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "verbose output (default false)") + + goflag.CommandLine.VisitAll(func(goflag *goflag.Flag) { + RootCmd.PersistentFlags().AddGoFlag(goflag) + }) + } // initConfig reads in config file and ENV variables if set. @@ -103,5 +112,4 @@ func initConfig() { colorPrint(check.FAIL, fmt.Sprintf("Failed to read config file: %v\n", err)) os.Exit(1) } - } diff --git a/cmd/util.go b/cmd/util.go index 6a1f790..88df3a7 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -8,6 +8,7 @@ import ( "github.com/aquasecurity/kube-bench/check" "github.com/fatih/color" + "github.com/golang/glog" ) var ( @@ -20,11 +21,35 @@ var ( } ) -func handleError(err error, context string) (errmsg string) { +func printWarn(msg string) { + fmt.Fprintf(os.Stderr, "[%s] %s\n", + colors[check.WARN].Sprintf("%s", check.WARN), + msg, + ) +} + +func printfWarn(msg string) string { + return fmt.Sprintf("[%s] %s", + colors[check.WARN].Sprintf("%s", check.WARN), + msg, + ) +} + +func exitWithError(err error) { + fmt.Fprintf(os.Stderr, "\n%v\n", err) + os.Exit(1) +} + +func continueWithError(err error, msg string) string { if err != nil { - errmsg = fmt.Sprintf("%s, error: %s\n", context, err) + glog.V(1).Info(err) } - return + + if msg != "" { + fmt.Fprintf(os.Stderr, "%s\n", msg) + } + + return "" } func cleanIDs(list string) []string { @@ -38,76 +63,85 @@ func cleanIDs(list string) []string { return ids } -func verifyConf(confPath ...string) []string { - var w []string +func verifyConf(confPath ...string) { + var missing string + for _, c := range confPath { if _, err := os.Stat(c); err != nil && os.IsNotExist(err) { - w = append(w, fmt.Sprintf("config file %s does not exist\n", c)) + continueWithError(err, "") + missing += c + ", " } } - return w + if len(missing) > 0 { + missing = strings.Trim(missing, ", ") + printWarn(fmt.Sprintf("Missing kubernetes config files: %s", missing)) + } + } -func verifyBin(binPath ...string) []string { - var w []string - var binList string +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 { - binList += b + "," _, err := exec.LookPath(b) - errmsgs += handleError( - err, - fmt.Sprintf("%s: command not found in path", b), - ) + bin = bin + "," + b + binSlice = append(binSlice, b) + if err != nil { + missing += b + ", " + continueWithError(err, "") + } + } + 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), "") } - binList = strings.Trim(binList, ",") - - // Run ps command - cmd := exec.Command("ps", "-C", binList, "-o", "cmd", "--no-headers") - out, err := cmd.Output() - errmsgs += handleError( - err, - fmt.Sprintf("failed to run: %s", cmd.Args), - ) - - // Actual verification - for _, b := range binPath { + for _, b := range binSlice { matched := strings.Contains(string(out), b) if !matched { - w = append(w, fmt.Sprintf("%s is not running\n", b)) + notRunning += b + ", " } } - return w + if len(missing) > 0 { + missing = strings.Trim(missing, ", ") + printWarn(fmt.Sprintf("Missing kubernetes binaries: %s", missing)) + } + + if len(notRunning) > 0 { + notRunning = strings.Trim(notRunning, ", ") + printWarn(fmt.Sprintf("Kubernetes binaries not running: %s", notRunning)) + } } -func verifyKubeVersion(b string) []string { +func verifyKubeVersion(b string) { // These executables might not be on the user's path. // TODO! Check the version number using kubectl, which is more likely to be on the path. - var w []string _, err := exec.LookPath(b) - errmsgs += handleError( - err, - fmt.Sprintf("%s: command not found on path - version check skipped", b), - ) + if err != nil { + continueWithError(err, printfWarn("Kubernetes version check skipped")) + return + } - // Check version cmd := exec.Command(b, "--version") out, err := cmd.Output() - errmsgs += handleError( - err, - fmt.Sprintf("failed to run:%s", cmd.Args), - ) + if err != nil { + continueWithError(err, printfWarn("Kubernetes version check skipped")) + return + } matched := strings.Contains(string(out), kubeVersion) if !matched { - w = append(w, fmt.Sprintf("%s unsupported version\n", b)) + printWarn(fmt.Sprintf("Unsupported kubernetes version: %s", out)) } - - return w } From 82c92e00780f5469138920d37a1ad8262bdff160 Mon Sep 17 00:00:00 2001 From: Abubakr-Sadik Nii Nai Davis Date: Sun, 6 Aug 2017 14:25:02 +0000 Subject: [PATCH 02/13] Change function name to be clearer about the fact it returns a string. --- cmd/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/util.go b/cmd/util.go index 88df3a7..26e7441 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -28,7 +28,7 @@ func printWarn(msg string) { ) } -func printfWarn(msg string) string { +func printWarn(msg string) string { return fmt.Sprintf("[%s] %s", colors[check.WARN].Sprintf("%s", check.WARN), msg, From 43c1470c0ede3cd55abab6d8b28f43ca42a7cbd9 Mon Sep 17 00:00:00 2001 From: Abubakr-Sadik Nii Nai Davis Date: Sun, 6 Aug 2017 15:29:55 +0000 Subject: [PATCH 03/13] Add check type manual. Results of manual checks are forced to WARN to inform users to check manually. --- check/check.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/check/check.go b/check/check.go index e73c58f..b72bb60 100644 --- a/check/check.go +++ b/check/check.go @@ -60,6 +60,7 @@ type Check struct { ID string `yaml:"id" json:"id"` Text string Audit string `json:"omit"` + Type string `json:"type"` Commands []*exec.Cmd `json:"omit"` Tests *tests `json:"omit"` Set bool `json:"omit"` @@ -70,6 +71,12 @@ type Check struct { // Run executes the audit commands specified in a check and outputs // the results. func (c *Check) Run(verbose bool) { + // If check type is manual, force result to WARN. + if c.Type == "manual" { + c.State = WARN + return + } + var out bytes.Buffer var errmsgs string From 29122b82adc6cdf946f749fc815c794eadbb1aab Mon Sep 17 00:00:00 2001 From: Abubakr-Sadik Nii Nai Davis Date: Sun, 6 Aug 2017 16:12:47 +0000 Subject: [PATCH 04/13] Add master node manual check definitions. --- cfg/master.yaml | 91 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 84 insertions(+), 7 deletions(-) diff --git a/cfg/master.yaml b/cfg/master.yaml index 46aeb36..874b9cc 100644 --- a/cfg/master.yaml +++ b/cfg/master.yaml @@ -479,19 +479,14 @@ groups: parameter to \"--experimental-encryption-provider-config=\"" scored: true -# TODO: provide flag to WARN of manual tasks which we can't automate. - id: 1.1.35 text: "Ensure that the encryption provider is set to aescbc (Scored)" audit: "ps -ef | grep $apiserverbin | grep -v grep" - tests: - test_items: - - flag: "requires manual intervention" - set: true + type: "manual" remediation: "Follow the Kubernetes documentation and configure a EncryptionConfig file. In this file, choose aescbc as the encryption provider" scored: true - - id: 1.2 text: "Scheduler" checks: @@ -573,7 +568,13 @@ groups: KUBE_CONTROLLER_MANAGER_ARGS parameter to include --root-ca-file=" scored: true -# TODO: 1.3.6 is manual, provide way to WARN + - id: 1.3.6 + text: "Apply Security Context to Your Pods and Containers (Not Scored)" + type: "manual" + remediation: "Edit the /etc/kubernetes/controller-manager file on the master node and set the + KUBE_CONTROLLER_MANAGER_ARGS parameter to a value to include + \"--feature-gates=RotateKubeletServerCertificate=true\"" + scored: false - id: 1.3.7 text: " Ensure that the RotateKubeletServerCertificate argument is set to true (Scored)" @@ -717,6 +718,20 @@ groups: chmod 700 /var/lib/etcd/default.etcd" scored: true + - id: 1.4.12 + text: "Ensure that the etcd data directory ownership is set to etcd:etcd (Scored)" + audit: "ps -ef | grep $etcdbin | grep -v grep | grep -o data-dir=.* | cut -d= -f2 | xargs stat -c %U:%G" + tests: + test_items: + - flag: "etcd:etcd" + set: true + remediation: "On the etcd server node, get the etcd data directory, passed as an argument --data-dir , + from the below command:\n + ps -ef | grep etcd\n + Run the below command (based on the etcd data directory found above). For example,\n + chown etcd:etcd /var/lib/etcd/default.etcd" + scored: true + - id: 1.5 text: "etcd" checks: @@ -859,3 +874,65 @@ groups: remediation: "Follow the etcd documentation and create a dedicated certificate authority setup for the etcd service." scored: false + +- id: 1.6 + text: "General Security Primitives" + checks: + - id: 1.6.1 + text: "Ensure that the cluster-admin role is only used where required (Not Scored)" + type: "manual" + remediation: "Remove any unneeded clusterrolebindings: kubectl delete clusterrolebinding [name]" + scored: false + + - id: 1.6.2 + text: "Create Pod Security Policies for your cluster (Not Scored)" + type: "manual" + remediation: "Follow the documentation and create and enforce Pod Security Policies for your cluster. + Additionally, you could refer the \"CIS Security Benchmark for Docker\" and follow the + suggested Pod Security Policies for your environment." + scored: false + + - id: 1.6.3 + text: "Create administrative boundaries between resources using namespaces (Not Scored)" + type: "manual" + remediation: "Follow the documentation and create namespaces for objects in your deployment as you + need them." + scored: false + + - id: 1.6.4 + text: "Create network segmentation using Network Policies (Not Scored)" + type: "manual" + remediation: "Follow the documentation and create NetworkPolicy objects as you need them." + scored: false + + - id: 1.6.5 + text: "Ensure that the seccomp profile is set to docker/default in your pod definitions (Not Scored)" + type: "manual" + remediation: "Seccomp is an alpha feature currently. By default, all alpha features are disabled. So, you + would need to enable alpha features in the apiserver by passing \"--feature- + gates=AllAlpha=true\" argument.\n + Edit the $apiserverconf file on the master node and set the KUBE_API_ARGS + parameter to \"--feature-gates=AllAlpha=true\" + KUBE_API_ARGS=\"--feature-gates=AllAlpha=true\"" + scored: false + + - id: 1.6.6 + text: "Apply Security Context to Your Pods and Containers (Not Scored)" + type: "manual" + remediation: "Follow the Kubernetes documentation and apply security contexts to your pods. For a + suggested list of security contexts, you may refer to the CIS Security Benchmark for Docker + Containers." + scored: false + + - id: 1.6.7 + text: "Configure Image Provenance using ImagePolicyWebhook admission controller (Not Scored)" + type: "manual" + remediation: "Follow the Kubernetes documentation and setup image provenance." + scored: false + + - id: 1.6.8 + text: "Configure Network policies as appropriate (Not Scored)" + type: "manual" + remediation: "Follow the Kubernetes documentation and setup network policies as appropriate." + scored: false + From 9c563b0987627a984e8a739d7b770d4d9573c74f Mon Sep 17 00:00:00 2001 From: Abubakr-Sadik Nii Nai Davis Date: Sun, 6 Aug 2017 16:37:22 +0000 Subject: [PATCH 05/13] Remove misleading comment about manual checks in node check definition. --- cfg/node.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/cfg/node.yaml b/cfg/node.yaml index 94e48f7..346ddd1 100644 --- a/cfg/node.yaml +++ b/cfg/node.yaml @@ -285,7 +285,6 @@ 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)" @@ -298,7 +297,6 @@ groups: \nchmod 644 " 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" From 7bb66dd2da9c27059dc5f86aa334815962552710 Mon Sep 17 00:00:00 2001 From: Abubakr-Sadik Nii Nai Davis Date: Sun, 6 Aug 2017 16:59:03 +0000 Subject: [PATCH 06/13] Rename warning printing functions. printlnWarn: prints warning with a newline. sprintWarn: returns an optionally contextualized warning string. --- cmd/util.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/util.go b/cmd/util.go index 26e7441..1885952 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -21,14 +21,14 @@ var ( } ) -func printWarn(msg string) { +func printlnWarn(msg string) { fmt.Fprintf(os.Stderr, "[%s] %s\n", colors[check.WARN].Sprintf("%s", check.WARN), msg, ) } -func printWarn(msg string) string { +func sprintlnWarn(msg string) string { return fmt.Sprintf("[%s] %s", colors[check.WARN].Sprintf("%s", check.WARN), msg, @@ -75,7 +75,7 @@ func verifyConf(confPath ...string) { if len(missing) > 0 { missing = strings.Trim(missing, ", ") - printWarn(fmt.Sprintf("Missing kubernetes config files: %s", missing)) + printlnWarn(fmt.Sprintf("Missing kubernetes config files: %s", missing)) } } @@ -114,12 +114,12 @@ func verifyBin(binPath ...string) { if len(missing) > 0 { missing = strings.Trim(missing, ", ") - printWarn(fmt.Sprintf("Missing kubernetes binaries: %s", missing)) + printlnWarn(fmt.Sprintf("Missing kubernetes binaries: %s", missing)) } if len(notRunning) > 0 { notRunning = strings.Trim(notRunning, ", ") - printWarn(fmt.Sprintf("Kubernetes binaries not running: %s", notRunning)) + printlnWarn(fmt.Sprintf("Kubernetes binaries not running: %s", notRunning)) } } @@ -129,19 +129,19 @@ func verifyKubeVersion(b string) { _, err := exec.LookPath(b) if err != nil { - continueWithError(err, printfWarn("Kubernetes version check skipped")) + continueWithError(err, sprintlnWarn("Kubernetes version check skipped")) return } cmd := exec.Command(b, "--version") out, err := cmd.Output() if err != nil { - continueWithError(err, printfWarn("Kubernetes version check skipped")) + continueWithError(err, sprintlnWarn("Kubernetes version check skipped")) return } matched := strings.Contains(string(out), kubeVersion) if !matched { - printWarn(fmt.Sprintf("Unsupported kubernetes version: %s", out)) + printlnWarn(fmt.Sprintf("Unsupported kubernetes version: %s", out)) } } From b5f4876138a911bdd463405479e5a6a9ab041b7f Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Tue, 8 Aug 2017 22:00:06 +0100 Subject: [PATCH 07/13] Revert "Issue 19" --- cfg/master.yaml | 91 ++++--------------------------------------------- cfg/node.yaml | 2 ++ check/check.go | 9 +---- 3 files changed, 10 insertions(+), 92 deletions(-) diff --git a/cfg/master.yaml b/cfg/master.yaml index 874b9cc..46aeb36 100644 --- a/cfg/master.yaml +++ b/cfg/master.yaml @@ -479,14 +479,19 @@ groups: parameter to \"--experimental-encryption-provider-config=\"" scored: true +# TODO: provide flag to WARN of manual tasks which we can't automate. - id: 1.1.35 text: "Ensure that the encryption provider is set to aescbc (Scored)" audit: "ps -ef | grep $apiserverbin | grep -v grep" - type: "manual" + tests: + test_items: + - flag: "requires manual intervention" + set: true remediation: "Follow the Kubernetes documentation and configure a EncryptionConfig file. In this file, choose aescbc as the encryption provider" scored: true + - id: 1.2 text: "Scheduler" checks: @@ -568,13 +573,7 @@ groups: KUBE_CONTROLLER_MANAGER_ARGS parameter to include --root-ca-file=" scored: true - - id: 1.3.6 - text: "Apply Security Context to Your Pods and Containers (Not Scored)" - type: "manual" - remediation: "Edit the /etc/kubernetes/controller-manager file on the master node and set the - KUBE_CONTROLLER_MANAGER_ARGS parameter to a value to include - \"--feature-gates=RotateKubeletServerCertificate=true\"" - scored: false +# TODO: 1.3.6 is manual, provide way to WARN - id: 1.3.7 text: " Ensure that the RotateKubeletServerCertificate argument is set to true (Scored)" @@ -718,20 +717,6 @@ groups: chmod 700 /var/lib/etcd/default.etcd" scored: true - - id: 1.4.12 - text: "Ensure that the etcd data directory ownership is set to etcd:etcd (Scored)" - audit: "ps -ef | grep $etcdbin | grep -v grep | grep -o data-dir=.* | cut -d= -f2 | xargs stat -c %U:%G" - tests: - test_items: - - flag: "etcd:etcd" - set: true - remediation: "On the etcd server node, get the etcd data directory, passed as an argument --data-dir , - from the below command:\n - ps -ef | grep etcd\n - Run the below command (based on the etcd data directory found above). For example,\n - chown etcd:etcd /var/lib/etcd/default.etcd" - scored: true - - id: 1.5 text: "etcd" checks: @@ -874,65 +859,3 @@ groups: remediation: "Follow the etcd documentation and create a dedicated certificate authority setup for the etcd service." scored: false - -- id: 1.6 - text: "General Security Primitives" - checks: - - id: 1.6.1 - text: "Ensure that the cluster-admin role is only used where required (Not Scored)" - type: "manual" - remediation: "Remove any unneeded clusterrolebindings: kubectl delete clusterrolebinding [name]" - scored: false - - - id: 1.6.2 - text: "Create Pod Security Policies for your cluster (Not Scored)" - type: "manual" - remediation: "Follow the documentation and create and enforce Pod Security Policies for your cluster. - Additionally, you could refer the \"CIS Security Benchmark for Docker\" and follow the - suggested Pod Security Policies for your environment." - scored: false - - - id: 1.6.3 - text: "Create administrative boundaries between resources using namespaces (Not Scored)" - type: "manual" - remediation: "Follow the documentation and create namespaces for objects in your deployment as you - need them." - scored: false - - - id: 1.6.4 - text: "Create network segmentation using Network Policies (Not Scored)" - type: "manual" - remediation: "Follow the documentation and create NetworkPolicy objects as you need them." - scored: false - - - id: 1.6.5 - text: "Ensure that the seccomp profile is set to docker/default in your pod definitions (Not Scored)" - type: "manual" - remediation: "Seccomp is an alpha feature currently. By default, all alpha features are disabled. So, you - would need to enable alpha features in the apiserver by passing \"--feature- - gates=AllAlpha=true\" argument.\n - Edit the $apiserverconf file on the master node and set the KUBE_API_ARGS - parameter to \"--feature-gates=AllAlpha=true\" - KUBE_API_ARGS=\"--feature-gates=AllAlpha=true\"" - scored: false - - - id: 1.6.6 - text: "Apply Security Context to Your Pods and Containers (Not Scored)" - type: "manual" - remediation: "Follow the Kubernetes documentation and apply security contexts to your pods. For a - suggested list of security contexts, you may refer to the CIS Security Benchmark for Docker - Containers." - scored: false - - - id: 1.6.7 - text: "Configure Image Provenance using ImagePolicyWebhook admission controller (Not Scored)" - type: "manual" - remediation: "Follow the Kubernetes documentation and setup image provenance." - scored: false - - - id: 1.6.8 - text: "Configure Network policies as appropriate (Not Scored)" - type: "manual" - remediation: "Follow the Kubernetes documentation and setup network policies as appropriate." - scored: false - diff --git a/cfg/node.yaml b/cfg/node.yaml index 346ddd1..94e48f7 100644 --- a/cfg/node.yaml +++ b/cfg/node.yaml @@ -285,6 +285,7 @@ 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)" @@ -297,6 +298,7 @@ groups: \nchmod 644 " 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" diff --git a/check/check.go b/check/check.go index 2ac7b5b..5d8221a 100644 --- a/check/check.go +++ b/check/check.go @@ -61,7 +61,6 @@ type Check struct { ID string `yaml:"id" json:"id"` Text string Audit string `json:"omit"` - Type string `json:"type"` Commands []*exec.Cmd `json:"omit"` Tests *tests `json:"omit"` Set bool `json:"omit"` @@ -71,13 +70,7 @@ type Check struct { // Run executes the audit commands specified in a check and outputs // the results. -func (c *Check) Run(verbose bool) { - // If check type is manual, force result to WARN. - if c.Type == "manual" { - c.State = WARN - return - } - +func (c *Check) Run() { var out bytes.Buffer var errmsgs string From 09ca739dc0864874f1beee149cc58c6901b80215 Mon Sep 17 00:00:00 2001 From: Abubakr-Sadik Nii Nai Davis Date: Sun, 6 Aug 2017 15:29:55 +0000 Subject: [PATCH 08/13] Add check type manual. Results of manual checks are forced to WARN to inform users to check manually. --- check/check.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/check/check.go b/check/check.go index 5d8221a..2ac7b5b 100644 --- a/check/check.go +++ b/check/check.go @@ -61,6 +61,7 @@ type Check struct { ID string `yaml:"id" json:"id"` Text string Audit string `json:"omit"` + Type string `json:"type"` Commands []*exec.Cmd `json:"omit"` Tests *tests `json:"omit"` Set bool `json:"omit"` @@ -70,7 +71,13 @@ type Check struct { // Run executes the audit commands specified in a check and outputs // the results. -func (c *Check) Run() { +func (c *Check) Run(verbose bool) { + // If check type is manual, force result to WARN. + if c.Type == "manual" { + c.State = WARN + return + } + var out bytes.Buffer var errmsgs string From c39516581b0985fa2e5b1bccdc064b06d9051095 Mon Sep 17 00:00:00 2001 From: Abubakr-Sadik Nii Nai Davis Date: Sun, 6 Aug 2017 16:12:47 +0000 Subject: [PATCH 09/13] Add master node manual check definitions. --- cfg/master.yaml | 91 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 84 insertions(+), 7 deletions(-) diff --git a/cfg/master.yaml b/cfg/master.yaml index 46aeb36..874b9cc 100644 --- a/cfg/master.yaml +++ b/cfg/master.yaml @@ -479,19 +479,14 @@ groups: parameter to \"--experimental-encryption-provider-config=\"" scored: true -# TODO: provide flag to WARN of manual tasks which we can't automate. - id: 1.1.35 text: "Ensure that the encryption provider is set to aescbc (Scored)" audit: "ps -ef | grep $apiserverbin | grep -v grep" - tests: - test_items: - - flag: "requires manual intervention" - set: true + type: "manual" remediation: "Follow the Kubernetes documentation and configure a EncryptionConfig file. In this file, choose aescbc as the encryption provider" scored: true - - id: 1.2 text: "Scheduler" checks: @@ -573,7 +568,13 @@ groups: KUBE_CONTROLLER_MANAGER_ARGS parameter to include --root-ca-file=" scored: true -# TODO: 1.3.6 is manual, provide way to WARN + - id: 1.3.6 + text: "Apply Security Context to Your Pods and Containers (Not Scored)" + type: "manual" + remediation: "Edit the /etc/kubernetes/controller-manager file on the master node and set the + KUBE_CONTROLLER_MANAGER_ARGS parameter to a value to include + \"--feature-gates=RotateKubeletServerCertificate=true\"" + scored: false - id: 1.3.7 text: " Ensure that the RotateKubeletServerCertificate argument is set to true (Scored)" @@ -717,6 +718,20 @@ groups: chmod 700 /var/lib/etcd/default.etcd" scored: true + - id: 1.4.12 + text: "Ensure that the etcd data directory ownership is set to etcd:etcd (Scored)" + audit: "ps -ef | grep $etcdbin | grep -v grep | grep -o data-dir=.* | cut -d= -f2 | xargs stat -c %U:%G" + tests: + test_items: + - flag: "etcd:etcd" + set: true + remediation: "On the etcd server node, get the etcd data directory, passed as an argument --data-dir , + from the below command:\n + ps -ef | grep etcd\n + Run the below command (based on the etcd data directory found above). For example,\n + chown etcd:etcd /var/lib/etcd/default.etcd" + scored: true + - id: 1.5 text: "etcd" checks: @@ -859,3 +874,65 @@ groups: remediation: "Follow the etcd documentation and create a dedicated certificate authority setup for the etcd service." scored: false + +- id: 1.6 + text: "General Security Primitives" + checks: + - id: 1.6.1 + text: "Ensure that the cluster-admin role is only used where required (Not Scored)" + type: "manual" + remediation: "Remove any unneeded clusterrolebindings: kubectl delete clusterrolebinding [name]" + scored: false + + - id: 1.6.2 + text: "Create Pod Security Policies for your cluster (Not Scored)" + type: "manual" + remediation: "Follow the documentation and create and enforce Pod Security Policies for your cluster. + Additionally, you could refer the \"CIS Security Benchmark for Docker\" and follow the + suggested Pod Security Policies for your environment." + scored: false + + - id: 1.6.3 + text: "Create administrative boundaries between resources using namespaces (Not Scored)" + type: "manual" + remediation: "Follow the documentation and create namespaces for objects in your deployment as you + need them." + scored: false + + - id: 1.6.4 + text: "Create network segmentation using Network Policies (Not Scored)" + type: "manual" + remediation: "Follow the documentation and create NetworkPolicy objects as you need them." + scored: false + + - id: 1.6.5 + text: "Ensure that the seccomp profile is set to docker/default in your pod definitions (Not Scored)" + type: "manual" + remediation: "Seccomp is an alpha feature currently. By default, all alpha features are disabled. So, you + would need to enable alpha features in the apiserver by passing \"--feature- + gates=AllAlpha=true\" argument.\n + Edit the $apiserverconf file on the master node and set the KUBE_API_ARGS + parameter to \"--feature-gates=AllAlpha=true\" + KUBE_API_ARGS=\"--feature-gates=AllAlpha=true\"" + scored: false + + - id: 1.6.6 + text: "Apply Security Context to Your Pods and Containers (Not Scored)" + type: "manual" + remediation: "Follow the Kubernetes documentation and apply security contexts to your pods. For a + suggested list of security contexts, you may refer to the CIS Security Benchmark for Docker + Containers." + scored: false + + - id: 1.6.7 + text: "Configure Image Provenance using ImagePolicyWebhook admission controller (Not Scored)" + type: "manual" + remediation: "Follow the Kubernetes documentation and setup image provenance." + scored: false + + - id: 1.6.8 + text: "Configure Network policies as appropriate (Not Scored)" + type: "manual" + remediation: "Follow the Kubernetes documentation and setup network policies as appropriate." + scored: false + From 9c07527069ad818899a2fe5aa3015bfba2e99463 Mon Sep 17 00:00:00 2001 From: Abubakr-Sadik Nii Nai Davis Date: Sun, 6 Aug 2017 16:37:22 +0000 Subject: [PATCH 10/13] Remove misleading comment about manual checks in node check definition. --- cfg/node.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/cfg/node.yaml b/cfg/node.yaml index 94e48f7..346ddd1 100644 --- a/cfg/node.yaml +++ b/cfg/node.yaml @@ -285,7 +285,6 @@ 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)" @@ -298,7 +297,6 @@ groups: \nchmod 644 " 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" From 767e8eb83581de072a1ab1d98563f76408ac69b5 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Tue, 8 Aug 2017 22:22:47 +0100 Subject: [PATCH 11/13] Sorting out the bad merge --- check/check.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check/check.go b/check/check.go index 2ac7b5b..1f23a9e 100644 --- a/check/check.go +++ b/check/check.go @@ -71,7 +71,7 @@ type Check struct { // Run executes the audit commands specified in a check and outputs // the results. -func (c *Check) Run(verbose bool) { +func (c *Check) Run() { // If check type is manual, force result to WARN. if c.Type == "manual" { c.State = WARN From dee64c30ae1bf22a10619e3117169c10fbc431e9 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Fri, 11 Aug 2017 16:06:44 +0100 Subject: [PATCH 12/13] Create OWNERS --- OWNERS | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 OWNERS diff --git a/OWNERS b/OWNERS new file mode 100644 index 0000000..b95e22b --- /dev/null +++ b/OWNERS @@ -0,0 +1,3 @@ +approvers: + - lizrice + - jerbia From 96c469669c23df1bfdbe2d47aab7c0acb485fff6 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Fri, 11 Aug 2017 17:59:57 +0100 Subject: [PATCH 13/13] Use kubectl to check the kubernetes version --- cmd/common.go | 7 ++--- cmd/util.go | 53 ++++++++++++++++++++++++++++----- cmd/util_test.go | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 12 deletions(-) create mode 100644 cmd/util_test.go diff --git a/cmd/common.go b/cmd/common.go index 69bec28..d27dc1f 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -45,7 +45,8 @@ var ( errmsgs string // TODO: Consider specifying this in config file. - kubeVersion = "1.7.0" + kubeMajorVersion = "1" + kubeMinorVersion = "7" ) func runChecks(t check.NodeType) { @@ -77,6 +78,7 @@ func runChecks(t check.NodeType) { fedControllerManagerBin = viper.GetString("installation." + installation + ".federated.bin.controller-manager") // Run kubernetes installation validation checks. + verifyKubeVersion(kubeMajorVersion, kubeMinorVersion) verifyNodeType(t) switch t { @@ -150,15 +152,12 @@ func runChecks(t check.NodeType) { func verifyNodeType(t check.NodeType) { switch t { case check.MASTER: - verifyKubeVersion(apiserverBin) verifyBin(apiserverBin, schedulerBin, controllerManagerBin) verifyConf(apiserverConf, schedulerConf, controllerManagerConf) case check.NODE: - verifyKubeVersion(kubeletBin) verifyBin(kubeletBin, proxyBin) verifyConf(kubeletConf, proxyConf) case check.FEDERATED: - verifyKubeVersion(fedApiserverBin) verifyBin(fedApiserverBin, fedControllerManagerBin) } } diff --git a/cmd/util.go b/cmd/util.go index 1885952..937e3e0 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "os/exec" + "regexp" "strings" "github.com/aquasecurity/kube-bench/check" @@ -123,25 +124,61 @@ func verifyBin(binPath ...string) { } } -func verifyKubeVersion(b string) { +func verifyKubeVersion(major string, minor string) { // These executables might not be on the user's path. - // TODO! Check the version number using kubectl, which is more likely to be on the path. - _, err := exec.LookPath(b) + _, err := exec.LookPath("kubectl") if err != nil { continueWithError(err, sprintlnWarn("Kubernetes version check skipped")) return } - cmd := exec.Command(b, "--version") + cmd := exec.Command("kubectl", "version") out, err := cmd.Output() if err != nil { - continueWithError(err, sprintlnWarn("Kubernetes version check skipped")) + s := fmt.Sprintf("Kubernetes version check skipped with error %v", err) + continueWithError(err, sprintlnWarn(s)) return } - matched := strings.Contains(string(out), kubeVersion) - if !matched { - printlnWarn(fmt.Sprintf("Unsupported kubernetes version: %s", out)) + msg := checkVersion("Client", string(out), major, minor) + if msg != "" { + continueWithError(fmt.Errorf(msg), msg) + } + + msg = checkVersion("Server", string(out), major, minor) + if msg != "" { + continueWithError(fmt.Errorf(msg), msg) } } + +var regexVersionMajor = regexp.MustCompile("Major:\"([0-9]+)\"") +var regexVersionMinor = regexp.MustCompile("Minor:\"([0-9]+)\"") + +func checkVersion(x string, s string, expMajor string, expMinor string) string { + regexVersion, err := regexp.Compile(x + " Version: version.Info{(.*)}") + if err != nil { + return fmt.Sprintf("Error checking Kubernetes version: %v", err) + } + + ss := regexVersion.FindString(s) + major := versionMatch(regexVersionMajor, ss) + minor := versionMatch(regexVersionMinor, ss) + if major == "" || minor == "" { + return fmt.Sprintf("Couldn't find %s version from kubectl output '%s'", x, s) + } + + if major != expMajor || minor != expMinor { + return fmt.Sprintf("Unexpected %s version %s.%s", x, major, minor) + } + + return "" +} + +func versionMatch(r *regexp.Regexp, s string) string { + match := r.FindStringSubmatch(s) + if len(match) < 2 { + return "" + } + return match[1] +} diff --git a/cmd/util_test.go b/cmd/util_test.go new file mode 100644 index 0000000..74d955f --- /dev/null +++ b/cmd/util_test.go @@ -0,0 +1,77 @@ +// Copyright © 2017 Aqua Security Software Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cmd + +import ( + "regexp" + "testing" +) + +func TestCheckVersion(t *testing.T) { + kubeoutput := `Client Version: version.Info{Major:"1", Minor:"7", GitVersion:"v1.7.0", GitCommit:"d3ada0119e776222f11ec7945e6d860061339aad", GitTreeState:"clean", BuildDate:"2017-06-30T09:51:01Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"darwin/amd64"} + Server Version: version.Info{Major:"1", Minor:"7", GitVersion:"v1.7.0", GitCommit:"d3ada0119e776222f11ec7945e6d860061339aad", GitTreeState:"clean", BuildDate:"2017-07-26T00:12:31Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"linux/amd64"}` + cases := []struct { + t string + s string + major string + minor string + exp string + }{ + {t: "Client", s: kubeoutput, major: "1", minor: "7"}, + {t: "Server", s: kubeoutput, major: "1", minor: "7"}, + {t: "Client", s: kubeoutput, major: "1", minor: "6", exp: "Unexpected Client version 1.7"}, + {t: "Client", s: kubeoutput, major: "2", minor: "0", exp: "Unexpected Client version 1.7"}, + {t: "Server", s: "something unexpected", major: "2", minor: "0", exp: "Couldn't find Server version from kubectl output 'something unexpected'"}, + } + + for _, c := range cases { + t.Run("", 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) + } + }) + } + +} + +func TestVersionMatch(t *testing.T) { + minor := regexVersionMinor + major := regexVersionMajor + client := `Client Version: version.Info{Major:"1", Minor:"7", GitVersion:"v1.7.0", GitCommit:"d3ada0119e776222f11ec7945e6d860061339aad", GitTreeState:"clean", BuildDate:"2017-06-30T09:51:01Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"darwin/amd64"}` + server := `Server Version: version.Info{Major:"1", Minor:"7", GitVersion:"v1.7.0", GitCommit:"d3ada0119e776222f11ec7945e6d860061339aad", GitTreeState:"clean", BuildDate:"2017-07-26T00:12:31Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"linux/amd64"}` + + cases := []struct { + r *regexp.Regexp + s string + exp string + }{ + {r: major, s: server, exp: "1"}, + {r: minor, s: server, exp: "7"}, + {r: major, s: client, exp: "1"}, + {r: minor, s: client, exp: "7"}, + {r: major, s: "Some unexpected string"}, + {r: minor}, // Checking that we don't fall over if the string is empty + } + + for _, c := range cases { + t.Run("", func(t *testing.T) { + m := versionMatch(c.r, c.s) + if m != c.exp { + t.Fatalf("Got %s expected %s", m, c.exp) + } + }) + } +}