You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by mc...@apache.org on 2013/07/21 03:05:38 UTC
git commit: updated refs/heads/master to d423a75
Updated Branches:
refs/heads/master 98dd501a8 -> d423a755f
CLOUDSTACK-3274: API Refactoring: secretkey and accesskey of the backing
store is found in plaintext in the logs.
Conflicts:
server/src/com/cloud/api/ApiServer.java
server/src/com/cloud/api/ApiServlet.java
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/d423a755
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/d423a755
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/d423a755
Branch: refs/heads/master
Commit: d423a755f51814dcd5b09cb987704a1ef174b7f4
Parents: 98dd501
Author: Min Chen <mi...@citrix.com>
Authored: Sat Jul 20 18:01:49 2013 -0700
Committer: Min Chen <mi...@citrix.com>
Committed: Sat Jul 20 18:01:49 2013 -0700
----------------------------------------------------------------------
api/src/com/cloud/agent/api/to/S3TO.java | 10 +-
server/src/com/cloud/api/ApiServer.java | 25 ++---
server/src/com/cloud/api/ApiServlet.java | 17 +--
utils/src/com/cloud/utils/StringUtils.java | 16 +--
utils/test/com/cloud/utils/StringUtilsTest.java | 104 ++++++++++++++++++-
5 files changed, 141 insertions(+), 31 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d423a755/api/src/com/cloud/agent/api/to/S3TO.java
----------------------------------------------------------------------
diff --git a/api/src/com/cloud/agent/api/to/S3TO.java b/api/src/com/cloud/agent/api/to/S3TO.java
index 8a2f09d..b1b692a 100644
--- a/api/src/com/cloud/agent/api/to/S3TO.java
+++ b/api/src/com/cloud/agent/api/to/S3TO.java
@@ -18,6 +18,8 @@ package com.cloud.agent.api.to;
import java.util.Date;
+import com.cloud.agent.api.LogLevel;
+import com.cloud.agent.api.LogLevel.Log4jLevel;
import com.cloud.storage.DataStoreRole;
import com.cloud.utils.S3Utils;
@@ -25,7 +27,9 @@ public final class S3TO implements S3Utils.ClientOptions, DataStoreTO {
private Long id;
private String uuid;
+ @LogLevel(Log4jLevel.Off)
private String accessKey;
+ @LogLevel(Log4jLevel.Off)
private String secretKey;
private String endPoint;
private String bucketName;
@@ -68,10 +72,12 @@ public final class S3TO implements S3Utils.ClientOptions, DataStoreTO {
@Override
public boolean equals(final Object thatObject) {
- if (this == thatObject)
+ if (this == thatObject) {
return true;
- if (thatObject == null || getClass() != thatObject.getClass())
+ }
+ if (thatObject == null || getClass() != thatObject.getClass()) {
return false;
+ }
final S3TO thatS3TO = (S3TO) thatObject;
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d423a755/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 95f17af..57a4eaf 100755
--- a/server/src/com/cloud/api/ApiServer.java
+++ b/server/src/com/cloud/api/ApiServer.java
@@ -189,14 +189,14 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
public static ApiServer getInstance() {
return s_instance;
}
-
- @Override
- public boolean configure(String name, Map<String, Object> params)
- throws ConfigurationException {
- init();
- return true;
- }
-
+
+ @Override
+ public boolean configure(String name, Map<String, Object> params)
+ throws ConfigurationException {
+ init();
+ return true;
+ }
+
public void init() {
Integer apiPort = null; // api port, null by default
SearchCriteria<ConfigurationVO> sc = _configDao.createSearchCriteria();
@@ -293,12 +293,13 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
parameterMap.put(param.getName(), new String[] { param.getValue() });
}
- // Get the type of http method being used.
+ // Get the type of http method being used.
parameterMap.put("httpmethod", new String[] { request.getRequestLine().getMethod() });
// Check responseType, if not among valid types, fallback to JSON
- if (!(responseType.equals(BaseCmd.RESPONSE_TYPE_JSON) || responseType.equals(BaseCmd.RESPONSE_TYPE_XML)))
+ if (!(responseType.equals(BaseCmd.RESPONSE_TYPE_JSON) || responseType.equals(BaseCmd.RESPONSE_TYPE_XML))) {
responseType = BaseCmd.RESPONSE_TYPE_XML;
+ }
try {
// always trust commands from API port, user context will always be UID_SYSTEM/ACCOUNT_ID_SYSTEM
@@ -318,7 +319,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
throw e;
}
} finally {
- s_accessLogger.info(sb.toString());
+ s_accessLogger.info(StringUtils.cleanString(sb.toString()));
CallContext.unregister();
}
}
@@ -370,7 +371,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
cmdObj.configure();
cmdObj.setFullUrlParams(paramMap);
cmdObj.setResponseType(responseType);
- cmdObj.setHttpMethod(paramMap.get("httpmethod").toString());
+ cmdObj.setHttpMethod(paramMap.get("httpmethod").toString());
// This is where the command is either serialized, or directly dispatched
response = queueCommand(cmdObj, paramMap);
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d423a755/server/src/com/cloud/api/ApiServlet.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/api/ApiServlet.java b/server/src/com/cloud/api/ApiServlet.java
index 4b49ee4..22047ff 100755
--- a/server/src/com/cloud/api/ApiServlet.java
+++ b/server/src/com/cloud/api/ApiServlet.java
@@ -63,9 +63,9 @@ public class ApiServlet extends HttpServlet {
@Override
public void init(ServletConfig config) throws ServletException {
- SpringBeanAutowiringSupport.processInjectionBasedOnServletContext(this, config.getServletContext());
+ SpringBeanAutowiringSupport.processInjectionBasedOnServletContext(this, config.getServletContext());
}
-
+
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) {
processRequest(req, resp);
@@ -77,8 +77,9 @@ public class ApiServlet extends HttpServlet {
}
private void utf8Fixup(HttpServletRequest req, Map<String, Object[]> params) {
- if (req.getQueryString() == null)
+ if (req.getQueryString() == null) {
return;
+ }
String[] paramsInQueryString = req.getQueryString().split("&");
if (paramsInQueryString != null) {
@@ -325,14 +326,14 @@ public class ApiServlet extends HttpServlet {
}
} catch (ServerApiException se) {
String serializedResponseText = _apiServer.getSerializedApiError(se, params, responseType);
- resp.setHeader("X-Description", se.getDescription());
+ resp.setHeader("X-Description", se.getDescription());
writeResponse(resp, serializedResponseText, se.getErrorCode().getHttpCode(), responseType);
- auditTrailSb.append(" " + se.getErrorCode() + " " + se.getDescription());
+ auditTrailSb.append(" " + se.getErrorCode() + " " + se.getDescription());
} catch (Exception ex) {
- s_logger.error("unknown exception writing api response", ex);
- auditTrailSb.append(" unknown exception writing api response");
+ s_logger.error("unknown exception writing api response", ex);
+ auditTrailSb.append(" unknown exception writing api response");
} finally {
- s_accessLogger.info(auditTrailSb.toString());
+ s_accessLogger.info(StringUtils.cleanString(auditTrailSb.toString()));
if (s_logger.isDebugEnabled()) {
s_logger.debug("===END=== " + StringUtils.cleanString(reqStr));
}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d423a755/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 db32dd4..21c7220 100644
--- a/utils/src/com/cloud/utils/StringUtils.java
+++ b/utils/src/com/cloud/utils/StringUtils.java
@@ -91,10 +91,10 @@ public class StringUtils {
tags += ",";
}
}
- }
+ }
return tags;
- }
+ }
public static String getExceptionStackInfo(Throwable e) {
StringBuffer sb = new StringBuffer();
@@ -131,22 +131,24 @@ public class StringUtils {
}
public static String getMaskedPasswordForDisplay(String password) {
- if(password == null || password.isEmpty())
+ if(password == null || password.isEmpty()) {
return "*";
+ }
StringBuffer sb = new StringBuffer();
sb.append(password.charAt(0));
- for(int i = 1; i < password.length(); i++)
+ for(int i = 1; i < password.length(); i++) {
sb.append("*");
+ }
return sb.toString();
}
// removes a password request param and it's value
- private static final Pattern REGEX_PASSWORD_QUERYSTRING = Pattern.compile("&?password=.*?(?=[&'\"])");
+ private static final Pattern REGEX_PASSWORD_QUERYSTRING = Pattern.compile("&?(password|accesskey|secretkey)=.*?(?=[&'\"])");
- // removes a password property from a response json object
- private static final Pattern REGEX_PASSWORD_JSON = Pattern.compile("\"password\":\".*?\",?");
+ // removes a password/accesskey/ property from a response json object
+ private static final Pattern REGEX_PASSWORD_JSON = Pattern.compile("\"(password|accesskey|secretkey)\":\".*?\",?");
// Responsible for stripping sensitive content from request and response strings
public static String cleanString(String stringToClean){
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d423a755/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
index 796efba..ae37c24 100644
--- a/utils/test/com/cloud/utils/StringUtilsTest.java
+++ b/utils/test/com/cloud/utils/StringUtilsTest.java
@@ -24,7 +24,7 @@ public class StringUtilsTest {
@Test
public void testCleanPasswordFromJsonObjectAtEnd() {
String input = "{\"foo\":\"bar\",\"password\":\"test\"}";
- //TODO: It would be nice to clean up the regex in question to not
+ //TODO: It would be nice to clean up the regex in question to not
//have to return the trailing comma in the expected string below
String expected = "{\"foo\":\"bar\",}";
String result = StringUtils.cleanString(input);
@@ -78,7 +78,7 @@ public class StringUtilsTest {
String result = StringUtils.cleanString(input);
assertEquals(result, expected);
}
-
+
@Test
public void testCleanPasswordFromRequestStringMatchedAtEndSingleQuote() {
String input = "'username=foo&password=bar'";
@@ -108,4 +108,104 @@ public class StringUtilsTest {
assertEquals("a-b-c", StringUtils.join("-", "a", "b", "c"));
assertEquals("", StringUtils.join("-"));
}
+
+ @Test
+ public void testCleanSecretkeyFromJsonObjectAtEnd() {
+ String input = "{\"foo\":\"bar\",\"secretkey\":\"test\"}";
+ // TODO: It would be nice to clean up the regex in question to not
+ // have to return the trailing comma in the expected string below
+ String expected = "{\"foo\":\"bar\",}";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
+
+ @Test
+ public void testCleanSecretkeyFromJsonObjectInMiddle() {
+ String input = "{\"foo\":\"bar\",\"secretkey\":\"test\",\"test\":\"blah\"}";
+ String expected = "{\"foo\":\"bar\",\"test\":\"blah\"}";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
+
+ @Test
+ public void testCleanSecretkeyFromJsonObjectAlone() {
+ String input = "{\"secretkey\":\"test\"}";
+ String expected = "{}";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
+
+ @Test
+ public void testCleanSecretkeyFromJsonObjectAtStart() {
+ String input = "{\"secretkey\":\"test\",\"test\":\"blah\"}";
+ String expected = "{\"test\":\"blah\"}";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
+
+ @Test
+ public void testCleanSecretkeyFromJsonObjectWithMultiplePasswords() {
+ String input = "{\"description\":\"foo\"}],\"secretkey\":\"bar\",\"nic\":[{\"secretkey\":\"bar2\",\"id\":\"1\"}]}";
+ String expected = "{\"description\":\"foo\"}],\"nic\":[{\"id\":\"1\"}]}";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
+
+ @Test
+ public void testCleanAccesskeyFromJsonObjectAtEnd() {
+ String input = "{\"foo\":\"bar\",\"accesskey\":\"test\"}";
+ // TODO: It would be nice to clean up the regex in question to not
+ // have to return the trailing comma in the expected string below
+ String expected = "{\"foo\":\"bar\",}";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
+
+ @Test
+ public void testCleanAccesskeyFromJsonObjectInMiddle() {
+ String input = "{\"foo\":\"bar\",\"accesskey\":\"test\",\"test\":\"blah\"}";
+ String expected = "{\"foo\":\"bar\",\"test\":\"blah\"}";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
+
+ @Test
+ public void testCleanAccesskeyFromJsonObjectAlone() {
+ String input = "{\"accesskey\":\"test\"}";
+ String expected = "{}";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
+
+ @Test
+ public void testCleanAccesskeyFromJsonObjectAtStart() {
+ String input = "{\"accesskey\":\"test\",\"test\":\"blah\"}";
+ String expected = "{\"test\":\"blah\"}";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
+
+ @Test
+ public void testCleanAccesskeyFromJsonObjectWithMultiplePasswords() {
+ String input = "{\"description\":\"foo\"}],\"accesskey\":\"bar\",\"nic\":[{\"accesskey\":\"bar2\",\"id\":\"1\"}]}";
+ String expected = "{\"description\":\"foo\"}],\"nic\":[{\"id\":\"1\"}]}";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
+
+ @Test
+ public void testCleanAccesskeyFromRequestString() {
+ String input = "username=foo&accesskey=bar&url=foobar";
+ String expected = "username=foo&url=foobar";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
+
+ @Test
+ public void testCleanSecretkeyFromRequestString() {
+ String input = "username=foo&secretkey=bar&url=foobar";
+ String expected = "username=foo&url=foobar";
+ String result = StringUtils.cleanString(input);
+ assertEquals(result, expected);
+ }
}