Redact sensitive duplicate query parameter values in ApiServlet logs

server/src/main/java/com/cloud/api/ApiServlet.java

@@ -80,10 +80,12 @@
 
 @Component("apiServlet")
 public class ApiServlet extends HttpServlet {
-    protected static Logger LOGGER = LogManager.getLogger(ApiServlet.class);
-    private static final Logger ACCESSLOGGER = LogManager.getLogger("apiserver." + ApiServlet.class.getName());
-    private static final String REPLACEMENT = "_";
-    private static final String LOGGER_REPLACEMENTS = "[\n\r\t]";
+protected static Logger LOGGER = LogManager.getLogger(ApiServlet.class);
+private static final Logger ACCESSLOGGER = LogManager.getLogger("apiserver." + ApiServlet.class.getName());
+
+private static final String REPLACEMENT = "_";
+private static final String REDACTED = "REDACTED";
+private static final String LOGGER_REPLACEMENTS = "[\n\r\t]";
     public static final Pattern GET_REQUEST_COMMANDS = Pattern.compile("^(get|list|query|find)(\\w+)+$");
     private static final HashSet<String> POST_REQUESTS_TO_DISABLE_LOGGING = new HashSet<>(Set.of(
             "login",
@@ -197,12 +199,46 @@ private void checkSingleQueryParameterValue(Map<String, String[]> params) {
         params.forEach((k, v) -> {
             if (v.length > 1) {
                 String message = String.format("Query parameter '%s' has multiple values %s. Only the last value will be respected." +
-                    "It is advised to pass only a single parameter", k, Arrays.toString(v));
+                    "It is advised to pass only a single parameter", k, formatValuesForLog(k, v));
                 LOGGER.warn(message);
             }
         });
     }
 
+    private static final Set<String> SENSITIVE_PARAMETER_KEYWORDS = Set.of(
+        "password",
+        "privatekey",
+        "accesskey",
+        "secretkey",
+        "apikey",
+        "signature",
+        "sessionkey",
+        "token"
+    );
+
+    static boolean isSensitiveParameter(final String parameterName) {
+        if (parameterName == null) {
+            return false;
+        }
+
+        final String key = parameterName.toLowerCase(java.util.Locale.ROOT);
+
+        return SENSITIVE_PARAMETER_KEYWORDS.stream()
+            .anyMatch(key::contains);
+    }
+
+    static String formatValuesForLog(final String parameterName, final String[] values){
+        if (!isSensitiveParameter(parameterName)) {
+            return Arrays.toString(values);
+        }
+
+        final String[] masked = new String[values.length];
+
+        Arrays.fill(masked, REDACTED);
+
+        return Arrays.toString(masked);
+    }
+
     void processRequestInContext(final HttpServletRequest req, final HttpServletResponse resp) {
         InetAddress remoteAddress = null;
         try {
@@ -761,10 +797,7 @@ private String getCleanParamsString(Map<String, String[]> reqParams) {
             cleanParamsString.append(reqParam.getKey());
             cleanParamsString.append("=");
 
-            if (reqParam.getKey().toLowerCase().contains("password")
-                    || reqParam.getKey().toLowerCase().contains("privatekey")
-                    || reqParam.getKey().toLowerCase().contains("accesskey")
-                    || reqParam.getKey().toLowerCase().contains("secretkey")) {
+            if (isSensitiveParameter(reqParam.getKey())) {
                 cleanParamsString.append("\n");
                 continue;
             }

server/src/test/java/com/cloud/api/ApiServletTest.java

@@ -552,4 +552,30 @@ public void isStateChangingCommandNotUsingPOSTReturnsFalseForUpdateConfiguration
         boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params);
         Assert.assertTrue(result);
     }
+
+    @Test
+    public void testFormatValuesForLogMasksSensitiveParameters() {
+        String result = ApiServlet.formatValuesForLog(
+            "password",
+            new String[]{"SECRET_ONE", "SECRET_TWO"});
+
+        Assert.assertEquals("[REDACTED, REDACTED]", result);
+    }
+
+    @Test
+    public void testFormatValuesForLogDoesNotMaskNormalParameters() {
+        String result = ApiServlet.formatValuesForLog(
+            "username",
+            new String[]{"alice", "bob"});
+
+        Assert.assertEquals("[alice, bob]", result);
+    }
+
+    @Test
+    public void testIsSensitiveParameter() {
+        Assert.assertTrue(ApiServlet.isSensitiveParameter("adminpassword"));
+        Assert.assertTrue(ApiServlet.isSensitiveParameter("usersecretkey"));
+        Assert.assertTrue(ApiServlet.isSensitiveParameter("sessiontoken"));
+        Assert.assertFalse(ApiServlet.isSensitiveParameter("username"));
+    }
 }