etan39
/
cloudstack
mirror of https://github.com/apache/cloudstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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
This commit is contained in:
Min Chen 2013-07-20 18:01:49 -07:00
parent 98dd501a81
commit d423a755f5
5 changed files with 141 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
api/src/com/cloud/agent/api/to/S3TO.java
View File

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

25
server/src/com/cloud/api/ApiServer.java
View File

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

17
server/src/com/cloud/api/ApiServlet.java
View File

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

16
utils/src/com/cloud/utils/StringUtils.java
View File

@ -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){

104
utils/test/com/cloud/utils/StringUtilsTest.java
View File

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