Implement GOSec for security scanning Fix vulnerabilities (#227)

* Implement GOSec for security scanning Fix vulnerabilities

* Fix lint failure

* address PR comments and fix build break

* Fix test break in mqsc

.gitignore

@@ -11,3 +11,4 @@ vendor/github.com/prometheus/client_model/bin/
 vendor/github.com/prometheus/client_model/.classpath
 vendor/github.com/prometheus/client_model/.project
 vendor/github.com/prometheus/client_model/.settings*
+gosec_results.json

Makefile

@@ -141,6 +141,32 @@ lint: $(addsuffix /$(wildcard *.go), $(GO_PKG_DIRS))
 	@# As of 11/04/2018 there is an open issue to fix it: https://github.com/golang/lint/issues/320
 	golint -set_exit_status $(sort $(dir $(wildcard $(addsuffix /*/*.go, $(GO_PKG_DIRS)))))
 
+.PHONY: gosec
+gosec: $(info $(SPACER)$(shell printf "Running gosec test"$(END))) 
+	@gosec -fmt=json -out=gosec_results.json cmd/... internal/... 2> /dev/null ;\
+	cat "gosec_results.json" ;\
+	cat gosec_results.json | grep HIGH | grep severity > /dev/null ;\
+	if [ $$? -eq 0 ]; then \
+		printf "\nFAILURE: gosec found files containing HIGH severity issues - see results.json\n" ;\
+		exit 1 ;\
+	else \
+		printf "\ngosec found no HIGH severity issues\n" ;\
+	fi ;\
+	cat gosec_results.json | grep MEDIUM | grep severity > /dev/null ;\
+	if [ $$? -eq 0 ]; then \
+		printf "\nFAILURE: gosec found files containing MEDIUM severity issues - see results.json\n" ;\
+		exit 1 ;\
+	else \
+		printf "\ngosec found no MEDIUM severity issues\n" ;\
+	fi ;\
+	cat gosec_results.json | grep LOW | grep severity > /dev/null;\
+	if [ $$? -eq 0 ]; then \
+		printf "\nFAILURE: gosec found files containing LOW severity issues - see results.json\n" ;\
+		exit 1;\
+	else \
+		printf "\ngosec found no LOW severity issues\n" ;\
+fi ;\
+
 .PHONY: unknownos
 unknownos:
 	$(info $(SPACER)$(shell printf "ERROR: Unknown OS ("$(BASE_OS)") please run specific make targets"$(END)))

cmd/chkmqhealthy/main.go

@@ -32,6 +32,7 @@ func queueManagerHealthy() (bool, error) {
 		return false, err
 	}
 	// Specify the queue manager name, just in case someone's created a second queue manager
+	// #nosec G204
 	cmd := exec.Command("dspmq", "-n", "-m", name)
 	// Run the command and wait for completion
 	out, err := cmd.CombinedOutput()

cmd/chkmqready/main.go

@@ -37,5 +37,8 @@ func main() {
 		fmt.Println(err)
 		os.Exit(1)
 	}
-	conn.Close()
+	err = conn.Close()
+	if err != nil {
+		fmt.Println(err)
+	}
 }

cmd/runmqdevserver/keystore.go

@@ -65,11 +65,27 @@ func (ks *KeyStore) Create() error {
 			stashFile := ks.Filename[0:len(ks.Filename)-len(extension)] + ".sth"
 			rdbFile := ks.Filename[0:len(ks.Filename)-len(extension)] + ".rdb"
 			crlFile := ks.Filename[0:len(ks.Filename)-len(extension)] + ".crl"
-			os.Remove(stashFile)
+			err = os.Remove(stashFile)
-			os.Remove(rdbFile)
+			if err != nil {
-			os.Remove(crlFile)
+				log.Errorf("Error removing %s: %v", stashFile, err)
+				return err
+			}
+			err = os.Remove(rdbFile)
+			if err != nil {
+				log.Errorf("Error removing %s: %v", rdbFile, err)
+				return err
+			}
+			err = os.Remove(crlFile)
+			if err != nil {
+				log.Errorf("Error removing %s: %v", crlFile, err)
+				return err
+			}
+		}
+		err = os.Remove(ks.Filename)
+		if err != nil {
+			log.Errorf("Error removing %s: %v", ks.Filename, err)
+			return err
 		}
-		os.Remove(ks.Filename)
 	} else if !os.IsNotExist(err) {
 		// If the keystore exists but cannot be accessed then return the error
 		return err

cmd/runmqdevserver/main.go

@@ -30,13 +30,17 @@ import (
 var log *logger.Logger
 
 func setPassword(user string, password string) error {
+	// #nosec G204
 	cmd := exec.Command("chpasswd")
 	stdin, err := cmd.StdinPipe()
 	if err != nil {
 		return err
 	}
 	fmt.Fprintf(stdin, "%s:%s", user, password)
-	stdin.Close()
+	err = stdin.Close()
+	if err != nil {
+		log.Errorf("Error closing password stdin: %v", err)
+	}
 	_, _, err = command.RunCmd(cmd)
 	if err != nil {
 		return err
@@ -165,6 +169,10 @@ func main() {
 		osExit(1)
 	} else {
 		// Replace this process with runmqserver
-		syscall.Exec("/usr/local/bin/runmqserver", []string{"runmqserver"}, os.Environ())
+		// #nosec G204
+		err = syscall.Exec("/usr/local/bin/runmqserver", []string{"runmqserver"}, os.Environ())
+		if err != nil {
+			log.Errorf("Error replacing this process with runmqserver: %v", err)
+		}
 	}
 }

cmd/runmqdevserver/mqsc.go

@@ -35,7 +35,14 @@ func updateMQSC(appPasswordRequired bool) error {
 			return err
 		}
 	} else {
-		os.Remove(mqsc)
+		_, err := os.Stat(mqsc)
+		if !os.IsNotExist(err) {
+			err = os.Remove(mqsc)
+			if err != nil {
+				log.Errorf("Error removing file %s: %v", mqsc, err)
+				return err
+			}
+		}
 	}
 	return nil
 }

cmd/runmqdevserver/template.go

@@ -36,7 +36,11 @@ func processTemplateFile(templateFile, destFile string, data interface{}) error
 	_, err = os.Stat(dir)
 	if err != nil {
 		if os.IsNotExist(err) {
-			os.MkdirAll(dir, 0660)
+			err = os.MkdirAll(dir, 0660)
+			if err != nil {
+				log.Error(err)
+				return err
+			}
 			mqmUID, mqmGID, err := command.LookupMQM()
 			if err != nil {
 				log.Error(err)
@@ -51,6 +55,7 @@ func processTemplateFile(templateFile, destFile string, data interface{}) error
 			return err
 		}
 	}
+	// #nosec G302
 	f, err := os.OpenFile(destFile, os.O_CREATE|os.O_WRONLY, 0660)
 	defer f.Close()
 	err = t.Execute(f, data)

cmd/runmqdevserver/tls.go

@@ -85,6 +85,7 @@ func configureTLS(qmName string, inputFile string, passPhrase string) error {
 	_, err = os.Stat(dir)
 	if err != nil {
 		if os.IsNotExist(err) {
+			// #nosec G301
 			err = os.MkdirAll(dir, 0770)
 			if err != nil {
 				return err

cmd/runmqserver/crtmqvol.go

@@ -29,6 +29,7 @@ func createVolume(path string) error {
 	fi, err := os.Stat(dataPath)
 	if err != nil {
 		if os.IsNotExist(err) {
+			// #nosec G301
 			err = os.MkdirAll(dataPath, 0755)
 			if err != nil {
 				return err

cmd/runmqserver/license.go

@@ -78,6 +78,7 @@ func checkLicense() (bool, error) {
 		return true, nil
 	case ok && lic == "view":
 		file := filepath.Join("/opt/mqm/licenses", resolveLicenseFile())
+		// #nosec G304
 		buf, err := ioutil.ReadFile(file)
 		if err != nil {
 			log.Println(err)

cmd/runmqserver/logging.go

@@ -33,7 +33,7 @@ import (
 // var debug = false
 var log *logger.Logger
 
-var collectDiagOnFail bool = false
+var collectDiagOnFail = false
 
 func logTerminationf(format string, args ...interface{}) {
 	logTermination(fmt.Sprintf(format, args))
@@ -108,8 +108,12 @@ func configureLogger(name string) (mirrorFunc, error) {
 		return func(msg string) {
 			// Parse the JSON message, and print a simplified version
 			var obj map[string]interface{}
-			json.Unmarshal([]byte(msg), &obj)
+			err := json.Unmarshal([]byte(msg), &obj)
-			fmt.Printf(formatSimple(obj["ibm_datetime"].(string), obj["message"].(string)))
+			if err != nil {
+				fmt.Printf("Failed to Unmarshall JSON - %v", err)
+			} else {
+				fmt.Printf(formatSimple(obj["ibm_datetime"].(string), obj["message"].(string)))
+			}
 		}, nil
 	default:
 		log, err = logger.NewLogger(os.Stdout, d, false, name)
@@ -124,20 +128,27 @@ func logDiagnostics() {
 	log.Debug("--- Start Diagnostics ---")
 
 	// show the directory ownership/permissions
+	// #nosec G104
 	out, _, _ := command.Run("ls", "-l", "/mnt/")
 	log.Debugf("/mnt/:\n%s", out)
+	// #nosec G104
 	out, _, _ = command.Run("ls", "-l", "/mnt/mqm")
 	log.Debugf("/mnt/mqm:\n%s", out)
+	// #nosec G104
 	out, _, _ = command.Run("ls", "-l", "/mnt/mqm/data")
 	log.Debugf("/mnt/mqm/data:\n%s", out)
+	// #nosec G104
 	out, _, _ = command.Run("ls", "-l", "/var/mqm")
 	log.Debugf("/var/mqm:\n%s", out)
+	// #nosec G104
 	out, _, _ = command.Run("ls", "-l", "/var/mqm/errors")
 	log.Debugf("/var/mqm/errors:\n%s", out)
 
 	// Print out summary of any FDCs
+	// #nosec G204
 	cmd := exec.Command("/opt/mqm/bin/ffstsummary")
 	cmd.Dir = "/var/mqm/errors"
+	// #nosec G104
 	outB, _ := cmd.CombinedOutput()
 	log.Debugf("ffstsummary:\n%s", string(outB))
 

cmd/runmqserver/main.go

@@ -129,7 +129,11 @@ func doMain() error {
 		logTermination(err)
 		return err
 	}
-	configureQueueManager()
+	err = configureQueueManager()
+	if err != nil {
+		logTermination(err)
+		return err
+	}
 
 	enableMetrics := os.Getenv("MQ_ENABLE_METRICS")
 	if enableMetrics == "true" || enableMetrics == "1" {
@@ -145,7 +149,11 @@ func doMain() error {
 	// Reap zombies now, just in case we've already got some
 	signalControl <- reapNow
 	// Write a file to indicate that chkmqready should now work as normal
-	ready.Set()
+	err = ready.Set()
+	if err != nil {
+		logTermination(err)
+		return err
+	}
 	// Wait for terminate signal
 	<-signalControl
 	return nil

cmd/runmqserver/mirror.go

@@ -139,7 +139,10 @@ func mirrorLog(ctx context.Context, wg *sync.WaitGroup, path string, fromStart b
 		// Always start at the beginning if we've been told to go from the start
 		if offset != 0 && !fromStart {
 			log.Debugf("Seeking offset %v in file %v", offset, path)
-			f.Seek(offset, 0)
+			_, err = f.Seek(offset, 0)
+			if err != nil {
+				log.Errorf("Unable to return to offset %v: %v", offset, err)
+			}
 		}
 		closing := false
 		for {
@@ -159,7 +162,10 @@ func mirrorLog(ctx context.Context, wg *sync.WaitGroup, path string, fromStart b
 				// could skip all those messages.  This could happen with a very small
 				// MQ error log size.
 				mirrorAvailableMessages(f, mf)
-				f.Close()
+				err = f.Close()
+				if err != nil {
+					log.Errorf("Unable to close mirror file handle: %v", err)
+				}
 				// Re-open file
 				log.Debugf("Re-opening error log file %v", path)
 				f, err = os.OpenFile(path, os.O_RDONLY, 0)

cmd/runmqserver/mqconfig.go

@@ -24,19 +24,20 @@ import (
 	"github.com/genuinetools/amicontained/container"
 )
 
-func logContainerRuntime() error {
+func logContainerRuntime() {
 	r, err := container.DetectRuntime()
 	if err != nil {
-		return err
+		log.Printf("Failed to get container runtime: %v", err)
+		return
 	}
 	log.Printf("Container runtime: %v", r)
-	return nil
 }
 
-func logBaseImage() error {
+func logBaseImage() {
 	buf, err := ioutil.ReadFile("/etc/os-release")
 	if err != nil {
-		return err
+		log.Printf("Failed to read /etc/os-release: %v", err)
+		return
 	}
 	lines := strings.Split(string(buf), "\n")
 	for _, l := range lines {
@@ -44,41 +45,40 @@ func logBaseImage() error {
 			words := strings.Split(l, "\"")
 			if len(words) >= 2 {
 				log.Printf("Base image: %v", words[1])
-				return nil
+				return
 			}
 		}
 	}
-	return nil
 }
 
 // logCapabilities logs the Linux capabilities (e.g. setuid, setgid).  See https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities
-func logCapabilities() error {
+func logCapabilities() {
 	caps, err := container.Capabilities()
 	if err != nil {
-		return err
+		log.Printf("Failed to get container capabilities: %v", err)
+		return
 	}
 	for k, v := range caps {
 		if len(v) > 0 {
 			log.Printf("Capabilities (%s set): %v", strings.ToLower(k), strings.Join(v, ","))
 		}
 	}
-	return nil
 }
 
 // logSeccomp logs the seccomp enforcing mode, which affects which kernel calls can be made
-func logSeccomp() error {
+func logSeccomp() {
 	s, err := container.SeccompEnforcingMode()
 	if err != nil {
-		return err
+		log.Printf("Failed to get container SeccompEnforcingMode: %v", err)
+		return
 	}
 	log.Printf("seccomp enforcing mode: %v", s)
-	return nil
 }
 
 // logSecurityAttributes logs the security attributes of the current process.
 // The security attributes indicate whether AppArmor or SELinux are being used,
 // and what the level of confinement is.
-func logSecurityAttributes() error {
+func logSecurityAttributes() {
 	a, err := readProc("/proc/self/attr/current")
 	// On some systems, if AppArmor or SELinux are not installed, you get an
 	// error when you try and read `/proc/self/attr/current`, even though the
@@ -87,10 +87,10 @@ func logSecurityAttributes() error {
 		a = "none"
 	}
 	log.Printf("Process security attributes: %v", a)
-	return nil
 }
 
 func readProc(filename string) (value string, err error) {
+	// #nosec G304
 	buf, err := ioutil.ReadFile(filename)
 	if err != nil {
 		return "", err

cmd/runmqserver/qmgr.go

@@ -90,6 +90,7 @@ func configureQueueManager() error {
 	for _, file := range files {
 		if strings.HasSuffix(file.Name(), ".mqsc") {
 			abs := filepath.Join(configDir, file.Name())
+			// #nosec G204
 			cmd := exec.Command("runmqsc")
 			stdin, err := cmd.StdinPipe()
 			if err != nil {
@@ -97,6 +98,7 @@ func configureQueueManager() error {
 				return err
 			}
 			// Open the MQSC file for reading
+			// #nosec G304
 			f, err := os.Open(abs)
 			if err != nil {
 				log.Printf("Error opening %v: %v", abs, err)
@@ -104,10 +106,16 @@ func configureQueueManager() error {
 			// Copy the contents to stdin of the runmqsc process
 			_, err = io.Copy(stdin, f)
 			if err != nil {
-				log.Printf("Error reading %v: %v", abs, err)
+				log.Errorf("Error reading %v: %v", abs, err)
+			}
+			err = f.Close()
+			if err != nil {
+				log.Errorf("Failed to close MQSC file handle: %v", err)
+			}
+			err = stdin.Close()
+			if err != nil {
+				log.Errorf("Failed to close MQSC stdin: %v", err)
 			}
-			f.Close()
-			stdin.Close()
 			// Run the command and wait for completion
 			out, err := cmd.CombinedOutput()
 			if err != nil {

cmd/runmqserver/signals.go

@@ -43,7 +43,8 @@ func signalHandler(qmgr string) chan int {
 				log.Printf("Signal received: %v", sig)
 				signal.Stop(reapSignals)
 				signal.Stop(stopSignals)
-				metrics.StopMetricsGathering()
+				metrics.StopMetricsGathering(log)
+				// #nosec G104
 				stopQueueManager(qmgr)
 				// One final reap
 				reapZombies()

internal/command/command.go

@@ -53,11 +53,13 @@ func RunCmd(cmd *exec.Cmd) (string, int, error) {
 // Do not use this function to run shell built-ins (like "cd"), because
 // the error handling works differently
 func Run(name string, arg ...string) (string, int, error) {
+	// #nosec G204
 	return RunCmd(exec.Command(name, arg...))
 }
 
 // RunAsMQM runs the specified command as the mqm user
 func RunAsMQM(name string, arg ...string) (string, int, error) {
+	// #nosec G204
 	cmd := exec.Command(name, arg...)
 	cmd.SysProcAttr = &syscall.SysProcAttr{}
 	uid, gid, err := LookupMQM()

internal/metrics/metrics.go

@@ -44,7 +44,7 @@ func GatherMetrics(qmName string, log *logger.Logger) {
 	err := startMetricsGathering(qmName, log)
 	if err != nil {
 		log.Errorf("Metrics Error: %s", err.Error())
-		StopMetricsGathering()
+		StopMetricsGathering(log)
 	}
 }
 
@@ -76,6 +76,7 @@ func startMetricsGathering(qmName string, log *logger.Logger) error {
 	http.Handle("/metrics", prometheus.Handler())
 	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(200)
+		// #nosec G104
 		w.Write([]byte("Status: METRICS ACTIVE"))
 	})
 
@@ -83,7 +84,7 @@ func startMetricsGathering(qmName string, log *logger.Logger) error {
 		err = metricsServer.ListenAndServe()
 		if err != nil && err != http.ErrServerClosed {
 			log.Errorf("Metrics Error: Failed to handle metrics request: %v", err)
-			StopMetricsGathering()
+			StopMetricsGathering(log)
 		}
 	}()
 
@@ -91,7 +92,7 @@ func startMetricsGathering(qmName string, log *logger.Logger) error {
 }
 
 // StopMetricsGathering stops gathering metrics for the queue manager
-func StopMetricsGathering() {
+func StopMetricsGathering(log *logger.Logger) {
 
 	if metricsEnabled {
 
@@ -101,6 +102,9 @@ func StopMetricsGathering() {
 		// Shutdown HTTP server
 		timeout, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 		defer cancel()
-		metricsServer.Shutdown(timeout)
+		err := metricsServer.Shutdown(timeout)
+		if err != nil {
+			log.Errorf("Failed to shutdown metrics server: %v", err)
+		}
 	}
 }

internal/metrics/update.go

@@ -62,6 +62,7 @@ func processMetrics(log *logger.Logger, qmName string) {
 				firstConnect = false
 				startChannel <- true
 			}
+			// #nosec G104
 			metrics, _ = initialiseMetrics(log)
 		}