cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mc...@apache.org
Subject git commit: updated refs/heads/4.2 to ce6a5d7
Date Sun, 21 Jul 2013 00:31:02 GMT
Updated Branches:
  refs/heads/4.2 25b33ec80 -> ce6a5d718


CLOUDSTACK-3274: API Refactoring: secretkey and accesskey of the backing
store is found in plaintext in the logs.


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/ce6a5d71
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/ce6a5d71
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/ce6a5d71

Branch: refs/heads/4.2
Commit: ce6a5d7183c46ab77a089afb7436f407bca89235
Parents: 25b33ec
Author: Min Chen <min.chen@citrix.com>
Authored: Sat Jul 20 17:27:23 2013 -0700
Committer: Min Chen <min.chen@citrix.com>
Committed: Sat Jul 20 17:30:53 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/ce6a5d71/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/ce6a5d71/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 86b4cdd..a1841af 100755
--- a/server/src/com/cloud/api/ApiServer.java
+++ b/server/src/com/cloud/api/ApiServer.java
@@ -184,14 +184,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();
@@ -288,12 +288,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
@@ -313,7 +314,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler,
ApiSer
                 throw e;
             }
         } finally {
-            s_accessLogger.info(sb.toString());
+            s_accessLogger.info(StringUtils.cleanString(sb.toString()));
             UserContext.unregisterContext();
         }
     }
@@ -365,7 +366,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/ce6a5d71/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 7525b61..fa02041 100755
--- a/server/src/com/cloud/api/ApiServlet.java
+++ b/server/src/com/cloud/api/ApiServlet.java
@@ -58,9 +58,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);
@@ -72,8 +72,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) {
@@ -318,14 +319,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/ce6a5d71/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/ce6a5d71/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);
+    }
 }


Mime
View raw message