Expand redaction system to handle uncommon MQSC cases (#304)

* Expand redaction system to handle uncommon MQSC cases

* add invalidMQSC fixes

* address PR comments

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Change log
 
+## vNext
+
+* BREAKING CHANGE: MQSC files supplied will be verified before being run. Files containing invalid MQSC will cause the container to fail to start
+* Security Fixes
+
 ## 9.1.2.0 (2019-03-21)
 
 * Now runs using the "mqm" user instead of root.  See new [security doc](https://github.com/ibm-messaging/mq-container/blob/master/docs/security.md)

cmd/runmqserver/qmgr.go

@@ -17,14 +17,15 @@ package main
 
 import (
 	"bytes"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
-	"regexp"
 	"strings"
 
 	"github.com/ibm-messaging/mq-container/internal/command"
+	"github.com/ibm-messaging/mq-container/internal/mqscredact"
 )
 
 // createDirStructure creates the default MQ directory structure under /var/mqm
@@ -91,6 +92,7 @@ func configureQueueManager() error {
 		if strings.HasSuffix(file.Name(), ".mqsc") {
 			abs := filepath.Join(configDir, file.Name())
 			// #nosec G204
+			verify := exec.Command("runmqsc", "-v", "-e")
 			cmd := exec.Command("runmqsc")
 			// Read mqsc file into variable
 			mqsc, err := ioutil.ReadFile(abs)
@@ -105,10 +107,21 @@ func configureQueueManager() error {
 				log.Printf("Error writing MQSC file %v to buffer: %v", abs, err)
 				continue
 			}
+			verifyBuffer := buffer
+
 			// Buffer mqsc to stdin of runmqsc
 			cmd.Stdin = &buffer
+			verify.Stdin = &verifyBuffer
+
+			// Verify the MQSC commands
+			out, err := verify.CombinedOutput()
+			if err != nil {
+				log.Errorf("Error verifying MQSC file %v (%v):\n\t%v", file.Name(), err, formatMQSCOutput(string(out)))
+				return fmt.Errorf("Error verifying MQSC file %v (%v):\n\t%v", file.Name(), err, formatMQSCOutput(string(out)))
+			}
+
 			// Run runmqsc command
-			out, err := cmd.CombinedOutput()
+			out, err = cmd.CombinedOutput()
 			if err != nil {
 				log.Errorf("Error running MQSC file %v (%v):\n\t%v", file.Name(), err, formatMQSCOutput(string(out)))
 				continue
@@ -134,12 +147,7 @@ func stopQueueManager(name string) error {
 
 func formatMQSCOutput(out string) string {
 	// redact sensitive information
-	pattern, _ := regexp.Compile("(?i)LDAPPWD\\s*?\\((.*?)\\)")
+	out, _ = mqscredact.Redact(out)
-	out = pattern.ReplaceAllString(out, "LDAPPWD(*********)")
-	pattern, _ = regexp.Compile("(?i)PASSWORD\\s*?\\((.*?)\\)")
-	out = pattern.ReplaceAllString(out, "PASSWORD(*********)")
-	pattern, _ = regexp.Compile("(?i)SSLCRYP\\s*?\\((.*?)\\)")
-	out = pattern.ReplaceAllString(out, "SSLCRYP(*********)")
 
 	// add tab characters to make it more readable as part of the log
 	return strings.Replace(string(out), "\n", "\n\t", -1)

internal/mqscredact/mqscredact.go

@@ -0,0 +1,264 @@
+/*
+© 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 mqscredact
+
+import (
+	"bufio"
+	"io"
+	"regexp"
+	"strings"
+)
+
+/* List of sensitive MQ Parameters */
+var sensitiveParameters = []string{"LDAPPWD", "PASSWORD", "SSLCRYP"}
+
+// redactionString is what sensitive paramters will be replaced with
+const redactionString = "(*********)"
+
+func findEndOfParamterString(stringDenoter rune, r *bufio.Reader) string {
+	parameter := ""
+	for {
+		char, _, err := r.ReadRune()
+		if err != nil {
+			return parameter
+		}
+		parameter = parameter + string(char)
+		if char == stringDenoter {
+			break
+		} else if char == '\n' {
+			// Check if we're on a comment line
+		NewLineLoop:
+			for {
+				// Look at next character without moving buffer forwards
+				chars, err := r.Peek(1)
+				if err != nil {
+					return parameter
+				}
+				// Check if we're at the beginning of some data.
+				startOutput, _ := regexp.MatchString(`[^:0-9\s]`, string(chars[0]))
+				if startOutput {
+					// We are at the start, check if we're on a comment line
+					if chars[0] == '*' {
+						// found a comment line. go to the next newline chraracter
+					CommentLoop:
+						for {
+							char, _, err = r.ReadRune()
+							if err != nil {
+								return parameter
+							}
+							parameter = parameter + string(char)
+							if char == '\n' {
+								break CommentLoop
+							}
+						}
+						// Go round again as we're now on a new line
+						continue NewLineLoop
+					}
+					// We've checked for comment and it isn't a comment line so break without moving buffer forwards
+					break NewLineLoop
+				}
+				// Move the buffer forward and try again
+				char, _, _ = r.ReadRune()
+				parameter = parameter + string(char)
+			}
+		}
+	}
+
+	return parameter
+}
+
+// getParameterString reads from r in order to find the end of the MQSC Parameter value. This is enclosed in ( ).
+// This function will return what it finds and will increment the reader pointer along as it goes.
+func getParameterString(r *bufio.Reader) string {
+	// Add the ( in as it will have been dropped before.
+	parameter := "("
+Loop:
+	for {
+		char, _, err := r.ReadRune()
+		if err != nil {
+			return parameter
+		}
+
+		parameter = parameter + string(char)
+
+		switch char {
+		case ')':
+			break Loop
+		// TODO: Duplicate code..
+		case '\'', '"':
+			parameter = parameter + findEndOfParamterString(char, r)
+		}
+	}
+
+	return parameter
+}
+
+func resetAllParameters(currentVerb, originalString *string, lineContinuation, foundGap, parameterNext, redacting, checkComment *bool) {
+	*currentVerb = ""
+	*originalString = ""
+	*lineContinuation = false
+	*foundGap = false
+	*parameterNext = false
+	*redacting = false
+	*checkComment = true
+}
+
+// Redact is the main function for redacting sensitive parameters in MQSC strings
+// It accepts a string and redacts sensitive paramters such as LDAPPWD or PASSWORD
+func Redact(out string) (string, error) {
+	out = strings.TrimSpace(out)
+	var returnStr, currentVerb, originalString string
+	var lineContinuation, foundGap, parameterNext, redacting, checkComment bool
+	newline := true
+	resetAllParameters(&currentVerb, &originalString, &lineContinuation, &foundGap, &parameterNext, &redacting, &checkComment)
+	r := bufio.NewReader(strings.NewReader(out))
+
+MainLoop:
+	for {
+		// We have found a opening ( so use special parameter parsing
+		if parameterNext {
+			parameterStr := getParameterString(r)
+			if !redacting {
+				returnStr = returnStr + parameterStr
+			} else {
+				returnStr = returnStr + redactionString
+			}
+
+			resetAllParameters(&currentVerb, &originalString, &lineContinuation, &foundGap, &parameterNext, &redacting, &checkComment)
+		}
+
+		// Loop round getting hte next parameter
+		char, _, err := r.ReadRune()
+		if err == io.EOF {
+			if originalString != "" {
+				returnStr = returnStr + originalString
+			}
+			break
+		} else if err != nil {
+			return returnStr, err
+		}
+
+		/* We need to push forward until we find a non-whitespace, digit or colon character */
+		if newline {
+			startOutput, _ := regexp.MatchString(`[^:0-9\s]`, string(char))
+			if !startOutput {
+				originalString = originalString + string(char)
+				continue MainLoop
+			}
+			newline = false
+		}
+
+		switch char {
+		// Found a line continuation character
+		case '+', '-':
+			lineContinuation = true
+			foundGap = false
+			originalString = originalString + string(char)
+			continue MainLoop
+
+		// Found whitespace/new line
+		case '\n':
+			checkComment = true
+			newline = true
+			fallthrough
+		case '\t', '\r', ' ':
+			if !lineContinuation {
+				foundGap = true
+			}
+			originalString = originalString + string(char)
+			continue MainLoop
+
+		// Found a paramter value
+		case '(':
+			parameterNext = true
+			/* Do not continue as we need to do some checks */
+
+		// Found a comment, parse in a special manner
+		case '*':
+			if checkComment {
+				originalString = originalString + string(char)
+				// Loop round until we find the new line character that marks the end of the comment
+			CommentLoop:
+				for {
+					char, _, err := r.ReadRune()
+					if err == io.EOF {
+						if originalString != "" {
+							returnStr = returnStr + originalString
+						}
+						break MainLoop
+					} else if err != nil {
+						return returnStr, err
+					}
+					originalString = originalString + string(char)
+
+					if char == '\n' {
+						break CommentLoop
+					}
+
+				}
+
+				//Comment has been read and added to original string, go back to start
+				checkComment = true
+				newline = true
+				continue MainLoop
+			}
+			/* Do not continue as we need to do some checks */
+
+		} //end of switch
+
+		checkComment = false
+
+		if lineContinuation {
+			lineContinuation = false
+		}
+		if foundGap || parameterNext {
+			// we've completed an parameter so check whether it is sensitive
+			currentVerb = strings.ToUpper(currentVerb)
+
+			if isSensitiveCommand(currentVerb) {
+				redacting = true
+			}
+
+			// Add the unedited string to the return string
+			returnStr = returnStr + originalString
+
+			//reset some of the parameters
+			originalString = ""
+			currentVerb = ""
+			foundGap = false
+			lineContinuation = false
+		}
+
+		originalString = originalString + string(char)
+		currentVerb = currentVerb + string(char)
+	}
+
+	return returnStr, nil
+}
+
+// isSensitiveCommand checks whether the given string contains a sensitive parameter.
+// We use contains here because we can't determine whether a line continuation seperates
+// parts of a parameter or two different parameters.
+func isSensitiveCommand(command string) bool {
+	for _, v := range sensitiveParameters {
+		if strings.Contains(command, v) {
+			return true
+		}
+	}
+
+	return false
+}

internal/mqscredact/mqscredact_test.go

@@ -0,0 +1,171 @@
+/*
+© 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 mqscredact
+
+import (
+	"strings"
+	"testing"
+)
+
+const passwordString = passwordHalf1 + passwordHalf2
+const passwordHalf1 = "hippo"
+const passwordHalf2 = "123456"
+
+var testStrings = [...]string{
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD('" + passwordString + "')",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD(\"" + passwordString + "\")",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD     ('" + passwordString + "')",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD\t\t('" + passwordString + "')",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) ldappwd('" + passwordString + "')",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LdApPwD('" + passwordString + "')",
+	"DEFINE CHANNEL(CHL) CHLTYPE(SOMETHING) PASSWORD('" + passwordString + "')",
+	"DEFINE CHANNEL(CHL) CHLTYPE(SOMETHING) PASSWORD(\"" + passwordString + "\")",
+	"DEFINE CHANNEL(CHL) CHLTYPE(SOMETHING) PASSWORD     ('" + passwordString + "')",
+	"DEFINE CHANNEL(CHL) CHLTYPE(SOMETHING) PASSWORD\t\t('" + passwordString + "')",
+	"DEFINE CHANNEL(CHL) CHLTYPE(SOMETHING) password('" + passwordString + "')",
+	"DEFINE CHANNEL(CHL) CHLTYPE(SOMETHING) pAsSwOrD('" + passwordString + "')",
+	"ALTER QMGR SSLCRYP('" + passwordString + "')",
+	"ALTER QMGR SSLCRYP(\"" + passwordString + "\")",
+	"ALTER QMGR SSLCRYP     ('" + passwordString + "')",
+	"ALTER QMGR SSLCRYP\t\t('" + passwordString + "')",
+	"ALTER QMGR sslcryp('" + passwordString + "')",
+	"ALTER QMGR sslCRYP('" + passwordString + "')",
+
+	// Line continuation ones
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD(\"" + passwordHalf1 + "+\n " + passwordHalf2 + "\")",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD(\"" + passwordHalf1 + "+\n\t" + passwordHalf2 + "\")",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD(\"" + passwordHalf1 + "+\n\t   " + passwordHalf2 + "\")",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD('" + passwordHalf1 + "+\n " + passwordHalf2 + "')",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD('" + passwordHalf1 + "+\n\t" + passwordHalf2 + "')",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD('" + passwordHalf1 + "+\n\t   " + passwordHalf2 + "')",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD(\"" + passwordHalf1 + "-\n" + passwordHalf2 + "\")",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD('" + passwordHalf1 + "-\n" + passwordHalf2 + "')",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD(\"" + passwordHalf1 + "+  \n " + passwordHalf2 + "\")",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD(\"" + passwordHalf1 + "+\t\n " + passwordHalf2 + "\")",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD(\"" + passwordHalf1 + "-  \n" + passwordHalf2 + "\")",
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD(\"" + passwordHalf1 + "-\t\n" + passwordHalf2 + "\")",
+
+	"DEFINE CHANNEL(CHL) CHLTYPE(SOMETHING) PASSWORD(\"" + passwordHalf1 + "+\n " + passwordHalf2 + "\")",
+
+	"ALTER QMGR SSLCRYP(\"" + passwordHalf1 + "+\n " + passwordHalf2 + "\")",
+
+	//edge cases
+	"ALTER QMGR SSLCRYP(\"" + passwordHalf1 + "+\n 123+\n 456\")",
+	"ALTER QMGR SSLCRYP(\"" + passwordHalf1 + "-\n123-\n456\")",
+
+	"ALTER QMGR SSLCRYP(\"" + passwordHalf1 + "+\n 1+\n 2+\n 3+\n   4+\n  5+\n  6\")",
+	"ALTER QMGR SSLCRYP(\"" + passwordHalf1 + "-\n1-\n2-\n3-\n4-\n5-\n6\")",
+
+	"ALTER QMGR SSLCRYP  + \n  (\"" + passwordHalf1 + "+\n 1+\n 2+\n 3+\n   4+\n  5+\n  6\")",
+	"ALTER QMGR SSLCRYP  -  \n(\"" + passwordHalf1 + "-\n1-\n2-\n3-\n4-\n5-\n6\")",
+
+	"ALTER QMGR SSL  +  \n    CRYP(\"" + passwordHalf1 + "+\n 1+\n 2+\n 3+\n   4+\n  5+\n  6\")",
+	"ALTER QMGR SSL  -     \nCRYP(\"" + passwordHalf1 + "-\n1-\n2-\n3-\n4-\n5-\n6\")",
+
+	"ALTER QMGR +   \n   SSL +\n CRYP(\"" + passwordHalf1 + "+\n 1+\n 2+\n 3+\n   4+\n  5+\n  6\") +\n TEST(1234)",
+	"ALTER QMGR    -\nSSL -\nCRYP(\"" + passwordHalf1 + "-\n1-\n2-\n3-\n4-\n5-\n6\") -\nTEST(1234)",
+
+	"ALTER QMGR +\n * COMMENT\n SSL +\n * COMMENT IN MIDDLE\n CRYP('" + passwordString + "')",
+
+	" 1: ALTER CHANNEL(TEST2) CHLTYPE(SDR) PASS+\n   : *test comment\n   : WORD('" + passwordString + "')",
+	" 2: ALTER CHANNEL(TEST3) CHLTYPE(SDR) PASSWORD('" + passwordHalf1 + "-\n*commentinmiddle with ' \n" + passwordHalf2 + "')",
+	" 3: ALTER CHANNEL(TEST3) CHLTYPE(SDR) PASSWORD('" + passwordHalf1 + "-\n*commentinmiddle with ') \n" + passwordHalf2 + "')",
+}
+
+var expected = [...]string{
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD" + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD" + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD     " + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD\t\t" + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) ldappwd" + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LdApPwD" + redactionString,
+	"DEFINE CHANNEL(CHL) CHLTYPE(SOMETHING) PASSWORD" + redactionString,
+	"DEFINE CHANNEL(CHL) CHLTYPE(SOMETHING) PASSWORD" + redactionString,
+	"DEFINE CHANNEL(CHL) CHLTYPE(SOMETHING) PASSWORD     " + redactionString,
+	"DEFINE CHANNEL(CHL) CHLTYPE(SOMETHING) PASSWORD\t\t" + redactionString,
+	"DEFINE CHANNEL(CHL) CHLTYPE(SOMETHING) password" + redactionString,
+	"DEFINE CHANNEL(CHL) CHLTYPE(SOMETHING) pAsSwOrD" + redactionString,
+	"ALTER QMGR SSLCRYP" + redactionString,
+	"ALTER QMGR SSLCRYP" + redactionString,
+	"ALTER QMGR SSLCRYP     " + redactionString,
+	"ALTER QMGR SSLCRYP\t\t" + redactionString,
+	"ALTER QMGR sslcryp" + redactionString,
+	"ALTER QMGR sslCRYP" + redactionString,
+
+	// Line continuation ones
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD" + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD" + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD" + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD" + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD" + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD" + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD" + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD" + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD" + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD" + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD" + redactionString,
+	"DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD" + redactionString,
+
+	"DEFINE CHANNEL(CHL) CHLTYPE(SOMETHING) PASSWORD" + redactionString,
+
+	"ALTER QMGR SSLCRYP" + redactionString,
+
+	//edge cases
+	"ALTER QMGR SSLCRYP" + redactionString,
+	"ALTER QMGR SSLCRYP" + redactionString,
+
+	"ALTER QMGR SSLCRYP" + redactionString,
+	"ALTER QMGR SSLCRYP" + redactionString,
+
+	"ALTER QMGR SSLCRYP  + \n    \t  " + redactionString,
+	"ALTER QMGR SSLCRYP  -  \n    " + redactionString,
+
+	"ALTER QMGR SSL  +  \n    CRYP" + redactionString,
+	"ALTER QMGR SSL  -     \nCRYP" + redactionString,
+
+	"ALTER QMGR +   \n   SSL +\n CRYP" + redactionString + " +\n TEST(1234)",
+	"ALTER QMGR    -\nSSL -\nCRYP" + redactionString + " -\nTEST(1234)",
+
+	"ALTER QMGR +\n * COMMENT\n SSL +\n * COMMENT IN MIDDLE\n CRYP" + redactionString,
+
+	"1: ALTER CHANNEL(TEST2) CHLTYPE(SDR) PASS+\n   : *test comment\n   : WORD" + redactionString,
+	"2: ALTER CHANNEL(TEST3) CHLTYPE(SDR) PASSWORD" + redactionString,
+	"3: ALTER CHANNEL(TEST3) CHLTYPE(SDR) PASSWORD" + redactionString,
+}
+
+// Returns true if the 2 strings are equal ignoring whitespace characters
+func compareIgnoreWhiteSpace(str1, str2 string) bool {
+	whiteSpaces := [...]string{" ", "\t", "\n", "\r"}
+	for _, w := range whiteSpaces {
+		str1 = strings.Replace(str1, w, "", -1)
+		str2 = strings.Replace(str2, w, "", -1)
+	}
+
+	return str1 == str2
+}
+
+func TestAll(t *testing.T) {
+	for i, v := range testStrings {
+		back, _ := Redact(v)
+		if strings.Contains(back, passwordHalf1) || strings.Contains(back, passwordHalf2) || strings.Contains(back, passwordString) {
+			t.Errorf("MAJOR FAIL[%d]: Found an instance of the password. ", i)
+		}
+
+		if !compareIgnoreWhiteSpace(back, expected[i]) {
+			t.Errorf("FAIL[%d]:\nGave    :%s\nexpected:%s\ngot     :%s", i, v, expected[i], back)
+		}
+	}
+}

test/docker/docker_api_test.go

@@ -298,7 +298,6 @@ func TestNoVolumeWithRestart(t *testing.T) {
 // where `runmqserver -i` is run to initialize the storage.  Then the
 // container can be run as normal.
 func TestVolumeRequiresRoot(t *testing.T) {
-
 	cli, err := client.NewEnvClient()
 	if err != nil {
 		t.Fatal(err)
@@ -598,9 +597,9 @@ func TestLargeMQSC(t *testing.T) {
 	}
 }
 
-// TestRedactMQSC creates a new image with a MQSC file that contains sensitive information, starts a container based
+// TestRedactValidMQSC creates a new image with a Valid MQSC file that contains sensitive information, starts a container based
 // on that image, and checks that the MQSC has been redacted in the logs.
-func TestRedactMQSC(t *testing.T) {
+func TestRedactValidMQSC(t *testing.T) {
 	t.Parallel()
 
 	cli, err := client.NewEnvClient()
@@ -608,11 +607,38 @@ func TestRedactMQSC(t *testing.T) {
 		t.Fatal(err)
 	}
 	var buf bytes.Buffer
-	sslcryp := "GSK_PKCS11=/usr/lib/pkcs11/PKCS11_API.so;token-label;token-password;SYMMETRIC_CIPHER_ON;"
+	passwords := "hippoman4567"
-	fmt.Fprintf(&buf, "*TEST-REDACT-MQSC: A(1) LDAPPWD(abcdefgh) B(2) PASSWORD(abcdefgh) C(3) SSLCRYP(%v) D(4)\n", sslcryp)
+	sslcryp := fmt.Sprintf("GSK_PKCS11=/usr/lib/pkcs11/PKCS11_API.so;token-label;%s;SYMMETRIC_CIPHER_ON;", passwords)
-	fmt.Fprintf(&buf, "*TEST-REDACT-MQSC: A(1) ldappwd(12345678) B(2) password(12345678) C(3) sslcryp(%v) D(4)\n", sslcryp)
+
-	fmt.Fprintf(&buf, "*TEST-REDACT-MQSC: A(1) LdapPwd('12?@!$Gh') B(2) Password('12?@!$Gh') C(3) SSLCryp(%v) D(4)\n", sslcryp)
+	/* LDAPPWD*/
-	fmt.Fprintf(&buf, "*TEST-REDACT-MQSC: A(1) LDAPPWD (abcdefgh) B(2) PASSWORD\t(abcdefgh) C(3) SSLCRYP \t (%v) D(4)", sslcryp)
+	fmt.Fprintf(&buf, "DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) CONNAME('test(24)') SHORTUSR('sn') LDAPUSER('user') LDAPPWD('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) ldappwd('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) lDaPpWd('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD \t('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) +\n LDAP+\n PWD('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) -\nLDAPP-\nWD('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) +\n*test comment\n LDAPP-\n*test comment2\nWD('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD(%v)\n", passwords)
+
+	/* PASSWORD */
+	fmt.Fprintf(&buf, "DEFINE CHANNEL(TEST2) CHLTYPE(SDR) CONNAME('test(24)') XMITQ('fake') PASSWORD('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER CHANNEL(TEST2) CHLTYPE(SDR) password('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER CHANNEL(TEST2) CHLTYPE(SDR) pAsSwOrD('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER CHANNEL(TEST2) CHLTYPE(SDR) PASSWORD \t('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER CHANNEL(TEST2) +\n CHLTYPE(SDR) PASS+\n WORD+\n ('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER CHANNEL(TEST2) -\nCHLTYPE(SDR) PASS-\nWORD-\n('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER CHANNEL(TEST2) +\n CHLTYPE(SDR) PASS-\n*comemnt 2\nWORD+\n*test comment\n ('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER CHANNEL(TEST2) CHLTYPE(SDR) PASSWORD(%s)\n", passwords)
+
+	/* SSLCRYP */
+	fmt.Fprintf(&buf, "ALTER QMGR SSLCRYP('%v')\n", sslcryp)
+	fmt.Fprintf(&buf, "ALTER QMGR sslcryp('%v')\n", sslcryp)
+	fmt.Fprintf(&buf, "ALTER QMGR SsLcRyP('%v')\n", sslcryp)
+	fmt.Fprintf(&buf, "ALTER QMGR SSLCRYP \t('%v')\n", sslcryp)
+	fmt.Fprintf(&buf, "ALTER QMGR +\n SSL+\n CRYP+\n ('%v')\n", sslcryp)
+	fmt.Fprintf(&buf, "ALTER QMGR -\nSSLC-\nRYP-\n('%v')\n", sslcryp)
+	fmt.Fprintf(&buf, "ALTER QMGR +\n*commenttime\n SSL-\n*commentagain\nCRYP+\n*last comment\n ('%v')\n", sslcryp)
+
 	var files = []struct {
 		Name, Body string
 	}{
@@ -637,10 +663,83 @@ func TestRedactMQSC(t *testing.T) {
 	waitForReady(t, cli, id)
 	stopContainer(t, cli, id)
 	scanner := bufio.NewScanner(strings.NewReader(inspectLogs(t, cli, id)))
-	expectedOutput := "*TEST-REDACT-MQSC: A(1) LDAPPWD(*********) B(2) PASSWORD(*********) C(3) SSLCRYP(*********) D(4)"
 	for scanner.Scan() {
 		s := scanner.Text()
-		if strings.Contains(s, "*TEST-REDACT-MQSC:") && !strings.Contains(s, expectedOutput) {
+		if strings.Contains(s, sslcryp) || strings.Contains(s, passwords) {
+			t.Fatalf("Expected redacted MQSC output, got: %v", s)
+		}
+	}
+	err = scanner.Err()
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+// TestRedactValidMQSC creates a new image with a Invalid MQSC file that contains sensitive information, starts a container based
+// on that image, and checks that the MQSC has been redacted in the logs.
+func TestRedactInvalidMQSC(t *testing.T) {
+	t.Parallel()
+
+	cli, err := client.NewEnvClient()
+	if err != nil {
+		t.Fatal(err)
+	}
+	var buf bytes.Buffer
+	passwords := "hippoman4567"
+	sslcryp := fmt.Sprintf("GSK_PKCS11=/usr/lib/pkcs11/PKCS11_API.so;token-label;%s;SYMMETRIC_CIPHER_ON;", passwords)
+
+	/* LDAPPWD*/
+	fmt.Fprintf(&buf, "DEFINE AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) CONNAME('test(24)') SHORTUSR('sn') LDAPUSER('user') LDAPPWD('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPPPPPP('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD['%v']\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(ARGHHH) LDAPPWD('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) LDAPPWD('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) ARGHAHA(IDPWLDAP) LDAPPWD('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD '%v'\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD('%v') badvalues\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) badvales LDAPPWD('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD{'%v'}\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD<'%v'>\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD('%v'+\n p['il6])\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) AUTHTYPE(IDPWLDAP) LDAPPWD('%v'/653***)\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) LDAPPWD('%v'\n DISPLAY QMGR", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) LDAPPWD('%v💩')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) LDAPPWD💩('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) LDAP+\n 💩PWD('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) 💩 LDAPPWD('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) LDAPPWD 💩 ('%v')\n", passwords)
+	fmt.Fprintf(&buf, "ALTER AUTHINFO(TEST) LDAPPWD('%v') 💩\n", passwords)
+	fmt.Fprintf(&buf, "ALTER 💩 AUTHINFO(TEST) LDAPPWD('%v')\n", passwords)
+
+	var files = []struct {
+		Name, Body string
+	}{
+		{"Dockerfile", fmt.Sprintf(`
+		  FROM %v
+		  USER root
+		  RUN rm -f /etc/mqm/*.mqsc
+		  ADD test.mqsc /etc/mqm/
+		  RUN chmod 0660 /etc/mqm/test.mqsc
+		  USER mqm`, imageName())},
+		{"test.mqsc", buf.String()},
+	}
+	tag := createImage(t, cli, files)
+	defer deleteImage(t, cli, tag)
+
+	containerConfig := container.Config{
+		Env:   []string{"LICENSE=accept", "MQ_QMGR_NAME=qm1"},
+		Image: tag,
+	}
+	id := runContainer(t, cli, &containerConfig)
+	defer cleanContainer(t, cli, id)
+	rc := waitForContainer(t, cli, id, 20*time.Second)
+	if rc != 1 {
+		t.Errorf("Expected rc=1, got rc=%v", rc)
+	}
+	scanner := bufio.NewScanner(strings.NewReader(inspectLogs(t, cli, id)))
+	for scanner.Scan() {
+		s := scanner.Text()
+		if strings.Contains(s, sslcryp) || strings.Contains(s, passwords) {
 			t.Fatalf("Expected redacted MQSC output, got: %v", s)
 		}
 	}
@@ -652,39 +751,39 @@ func TestRedactMQSC(t *testing.T) {
 
 // TestInvalidMQSC creates a new image with an MQSC file containing invalid MQSC,
 // tries to start a container based on that image, and checks that container terminates
-// func TestInvalidMQSC(t *testing.T) {
+func TestInvalidMQSC(t *testing.T) {
-// 	t.Parallel()
+	t.Parallel()
-// 	cli, err := client.NewEnvClient()
+	cli, err := client.NewEnvClient()
-// 	if err != nil {
+	if err != nil {
-// 		t.Fatal(err)
+		t.Fatal(err)
-// 	}
+	}
-// 	var files = []struct {
+	var files = []struct {
-// 		Name, Body string
+		Name, Body string
-// 	}{
+	}{
-// 		{"Dockerfile", fmt.Sprintf(`
+		{"Dockerfile", fmt.Sprintf(`
-// 		FROM %v
+		FROM %v
-// 		USER root
+		USER root
-// 		RUN rm -f /etc/mqm/*.mqsc
+		RUN rm -f /etc/mqm/*.mqsc
-// 		ADD mqscTest.mqsc /etc/mqm/
+		ADD mqscTest.mqsc /etc/mqm/
-// 		RUN chmod 0660 /etc/mqm/mqscTest.mqsc
+		RUN chmod 0660 /etc/mqm/mqscTest.mqsc
-// 		USER mqm`, imageName())},
+		USER mqm`, imageName())},
-// 		{"mqscTest.mqsc", "DEFINE INVALIDLISTENER('TEST.LISTENER.TCP') TRPTYPE(TCP) PORT(1414) CONTROL(QMGR) REPLACE"},
+		{"mqscTest.mqsc", "DEFINE INVALIDLISTENER('TEST.LISTENER.TCP') TRPTYPE(TCP) PORT(1414) CONTROL(QMGR) REPLACE"},
-// 	}
+	}
-// 	tag := createImage(t, cli, files)
+	tag := createImage(t, cli, files)
-// 	defer deleteImage(t, cli, tag)
+	defer deleteImage(t, cli, tag)
 
-// 	containerConfig := container.Config{
+	containerConfig := container.Config{
-// 		Env:   []string{"LICENSE=accept", "MQ_QMGR_NAME=qm1"},
+		Env:   []string{"LICENSE=accept", "MQ_QMGR_NAME=qm1"},
-// 		Image: tag,
+		Image: tag,
-// 	}
+	}
-// 	id := runContainer(t, cli, &containerConfig)
+	id := runContainer(t, cli, &containerConfig)
-// 	defer cleanContainer(t, cli, id)
+	defer cleanContainer(t, cli, id)
-// 	rc := waitForContainer(t, cli, id, 60*time.Second)
+	rc := waitForContainer(t, cli, id, 60*time.Second)
-// 	if rc != 1 {
+	if rc != 1 {
-// 		t.Errorf("Expected rc=1, got rc=%v", rc)
+		t.Errorf("Expected rc=1, got rc=%v", rc)
-// 	}
+	}
-// 	expectTerminationMessage(t, cli, id)
+	expectTerminationMessage(t, cli, id)
-// }
+}
 
 // TestReadiness creates a new image with large amounts of MQSC in, to
 // ensure that the readiness check doesn't pass until configuration has finished.