From 9114e139cfb341506b7b0f36012f48324c145077 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Wed, 30 Aug 2017 12:07:46 +0100 Subject: [PATCH 01/19] Function to find which of a set of executables is running --- cmd/util.go | 11 +++++++++++ cmd/util_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/cmd/util.go b/cmd/util.go index 478ae21..10a13c2 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -90,6 +90,17 @@ func verifyBin(bin string, psFunc func(string) string) bool { return strings.Contains(out, bin) } +// findExecutable looks through a list of possible executable names and finds the first one that's running +func findExecutable(candidates []string, psFunc func(string) string) (string, error) { + for _, c := range candidates { + if verifyBin(c, psFunc) { + return c, nil + } + } + + return "", fmt.Errorf("no candidates running") +} + func verifyKubeVersion(major string, minor string) { // These executables might not be on the user's path. diff --git a/cmd/util_test.go b/cmd/util_test.go index dbd434b..5da91de 100644 --- a/cmd/util_test.go +++ b/cmd/util_test.go @@ -108,6 +108,39 @@ func TestVerifyBin(t *testing.T) { } } +func TestFindExecutable(t *testing.T) { + cases := []struct { + candidates []string // list of executables we'd consider + psOut string // fake output from ps + exp string // the one we expect to find in the (fake) ps output + expErr bool + }{ + {candidates: []string{"one", "two", "three"}, psOut: "two", exp: "two"}, + {candidates: []string{"one", "two", "three"}, psOut: "two three", exp: "two"}, + {candidates: []string{"one double", "two double", "three double"}, psOut: "two double is running", exp: "two double"}, + {candidates: []string{"one", "two", "three"}, psOut: "blah", expErr: true}, + {candidates: []string{"one double", "two double", "three double"}, psOut: "two", expErr: true}, + } + + for id, c := range cases { + t.Run(strconv.Itoa(id), func(t *testing.T) { + g = c.psOut + e, err := findExecutable(c.candidates, fakeps) + if e != c.exp { + t.Fatalf("Expected %v got %v", c.exp, e) + } + + if err == nil && c.expErr { + t.Fatalf("Expected error") + } + + if err != nil && !c.expErr { + t.Fatalf("Didn't expect error: %v", err) + } + }) + } +} + func TestMultiWordReplace(t *testing.T) { cases := []struct { input string From 0bc00e003675e960ff923f0bc847c6f1aee81c6a Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Wed, 30 Aug 2017 17:48:12 +0100 Subject: [PATCH 02/19] Slightly more robust looking for running executables --- cmd/util.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/cmd/util.go b/cmd/util.go index 10a13c2..bd12398 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -87,7 +87,19 @@ func verifyBin(bin string, psFunc func(string) string) bool { proc := strings.Fields(bin)[0] out := psFunc(proc) - return strings.Contains(out, bin) + if !strings.Contains(out, bin) { + return false + } + + // Make sure we're not just matching on a partial word (e.g. if we're looking for apiserver, don't match on kube-apiserver) + // This will give a false positive for matching "one two" against "zero one two-x" but it will do for now + for _, f := range strings.Fields(out) { + if f == proc { + return true + } + } + + return false } // findExecutable looks through a list of possible executable names and finds the first one that's running From 7600dd9dd676e9414c27f1e4dc8dd0dab1f88601 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Wed, 30 Aug 2017 17:51:28 +0100 Subject: [PATCH 03/19] =?UTF-8?q?Make=20the=20ps=20/=20fakeps=20function?= =?UTF-8?q?=20global=20so=20we=20don=E2=80=99t=20have=20to=20pass=20it=20a?= =?UTF-8?q?round=20so=20much?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/common.go | 2 +- cmd/util.go | 14 +++++++++++--- cmd/util_test.go | 8 ++++++-- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/cmd/common.go b/cmd/common.go index 89f45bc..b3df7b9 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -165,7 +165,7 @@ func verifyNodeType(t check.NodeType) { } for _, bin := range bins { - if !verifyBin(bin, ps) { + if !verifyBin(bin) { printlnWarn(fmt.Sprintf("%s is not running", bin)) } } diff --git a/cmd/util.go b/cmd/util.go index bd12398..87b13f0 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -22,6 +22,14 @@ var ( } ) +var psFunc func(string) string +var statFunc func(string) (os.FileInfo, error) + +func init() { + psFunc = ps + statFunc = os.Stat +} + func printlnWarn(msg string) { fmt.Fprintf(os.Stderr, "[%s] %s\n", colors[check.WARN].Sprintf("%s", check.WARN), @@ -76,7 +84,7 @@ func ps(proc string) string { } // verifyBin checks that the binary specified is running -func verifyBin(bin string, psFunc func(string) string) bool { +func verifyBin(bin string) bool { // Strip any quotes bin = strings.Trim(bin, "'\"") @@ -103,9 +111,9 @@ func verifyBin(bin string, psFunc func(string) string) bool { } // findExecutable looks through a list of possible executable names and finds the first one that's running -func findExecutable(candidates []string, psFunc func(string) string) (string, error) { +func findExecutable(candidates []string) (string, error) { for _, c := range candidates { - if verifyBin(c, psFunc) { + if verifyBin(c) { return c, nil } } diff --git a/cmd/util_test.go b/cmd/util_test.go index 5da91de..659dd2e 100644 --- a/cmd/util_test.go +++ b/cmd/util_test.go @@ -97,10 +97,11 @@ func TestVerifyBin(t *testing.T) { {proc: "cmd param", psOut: "cmd", exp: false}, } + psFunc = fakeps for id, c := range cases { t.Run(strconv.Itoa(id), func(t *testing.T) { g = c.psOut - v := verifyBin(c.proc, fakeps) + v := verifyBin(c.proc) if v != c.exp { t.Fatalf("Expected %v got %v", c.exp, v) } @@ -120,12 +121,15 @@ func TestFindExecutable(t *testing.T) { {candidates: []string{"one double", "two double", "three double"}, psOut: "two double is running", exp: "two double"}, {candidates: []string{"one", "two", "three"}, psOut: "blah", expErr: true}, {candidates: []string{"one double", "two double", "three double"}, psOut: "two", expErr: true}, + {candidates: []string{"apiserver", "kube-apiserver"}, psOut: "kube-apiserver", exp: "kube-apiserver"}, + {candidates: []string{"apiserver", "kube-apiserver", "hyperkube-apiserver"}, psOut: "kube-apiserver", exp: "kube-apiserver"}, } + psFunc = fakeps for id, c := range cases { t.Run(strconv.Itoa(id), func(t *testing.T) { g = c.psOut - e, err := findExecutable(c.candidates, fakeps) + e, err := findExecutable(c.candidates) if e != c.exp { t.Fatalf("Expected %v got %v", c.exp, e) } From f5cef922cc69d35d34292cb75a91980b551e4052 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Wed, 30 Aug 2017 18:01:53 +0100 Subject: [PATCH 04/19] Functions and tests for finding binaries and config files --- cmd/util.go | 47 ++++++++++++++++++++ cmd/util_test.go | 109 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+) diff --git a/cmd/util.go b/cmd/util.go index 87b13f0..0bc40be 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -10,6 +10,7 @@ import ( "github.com/aquasecurity/kube-bench/check" "github.com/fatih/color" "github.com/golang/glog" + "github.com/spf13/viper" ) var ( @@ -83,6 +84,37 @@ func ps(proc string) string { return string(out) } +// getBinaries finds which of the set of candidate executables are running +func getBinaries(v *viper.Viper) map[string]string { + binmap := make(map[string]string) + + for _, exeType := range v.AllKeys() { + bin, err := findExecutable(v.GetStringSlice(exeType)) + if err != nil { + exitWithError(fmt.Errorf("looking for %s executable but none of the candidates are running", exeType)) + } + + binmap[exeType] = bin + } + return binmap +} + +// getConfigFiles finds which of the set of candidate config files exist +func getConfigFiles(v *viper.Viper) map[string]string { + confmap := make(map[string]string) + + for _, confType := range v.AllKeys() { + conf := findConfigFile(v.GetStringSlice(confType)) + if conf == "" { + printlnWarn(fmt.Sprintf("Missing kubernetes config file for %s", confType)) + } else { + confmap[confType] = conf + } + } + + return confmap +} + // verifyBin checks that the binary specified is running func verifyBin(bin string) bool { @@ -110,6 +142,21 @@ func verifyBin(bin string) bool { return false } +// fundConfigFile looks through a list of possible config files and finds the first one that exists +func findConfigFile(candidates []string) string { + for _, c := range candidates { + _, err := statFunc(c) + if err == nil { + return c + } + if !os.IsNotExist(err) { + exitWithError(fmt.Errorf("error looking for file %s: %v", c, err)) + } + } + + return "" +} + // findExecutable looks through a list of possible executable names and finds the first one that's running func findExecutable(candidates []string) (string, error) { for _, c := range candidates { diff --git a/cmd/util_test.go b/cmd/util_test.go index 659dd2e..d5192a6 100644 --- a/cmd/util_test.go +++ b/cmd/util_test.go @@ -15,9 +15,13 @@ package cmd import ( + "os" + "reflect" "regexp" "strconv" "testing" + + "github.com/spf13/viper" ) func TestCheckVersion(t *testing.T) { @@ -78,10 +82,19 @@ func TestVersionMatch(t *testing.T) { } var g string +var e []error +var eIndex int func fakeps(proc string) string { return g } + +func fakestat(file string) (os.FileInfo, error) { + err := e[eIndex] + eIndex++ + return nil, err +} + func TestVerifyBin(t *testing.T) { cases := []struct { proc string @@ -145,6 +158,41 @@ func TestFindExecutable(t *testing.T) { } } +func TestGetBinaries(t *testing.T) { + cases := []struct { + config map[string]interface{} + psOut string + exp map[string]string + }{ + { + config: map[string]interface{}{"apiserver": []string{"apiserver", "kube-apiserver"}}, + psOut: "kube-apiserver", + exp: map[string]string{"apiserver": "kube-apiserver"}, + }, + { + config: map[string]interface{}{"apiserver": []string{"apiserver", "kube-apiserver"}, "thing": []string{"something else", "thing"}}, + psOut: "kube-apiserver thing", + exp: map[string]string{"apiserver": "kube-apiserver", "thing": "thing"}, + }, + } + + v := viper.New() + psFunc = fakeps + + for id, c := range cases { + t.Run(strconv.Itoa(id), func(t *testing.T) { + g = c.psOut + for k, val := range c.config { + v.Set(k, val) + } + m := getBinaries(v) + if !reflect.DeepEqual(m, c.exp) { + t.Fatalf("Got %v\nExpected %v", m, c.exp) + } + }) + } +} + func TestMultiWordReplace(t *testing.T) { cases := []struct { input string @@ -166,3 +214,64 @@ func TestMultiWordReplace(t *testing.T) { }) } } + +func TestFindConfigFile(t *testing.T) { + cases := []struct { + input []string + statResults []error + exp string + }{ + {input: []string{"myfile"}, statResults: []error{nil}, exp: "myfile"}, + {input: []string{"thisfile", "thatfile"}, statResults: []error{os.ErrNotExist, nil}, exp: "thatfile"}, + {input: []string{"thisfile", "thatfile"}, statResults: []error{os.ErrNotExist, os.ErrNotExist}, exp: ""}, + } + + statFunc = fakestat + for id, c := range cases { + t.Run(strconv.Itoa(id), func(t *testing.T) { + e = c.statResults + eIndex = 0 + conf := findConfigFile(c.input) + if conf != c.exp { + t.Fatalf("Got %s expected %s", conf, c.exp) + } + }) + } +} + +func TestGetConfigFiles(t *testing.T) { + cases := []struct { + config map[string]interface{} + exp map[string]string + statResults []error + }{ + { + config: map[string]interface{}{"apiserver": []string{"apiserver", "kube-apiserver"}}, + statResults: []error{os.ErrNotExist, nil}, + exp: map[string]string{"apiserver": "kube-apiserver"}, + }, + { + config: map[string]interface{}{"apiserver": []string{"apiserver", "kube-apiserver"}, "thing": []string{"/my/file/thing"}}, + statResults: []error{os.ErrNotExist, nil, nil}, + exp: map[string]string{"apiserver": "kube-apiserver", "thing": "/my/file/thing"}, + }, + } + + v := viper.New() + statFunc = fakestat + + for id, c := range cases { + t.Run(strconv.Itoa(id), func(t *testing.T) { + for k, val := range c.config { + v.Set(k, val) + } + e = c.statResults + eIndex = 0 + + m := getConfigFiles(v) + if !reflect.DeepEqual(m, c.exp) { + t.Fatalf("Got %v\nExpected %v", m, c.exp) + } + }) + } +} From e4e41683c4b751eceb20df4cdde180f9520a2816 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Wed, 30 Aug 2017 18:36:00 +0100 Subject: [PATCH 05/19] Update the config file --- cfg/config.yaml | 164 +++++++++++++++++++----------------------------- 1 file changed, 64 insertions(+), 100 deletions(-) diff --git a/cfg/config.yaml b/cfg/config.yaml index 5c21e37..766ece3 100644 --- a/cfg/config.yaml +++ b/cfg/config.yaml @@ -7,106 +7,70 @@ # nodeControls: ./cfg/node.yaml # federatedControls: ./cfg/federated.yaml -## Support components -etcd: - bin: etcd - conf: /etc/etcd/etcd.conf +master: + bins: + apiserver: + - "kube-apiserver" + - "hyperkube apiserver" + - "apiserver" + scheduler: + - "kube-scheduler" + - "hyperkube scheduler" + - "scheduler" + controllermanager: + - "kube-controller-manager" + - "hyperkube controller-manager" + - "controller-manager" + confs: + apiserver: + - /etc/kubernetes/admin.conf + - /etc/kubernetes/apiserver + - /etc/kubernetes/manifests/kube-apiserver.yaml + scheduler: + - /etc/kubernetes/scheduler.conf + - /etc/kubernetes/scheduler + - /etc/kubernetes/manifests/kube-scheduler.yaml + controller-manager: + - /etc/kubernetes/controller-manager.conf + - /etc/kubernetes/controller-manager + - /etc/kubernetes/manifests/kube-controller-manager.yaml + etcd: + - /etc/etcd/etcd.conf + flanneld: + - /etc/sysconfig/flanneld -flanneld: - bin: flanneld - conf: /etc/sysconfig/flanneld +node: + bins: + kubelet: + - "hyperkube kubelet" + - "kubelet" + proxy: + - "kube-proxy" + - "hyperkube proxy" + - "proxy" + confs: + kubelet: + - /etc/kubernetes/kubelet.conf + - /etc/kubernetes/kubelet + proxy: + - /etc/kubernetes/proxy.conf + - /etc/kubernetes/proxy + - /etc/kubernetes/addons/kube-proxy-daemonset.yaml -# Installation -# Configure kubernetes component binaries and paths to their configuration files. -installation: - default: - config: /etc/kubernetes/config - master: - bin: - apiserver: apiserver - scheduler: scheduler - controller-manager: controller-manager - conf: - apiserver: /etc/kubernetes/apiserver - scheduler: /etc/kubernetes/scheduler - controller-manager: /etc/kubernetes/controller-manager - node: - bin: - kubelet: kubelet - proxy: proxy - conf: - kubelet: /etc/kubernetes/kubelet - proxy: /etc/kubernetes/proxy - federated: - bin: - apiserver: federation-apiserver - controller-manager: federation-controller-manager +federated: + bins: + fedapiserver: + - "hyperkube federation-apiserver" + - "kube-federation-apiserver" + - "federation-apiserver" + fedcontrollermanager: + - "hyperkube federation-controller-manager" + - "kube-federation-controller-manager" + - "federation-controller-manager" - kops: - config: /etc/kubernetes/config - master: - bin: - apiserver: apiserver - scheduler: scheduler - controller-manager: controller-manager - conf: - apiserver: /etc/kubernetes/apiserver - scheduler: /etc/kubernetes/scheduler - controller-manager: /etc/kubernetes/apiserver - node: - bin: - kubelet: kubelet - proxy: proxy - conf: - kubelet: /etc/kubernetes/kubelet - proxy: /etc/kubernetes/proxy - federated: - bin: - apiserver: federation-apiserver - controller-manager: federation-controller-manager - - hyperkube: - config: /etc/kubernetes/config - master: - bin: - apiserver: hyperkube apiserver - scheduler: hyperkube scheduler - controller-manager: hyperkube controller-manager - conf: - apiserver: /etc/kubernetes/manifests/kube-apiserver.yaml - scheduler: /etc/kubernetes/manifests/kube-scheduler.yaml - controller-manager: /etc/kubernetes/manifests/kube-controller-manager.yaml - node: - bin: - kubelet: hyperkube kubelet - proxy: hyperkube proxy - conf: - kubelet: /etc/kubernetes/kubelet - proxy: /etc/kubernetes/addons/kube-proxy-daemonset.yaml - federated: - bin: - apiserver: hyperkube federation-apiserver - controller-manager: hyperkube federation-controller-manager - - kubeadm: - config: /etc/kubernetes/config - master: - bin: - apiserver: kube-apiserver - scheduler: kube-scheduler - controller-manager: kube-controller-manager - conf: - apiserver: /etc/kubernetes/admin.conf - scheduler: /etc/kubernetes/scheduler.conf - controller-manager: /etc/kubernetes/controller-manager.conf - node: - bin: - kubelet: kubelet - proxy: kube-proxy - conf: - kubelet: /etc/kubernetes/kubelet.conf - proxy: /etc/kubernetes/proxy.conf - federated: - bin: - apiserver: kube-federation-apiserver - controller-manager: kube-federation-controller-manager +optional: + bins: + etcd: + - "etcd" + flanneld: + - "flanneld" From 6a5a62b27876ea7ed9c4d8098a7a09a2fee894e9 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Wed, 30 Aug 2017 18:37:01 +0100 Subject: [PATCH 06/19] Autodetect the binaries and config files from a set of options --- cmd/common.go | 63 ++++++++++++++++-------------------------------- cmd/util.go | 18 +++++++++++--- cmd/util_test.go | 22 ++++++++++++++++- 3 files changed, 57 insertions(+), 46 deletions(-) diff --git a/cmd/common.go b/cmd/common.go index b3df7b9..51ffda2 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -52,30 +52,25 @@ var ( func runChecks(t check.NodeType) { var summary check.Summary var file string + var err error + var typeConf *viper.Viper - // Master variables - apiserverBin = viper.GetString("installation." + installation + ".master.bin.apiserver") - apiserverConf = viper.GetString("installation." + installation + ".master.conf.apiserver") - 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.controller-manager") - config = viper.GetString("installation." + installation + ".config") + switch t { + case check.MASTER: + file = masterFile + typeConf = viper.Sub("master") + case check.NODE: + file = nodeFile + typeConf = viper.Sub("node") + case check.FEDERATED: + file = federatedFile + typeConf = viper.Sub("federated") + } - etcdBin = viper.GetString("etcd.bin") - etcdConf = viper.GetString("etcd.conf") - flanneldBin = viper.GetString("flanneld.bin") - flanneldConf = viper.GetString("flanneld.conf") - - // Node variables - kubeletBin = viper.GetString("installation." + installation + ".node.bin.kubelet") - kubeletConf = viper.GetString("installation." + installation + ".node.conf.kubelet") - proxyBin = viper.GetString("installation." + installation + ".node.bin.proxy") - proxyConf = viper.GetString("installation." + installation + ".node.conf.proxy") - - // Federated - fedApiserverBin = viper.GetString("installation." + installation + ".federated.bin.apiserver") - fedControllerManagerBin = viper.GetString("installation." + installation + ".federated.bin.controller-manager") + // Get the set of exectuables we care about on this type of node + binmap := getBinaries(typeConf.Sub("bins"), false) + extrasmap := getBinaries(viper.Sub("optional"), true) + confmap := getConfigFiles(typeConf.Sub("confs")) // Run kubernetes installation validation checks. verifyKubeVersion(kubeMajorVersion, kubeMinorVersion) @@ -96,26 +91,10 @@ func runChecks(t check.NodeType) { } // Variable substitutions. Replace all occurrences of variables in controls files. - s := multiWordReplace(string(in), "$apiserverbin", apiserverBin) - s = multiWordReplace(s, "$apiserverconf", apiserverConf) - s = multiWordReplace(s, "$schedulerbin", schedulerBin) - s = multiWordReplace(s, "$schedulerconf", schedulerConf) - s = multiWordReplace(s, "$controllermanagerbin", controllerManagerBin) - s = multiWordReplace(s, "$controllermanagerconf", controllerManagerConf) - s = multiWordReplace(s, "$config", config) - - s = multiWordReplace(s, "$etcdbin", etcdBin) - s = multiWordReplace(s, "$etcdconf", etcdConf) - s = multiWordReplace(s, "$flanneldbin", flanneldBin) - s = multiWordReplace(s, "$flanneldconf", flanneldConf) - - s = multiWordReplace(s, "$kubeletbin", kubeletBin) - s = multiWordReplace(s, "$kubeletconf", kubeletConf) - s = multiWordReplace(s, "$proxybin", proxyBin) - s = multiWordReplace(s, "$proxyconf", proxyConf) - - s = multiWordReplace(s, "$fedapiserverbin", fedApiserverBin) - s = multiWordReplace(s, "$fedcontrollermanagerbin", fedControllerManagerBin) + s := string(in) + s = makeSubstitutions(s, "bin", binmap) + s = makeSubstitutions(s, "bin", extrasmap) + s = makeSubstitutions(s, "conf", confmap) controls, err := check.NewControls(t, []byte(s)) if err != nil { diff --git a/cmd/util.go b/cmd/util.go index 0bc40be..686fb52 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -52,7 +52,7 @@ func exitWithError(err error) { func continueWithError(err error, msg string) string { if err != nil { - glog.V(1).Info(err) + glog.V(2).Info(err) } if msg != "" { @@ -85,12 +85,12 @@ func ps(proc string) string { } // getBinaries finds which of the set of candidate executables are running -func getBinaries(v *viper.Viper) map[string]string { +func getBinaries(v *viper.Viper, optional bool) map[string]string { binmap := make(map[string]string) for _, exeType := range v.AllKeys() { bin, err := findExecutable(v.GetStringSlice(exeType)) - if err != nil { + if err != nil && !optional { exitWithError(fmt.Errorf("looking for %s executable but none of the candidates are running", exeType)) } @@ -162,6 +162,8 @@ func findExecutable(candidates []string) (string, error) { for _, c := range candidates { if verifyBin(c) { return c, nil + } else { + glog.V(1).Info(fmt.Sprintf("executable '%s' not running", c)) } } @@ -237,3 +239,13 @@ func multiWordReplace(s string, subname string, sub string) string { return strings.Replace(s, subname, sub, -1) } + +func makeSubstitutions(s string, ext string, m map[string]string) string { + for k, v := range m { + subst := "$" + k + ext + glog.V(1).Info(fmt.Sprintf("Substituting %s with '%s'\n", subst, v)) + s = multiWordReplace(s, subst, v) + } + + return s +} diff --git a/cmd/util_test.go b/cmd/util_test.go index d5192a6..35b08d5 100644 --- a/cmd/util_test.go +++ b/cmd/util_test.go @@ -185,7 +185,7 @@ func TestGetBinaries(t *testing.T) { for k, val := range c.config { v.Set(k, val) } - m := getBinaries(v) + m := getBinaries(v, false) if !reflect.DeepEqual(m, c.exp) { t.Fatalf("Got %v\nExpected %v", m, c.exp) } @@ -275,3 +275,23 @@ func TestGetConfigFiles(t *testing.T) { }) } } + +func TestMakeSubsitutions(t *testing.T) { + cases := []struct { + input string + subst map[string]string + exp string + }{ + {input: "Replace $thisbin", subst: map[string]string{"this": "that"}, exp: "Replace that"}, + {input: "Replace $thisbin", subst: map[string]string{"this": "that", "here": "there"}, exp: "Replace that"}, + {input: "Replace $thisbin and $herebin", subst: map[string]string{"this": "that", "here": "there"}, exp: "Replace that and there"}, + } + for _, c := range cases { + t.Run(c.input, func(t *testing.T) { + s := makeSubstitutions(c.input, "bin", c.subst) + if s != c.exp { + t.Fatalf("Got %s expected %s", s, c.exp) + } + }) + } +} From 0e9c11ebd5d5de342e010a1d26e33872daa149ee Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Thu, 31 Aug 2017 14:41:52 +0100 Subject: [PATCH 07/19] Remove empty error messages that manifested as "%s" --- check/check.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/check/check.go b/check/check.go index 4f91340..16c3984 100644 --- a/check/check.go +++ b/check/check.go @@ -156,7 +156,9 @@ func (c *Check) Run() { i++ } - glog.V(2).Info("%s\n", errmsgs) + if errmsgs != "" { + glog.V(2).Info(errmsgs) + } res := c.Tests.execute(out.String()) if res { From f5550fd8bdf162abaaa1baa2f0f5b7f7258f3137 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Thu, 31 Aug 2017 14:43:35 +0100 Subject: [PATCH 08/19] Node type is now verified by looking for running binaries from a set of options --- cmd/common.go | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/cmd/common.go b/cmd/common.go index 51ffda2..33c0a03 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -74,7 +74,6 @@ func runChecks(t check.NodeType) { // Run kubernetes installation validation checks. verifyKubeVersion(kubeMajorVersion, kubeMinorVersion) - verifyNodeType(t) switch t { case check.MASTER: @@ -126,41 +125,6 @@ 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: - bins = []string{apiserverBin, schedulerBin, controllerManagerBin} - confs = []string{apiserverConf, schedulerConf, controllerManagerConf} - case check.NODE: - bins = []string{kubeletBin, proxyBin} - confs = []string{kubeletConf, proxyConf} - case check.FEDERATED: - bins = []string{fedApiserverBin, fedControllerManagerBin} - } - - for _, bin := range bins { - if !verifyBin(bin) { - 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)) - } - } - } -} - // colorPrint outputs the state in a specific colour, along with a message string func colorPrint(state check.State, s string) { colors[state].Printf("[%s] ", state) From e4b905e360901b974725660294a16b384bfa60d9 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Thu, 31 Aug 2017 14:43:59 +0100 Subject: [PATCH 09/19] =?UTF-8?q?Log=20when=20there=E2=80=99s=20no=20subst?= =?UTF-8?q?itution?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/util.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/util.go b/cmd/util.go index 686fb52..a89a1f0 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -243,6 +243,10 @@ func multiWordReplace(s string, subname string, sub string) string { func makeSubstitutions(s string, ext string, m map[string]string) string { for k, v := range m { subst := "$" + k + ext + if v == "" { + glog.V(2).Info(fmt.Sprintf("No subsitution for '%s'\n", subst)) + continue + } glog.V(1).Info(fmt.Sprintf("Substituting %s with '%s'\n", subst, v)) s = multiWordReplace(s, subst, v) } From a3197f8efe07f0f212f24f6395704d244cda4062 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Thu, 31 Aug 2017 14:45:16 +0100 Subject: [PATCH 10/19] =?UTF-8?q?Reorder=20YAML=20to=20make=20a=20bit=20mo?= =?UTF-8?q?re=20sense.=20Allow=20for=20optional=20components,=20and=20a=20?= =?UTF-8?q?config=20file=20that=20we=20don=E2=80=99t=20think=20exists.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cfg/config.yaml | 107 ++++++++++++++++++++++++++++++++---------------- cmd/common.go | 10 ++--- cmd/util.go | 56 +++++++++++++++++++------ 3 files changed, 120 insertions(+), 53 deletions(-) diff --git a/cfg/config.yaml b/cfg/config.yaml index 766ece3..6836fd5 100644 --- a/cfg/config.yaml +++ b/cfg/config.yaml @@ -8,69 +8,106 @@ # federatedControls: ./cfg/federated.yaml master: - bins: - apiserver: + components: + - apiserver + - scheduler + - controllermanager + - etcd + - flanneld + # kubernetes is a component to cover the config file /etc/kubernetes/config that is referred to in the + # benchmark but is believed to now be obselete + - kubernetes + + kubernetes: + defaultconf: /etc/kubernetes/config + + apiserver: + bins: - "kube-apiserver" - "hyperkube apiserver" - "apiserver" - scheduler: + confs: + - /etc/kubernetes/manifests/kube-apiserver.yaml + - /etc/kubernetes/apiserver.conf + - /etc/kubernetes/apiserver + defaultconf: /etc/kubernetes/apiserver + + scheduler: + bins: - "kube-scheduler" - "hyperkube scheduler" - "scheduler" - controllermanager: + confs: + - /etc/kubernetes/manifests/kube-scheduler.yaml + - /etc/kubernetes/scheduler.conf + - /etc/kubernetes/scheduler + defaultconf: /etc/kubernetes/scheduler + + controllermanager: + bins: - "kube-controller-manager" - "hyperkube controller-manager" - "controller-manager" - confs: - apiserver: - - /etc/kubernetes/admin.conf - - /etc/kubernetes/apiserver - - /etc/kubernetes/manifests/kube-apiserver.yaml - scheduler: - - /etc/kubernetes/scheduler.conf - - /etc/kubernetes/scheduler - - /etc/kubernetes/manifests/kube-scheduler.yaml - controller-manager: + confs: + - /etc/kubernetes/manifests/kube-controller-manager.yaml - /etc/kubernetes/controller-manager.conf - /etc/kubernetes/controller-manager - - /etc/kubernetes/manifests/kube-controller-manager.yaml - etcd: + defaultconf: /etc/kubernetes/controller-manager + + etcd: + optional: true + bins: + - "etcd" + confs: + - /etc/kubernetes/manifests/etcd.yaml - /etc/etcd/etcd.conf - flanneld: - - /etc/sysconfig/flanneld + defaultconf: /etc/etcd/etcd.conf + + flanneld: + optional: true + bins: + - flanneld + defaultconf: /etc/sysconfig/flanneld + node: - bins: - kubelet: + components: + - kubelet + - proxy + + kubelet: + bins: - "hyperkube kubelet" - "kubelet" - proxy: + confs: + - /etc/kubernetes/kubelet.conf + - /etc/kubernetes/kubelet + + proxy: + bins: - "kube-proxy" - "hyperkube proxy" - "proxy" - confs: - kubelet: - - /etc/kubernetes/kubelet.conf - - /etc/kubernetes/kubelet - proxy: + confs: - /etc/kubernetes/proxy.conf - /etc/kubernetes/proxy - /etc/kubernetes/addons/kube-proxy-daemonset.yaml federated: - bins: - fedapiserver: + components: + - fedapiserver + - fedcontrollermanager + + fedapiserver: + bins: - "hyperkube federation-apiserver" - "kube-federation-apiserver" - "federation-apiserver" - fedcontrollermanager: + + fedcontrollermanager: + bins: - "hyperkube federation-controller-manager" - "kube-federation-controller-manager" - "federation-controller-manager" -optional: - bins: - etcd: - - "etcd" - flanneld: - - "flanneld" + diff --git a/cmd/common.go b/cmd/common.go index 33c0a03..14f2998 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -17,7 +17,6 @@ package cmd import ( "fmt" "io/ioutil" - "os" "github.com/aquasecurity/kube-bench/check" "github.com/spf13/viper" @@ -67,10 +66,10 @@ func runChecks(t check.NodeType) { typeConf = viper.Sub("federated") } - // Get the set of exectuables we care about on this type of node - binmap := getBinaries(typeConf.Sub("bins"), false) - extrasmap := getBinaries(viper.Sub("optional"), true) - confmap := getConfigFiles(typeConf.Sub("confs")) + // Get the set of exectuables and config files we care about on this type of node. This also + // checks that the executables we need for the node type are running. + binmap := getBinaries(typeConf) + confmap := getConfigFiles(typeConf) // Run kubernetes installation validation checks. verifyKubeVersion(kubeMajorVersion, kubeMinorVersion) @@ -92,7 +91,6 @@ func runChecks(t check.NodeType) { // Variable substitutions. Replace all occurrences of variables in controls files. s := string(in) s = makeSubstitutions(s, "bin", binmap) - s = makeSubstitutions(s, "bin", extrasmap) s = makeSubstitutions(s, "conf", confmap) controls, err := check.NewControls(t, []byte(s)) diff --git a/cmd/util.go b/cmd/util.go index a89a1f0..52010f4 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -85,17 +85,34 @@ func ps(proc string) string { } // getBinaries finds which of the set of candidate executables are running -func getBinaries(v *viper.Viper, optional bool) map[string]string { +func getBinaries(v *viper.Viper) map[string]string { binmap := make(map[string]string) - for _, exeType := range v.AllKeys() { - bin, err := findExecutable(v.GetStringSlice(exeType)) - if err != nil && !optional { - exitWithError(fmt.Errorf("looking for %s executable but none of the candidates are running", exeType)) + for _, component := range v.GetStringSlice("components") { + s := v.Sub(component) + if s == nil { + continue } - binmap[exeType] = bin + optional := s.GetBool("optional") + bins := s.GetStringSlice("bins") + if len(bins) > 0 { + bin, err := findExecutable(bins) + if err != nil && !optional { + exitWithError(fmt.Errorf("need %s executable but none of the candidates are running", component)) + } + + // Default the executable name that we'll substitute to the name of the component + if bin == "" { + bin = component + glog.V(2).Info(fmt.Sprintf("Component %s not running", component)) + } else { + glog.V(2).Info(fmt.Sprintf("Component %s uses running binary %s", component, bin)) + } + binmap[component] = bin + } } + return binmap } @@ -103,13 +120,28 @@ func getBinaries(v *viper.Viper, optional bool) map[string]string { func getConfigFiles(v *viper.Viper) map[string]string { confmap := make(map[string]string) - for _, confType := range v.AllKeys() { - conf := findConfigFile(v.GetStringSlice(confType)) - if conf == "" { - printlnWarn(fmt.Sprintf("Missing kubernetes config file for %s", confType)) - } else { - confmap[confType] = conf + for _, component := range v.GetStringSlice("components") { + s := v.Sub(component) + if s == nil { + continue } + + // See if any of the candidate config files exist + conf := findConfigFile(s.GetStringSlice("confs")) + if conf == "" { + if s.IsSet("defaultconf") { + conf = s.GetString("defaultconf") + glog.V(2).Info(fmt.Sprintf("Using default config file name '%s' for component %s", conf, component)) + } else { + // Default the config file name that we'll substitute to the name of the component + printlnWarn(fmt.Sprintf("Missing config file for %s", component)) + conf = component + } + } else { + glog.V(2).Info(fmt.Sprintf("Component %s uses config file '%s'", component, conf)) + } + + confmap[component] = conf } return confmap From d637d8714adc7f9d68087fa148b0ec9ff140b161 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Thu, 31 Aug 2017 15:22:30 +0100 Subject: [PATCH 11/19] Fix and add tests --- cmd/util_test.go | 60 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 5 deletions(-) diff --git a/cmd/util_test.go b/cmd/util_test.go index 35b08d5..c95e6e4 100644 --- a/cmd/util_test.go +++ b/cmd/util_test.go @@ -165,13 +165,32 @@ func TestGetBinaries(t *testing.T) { exp map[string]string }{ { - config: map[string]interface{}{"apiserver": []string{"apiserver", "kube-apiserver"}}, + config: map[string]interface{}{"components": []string{"apiserver"}, "apiserver": map[string]interface{}{"bins": []string{"apiserver", "kube-apiserver"}}}, psOut: "kube-apiserver", exp: map[string]string{"apiserver": "kube-apiserver"}, }, { - config: map[string]interface{}{"apiserver": []string{"apiserver", "kube-apiserver"}, "thing": []string{"something else", "thing"}}, + // "thing" is not in the list of components + config: map[string]interface{}{"components": []string{"apiserver"}, "apiserver": map[string]interface{}{"bins": []string{"apiserver", "kube-apiserver"}}, "thing": map[string]interface{}{"bins": []string{"something else", "thing"}}}, psOut: "kube-apiserver thing", + exp: map[string]string{"apiserver": "kube-apiserver"}, + }, + { + // "anotherthing" in list of components but doesn't have a defintion + config: map[string]interface{}{"components": []string{"apiserver", "anotherthing"}, "apiserver": map[string]interface{}{"bins": []string{"apiserver", "kube-apiserver"}}, "thing": map[string]interface{}{"bins": []string{"something else", "thing"}}}, + psOut: "kube-apiserver thing", + exp: map[string]string{"apiserver": "kube-apiserver"}, + }, + { + // more than one component + config: map[string]interface{}{"components": []string{"apiserver", "thing"}, "apiserver": map[string]interface{}{"bins": []string{"apiserver", "kube-apiserver"}}, "thing": map[string]interface{}{"bins": []string{"something else", "thing"}}}, + psOut: "kube-apiserver thing", + exp: map[string]string{"apiserver": "kube-apiserver", "thing": "thing"}, + }, + { + // default binary to component name + config: map[string]interface{}{"components": []string{"apiserver", "thing"}, "apiserver": map[string]interface{}{"bins": []string{"apiserver", "kube-apiserver"}}, "thing": map[string]interface{}{"bins": []string{"something else", "thing"}, "optional": true}}, + psOut: "kube-apiserver otherthing", exp: map[string]string{"apiserver": "kube-apiserver", "thing": "thing"}, }, } @@ -185,7 +204,7 @@ func TestGetBinaries(t *testing.T) { for k, val := range c.config { v.Set(k, val) } - m := getBinaries(v, false) + m := getBinaries(v) if !reflect.DeepEqual(m, c.exp) { t.Fatalf("Got %v\nExpected %v", m, c.exp) } @@ -246,15 +265,46 @@ func TestGetConfigFiles(t *testing.T) { statResults []error }{ { - config: map[string]interface{}{"apiserver": []string{"apiserver", "kube-apiserver"}}, + config: map[string]interface{}{"components": []string{"apiserver"}, "apiserver": map[string]interface{}{"confs": []string{"apiserver", "kube-apiserver"}}}, statResults: []error{os.ErrNotExist, nil}, exp: map[string]string{"apiserver": "kube-apiserver"}, }, { - config: map[string]interface{}{"apiserver": []string{"apiserver", "kube-apiserver"}, "thing": []string{"/my/file/thing"}}, + // Component "thing" isn't included in the list of components + config: map[string]interface{}{ + "components": []string{"apiserver"}, + "apiserver": map[string]interface{}{"confs": []string{"apiserver", "kube-apiserver"}}, + "thing": map[string]interface{}{"confs": []string{"/my/file/thing"}}}, + statResults: []error{os.ErrNotExist, nil}, + exp: map[string]string{"apiserver": "kube-apiserver"}, + }, + { + // More than one component + config: map[string]interface{}{ + "components": []string{"apiserver", "thing"}, + "apiserver": map[string]interface{}{"confs": []string{"apiserver", "kube-apiserver"}}, + "thing": map[string]interface{}{"confs": []string{"/my/file/thing"}}}, statResults: []error{os.ErrNotExist, nil, nil}, exp: map[string]string{"apiserver": "kube-apiserver", "thing": "/my/file/thing"}, }, + { + // Default thing to specified default config + config: map[string]interface{}{ + "components": []string{"apiserver", "thing"}, + "apiserver": map[string]interface{}{"confs": []string{"apiserver", "kube-apiserver"}}, + "thing": map[string]interface{}{"confs": []string{"/my/file/thing"}, "defaultconf": "another/thing"}}, + statResults: []error{os.ErrNotExist, nil, os.ErrNotExist}, + exp: map[string]string{"apiserver": "kube-apiserver", "thing": "another/thing"}, + }, + { + // Default thing to component name + config: map[string]interface{}{ + "components": []string{"apiserver", "thing"}, + "apiserver": map[string]interface{}{"confs": []string{"apiserver", "kube-apiserver"}}, + "thing": map[string]interface{}{"confs": []string{"/my/file/thing"}}}, + statResults: []error{os.ErrNotExist, nil, os.ErrNotExist}, + exp: map[string]string{"apiserver": "kube-apiserver", "thing": "thing"}, + }, } v := viper.New() From 8380ad1ef3b9b3997fdcbac2bb2018806152cec3 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Thu, 31 Aug 2017 16:01:31 +0100 Subject: [PATCH 12/19] Better detection of running executables --- cmd/util.go | 16 ++++++++-------- cmd/util_test.go | 5 +++++ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/cmd/util.go b/cmd/util.go index 52010f4..4d96c51 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -159,14 +159,14 @@ func verifyBin(bin string) bool { proc := strings.Fields(bin)[0] out := psFunc(proc) - if !strings.Contains(out, bin) { - return false - } - - // Make sure we're not just matching on a partial word (e.g. if we're looking for apiserver, don't match on kube-apiserver) - // This will give a false positive for matching "one two" against "zero one two-x" but it will do for now - for _, f := range strings.Fields(out) { - if f == proc { + // There could be multiple lines in the ps output + // The binary needs to be the first word in the ps output, except that it could be preceded by a path + // e.g. /usr/bin/kubelet is a match for kubelet + // but apiserver is not a match for kube-apiserver + reFirstWord := regexp.MustCompile(`^(\S*\/)*` + bin) + lines := strings.Split(out, "\n") + for _, l := range lines { + if reFirstWord.Match([]byte(l)) { return true } } diff --git a/cmd/util_test.go b/cmd/util_test.go index c95e6e4..eb1b90f 100644 --- a/cmd/util_test.go +++ b/cmd/util_test.go @@ -108,6 +108,11 @@ func TestVerifyBin(t *testing.T) { {proc: "cmd", psOut: "cmd param1 param2", exp: true}, {proc: "cmd param", psOut: "cmd param1 param2", exp: true}, {proc: "cmd param", psOut: "cmd", exp: false}, + {proc: "cmd", psOut: "cmd x \ncmd y", exp: true}, + {proc: "cmd y", psOut: "cmd x \ncmd y", exp: true}, + {proc: "cmd", psOut: "/usr/bin/cmd", exp: true}, + {proc: "cmd", psOut: "kube-cmd", exp: false}, + {proc: "cmd", psOut: "/usr/bin/kube-cmd", exp: false}, } psFunc = fakeps From e4a89123e02b9d58591116624f42812dd25fb542 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Thu, 31 Aug 2017 17:38:11 +0100 Subject: [PATCH 13/19] =?UTF-8?q?Move=20message=20about=20which=20config?= =?UTF-8?q?=20file=20we=E2=80=99re=20using=20into=20a=20log=20at=20the=20s?= =?UTF-8?q?tart?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/common.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/common.go b/cmd/common.go index 14f2998..3708d1f 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -19,6 +19,7 @@ import ( "io/ioutil" "github.com/aquasecurity/kube-bench/check" + "github.com/golang/glog" "github.com/spf13/viper" ) @@ -54,6 +55,8 @@ func runChecks(t check.NodeType) { var err error var typeConf *viper.Viper + glog.V(1).Info(fmt.Sprintf("Using config file: %s\n", viper.ConfigFileUsed())) + switch t { case check.MASTER: file = masterFile @@ -131,8 +134,6 @@ func colorPrint(state check.State, s string) { // prettyPrint outputs the results to stdout in human-readable format func prettyPrint(r *check.Controls, summary check.Summary) { - colorPrint(check.INFO, fmt.Sprintf("Using config file: %s\n", viper.ConfigFileUsed())) - 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)) From a6036bcfcf9652dfb850754d32a583686420e492 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Thu, 31 Aug 2017 17:39:40 +0100 Subject: [PATCH 14/19] =?UTF-8?q?Corrections=20to=20config=20file=20substi?= =?UTF-8?q?tutions.=20Use=20=E2=80=9Ckubernetes=E2=80=9D=20as=20a=20fake?= =?UTF-8?q?=20component=20name=20so=20we=20can=20more=20easily=20substitut?= =?UTF-8?q?e=20=E2=80=9Ckubernetesconf=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cfg/config.yaml | 11 ++++++++--- cfg/master.yaml | 8 ++++---- cfg/node.yaml | 14 +++++++------- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/cfg/config.yaml b/cfg/config.yaml index 6836fd5..8ad2e89 100644 --- a/cfg/config.yaml +++ b/cfg/config.yaml @@ -14,8 +14,7 @@ master: - controllermanager - etcd - flanneld - # kubernetes is a component to cover the config file /etc/kubernetes/config that is referred to in the - # benchmark but is believed to now be obselete + # kubernetes is a component to cover the config file /etc/kubernetes/config that is referred to in the benchmark - kubernetes kubernetes: @@ -74,6 +73,11 @@ node: components: - kubelet - proxy + # kubernetes is a component to cover the config file /etc/kubernetes/config that is referred to in the benchmark + - kubernetes + + kubernetes: + defaultconf: /etc/kubernetes/config kubelet: bins: @@ -81,7 +85,8 @@ node: - "kubelet" confs: - /etc/kubernetes/kubelet.conf - - /etc/kubernetes/kubelet + - /etc/kubernetes/kubelet + defaultconf: "/etc/kubernetes/kubelet.conf" proxy: bins: diff --git a/cfg/master.yaml b/cfg/master.yaml index f54bf6a..dc9295e 100644 --- a/cfg/master.yaml +++ b/cfg/master.yaml @@ -636,7 +636,7 @@ groups: - id: 1.4.3 text: "Ensure that the config file permissions are set to 644 or more restrictive (Scored)" - audit: "/bin/sh -c 'if test -e $config; then stat -c %a $config; fi'" + audit: "/bin/sh -c 'if test -e $kubernetesconf; then stat -c %a $kubernetesconf; fi'" tests: bin_op: or test_items: @@ -656,12 +656,12 @@ groups: value: "600" set: true remediation: "Run the below command (based on the file location on your system) on the master node. - \nFor example, chmod 644 $config" + \nFor example, chmod 644 $kubernetesconf" scored: true - id: 1.4.4 text: "Ensure that the config file ownership is set to root:root (Scored)" - audit: "/bin/sh -c 'if test -e $config; then stat -c %U:%G $config; fi'" + audit: "/bin/sh -c 'if test -e $kubernetesconf; then stat -c %U:%G $kubernetesconf; fi'" tests: test_items: - flag: "root:root" @@ -670,7 +670,7 @@ groups: 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" + \nFor example, chown root:root $kubernetesconf" scored: true - id: 1.4.5 diff --git a/cfg/node.yaml b/cfg/node.yaml index a146baa..0a1c0b6 100644 --- a/cfg/node.yaml +++ b/cfg/node.yaml @@ -17,7 +17,7 @@ groups: op: eq value: false set: true - remediation: "Edit the $config file on each node and set the KUBE_ALLOW_PRIV + remediation: "Edit the $kubeletconf file on each node and set the KUBE_ALLOW_PRIV parameter to \"--allow-privileged=false\"" scored: true @@ -199,7 +199,7 @@ groups: op: eq value: true set: true - remediation: "Edit the /etc/kubernetes/kubelet file on each node and set the KUBELET_ARGS parameter + remediation: "Edit the $kubeletconf file on each node and set the KUBELET_ARGS parameter to a value to include \"--feature-gates=RotateKubeletClientCertificate=true\"." scored: true @@ -213,7 +213,7 @@ groups: op: eq value: true set: true - remediation: "Edit the /etc/kubernetes/kubelet file on each node and set the KUBELET_ARGS parameter + remediation: "Edit the $kubeletconf file on each node and set the KUBELET_ARGS parameter to a value to include \"--feature-gates=RotateKubeletServerCertificate=true\"." scored: true @@ -222,7 +222,7 @@ groups: checks: - id: 2.2.1 text: "Ensure that the config file permissions are set to 644 or more restrictive (Scored)" - audit: "/bin/sh -c 'if test -e $config; then stat -c %a $config; fi'" + audit: "/bin/sh -c 'if test -e $kubernetesconf; then stat -c %a $kubernetesconf; fi'" tests: bin_op: or test_items: @@ -242,12 +242,12 @@ groups: value: "600" set: true remediation: "Run the below command (based on the file location on your system) on the each worker node. - \nFor example, chmod 644 $config" + \nFor example, chmod 644 $kubernetesconf" scored: true - id: 2.2.2 text: "Ensure that the config file ownership is set to root:root (Scored)" - audit: "/bin/sh -c 'if test -e $config; then stat -c %U:%G $config; fi'" + audit: "/bin/sh -c 'if test -e $kubernetesconf; then stat -c %U:%G $kubernetesconf; fi'" tests: test_items: - flag: "root:root" @@ -256,7 +256,7 @@ groups: 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" + \nFor example, chown root:root $kubernetesconf" scored: true - id: 2.2.3 From de128299237b4dd14c25b8991f5e2de6f4cfe085 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Thu, 31 Aug 2017 17:43:07 +0100 Subject: [PATCH 15/19] Correct test to cope with multi-line ps output --- cmd/util_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/util_test.go b/cmd/util_test.go index eb1b90f..1f3e5a2 100644 --- a/cmd/util_test.go +++ b/cmd/util_test.go @@ -189,13 +189,13 @@ func TestGetBinaries(t *testing.T) { { // more than one component config: map[string]interface{}{"components": []string{"apiserver", "thing"}, "apiserver": map[string]interface{}{"bins": []string{"apiserver", "kube-apiserver"}}, "thing": map[string]interface{}{"bins": []string{"something else", "thing"}}}, - psOut: "kube-apiserver thing", + psOut: "kube-apiserver \nthing", exp: map[string]string{"apiserver": "kube-apiserver", "thing": "thing"}, }, { // default binary to component name config: map[string]interface{}{"components": []string{"apiserver", "thing"}, "apiserver": map[string]interface{}{"bins": []string{"apiserver", "kube-apiserver"}}, "thing": map[string]interface{}{"bins": []string{"something else", "thing"}, "optional": true}}, - psOut: "kube-apiserver otherthing", + psOut: "kube-apiserver \notherthing some params", exp: map[string]string{"apiserver": "kube-apiserver", "thing": "thing"}, }, } From c4be4a124073e8b072fd9e8983a73acdf050c8f7 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Thu, 31 Aug 2017 17:52:21 +0100 Subject: [PATCH 16/19] Remove installation flag and some other unused variables --- cmd/root.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index aed8b42..601804e 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -34,14 +34,6 @@ var ( masterFile string nodeFile string federatedFile string - - loud bool - - kubeConfDir string - etcdConfDir string - flanneldConfDir string - - installation string ) // RootCmd represents the base command when called without any subcommands @@ -67,12 +59,6 @@ func init() { cobra.OnInitialize(initConfig) RootCmd.PersistentFlags().BoolVar(&jsonFmt, "json", false, "Prints the results as JSON") - RootCmd.PersistentFlags().StringVar( - &installation, - "installation", - "default", - "Specify how kubernetes cluster was installed. Possible values are default,hyperkube,kops,kubeadm", - ) RootCmd.PersistentFlags().StringVarP( &checkList, "check", From 9a500229a45b2857d965f3b1593237c6a21d3721 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Mon, 4 Sep 2017 10:11:34 +0100 Subject: [PATCH 17/19] Update README for auto-detection of executables and config files --- README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 12a3509..39c0209 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,10 @@ Flags: ## Configuration Kubernetes config and binary file locations and names can vary from installation to installation, so these are configurable in the `cfg/config.yaml` file. -They also tend to vary according to which tool was used to install Kubernetes. You can use the `--installation` flag to pick up a different default set of file names and locations. Again these defaults are configurable through `cfg/config.yaml` (and pull requests to correct or add default file locations are especially welcome). +For each type of node (*master*, *node* or *federated*) there is a list of components, and for each component there is a set of binaries (*bins*) and config files (*confs*) that kube-bench will look for (in the order they are listed). If your installation uses a different binary name or config file location for a Kubernetes component, you can add it to `cfg/config.yaml`. + +* **bins** - If there is a *bins* list for a component, at least one of these binaries must be running. The tests will consider the parameters for the first binary in the list found to be running. +* **confs** - If one of the listed config files is found, this will be considered for the test. Tests can continue even if no config file is found. If no file is found at any of the listed locations, and a *defaultconf* location is given for the component, the test will give remediation advice using the *defaultconf* location. ## Test config YAML representation The tests are represented as YAML documents (installed by default into ./cfg). From 44994ced33a47841eee0f7b00c1c9b3cbfd54887 Mon Sep 17 00:00:00 2001 From: Juned Memon Date: Wed, 13 Sep 2017 04:31:43 +0530 Subject: [PATCH 18/19] Fixed issue of The controls for master - admission control showing wrong status #49 --- check/test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/check/test.go b/check/test.go index 06c1b93..eaf0a12 100644 --- a/check/test.go +++ b/check/test.go @@ -62,7 +62,8 @@ func (t *testItem) execute(s string) (result bool) { // --flag=somevalue // --flag // somevalue - pttn := `(` + t.Flag + `)(=)*([^\s,]*) *` + //pttn := `(` + t.Flag + `)(=)*([^\s,]*) *` + pttn := `(` + t.Flag + `)(=)*([^\s]*) *` flagRe := regexp.MustCompile(pttn) vals := flagRe.FindStringSubmatch(s) From e8579ade6c6228052d4818d4ec57adf79eae3f7d Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Wed, 13 Sep 2017 15:32:33 +0100 Subject: [PATCH 19/19] Add tests for #50 --- check/data | 42 ++++++++++++++++++++++++++++++++++++++++++ check/test_test.go | 16 ++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/check/data b/check/data index 1e88841..435940e 100644 --- a/check/data +++ b/check/data @@ -116,3 +116,45 @@ groups: op: eq value: "600" set: true + + - id: 10 + text: "flag value includes some value in a comma-separated list, value is last in list" + tests: + test_items: + - flag: "--admission-control" + compare: + op: has + value: RBAC + set: true + + - id: 11 + text: "flag value includes some value in a comma-separated list, value is first in list" + tests: + test_items: + - flag: "--admission-control" + compare: + op: has + value: WebHook + set: true + + - id: 12 + text: "flag value includes some value in a comma-separated list, value middle of list" + tests: + test_items: + - flag: "--admission-control" + compare: + op: has + value: Something + set: true + + - id: 13 + text: "flag value includes some value in a comma-separated list, value only one in list" + tests: + test_items: + - flag: "--admission-control" + compare: + op: has + value: Something + set: true + + diff --git a/check/test_test.go b/check/test_test.go index a0228c2..b2d9ac8 100644 --- a/check/test_test.go +++ b/check/test_test.go @@ -94,6 +94,22 @@ func TestTestExecute(t *testing.T) { controls.Groups[0].Checks[9], "600", }, + { + controls.Groups[0].Checks[10], + "2:45 ../kubernetes/kube-apiserver --option --admission-control=WebHook,RBAC ---audit-log-maxage=40", + }, + { + controls.Groups[0].Checks[11], + "2:45 ../kubernetes/kube-apiserver --option --admission-control=WebHook,RBAC ---audit-log-maxage=40", + }, + { + controls.Groups[0].Checks[12], + "2:45 ../kubernetes/kube-apiserver --option --admission-control=WebHook,Something,RBAC ---audit-log-maxage=40", + }, + { + controls.Groups[0].Checks[13], + "2:45 ../kubernetes/kube-apiserver --option --admission-control=Something ---audit-log-maxage=40", + }, } for _, c := range cases {