CLOUDSTACK-5355: addImageStore should not log password in clear text in

minchen07 · minchen07 · commit f420b748903e · 2013-12-03T16:55:26.000-08:00
the log.
diff --git a/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackImageStoreLifeCycleImpl.java b/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackImageStoreLifeCycleImpl.java
@@ -26,6 +26,8 @@
 
 import org.apache.log4j.Logger;
 
+import com.ibm.wsdl.util.StringUtils;
+
 import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.HostScope;
@@ -84,13 +86,13 @@ public DataStore initialize(Map<String, Object> dsInfos) {
         DataStoreRole role = (DataStoreRole)dsInfos.get("role");
         Map<String, String> details = (Map<String, String>)dsInfos.get("details");
 
-        s_logger.info("Trying to add a new data store at " + url + " to data center " + dcId);
+        s_logger.info("Trying to add a new data store at " + StringUtils.cleanString(url) + " to data center " + dcId);
 
         URI uri = null;
         try {
             uri = new URI(UriUtils.encodeURIComponent(url));
             if (uri.getScheme() == null) {
-                throw new InvalidParameterValueException("uri.scheme is null " + url + ", add nfs:// (or cifs://) as a prefix");
+                throw new InvalidParameterValueException("uri.scheme is null " + StringUtils.cleanString(url) + ", add nfs:// (or cifs://) as a prefix");
             } else if (uri.getScheme().equalsIgnoreCase("nfs")) {
                 if (uri.getHost() == null || uri.getHost().equalsIgnoreCase("") || uri.getPath() == null || uri.getPath().equalsIgnoreCase("")) {
                     throw new InvalidParameterValueException("Your host and/or path is wrong.  Make sure it's of the format nfs://hostname/path");
diff --git a/utils/src/com/cloud/utils/StringUtils.java b/utils/src/com/cloud/utils/StringUtils.java
@@ -152,8 +152,8 @@ public static String getMaskedPasswordForDisplay(String password) {
         return sb.toString();
     }
 
-    // removes a password request param and it's value
-    private static final Pattern REGEX_PASSWORD_QUERYSTRING = Pattern.compile("&?(password|accesskey|secretkey)=.*?(?=[&'\"])");
+    // removes a password request param and it's value, also considering password is in query parameter value which has been url encoded
+    private static final Pattern REGEX_PASSWORD_QUERYSTRING = Pattern.compile("(&|%26)?(password|accesskey|secretkey)(=|%3D).*?(?=(%26|[&'\"]))");
 
     // removes a password/accesskey/ property from a response json object
     private static final Pattern REGEX_PASSWORD_JSON = Pattern.compile("\"(password|accesskey|secretkey)\":\".*?\",?");
diff --git a/utils/test/com/cloud/utils/StringUtilsTest.java b/utils/test/com/cloud/utils/StringUtilsTest.java
@@ -71,6 +71,14 @@ public void testCleanPasswordFromRequestString() {
         assertEquals(result, expected);
     }
 
+    @Test
+    public void testCleanPasswordFromEncodedRequestString() {
+        String input = "name=SS1&provider=SMB&zoneid=5a60af2b-3025-4f2a-9ecc-8e33bf2b94e3&url=cifs%3A%2F%2F10.102.192.150%2FSMB-Share%2Fsowmya%2Fsecondary%3Fuser%3Dsowmya%26password%3DXXXXX%40123%26domain%3DBLR";
+        String expected = "name=SS1&provider=SMB&zoneid=5a60af2b-3025-4f2a-9ecc-8e33bf2b94e3&url=cifs%3A%2F%2F10.102.192.150%2FSMB-Share%2Fsowmya%2Fsecondary%3Fuser%3Dsowmya%26domain%3DBLR";
+        String result = StringUtils.cleanString(input);
+        assertEquals(result, expected);
+    }
+
     @Test
     public void testCleanPasswordFromRequestStringWithMultiplePasswords() {
         String input = "username=foo&password=bar&url=foobar&password=bar2&test=4";

Original file line number	Diff line number	Diff line change
`@@ -152,8 +152,8 @@ public static String getMaskedPasswordForDisplay(String password) {`
`152`	`152`	`return sb.toString();`
`153`	`153`	`}`
`154`	`154`
`155`		`- // removes a password request param and it's value`
`156`		`- private static final Pattern REGEX_PASSWORD_QUERYSTRING = Pattern.compile("&?(password\|accesskey\|secretkey)=.*?(?=[&'\"])");`
	`155`	`+ // removes a password request param and it's value, also considering password is in query parameter value which has been url encoded`
	`156`	`+ private static final Pattern REGEX_PASSWORD_QUERYSTRING = Pattern.compile("(&\|%26)?(password\|accesskey\|secretkey)(=\|%3D).*?(?=(%26\|[&'\"]))");`
`157`	`157`
`158`	`158`	`// removes a password/accesskey/ property from a response json object`
`159`	`159`	`private static final Pattern REGEX_PASSWORD_JSON = Pattern.compile("\"(password\|accesskey\|secretkey)\":\".*?\",?");`