Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement GOSec for security scanning Fix vulnerabilities #227

Merged
merged 4 commits into from
Oct 11, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
26 changes: 26 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down
1 change: 1 addition & 0 deletions cmd/chkmqhealthy/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
5 changes: 4 additions & 1 deletion cmd/chkmqready/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,8 @@ func main() {
fmt.Println(err)
os.Exit(1)
}
conn.Close()
err = conn.Close()
if err != nil {
fmt.Println(err)
}
}
20 changes: 16 additions & 4 deletions cmd/runmqdevserver/keystore.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,23 @@ 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)
os.Remove(rdbFile)
os.Remove(crlFile)
err = os.Remove(stashFile)
if err != nil {
log.Debugf("Error removing %s: %v", stashFile, err)
}
err = os.Remove(rdbFile)
if err != nil {
log.Debugf("Error removing %s: %v", rdbFile, err)
}
err = os.Remove(crlFile)
if err != nil {
log.Debugf("Error removing %s: %v", crlFile, err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these should be regular log messages, not debug ones. Also, in the case of removing files failing, maybe we should just fail.

}
}
err = os.Remove(ks.Filename)
if err != nil {
log.Debugf("Error removing %s: %v", ks.Filename, err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log normally

}
os.Remove(ks.Filename)
} else if !os.IsNotExist(err) {
// If the keystore exists but cannot be accessed then return the error
return err
Expand Down
12 changes: 10 additions & 2 deletions cmd/runmqdevserver/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.Debugf("Error closing password stdin: %v", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log normally

}
_, _, err = command.RunCmd(cmd)
if err != nil {
return err
Expand Down Expand Up @@ -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.Debugf("Error replacing this process with runmqserver: %v", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log normally

}
}
}
5 changes: 4 additions & 1 deletion cmd/runmqdevserver/mqsc.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ func updateMQSC(appPasswordRequired bool) error {
return err
}
} else {
os.Remove(mqsc)
err := os.Remove(mqsc)
if err != nil {
log.Debugf("Error removing file %s: %v", mqsc, err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log normally

}
}
return nil
}
7 changes: 6 additions & 1 deletion cmd/runmqdevserver/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
1 change: 1 addition & 0 deletions cmd/runmqdevserver/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions cmd/runmqserver/crtmqvol.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions cmd/runmqserver/license.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 14 additions & 3 deletions cmd/runmqserver/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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)
fmt.Printf(formatSimple(obj["ibm_datetime"].(string), obj["message"].(string)))
err := json.Unmarshal([]byte(msg), &obj)
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)
Expand All @@ -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))

Expand Down
12 changes: 10 additions & 2 deletions cmd/runmqserver/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand All @@ -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
Expand Down
10 changes: 8 additions & 2 deletions cmd/runmqserver/mirror.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.Debugf("Unable to close mirror file handle: %v", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normal log entry

}
// Re-open file
log.Debugf("Re-opening error log file %v", path)
f, err = os.OpenFile(path, os.O_RDONLY, 0)
Expand Down
25 changes: 11 additions & 14 deletions cmd/runmqserver/mqconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,52 +24,49 @@ 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)
}
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)
}
lines := strings.Split(string(buf), "\n")
for _, l := range lines {
if strings.HasPrefix(l, "PRETTY_NAME=") {
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)
}
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)
}
log.Printf("seccomp enforcing mode: %v", s)
return nil
Expand All @@ -78,7 +75,7 @@ func logSeccomp() error {
// 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
Expand All @@ -87,10 +84,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
Expand Down
12 changes: 10 additions & 2 deletions cmd/runmqserver/qmgr.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,15 @@ 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 {
log.Println(err)
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)
Expand All @@ -106,8 +108,14 @@ func configureQueueManager() error {
if err != nil {
log.Printf("Error reading %v: %v", abs, err)
}
f.Close()
stdin.Close()
err = f.Close()
if err != nil {

This comment was marked as resolved.

log.Debugf("Failed to close MQSC file handle: %v", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log normally

}
err = stdin.Close()
if err != nil {
log.Debugf("Failed to close MQSC stdin: %v", err)
}
// Run the command and wait for completion
out, err := cmd.CombinedOutput()
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions cmd/runmqserver/signals.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func signalHandler(qmgr string) chan int {
signal.Stop(reapSignals)
signal.Stop(stopSignals)
metrics.StopMetricsGathering()
// #nosec G104
stopQueueManager(qmgr)
// One final reap
reapZombies()
Expand Down
Loading