# HG changeset patch
# User Severin Gehwolf <sgehwolf@redhat.com>
# Date 1386080702 -3600
# Node ID b4c7ee6fb5c50d90a159b1a9d52098097e093d91
# Parent  b9dc33ef549a2100ea33605bc6ec580e402705ed
Filter security sensitive request parameters.
Reviewed-by: omajid
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-December/009003.html
PR1620

diff -r b9dc33ef549a -r b4c7ee6fb5c5 common/command/src/main/java/com/redhat/thermostat/common/command/Request.java
--- a/common/command/src/main/java/com/redhat/thermostat/common/command/Request.java	Mon Dec 02 18:28:36 2013 +0100
+++ b/common/command/src/main/java/com/redhat/thermostat/common/command/Request.java	Tue Dec 03 15:25:02 2013 +0100
@@ -40,7 +40,9 @@
 import java.net.SocketAddress;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 import java.util.TreeMap;
 import java.util.concurrent.CopyOnWriteArrayList;
 
@@ -85,8 +87,23 @@
  */
 public class Request implements Message {
     
+    private static final String RECEIVER = "receiver";
+    
+    public static final String CLIENT_TOKEN = "client-token";
+    public static final String AUTH_TOKEN = "auth-token";
+    public static final String ACTION = "action-name";
     public static final String UNKNOWN_HOSTNAME = "";
 
+    private static final String FILTERED_PARAM_VALUE = "<filtered>";
+    private static final Set<String> FILTERED_PARAMS;
+    
+    static {
+        FILTERED_PARAMS = new HashSet<>();
+        FILTERED_PARAMS.add(AUTH_TOKEN);
+        FILTERED_PARAMS.add(CLIENT_TOKEN);
+    }
+    
+
     public enum RequestType implements MessageType {
         NO_RESPONSE_EXPECTED,
         RESPONSE_EXPECTED,
@@ -98,11 +115,6 @@
     private final InetSocketAddress target;
     private final Collection<RequestResponseListener> listeners;
 
-    private static final String RECEIVER = "receiver";
-
-    public static final String CLIENT_TOKEN = "client-token";
-    public static final String AUTH_TOKEN = "auth-token";
-    public static final String ACTION = "action-name";
 
     public Request(RequestType type, InetSocketAddress target) {
         this.type = type;
@@ -154,7 +166,25 @@
     
     @Override
     public String toString() {
-        return "{ Request: {target = " + target.toString() + "}, {type = " + type.name() + "}, {parameters = " + parameters.toString() + "} }";
+        Map<String, String> filteredParams = getFilteredParams(parameters);
+        return "{ Request: {target = " + target.toString() + "}, {type = " +
+                type.name() + "}, {parameters = " + filteredParams +
+                "} }";
     }
+    
+    // package-private for testing
+    Map<String, String> getFilteredParams(Map<String, String> unfiltered) {
+        Map<String, String> filtered = new TreeMap<>();
+        for (String key: unfiltered.keySet()) {
+            if (FILTERED_PARAMS.contains(key)) {
+                // actual value may be security sensitive
+                filtered.put(key, FILTERED_PARAM_VALUE);
+            } else {
+                filtered.put(key, unfiltered.get(key));
+            }
+        }
+        return filtered;
+    }
+    
 }
 
diff -r b9dc33ef549a -r b4c7ee6fb5c5 common/command/src/test/java/com/redhat/thermostat/common/command/RequestTest.java
--- a/common/command/src/test/java/com/redhat/thermostat/common/command/RequestTest.java	Mon Dec 02 18:28:36 2013 +0100
+++ b/common/command/src/test/java/com/redhat/thermostat/common/command/RequestTest.java	Tue Dec 03 15:25:02 2013 +0100
@@ -42,6 +42,10 @@
 
 import java.net.InetSocketAddress;
 import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
 
 import org.junit.Test;
 
@@ -117,5 +121,66 @@
         Request request = new Request(RequestType.RESPONSE_EXPECTED, addr);
         assertEquals("{ Request: {target = "+ addr.toString() + "}, {type = RESPONSE_EXPECTED}, {parameters = {}} }", request.toString());
     }
+    
+    /*
+     * It is important that request parameters won't get logged (at any log
+     * level). Since toString() is used in some log statements it should be
+     * sufficient to verify toString() filters parameters appropriately.
+     */
+    @Test
+    public void testToStringFiltersParams() {
+        InetSocketAddress addr = new InetSocketAddress(1234);
+        Request request = new Request(RequestType.RESPONSE_EXPECTED, addr);
+        request.setParameter(Request.AUTH_TOKEN, "foo-auth-token");
+        request.setParameter(Request.CLIENT_TOKEN, "bar-client-token");
+        String preamble = "{ Request: {target = "+ addr.toString() + 
+                "}, {type = RESPONSE_EXPECTED}, ";
+        String postfix = " }";
+        // maps aren't ordered. work around it by using a set assert.
+        String expectedParamOrdering1 = "{parameters = {" +
+                Request.AUTH_TOKEN +"=<filtered>, " +
+                Request.CLIENT_TOKEN +"=<filtered>}}";
+        String expectedParamOrdering2 = "{parameters = {" +
+                Request.CLIENT_TOKEN +"=<filtered>, " +
+                Request.AUTH_TOKEN +"=<filtered>}}";
+        Set<String> expectedStrings = new HashSet<>();
+        expectedStrings.add(preamble + expectedParamOrdering1 + postfix);
+        expectedStrings.add(preamble + expectedParamOrdering2 + postfix);
+        String actual = request.toString();
+        assertTrue("Security sensitive parameters should be filtered! String was: "
+                + actual, expectedStrings.contains(actual));
+    }
+    
+    @Test
+    public void testFilterParams() {
+        Map<String, String> origParams = new HashMap<>();
+        origParams.put(Request.AUTH_TOKEN, "foo-auth-token");
+        origParams.put(Request.CLIENT_TOKEN, "bar-client-token");
+        origParams.put("foo-param", "something");
+        
+        InetSocketAddress addr = new InetSocketAddress(1234);
+        Request request = new Request(RequestType.RESPONSE_EXPECTED, addr);
+        Map<String, String> filteredParams = request.getFilteredParams(origParams);
+        assertEquals(Request.AUTH_TOKEN + " should be filtered", "<filtered>", filteredParams.get(Request.AUTH_TOKEN));
+        assertEquals(Request.CLIENT_TOKEN + " should be filtered", "<filtered>", filteredParams.get(Request.CLIENT_TOKEN));
+        assertEquals("something", filteredParams.get("foo-param"));
+        assertEquals(3, filteredParams.size());
+        
+        origParams.clear();
+        origParams.put(Request.AUTH_TOKEN, "foo-auth-token");
+        origParams.put("foo-param", "something-new");
+        filteredParams = request.getFilteredParams(origParams);
+        assertEquals(Request.AUTH_TOKEN + " should be filtered", "<filtered>", filteredParams.get(Request.AUTH_TOKEN));
+        assertEquals("something-new", filteredParams.get("foo-param"));
+        assertEquals(2, filteredParams.size());
+        
+        origParams.clear();
+        origParams.put("bar-param", "should-be-unchanged");
+        origParams.put(Request.CLIENT_TOKEN, "client-token");
+        filteredParams = request.getFilteredParams(origParams);
+        assertEquals(Request.CLIENT_TOKEN + " should be filtered", "<filtered>", filteredParams.get(Request.CLIENT_TOKEN));
+        assertEquals("should-be-unchanged", filteredParams.get("bar-param"));
+        assertEquals(2, filteredParams.size());
+    }
 }