You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by jz...@apache.org on 2013/01/25 04:02:31 UTC
[1/3] git commit: CLOUDSTACK-505: Reworked approach to cleaning
request / response strings
CLOUDSTACK-505: Reworked approach to cleaning request / response strings
As noted in the bug, several of the API command in question
are async calls. I've added a simple regex-based string cleaning
function, and have the request and response strings running through
it prior to being appended to the audit log.
Unit tests added for the new cleaning function as well.
The call to skip logging the createSSHKeyPair response remains intact
for now, although it should probably be scrubbed similarly to the
password fields.
Signed-off-by: Chip Childers <ch...@gmail.com>
Conflicts:
utils/src/com/cloud/utils/StringUtils.java
Project: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/commit/d5c9ad0f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/d5c9ad0f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/d5c9ad0f
Branch: refs/heads/4.0
Commit: d5c9ad0f94f0b42a47ff54c0e7481cc035f8d1e6
Parents: 7d37e56
Author: Chip Childers <ch...@gmail.com>
Authored: Mon Dec 17 13:26:40 2012 -0500
Committer: Joe Brockmeier <jz...@zonker.net>
Committed: Thu Jan 24 19:20:02 2013 -0600
----------------------------------------------------------------------
server/src/com/cloud/api/ApiServer.java | 21 ++-----
utils/src/com/cloud/utils/StringUtils.java | 12 ++++
utils/test/com/cloud/utils/StringUtilsTest.java | 56 ++++++++++++++++++
3 files changed, 73 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/d5c9ad0f/server/src/com/cloud/api/ApiServer.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/api/ApiServer.java b/server/src/com/cloud/api/ApiServer.java
index 52ae314..8a5d3de 100755
--- a/server/src/com/cloud/api/ApiServer.java
+++ b/server/src/com/cloud/api/ApiServer.java
@@ -108,6 +108,7 @@ import com.cloud.user.UserVO;
import com.cloud.utils.IdentityProxy;
import com.cloud.utils.Pair;
import com.cloud.utils.PropertiesUtil;
+import com.cloud.utils.StringUtils;
import com.cloud.utils.component.ComponentLocator;
import com.cloud.utils.component.PluggableService;
import com.cloud.utils.concurrency.NamedThreadFactory;
@@ -312,7 +313,7 @@ public class ApiServer implements HttpRequestHandler {
InetAddress remoteAddr = ((SocketHttpServerConnection) connObj).getRemoteAddress();
sb.append(remoteAddr.toString() + " -- ");
}
- sb.append(request.getRequestLine());
+ sb.append(StringUtils.cleanString(request.getRequestLine().toString()));
try {
String uri = request.getRequestLine().getUri();
@@ -590,25 +591,13 @@ public class ApiServer implements HttpRequestHandler {
return;
}
auditTrailSb.append(" " + HttpServletResponse.SC_OK + " ");
- if (command.equals("createSSHKeyPair") || command.equals("deployVirtualMachine") || command.equals("resetPasswordForVirtualMachine")){
+ if (command.equals("createSSHKeyPair")){
auditTrailSb.append("This result was not logged because it contains sensitive data.");
} else {
- auditTrailSb.append(result);
+ auditTrailSb.append(StringUtils.cleanString(result));
}
- /*
- * if (command.equals("queryAsyncJobResult")){ //For this command we need to also log job status and job
- * resultcode for
- * (Pair<String,Object> pair : resultValues){ String key = pair.first(); if (key.equals("jobstatus")){
- * auditTrailSb.append(" "); auditTrailSb.append(key); auditTrailSb.append("=");
- * auditTrailSb.append(pair.second());
- * }else if (key.equals("jobresultcode")){ auditTrailSb.append(" "); auditTrailSb.append(key);
- * auditTrailSb.append("=");
- * auditTrailSb.append(pair.second()); } } }else { for (Pair<String,Object> pair : resultValues){ if
- * (pair.first().equals("jobid")){ // Its an async job so report the jobid auditTrailSb.append(" ");
- * auditTrailSb.append(pair.first()); auditTrailSb.append("="); auditTrailSb.append(pair.second()); } } }
- */
}
-
+
private static boolean isCommandAvailable(String commandName) {
boolean isCommandAvailable = false;
isCommandAvailable = s_allCommands.contains(commandName);
http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/d5c9ad0f/utils/src/com/cloud/utils/StringUtils.java
----------------------------------------------------------------------
diff --git a/utils/src/com/cloud/utils/StringUtils.java b/utils/src/com/cloud/utils/StringUtils.java
index 0f0ef05..f0acc23 100644
--- a/utils/src/com/cloud/utils/StringUtils.java
+++ b/utils/src/com/cloud/utils/StringUtils.java
@@ -128,4 +128,16 @@ public class StringUtils {
return sb.toString();
}
+
+ // Responsible for stripping sensitive content from request and response strings
+ public static String cleanString(String stringToClean){
+ String cleanResult = "";
+ // removes a password request param and it's value
+ cleanResult = stringToClean.replaceAll("password=.*?&", "");
+ // removes a password property from a response json object
+ cleanResult = cleanResult.replaceAll("\"password\":\".*?\",", "");
+ return cleanResult;
+ }
+
+
}
http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/d5c9ad0f/utils/test/com/cloud/utils/StringUtilsTest.java
----------------------------------------------------------------------
diff --git a/utils/test/com/cloud/utils/StringUtilsTest.java b/utils/test/com/cloud/utils/StringUtilsTest.java
new file mode 100644
index 0000000..f25db97
--- /dev/null
+++ b/utils/test/com/cloud/utils/StringUtilsTest.java
@@ -0,0 +1,56 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// 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 com.cloud.utils;
+
+import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+import com.cloud.utils.StringUtils;
+
+public class StringUtilsTest {
+ @Test
+ public void testCleanJsonObject() {
+ String input = "{\"description\":\"foo\"}],\"password\":\"bar\",\"nic\":[{\"id\":\"1\"}]}";
+ String expected = "{\"description\":\"foo\"}],\"nic\":[{\"id\":\"1\"}]}";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
+
+ @Test
+ public void testCleanJsonObjectWithMultiplePasswords() {
+ String input = "{\"description\":\"foo\"}],\"password\":\"bar\",\"nic\":[{\"password\":\"bar2\",\"id\":\"1\"}]}";
+ String expected = "{\"description\":\"foo\"}],\"nic\":[{\"id\":\"1\"}]}";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
+
+ @Test
+ public void testCleanRequestObject() {
+ String input = "username=foo&password=bar&url=foobar";
+ String expected = "username=foo&url=foobar";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
+
+ @Test
+ public void testCleanRequestObjectWithMultiplePasswords() {
+ String input = "username=foo&password=bar&url=foobar&password=bar2&test=4";
+ String expected = "username=foo&url=foobar&test=4";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
+
+}