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 <chip.childers@gmail.com>
This commit is contained in:
Chip Childers 2012-12-17 13:26:40 -05:00
parent d6694abf4d
commit 44da7b1841
3 changed files with 72 additions and 16 deletions

View File

@ -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;
@ -313,7 +314,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();
@ -586,25 +587,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);

View File

@ -134,5 +134,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;
}
}

View File

@ -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);
}
}