From 52ebfa5b5a739433aa1bf08cacaf712079896a7c Mon Sep 17 00:00:00 2001 From: Huang Huang Date: Wed, 24 Jun 2020 17:13:10 +0800 Subject: [PATCH] Fix invalid JSON output (#629) * Fix invalid JSON output Fixes #622 * Apply suggestions from code review Co-authored-by: Liz Rice * Add tests Co-authored-by: Liz Rice --- cmd/common.go | 95 +++++++++++++-------- cmd/common_test.go | 48 +++++++++++ cmd/master.go | 1 + cmd/node.go | 1 + cmd/root.go | 2 + cmd/run.go | 1 + cmd/testdata/controlsCollection.json | 114 ++++++++++++++++++++++++++ cmd/testdata/result.json | 114 ++++++++++++++++++++++++++ integration/testdata/cis-1.5/job.data | 30 +++---- 9 files changed, 358 insertions(+), 48 deletions(-) create mode 100644 cmd/testdata/controlsCollection.json create mode 100644 cmd/testdata/result.json diff --git a/cmd/common.go b/cmd/common.go index 159c2ec..b979bb7 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -16,10 +16,13 @@ package cmd import ( "bufio" + "encoding/json" "fmt" "io/ioutil" "os" "path/filepath" + "sort" + "strconv" "strings" "github.com/aquasecurity/kube-bench/check" @@ -63,8 +66,6 @@ func NewRunFilter(opts FilterOpts) (check.Predicate, error) { } func runChecks(nodetype check.NodeType, testYamlFile string) { - var summary check.Summary - // Verify config file was loaded into Viper during Cobra sub-command initialization. if configFileError != nil { colorPrint(check.FAIL, fmt.Sprintf("Failed to read config file: %v\n", configFileError)) @@ -117,36 +118,8 @@ func runChecks(nodetype check.NodeType, testYamlFile string) { exitWithError(fmt.Errorf("error setting up run filter: %v", err)) } - summary = controls.RunChecks(runner, filter) - - if (summary.Fail > 0 || summary.Warn > 0 || summary.Pass > 0 || summary.Info > 0) && junitFmt { - out, err := controls.JUnit() - if err != nil { - exitWithError(fmt.Errorf("failed to output in JUnit format: %v", err)) - } - - PrintOutput(string(out), outputFile) - // if we successfully ran some tests and it's json format, ignore the warnings - } else if (summary.Fail > 0 || summary.Warn > 0 || summary.Pass > 0 || summary.Info > 0) && jsonFmt { - out, err := controls.JSON() - if err != nil { - exitWithError(fmt.Errorf("failed to output in JSON format: %v", err)) - } - - PrintOutput(string(out), outputFile) - } else { - // if we want to store in PostgreSQL, convert to JSON and save it - if (summary.Fail > 0 || summary.Warn > 0 || summary.Pass > 0 || summary.Info > 0) && pgSQL { - out, err := controls.JSON() - if err != nil { - exitWithError(fmt.Errorf("failed to output in JSON format: %v", err)) - } - - savePgsql(string(out)) - } else { - prettyPrint(controls, summary) - } - } + controls.RunChecks(runner, filter) + controlsCollection = append(controlsCollection, controls) } // colorPrint outputs the state in a specific colour, along with a message string @@ -185,7 +158,7 @@ func prettyPrint(r *check.Controls, summary check.Summary) { } if c.State == check.WARN { // Print the error if test failed due to problem with the audit command - if c.Reason != "" && c.Type != "manual"{ + if c.Reason != "" && c.Type != "manual" { fmt.Printf("%s audit test did not run: %s\n", c.ID, c.Reason) } else { fmt.Printf("%s %s\n", c.ID, c.Remediation) @@ -359,6 +332,62 @@ func isThisNodeRunning(nodeType check.NodeType) bool { return true } +func writeOutput(controlsCollection []*check.Controls) { + sort.Slice(controlsCollection, func(i, j int) bool { + iid, _ := strconv.Atoi(controlsCollection[i].ID) + jid, _ := strconv.Atoi(controlsCollection[j].ID) + return iid < jid + }) + if junitFmt { + writeJunitOutput(controlsCollection) + return + } + if jsonFmt { + writeJsonOutput(controlsCollection) + return + } + if pgSQL { + writePgsqlOutput(controlsCollection) + return + } + writeStdoutOutput(controlsCollection) +} + +func writeJsonOutput(controlsCollection []*check.Controls) { + out, err := json.Marshal(controlsCollection) + if err != nil { + exitWithError(fmt.Errorf("failed to output in JSON format: %v", err)) + } + PrintOutput(string(out), outputFile) +} + +func writeJunitOutput(controlsCollection []*check.Controls) { + for _, controls := range controlsCollection { + out, err := controls.JUnit() + if err != nil { + exitWithError(fmt.Errorf("failed to output in JUnit format: %v", err)) + } + PrintOutput(string(out), outputFile) + } +} + +func writePgsqlOutput(controlsCollection []*check.Controls) { + for _, controls := range controlsCollection { + out, err := controls.JSON() + if err != nil { + exitWithError(fmt.Errorf("failed to output in Postgresql format: %v", err)) + } + savePgsql(string(out)) + } +} + +func writeStdoutOutput(controlsCollection []*check.Controls) { + for _, controls := range controlsCollection { + summary := controls.Summary + prettyPrint(controls, summary) + } +} + func printRawOutput(output string) { for _, row := range strings.Split(output, "\n") { fmt.Println(fmt.Sprintf("\t %s", row)) diff --git a/cmd/common_test.go b/cmd/common_test.go index 5521fff..a3c4915 100644 --- a/cmd/common_test.go +++ b/cmd/common_test.go @@ -15,12 +15,15 @@ package cmd import ( + "encoding/json" "errors" "fmt" "io/ioutil" "os" + "path" "path/filepath" "testing" + "time" "github.com/aquasecurity/kube-bench/check" "github.com/spf13/viper" @@ -475,6 +478,51 @@ func TestIsEtcd(t *testing.T) { } } +func TestWriteResultToJsonFile(t *testing.T) { + defer func() { + controlsCollection = []*check.Controls{} + jsonFmt = false + outputFile = "" + }() + var err error + jsonFmt = true + outputFile = path.Join(os.TempDir(), fmt.Sprintf("%d", time.Now().UnixNano())) + + controlsCollection, err = parseControlsJsonFile("./testdata/controlsCollection.json") + if err != nil { + t.Error(err) + } + writeOutput(controlsCollection) + + var expect []*check.Controls + var result []*check.Controls + result, err = parseControlsJsonFile(outputFile) + if err != nil { + t.Error(err) + } + expect, err = parseControlsJsonFile("./testdata/result.json") + if err != nil { + t.Error(err) + } + + assert.Equal(t, expect, result) +} + +func parseControlsJsonFile(filepath string) ([]*check.Controls, error) { + var result []*check.Controls + + d, err := ioutil.ReadFile(filepath) + if err != nil { + return nil, err + } + err = json.Unmarshal(d, &result) + if err != nil { + return nil, err + } + + return result, nil +} + func loadConfigForTest() (*viper.Viper, error) { viperWithData := viper.New() viperWithData.SetConfigFile(filepath.Join("..", cfgDir, "config.yaml")) diff --git a/cmd/master.go b/cmd/master.go index fd903a7..76ccd5b 100644 --- a/cmd/master.go +++ b/cmd/master.go @@ -27,6 +27,7 @@ var masterCmd = &cobra.Command{ Run: func(cmd *cobra.Command, args []string) { filename := loadConfig(check.MASTER) runChecks(check.MASTER, filename) + writeOutput(controlsCollection) }, } diff --git a/cmd/node.go b/cmd/node.go index d09b47b..d2f0b72 100644 --- a/cmd/node.go +++ b/cmd/node.go @@ -27,6 +27,7 @@ var nodeCmd = &cobra.Command{ Run: func(cmd *cobra.Command, args []string) { filename := loadConfig(check.NODE) runChecks(check.NODE, filename) + writeOutput(controlsCollection) }, } diff --git a/cmd/root.go b/cmd/root.go index 1f135dc..6bb3d32 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -55,6 +55,7 @@ var ( includeTestOutput bool outputFile string configFileError error + controlsCollection []*check.Controls ) // RootCmd represents the base command when called without any subcommands @@ -104,6 +105,7 @@ var RootCmd = &cobra.Command{ runChecks(check.MANAGEDSERVICES, loadConfig(check.MANAGEDSERVICES)) } + writeOutput(controlsCollection) }, } diff --git a/cmd/run.go b/cmd/run.go index 4b7e3d0..72a2f23 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -67,6 +67,7 @@ func run(targets []string, benchmarkVersion string) (err error) { runChecks(testType, yamlFile) } + writeOutput(controlsCollection) return nil } diff --git a/cmd/testdata/controlsCollection.json b/cmd/testdata/controlsCollection.json new file mode 100644 index 0000000..dd00c49 --- /dev/null +++ b/cmd/testdata/controlsCollection.json @@ -0,0 +1,114 @@ +[ + { + "id": "2", + "version": "1.15", + "text": "Etcd Node Configuration", + "node_type": "etcd", + "tests": [ + { + "section": "2", + "pass": 7, + "fail": 0, + "warn": 0, + "info": 0, + "desc": "Etcd Node Configuration Files", + "results": [ + { + "test_number": "2.1", + "test_desc": "Ensure that the --cert-file and --key-file arguments are set as appropriate (Scored)", + "audit": "/bin/ps -ef | /bin/grep etcd | /bin/grep -v grep", + "AuditConfig": "", + "type": "", + "remediation": "Follow the etcd service documentation and configure TLS encryption.\nThen, edit the etcd pod specification file /etc/kubernetes/manifests/etcd.yaml\non the master node and set the below parameters.\n--cert-file=\n--key-file=\n", + "test_info": [ + "Follow the etcd service documentation and configure TLS encryption.\nThen, edit the etcd pod specification file /etc/kubernetes/manifests/etcd.yaml\non the master node and set the below parameters.\n--cert-file=\n--key-file=\n" + ], + "status": "PASS", + "actual_value": "root 3277 3218 3 Apr19 ? 03:57:52 etcd --advertise-client-urls=https://192.168.64.4:2379 --cert-file=/var/lib/minikube/certs/etcd/server.crt --client-cert-auth=true --data-dir=/var/lib/minikube/etcd --initial-advertise-peer-urls=https://192.168.64.4:2380 --initial-cluster=minikube=https://192.168.64.4:2380 --key-file=/var/lib/minikube/certs/etcd/server.key --listen-client-urls=https://127.0.0.1:2379,https://192.168.64.4:2379 --listen-metrics-urls=http://127.0.0.1:2381 --listen-peer-urls=https://192.168.64.4:2380 --name=minikube --peer-cert-file=/var/lib/minikube/certs/etcd/peer.crt --peer-client-cert-auth=true --peer-key-file=/var/lib/minikube/certs/etcd/peer.key --peer-trusted-ca-file=/var/lib/minikube/certs/etcd/ca.crt --snapshot-count=10000 --trusted-ca-file=/var/lib/minikube/certs/etcd/ca.crt\nroot 4624 4605 8 Apr21 ? 04:55:10 kube-apiserver --advertise-address=192.168.64.4 --allow-privileged=true --authorization-mode=Node,RBAC --client-ca-file=/var/lib/minikube/certs/ca.crt --enable-admission-plugins=NamespaceLifecycle,LimitRanger,ServiceAccount,DefaultStorageClass,DefaultTolerationSeconds,NodeRestriction,MutatingAdmissionWebhook,ValidatingAdmissionWebhook,ResourceQuota,PodSecurityPolicy --enable-bootstrap-token-auth=true --etcd-cafile=/var/lib/minikube/certs/etcd/ca.crt --etcd-certfile=/var/lib/minikube/certs/apiserver-etcd-client.crt --etcd-keyfile=/var/lib/minikube/certs/apiserver-etcd-client.key --etcd-servers=https://127.0.0.1:2379 --insecure-port=0 --kubelet-client-certificate=/var/lib/minikube/certs/apiserver-kubelet-client.crt --kubelet-client-key=/var/lib/minikube/certs/apiserver-kubelet-client.key --kubelet-preferred-address-types=InternalIP,ExternalIP,Hostname --proxy-client-cert-file=/var/lib/minikube/certs/front-proxy-client.crt --proxy-client-key-file=/var/lib/minikube/certs/front-proxy-client.key --requestheader-allowed-names=front-proxy-client --requestheader-client-ca-file=/var/lib/minikube/certs/front-proxy-ca.crt --requestheader-extra-headers-prefix=X-Remote-Extra- --requestheader-group-headers=X-Remote-Group --requestheader-username-headers=X-Remote-User --secure-port=8443 --service-account-key-file=/var/lib/minikube/certs/sa.pub --service-cluster-ip-range=10.96.0.0/12 --tls-cert-file=/var/lib/minikube/certs/apiserver.crt --tls-private-key-file=/var/lib/minikube/certs/apiserver.key\n", + "scored": true, + "expected_result": "'--cert-file' is present AND '--key-file' is present" + } + ] + } + ], + "total_pass": 7, + "total_fail": 0, + "total_warn": 0, + "total_info": 0 + }, + { + "id": "3", + "version": "1.5", + "text": "Control Plane Configuration", + "node_type": "controlplane", + "tests": [ + { + "section": "3.1", + "pass": 0, + "fail": 0, + "warn": 1, + "info": 0, + "desc": "Authentication and Authorization", + "results": [ + { + "test_number": "3.1.1", + "test_desc": "Client certificate authentication should not be used for users (Not Scored)", + "audit": "", + "AuditConfig": "", + "type": "manual", + "remediation": "Alternative mechanisms provided by Kubernetes such as the use of OIDC should be\nimplemented in place of client certificates.\n", + "test_info": [ + "Alternative mechanisms provided by Kubernetes such as the use of OIDC should be\nimplemented in place of client certificates.\n" + ], + "status": "WARN", + "actual_value": "", + "scored": false, + "expected_result": "", + "reason": "Test marked as a manual test" + } + ] + } + ], + "total_pass": 0, + "total_fail": 0, + "total_warn": 3, + "total_info": 0 + }, + { + "id": "1", + "version": "1.5", + "text": "Master Node Security Configuration", + "node_type": "master", + "tests": [ + { + "section": "1.1", + "pass": 15, + "fail": 1, + "warn": 5, + "info": 0, + "desc": "Master Node Configuration Files", + "results": [ + { + "test_number": "1.1.1", + "test_desc": "Ensure that the API server pod specification file permissions are set to 644 or more restrictive (Scored)", + "audit": "/bin/sh -c 'if test -e /etc/kubernetes/manifests/kube-apiserver.yaml; then stat -c permissions=%a /etc/kubernetes/manifests/kube-apiserver.yaml; fi'", + "AuditConfig": "", + "type": "", + "remediation": "Run the below command (based on the file location on your system) on the\nmaster node.\nFor example, chmod 644 /etc/kubernetes/manifests/kube-apiserver.yaml\n", + "test_info": [ + "Run the below command (based on the file location on your system) on the\nmaster node.\nFor example, chmod 644 /etc/kubernetes/manifests/kube-apiserver.yaml\n" + ], + "status": "PASS", + "actual_value": "permissions=600\n", + "scored": true, + "expected_result": "bitmask '600' AND '644'" + } + ] + } + ], + "total_pass": 42, + "total_fail": 12, + "total_warn": 11, + "total_info": 0 + } +] diff --git a/cmd/testdata/result.json b/cmd/testdata/result.json new file mode 100644 index 0000000..9d79fd0 --- /dev/null +++ b/cmd/testdata/result.json @@ -0,0 +1,114 @@ +[ + { + "id": "1", + "version": "1.5", + "text": "Master Node Security Configuration", + "node_type": "master", + "tests": [ + { + "section": "1.1", + "pass": 15, + "fail": 1, + "warn": 5, + "info": 0, + "desc": "Master Node Configuration Files", + "results": [ + { + "test_number": "1.1.1", + "test_desc": "Ensure that the API server pod specification file permissions are set to 644 or more restrictive (Scored)", + "audit": "/bin/sh -c 'if test -e /etc/kubernetes/manifests/kube-apiserver.yaml; then stat -c permissions=%a /etc/kubernetes/manifests/kube-apiserver.yaml; fi'", + "AuditConfig": "", + "type": "", + "remediation": "Run the below command (based on the file location on your system) on the\nmaster node.\nFor example, chmod 644 /etc/kubernetes/manifests/kube-apiserver.yaml\n", + "test_info": [ + "Run the below command (based on the file location on your system) on the\nmaster node.\nFor example, chmod 644 /etc/kubernetes/manifests/kube-apiserver.yaml\n" + ], + "status": "PASS", + "actual_value": "permissions=600\n", + "scored": true, + "expected_result": "bitmask '600' AND '644'" + } + ] + } + ], + "total_pass": 42, + "total_fail": 12, + "total_warn": 11, + "total_info": 0 + }, + { + "id": "2", + "version": "1.15", + "text": "Etcd Node Configuration", + "node_type": "etcd", + "tests": [ + { + "section": "2", + "pass": 7, + "fail": 0, + "warn": 0, + "info": 0, + "desc": "Etcd Node Configuration Files", + "results": [ + { + "test_number": "2.1", + "test_desc": "Ensure that the --cert-file and --key-file arguments are set as appropriate (Scored)", + "audit": "/bin/ps -ef | /bin/grep etcd | /bin/grep -v grep", + "AuditConfig": "", + "type": "", + "remediation": "Follow the etcd service documentation and configure TLS encryption.\nThen, edit the etcd pod specification file /etc/kubernetes/manifests/etcd.yaml\non the master node and set the below parameters.\n--cert-file=\n--key-file=\n", + "test_info": [ + "Follow the etcd service documentation and configure TLS encryption.\nThen, edit the etcd pod specification file /etc/kubernetes/manifests/etcd.yaml\non the master node and set the below parameters.\n--cert-file=\n--key-file=\n" + ], + "status": "PASS", + "actual_value": "root 3277 3218 3 Apr19 ? 03:57:52 etcd --advertise-client-urls=https://192.168.64.4:2379 --cert-file=/var/lib/minikube/certs/etcd/server.crt --client-cert-auth=true --data-dir=/var/lib/minikube/etcd --initial-advertise-peer-urls=https://192.168.64.4:2380 --initial-cluster=minikube=https://192.168.64.4:2380 --key-file=/var/lib/minikube/certs/etcd/server.key --listen-client-urls=https://127.0.0.1:2379,https://192.168.64.4:2379 --listen-metrics-urls=http://127.0.0.1:2381 --listen-peer-urls=https://192.168.64.4:2380 --name=minikube --peer-cert-file=/var/lib/minikube/certs/etcd/peer.crt --peer-client-cert-auth=true --peer-key-file=/var/lib/minikube/certs/etcd/peer.key --peer-trusted-ca-file=/var/lib/minikube/certs/etcd/ca.crt --snapshot-count=10000 --trusted-ca-file=/var/lib/minikube/certs/etcd/ca.crt\nroot 4624 4605 8 Apr21 ? 04:55:10 kube-apiserver --advertise-address=192.168.64.4 --allow-privileged=true --authorization-mode=Node,RBAC --client-ca-file=/var/lib/minikube/certs/ca.crt --enable-admission-plugins=NamespaceLifecycle,LimitRanger,ServiceAccount,DefaultStorageClass,DefaultTolerationSeconds,NodeRestriction,MutatingAdmissionWebhook,ValidatingAdmissionWebhook,ResourceQuota,PodSecurityPolicy --enable-bootstrap-token-auth=true --etcd-cafile=/var/lib/minikube/certs/etcd/ca.crt --etcd-certfile=/var/lib/minikube/certs/apiserver-etcd-client.crt --etcd-keyfile=/var/lib/minikube/certs/apiserver-etcd-client.key --etcd-servers=https://127.0.0.1:2379 --insecure-port=0 --kubelet-client-certificate=/var/lib/minikube/certs/apiserver-kubelet-client.crt --kubelet-client-key=/var/lib/minikube/certs/apiserver-kubelet-client.key --kubelet-preferred-address-types=InternalIP,ExternalIP,Hostname --proxy-client-cert-file=/var/lib/minikube/certs/front-proxy-client.crt --proxy-client-key-file=/var/lib/minikube/certs/front-proxy-client.key --requestheader-allowed-names=front-proxy-client --requestheader-client-ca-file=/var/lib/minikube/certs/front-proxy-ca.crt --requestheader-extra-headers-prefix=X-Remote-Extra- --requestheader-group-headers=X-Remote-Group --requestheader-username-headers=X-Remote-User --secure-port=8443 --service-account-key-file=/var/lib/minikube/certs/sa.pub --service-cluster-ip-range=10.96.0.0/12 --tls-cert-file=/var/lib/minikube/certs/apiserver.crt --tls-private-key-file=/var/lib/minikube/certs/apiserver.key\n", + "scored": true, + "expected_result": "'--cert-file' is present AND '--key-file' is present" + } + ] + } + ], + "total_pass": 7, + "total_fail": 0, + "total_warn": 0, + "total_info": 0 + }, + { + "id": "3", + "version": "1.5", + "text": "Control Plane Configuration", + "node_type": "controlplane", + "tests": [ + { + "section": "3.1", + "pass": 0, + "fail": 0, + "warn": 1, + "info": 0, + "desc": "Authentication and Authorization", + "results": [ + { + "test_number": "3.1.1", + "test_desc": "Client certificate authentication should not be used for users (Not Scored)", + "audit": "", + "AuditConfig": "", + "type": "manual", + "remediation": "Alternative mechanisms provided by Kubernetes such as the use of OIDC should be\nimplemented in place of client certificates.\n", + "test_info": [ + "Alternative mechanisms provided by Kubernetes such as the use of OIDC should be\nimplemented in place of client certificates.\n" + ], + "status": "WARN", + "actual_value": "", + "scored": false, + "expected_result": "", + "reason": "Test marked as a manual test" + } + ] + } + ], + "total_pass": 0, + "total_fail": 0, + "total_warn": 3, + "total_info": 0 + } +] diff --git a/integration/testdata/cis-1.5/job.data b/integration/testdata/cis-1.5/job.data index 1356f9d..4800169 100644 --- a/integration/testdata/cis-1.5/job.data +++ b/integration/testdata/cis-1.5/job.data @@ -186,6 +186,21 @@ on the master node and set the below parameter. 13 checks FAIL 11 checks WARN 0 checks INFO +[INFO] 2 Etcd Node Configuration +[INFO] 2 Etcd Node Configuration Files +[PASS] 2.1 Ensure that the --cert-file and --key-file arguments are set as appropriate (Scored) +[PASS] 2.2 Ensure that the --client-cert-auth argument is set to true (Scored) +[PASS] 2.3 Ensure that the --auto-tls argument is not set to true (Scored) +[PASS] 2.4 Ensure that the --peer-cert-file and --peer-key-file arguments are set as appropriate (Scored) +[PASS] 2.5 Ensure that the --peer-client-cert-auth argument is set to true (Scored) +[PASS] 2.6 Ensure that the --peer-auto-tls argument is not set to true (Scored) +[PASS] 2.7 Ensure that a unique Certificate Authority is used for etcd (Not Scored) + +== Summary == +7 checks PASS +0 checks FAIL +0 checks WARN +0 checks INFO [INFO] 3 Control Plane Configuration [INFO] 3.1 Authentication and Authorization [WARN] 3.1.1 Client certificate authentication should not be used for users (Not Scored) @@ -208,21 +223,6 @@ minimum. 0 checks FAIL 3 checks WARN 0 checks INFO -[INFO] 2 Etcd Node Configuration -[INFO] 2 Etcd Node Configuration Files -[PASS] 2.1 Ensure that the --cert-file and --key-file arguments are set as appropriate (Scored) -[PASS] 2.2 Ensure that the --client-cert-auth argument is set to true (Scored) -[PASS] 2.3 Ensure that the --auto-tls argument is not set to true (Scored) -[PASS] 2.4 Ensure that the --peer-cert-file and --peer-key-file arguments are set as appropriate (Scored) -[PASS] 2.5 Ensure that the --peer-client-cert-auth argument is set to true (Scored) -[PASS] 2.6 Ensure that the --peer-auto-tls argument is not set to true (Scored) -[PASS] 2.7 Ensure that a unique Certificate Authority is used for etcd (Not Scored) - -== Summary == -7 checks PASS -0 checks FAIL -0 checks WARN -0 checks INFO [INFO] 4 Worker Node Security Configuration [INFO] 4.1 Worker Node Configuration Files [PASS] 4.1.1 Ensure that the kubelet service file permissions are set to 644 or more restrictive (Scored)