From 44da7b1841446218fb363a809bd7fd7c02eec58f Mon Sep 17 00:00:00 2001 From: Chip Childers Date: Mon, 17 Dec 2012 13:26:40 -0500 Subject: [PATCH] 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 --- server/src/com/cloud/api/ApiServer.java | 21 ++----- utils/src/com/cloud/utils/StringUtils.java | 11 ++++ .../test/com/cloud/utils/StringUtilsTest.java | 56 +++++++++++++++++++ 3 files changed, 72 insertions(+), 16 deletions(-) create mode 100644 utils/test/com/cloud/utils/StringUtilsTest.java diff --git a/server/src/com/cloud/api/ApiServer.java b/server/src/com/cloud/api/ApiServer.java index 019931cea2a..f42025c1d23 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; @@ -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 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 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); diff --git a/utils/src/com/cloud/utils/StringUtils.java b/utils/src/com/cloud/utils/StringUtils.java index 17df3e10567..31b1a10c2d0 100644 --- a/utils/src/com/cloud/utils/StringUtils.java +++ b/utils/src/com/cloud/utils/StringUtils.java @@ -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; + } + } diff --git a/utils/test/com/cloud/utils/StringUtilsTest.java b/utils/test/com/cloud/utils/StringUtilsTest.java new file mode 100644 index 00000000000..f25db97ca13 --- /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); + } + +}