Skip to content

Commit 44da7b1

Browse files
chipchilderschipchilders
committed
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>
1 parent d6694ab commit 44da7b1

3 files changed

Lines changed: 72 additions & 16 deletions

File tree

‎server/src/com/cloud/api/ApiServer.java‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@
108108
import com.cloud.utils.IdentityProxy;
109109
import com.cloud.utils.Pair;
110110
import com.cloud.utils.PropertiesUtil;
111+
import com.cloud.utils.StringUtils;
111112
import com.cloud.utils.component.ComponentLocator;
112113
import com.cloud.utils.component.PluggableService;
113114
import com.cloud.utils.concurrency.NamedThreadFactory;
@@ -313,7 +314,7 @@ public void handle(HttpRequest request, HttpResponse response, HttpContext conte
313314
InetAddress remoteAddr = ((SocketHttpServerConnection) connObj).getRemoteAddress();
314315
sb.append(remoteAddr.toString() + " -- ");
315316
}
316-
sb.append(request.getRequestLine());
317+
sb.append(StringUtils.cleanString(request.getRequestLine().toString()));
317318

318319
try {
319320
String uri = request.getRequestLine().getUri();
@@ -586,25 +587,13 @@ private void buildAuditTrail(StringBuffer auditTrailSb, String command, String r
586587
return;
587588
}
588589
auditTrailSb.append(" " + HttpServletResponse.SC_OK + " ");
589-
if (command.equals("createSSHKeyPair") || command.equals("deployVirtualMachine") || command.equals("resetPasswordForVirtualMachine")){
590+
if (command.equals("createSSHKeyPair")){
590591
auditTrailSb.append("This result was not logged because it contains sensitive data.");
591592
} else {
592-
auditTrailSb.append(result);
593+
auditTrailSb.append(StringUtils.cleanString(result));
593594
}
594-
/*
595-
* if (command.equals("queryAsyncJobResult")){ //For this command we need to also log job status and job
596-
* resultcode for
597-
* (Pair<String,Object> pair : resultValues){ String key = pair.first(); if (key.equals("jobstatus")){
598-
* auditTrailSb.append(" "); auditTrailSb.append(key); auditTrailSb.append("=");
599-
* auditTrailSb.append(pair.second());
600-
* }else if (key.equals("jobresultcode")){ auditTrailSb.append(" "); auditTrailSb.append(key);
601-
* auditTrailSb.append("=");
602-
* auditTrailSb.append(pair.second()); } } }else { for (Pair<String,Object> pair : resultValues){ if
603-
* (pair.first().equals("jobid")){ // Its an async job so report the jobid auditTrailSb.append(" ");
604-
* auditTrailSb.append(pair.first()); auditTrailSb.append("="); auditTrailSb.append(pair.second()); } } }
605-
*/
606595
}
607-
596+
608597
private static boolean isCommandAvailable(String commandName) {
609598
boolean isCommandAvailable = false;
610599
isCommandAvailable = s_allCommands.contains(commandName);

‎utils/src/com/cloud/utils/StringUtils.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,5 +134,16 @@ public static String getMaskedPasswordForDisplay(String password) {
134134

135135
return sb.toString();
136136
}
137+
138+
// Responsible for stripping sensitive content from request and response strings
139+
public static String cleanString(String stringToClean){
140+
String cleanResult = "";
141+
// removes a password request param and it's value
142+
cleanResult = stringToClean.replaceAll("password=.*?&", "");
143+
// removes a password property from a response json object
144+
cleanResult = cleanResult.replaceAll("\"password\":\".*?\",", "");
145+
return cleanResult;
146+
}
147+
137148

138149
}

‎utils/test/com/cloud/utils/StringUtilsTest.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package com.cloud.utils;
18+
19+
import org.junit.Test;
20+
import static org.junit.Assert.assertEquals;
21+
import com.cloud.utils.StringUtils;
22+
23+
public class StringUtilsTest {
24+
@Test
25+
public void testCleanJsonObject() {
26+
String input = "{\"description\":\"foo\"}],\"password\":\"bar\",\"nic\":[{\"id\":\"1\"}]}";
27+
String expected = "{\"description\":\"foo\"}],\"nic\":[{\"id\":\"1\"}]}";
28+
String result = StringUtils.cleanString(input);
29+
assertEquals(result, expected);
30+
}
31+
32+
@Test
33+
public void testCleanJsonObjectWithMultiplePasswords() {
34+
String input = "{\"description\":\"foo\"}],\"password\":\"bar\",\"nic\":[{\"password\":\"bar2\",\"id\":\"1\"}]}";
35+
String expected = "{\"description\":\"foo\"}],\"nic\":[{\"id\":\"1\"}]}";
36+
String result = StringUtils.cleanString(input);
37+
assertEquals(result, expected);
38+
}
39+
40+
@Test
41+
public void testCleanRequestObject() {
42+
String input = "username=foo&password=bar&url=foobar";
43+
String expected = "username=foo&url=foobar";
44+
String result = StringUtils.cleanString(input);
45+
assertEquals(result, expected);
46+
}
47+
48+
@Test
49+
public void testCleanRequestObjectWithMultiplePasswords() {
50+
String input = "username=foo&password=bar&url=foobar&password=bar2&test=4";
51+
String expected = "username=foo&url=foobar&test=4";
52+
String result = StringUtils.cleanString(input);
53+
assertEquals(result, expected);
54+
}
55+
56+
}

0 commit comments

Comments
 (0)