Updates to resolve issues identified by gosec (#330)

* Updates to resolve issues identified by gosec

cmd/runmqdevserver/main.go

@@ -23,7 +23,7 @@ import (
 	"syscall"
 
 	"github.com/ibm-messaging/mq-container/internal/command"
-	containerruntimelogger "github.com/ibm-messaging/mq-container/internal/containerruntimelogger"
+	"github.com/ibm-messaging/mq-container/internal/containerruntimelogger"
 	"github.com/ibm-messaging/mq-container/internal/logger"
 	"github.com/ibm-messaging/mq-container/internal/mqtemplate"
 	"github.com/ibm-messaging/mq-container/internal/name"

cmd/runmqserver/main.go

@@ -24,7 +24,7 @@ import (
 	"os"
 	"sync"
 
-	containerruntimelogger "github.com/ibm-messaging/mq-container/internal/containerruntimelogger"
+	"github.com/ibm-messaging/mq-container/internal/containerruntimelogger"
 	"github.com/ibm-messaging/mq-container/internal/metrics"
 	"github.com/ibm-messaging/mq-container/internal/name"
 	"github.com/ibm-messaging/mq-container/internal/ready"

cmd/runmqserver/post_init.go

@@ -33,7 +33,10 @@ func postInit(name, keylabel string, p12Trust tls.KeyStoreData) error {
 		// Start the web server, in the background (if installed)
 		// WARNING: No error handling or health checking available for the web server
 		go func() {
-			startWebServer(keystore, p12Trust.Password)
+			err = startWebServer(keystore, p12Trust.Password)
+			if err != nil {
+				log.Printf("Error starting web server: %v", err)
+			}
 		}()
 	}
 	return nil

cmd/runmqserver/qmgr.go

@@ -171,8 +171,10 @@ func configureQueueManager() error {
 			abs := filepath.Join(configDir, file.Name())
 			// #nosec G204
 			verify := exec.Command("runmqsc", "-v", "-e")
+			// #nosec G204 - command is fixed, no injection vector
 			cmd := exec.Command("runmqsc")
 			// Read mqsc file into variable
+			// #nosec G304 - filename variable is derived from contents of 'configDir' which is a defined constant
 			mqsc, err := ioutil.ReadFile(abs)
 			if err != nil {
 				log.Printf("Error reading file %v: %v", abs, err)

cmd/runmqserver/webserver.go

@@ -38,6 +38,7 @@ func startWebServer(keystore, keystorepw string) error {
 		return nil
 	}
 	log.Println("Starting web server")
+	// #nosec G204 - command is fixed, no injection vector
 	cmd := exec.Command("strmqweb")
 	// Set a default app password for the web server, if one isn't already set
 	_, set := os.LookupEnv("MQ_APP_PASSWORD")
@@ -175,6 +176,7 @@ func configureWebServer(keyLabel string, p12Trust tls.KeyStoreData) (string, err
 		}
 		if info.IsDir() {
 			if !exists {
+				// #nosec G301 - write group permissions are required
 				err := os.MkdirAll(to, 0770)
 				if err != nil {
 					return err

internal/containerruntime/runtime.go

@@ -13,7 +13,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 */
-package runtime
+package containerruntime
 
 import (
 	"fmt"
@@ -107,3 +107,22 @@ func GetKernelVersion() (string, error) {
 func GetMaxFileHandles() (string, error) {
 	return readProc("/proc/sys/fs/file-max")
 }
+
+// SupportedFilesystem returns true if the supplied filesystem type is supported for MQ data
+func SupportedFilesystem(fsType string) bool {
+	switch fsType {
+	case "aufs", "overlayfs", "tmpfs":
+		return false
+	default:
+		return true
+	}
+}
+
+// ValidMultiInstanceFilesystem returns true if the supplied filesystem type is valid for a multi-instance queue manager
+func ValidMultiInstanceFilesystem(fsType string) bool {
+	if !SupportedFilesystem(fsType) {
+		return false
+	}
+	// TODO : check for non-shared filesystems & shared filesystems which are known not to work
+	return true
+}

internal/containerruntime/runtime_linux.go

@@ -15,7 +15,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 */
-package runtime
+package containerruntime
 
 import (
 	"golang.org/x/sys/unix"
@@ -113,22 +113,3 @@ func GetFilesystem(path string) (string, error) {
 	}
 	return t, nil
 }
-
-// SupportedFilesystem returns true if the supplied filesystem type is supported for MQ data
-func SupportedFilesystem(fsType string) bool {
-	switch fsType {
-	case "aufs", "overlayfs", "tmpfs":
-		return false
-	default:
-		return true
-	}
-}
-
-// ValidMultiInstanceFilesystem returns true if the supplied filesystem type is valid for a multi-instance queue manager
-func ValidMultiInstanceFilesystem(fsType string) bool {
-	if !SupportedFilesystem(fsType) {
-		return false
-	}
-	// TODO : check for non-shared filesystems & shared filesystems which are known not to work
-	return true
-}

internal/containerruntime/runtime_other.go

@@ -15,7 +15,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 */
-package runtime
+package containerruntime
 
 // Dummy version of this function, only for non-Linux systems.
 // Having this allows unit tests to be run on other platforms (e.g. macOS)

internal/containerruntimelogger/logruntime.go

@@ -13,7 +13,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 */
-package logruntime
+package containerruntimelogger
 
 import (
 	"fmt"
@@ -21,7 +21,7 @@ import (
 	"runtime"
 	"strings"
 
-	containerruntime "github.com/ibm-messaging/mq-container/internal/containerruntime"
+	"github.com/ibm-messaging/mq-container/internal/containerruntime"
 	"github.com/ibm-messaging/mq-container/internal/logger"
 	"github.com/ibm-messaging/mq-container/internal/user"
 )

internal/copy/copy.go

@@ -20,9 +20,18 @@ import (
 	"fmt"
 	"io"
 	"os"
+
+	"github.com/ibm-messaging/mq-container/internal/filecheck"
 )
 
 func CopyFileMode(src, dest string, perm os.FileMode) error {
+
+	err := filecheck.CheckFileSource(src)
+	if err != nil {
+		return fmt.Errorf("failed to open %s for copy: %v", src, err)
+	}
+
+	// #nosec G304 - filename variable 'src' is checked above to ensure it is valid
 	in, err := os.Open(src)
 	if err != nil {
 		return fmt.Errorf("failed to open %s for copy: %v", src, err)

internal/filecheck/filecheck.go

@@ -0,0 +1,37 @@
+/*
+© Copyright IBM Corporation 2019
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package filecheck
+
+import (
+	"fmt"
+	"path/filepath"
+	"strings"
+)
+
+// CheckFileSource checks the filename is valid
+func CheckFileSource(fileName string) error {
+
+	absFile, _ := filepath.Abs(fileName)
+
+	prefixes := []string{"bin", "boot", "dev", "lib", "lib64", "proc", "sbin", "sys"}
+	for _, prefix := range prefixes {
+		if strings.HasPrefix(absFile, filepath.Join("/", prefix)) {
+			return fmt.Errorf("Filename resolves to invalid path '%v'", absFile)
+		}
+	}
+	return nil
+}

internal/filecheck/filecheck_test.go

@@ -0,0 +1,40 @@
+/*
+© Copyright IBM Corporation 2019
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package filecheck
+
+import (
+	"testing"
+)
+
+func TestCheckFileSource(t *testing.T) {
+
+	invalidFilenames := []string{"/bin", "/boot", "/dev", "/lib", "/lib64", "/proc", "/sbin", "/sys", "/bin/myfile", "/boot/mydir/myfile", "/var/../dev", "/var/../lib/myfile"}
+	for _, invalidFilename := range invalidFilenames {
+		err := CheckFileSource(invalidFilename)
+		if err == nil {
+			t.Errorf("Expected to receive an error for filename '%v'", invalidFilename)
+		}
+	}
+
+	validFilenames := []string{"/var", "/mydir/dev", "/mydir/dev/myfile"}
+	for _, validFilename := range validFilenames {
+		err := CheckFileSource(validFilename)
+		if err != nil {
+			t.Errorf("Unexpected error received: %v", err)
+		}
+	}
+}

internal/tls/tls.go

@@ -26,10 +26,11 @@ import (
 	"time"
 
 	"crypto/rand"
-	"crypto/sha1"
+	"crypto/sha512"
 	"crypto/x509"
 	"encoding/pem"
 
+	"github.com/ibm-messaging/mq-container/internal/filecheck"
 	"github.com/ibm-messaging/mq-container/internal/keystore"
 	pkcs "software.sslmate.com/src/go-pkcs12"
 )
@@ -62,22 +63,22 @@ func getCertFingerPrint(block *pem.Block) (string, error) {
 	if err != nil {
 		return "", fmt.Errorf("could not parse x509 certificate: %v", err)
 	}
-	sha1Sum := sha1.Sum(cert.Raw)
-	sha1str := string(sha1Sum[:])
+	sha512Sum := sha512.Sum512(cert.Raw)
+	sha512str := string(sha512Sum[:])
 
-	return sha1str, nil
+	return sha512str, nil
 }
 
 // Add to Keystores known certs (if not already there) and add to the
 // Keystore if "addToKeystore" is true.
 func addCertToKeyData(block *pem.Block, keyData *KeyStoreData, addToKeystore bool) error {
-	sha1str, err := getCertFingerPrint(block)
+	sha512str, err := getCertFingerPrint(block)
 	if err != nil {
 		return err
 	}
 	known := false
 	for _, fingerprint := range keyData.KnownFingerPrints {
-		if fingerprint == sha1str {
+		if fingerprint == sha512str {
 			known = true
 			break
 		}
@@ -89,7 +90,7 @@ func addCertToKeyData(block *pem.Block, keyData *KeyStoreData, addToKeystore boo
 		if addToKeystore {
 			keyData.TrustedCerts = append(keyData.TrustedCerts, block)
 		}
-		keyData.KnownFingerPrints = append(keyData.KnownFingerPrints, sha1str)
+		keyData.KnownFingerPrints = append(keyData.KnownFingerPrints, sha512str)
 	}
 	return nil
 }
@@ -116,10 +117,14 @@ func generateAllStores(dir string) (KeyStoreData, KeyStoreData, error) {
 	p12TrustStore.Password = pw
 
 	// Create the keystore Directory (if it doesn't already exist)
-	os.MkdirAll(dir, 0775)
+	// #nosec G301 - write group permissions are required
+	err := os.MkdirAll(dir, 0770)
+	if err != nil {
+		return cmsKeystore, p12TrustStore, fmt.Errorf("Failed to create keystore directory: %v", err)
+	}
 
 	p12TrustStore.Keystore = keystore.NewPKCS12KeyStore(filepath.Join(dir, P12TrustStoreName), p12TrustStore.Password)
-	err := p12TrustStore.Keystore.Create()
+	err = p12TrustStore.Keystore.Create()
 	if err != nil {
 		return cmsKeystore, p12TrustStore, fmt.Errorf("Failed to create PKCS#12 TrustStore: %v", err)
 	}
@@ -161,6 +166,7 @@ func processKeys(keyDir, outputDir string, cmsKeyDB, p12TrustDB *KeyStoreData) (
 			// find the keyfile name
 			for _, a := range keys {
 				if strings.HasSuffix(a.Name(), ".key") {
+					// #nosec G304 - filename variable is derived from contents of 'keyDir' which is a defined constant
 					keyFile, err := ioutil.ReadFile(filepath.Join(keyDir, key.Name(), a.Name()))
 					if err != nil {
 						return firstLabel, p12s, fmt.Errorf("Could not read keyfile %s: %v", filepath.Join(keyDir, key.Name(), a.Name()), err)
@@ -196,6 +202,7 @@ func processKeys(keyDir, outputDir string, cmsKeyDB, p12TrustDB *KeyStoreData) (
 					continue
 				}
 				if strings.HasPrefix(a.Name(), prefix) && strings.HasSuffix(a.Name(), ".crt") {
+					// #nosec G304 - filename variable is derived from contents of 'keyDir' which is a defined constant
 					cert, err := ioutil.ReadFile(filepath.Join(keyDir, key.Name(), a.Name()))
 					if err != nil {
 						return firstLabel, p12s, fmt.Errorf("Could not read file %s: %v", filepath.Join(keyDir, key.Name(), a.Name()), err)
@@ -212,6 +219,7 @@ func processKeys(keyDir, outputDir string, cmsKeyDB, p12TrustDB *KeyStoreData) (
 					err = addCertToKeyData(block, cmsKeyDB, false)
 
 				} else if strings.HasSuffix(a.Name(), ".crt") {
+					// #nosec G304 - filename variable is derived from contents of 'keyDir' which is a defined constant
 					remainder, err := ioutil.ReadFile(filepath.Join(keyDir, key.Name(), a.Name()))
 					if err != nil {
 						return firstLabel, p12s, fmt.Errorf("Could not read file %s: %v", filepath.Join(keyDir, key.Name(), a.Name()), err)
@@ -309,6 +317,7 @@ func processTrustCertificates(trustDir string, cmsKeyDB, p12TrustDB *KeyStoreDat
 			certs, _ := ioutil.ReadDir(filepath.Join(trustDir, cert.Name()))
 			for _, a := range certs {
 				if strings.HasSuffix(a.Name(), ".crt") {
+					// #nosec G304 - filename variable is derived from contents of 'trustDir' which is a defined constant
 					remainder, err := ioutil.ReadFile(filepath.Join(trustDir, cert.Name(), a.Name()))
 					if err != nil {
 						return fmt.Errorf("Could not read file %s: %v", filepath.Join(trustDir, cert.Name(), a.Name()), err)
@@ -335,9 +344,15 @@ func processTrustCertificates(trustDir string, cmsKeyDB, p12TrustDB *KeyStoreDat
 	if len(p12TrustDB.TrustedCerts) > 0 {
 		// Do P12 TrustStore first
 		temporaryPemFile := filepath.Join("/tmp", "trust.pem")
-		os.Remove(temporaryPemFile)
+		_, err := os.Stat(temporaryPemFile)
+		if err == nil {
+			err = os.Remove(temporaryPemFile)
+			if err != nil {
+				return fmt.Errorf("Could not remove file %v: %v", temporaryPemFile, err)
+			}
+		}
 
-		err := writeCertsToFile(temporaryPemFile, p12TrustDB.TrustedCerts)
+		err = writeCertsToFile(temporaryPemFile, p12TrustDB.TrustedCerts)
 		if err != nil {
 			return err
 		}
@@ -368,7 +383,13 @@ func processTrustCertificates(trustDir string, cmsKeyDB, p12TrustDB *KeyStoreDat
 	if len(cmsKeyDB.TrustedCerts) > 0 {
 		// Now the CMS Keystore
 		temporaryPemFile := filepath.Join("/tmp", "cmsTrust.pem")
-		os.Remove(temporaryPemFile)
+		_, err := os.Stat(temporaryPemFile)
+		if err == nil {
+			err = os.Remove(temporaryPemFile)
+			if err != nil {
+				return fmt.Errorf("Could not remove file %v: %v", temporaryPemFile, err)
+			}
+		}
 
 		err = writeCertsToFile(temporaryPemFile, cmsKeyDB.TrustedCerts)
 		if err != nil {
@@ -398,7 +419,10 @@ func writeCertsToFile(file string, certs []*pem.Block) error {
 		if err != nil {
 			return fmt.Errorf("writeCertsToFile: Could not encode certificate number %d: %v", i, err)
 		}
-		w.Flush()
+		err = w.Flush()
+		if err != nil {
+			return fmt.Errorf("writeCertsToFile: Failed to write to file %s: %v", file, err)
+		}
 	}
 	return nil
 }
@@ -459,6 +483,12 @@ func expandOldTLSVariable(keyDir, outputDir string, cmsKeyDB, p12TrustDB *KeySto
 		return "", fmt.Errorf("File %s referenced by MQ_TLS_KEYSTORE does not exist", keyfile)
 	}
 
+	err = filecheck.CheckFileSource(keyfile)
+	if err != nil {
+		return "", fmt.Errorf("File %s referenced by MQ_TLS_KEYSTORE is invalid: %v", keyfile, err)
+	}
+
+	// #nosec G304 - filename variable 'keyfile' is checked above to ensure it is valid
 	readkey, err := ioutil.ReadFile(keyfile)
 	if err != nil {
 		return "", fmt.Errorf("Failed to read %s: %v", keyfile, err)