Repository: cxf
Updated Branches:
  refs/heads/3.0.x-fixes 25ae30750 -> da66c89c1


[CXF-7070] Avoiding logging the well-known senstive header values by default, 
patch from Andy McCright applied with a minor update, This closes #178


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

Branch: refs/heads/3.0.x-fixes
Commit: da66c89c10e60aa4faf6e05869529e9166ea6157
Parents: 25ae307
Author: Sergey Beryozkin <sberyoz...@gmail.com>
Authored: Thu Oct 13 12:50:34 2016 +0100
Committer: Sergey Beryozkin <sberyoz...@gmail.com>
Committed: Thu Oct 13 13:09:17 2016 +0100

----------------------------------------------------------------------
 .../org/apache/cxf/transport/http/Headers.java  | 110 +++++++++++-------
 .../apache/cxf/transport/http/HeadersTest.java  | 112 ++++++++++++++++---
 2 files changed, 163 insertions(+), 59 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cxf/blob/da66c89c/rt/transports/http/src/main/java/org/apache/cxf/transport/http/Headers.java
----------------------------------------------------------------------
diff --git 
a/rt/transports/http/src/main/java/org/apache/cxf/transport/http/Headers.java 
b/rt/transports/http/src/main/java/org/apache/cxf/transport/http/Headers.java
index 4879d97..dee9d61 100644
--- 
a/rt/transports/http/src/main/java/org/apache/cxf/transport/http/Headers.java
+++ 
b/rt/transports/http/src/main/java/org/apache/cxf/transport/http/Headers.java
@@ -29,7 +29,6 @@ import java.util.Date;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -62,8 +61,8 @@ public class Headers {
      * Each header value is added as a separate HTTP header, example, given A 
header with 'a' and 'b'
      * values, two A headers will be added as opposed to a single A header 
with the "a,b" value. 
      */
-    public static final String ADD_HEADERS_PROPERTY = 
"org.apache.cxf.http.add-headers";             
-       
+    public static final String ADD_HEADERS_PROPERTY = 
"org.apache.cxf.http.add-headers";
+
     public static final String PROTOCOL_HEADERS_CONTENT_TYPE = 
Message.CONTENT_TYPE.toLowerCase();
     public static final String HTTP_HEADERS_SETCOOKIE = "Set-Cookie";
     public static final String HTTP_HEADERS_LINK = "Link";
@@ -71,7 +70,10 @@ public class Headers {
     private static final String SET_EMPTY_REQUEST_CT_PROPERTY = 
"set.content.type.for.empty.request";
     private static final TimeZone TIME_ZONE_GMT = TimeZone.getTimeZone("GMT");
     private static final Logger LOG = LogUtils.getL7dLogger(Headers.class);
-    
+
+    private static final List<String> SENSITIVE_HEADERS = 
Arrays.asList("Authorization", "Proxy-Authorization");
+    private static final List<String> SENSITIVE_HEADER_MARKER = 
Arrays.asList("***");
+    private static final String ALLOW_LOGGING_SENSITIVE_HEADERS = 
"allow.logging.sensitive.headers";
     /**
      * Known HTTP headers whose values have to be represented as individual 
HTTP headers
      */
@@ -95,6 +97,25 @@ public class Headers {
         this.message = null;
     }
     
+    /**
+     * Returns a traceable string representation of the passed-in headers map.
+     * The value for any keys in the map that are in the 
<code>SENSITIVE_HEADERS</code>
+     * array will be filtered out of the returned string.
+     * Note that this method is expensive as it will copy the map (except for 
the
+     * filtered keys), so it should be used sparingly - i.e. only when debug is
+     * enabled.
+     */
+    static String toString(Map<String, List<String>> headers, boolean 
logSensitiveHeaders) {
+        Map<String, List<String>> filteredHeaders = new TreeMap<String, 
List<String>>(String.CASE_INSENSITIVE_ORDER);
+        filteredHeaders.putAll(headers);
+        if (!logSensitiveHeaders) {
+            for (String filteredKey : SENSITIVE_HEADERS) {
+                filteredHeaders.put(filteredKey, SENSITIVE_HEADER_MARKER);
+            }
+        }
+        return filteredHeaders.toString();
+    }
+
     public Map<String, List<String>> headerMap() {
         return headers;
     }
@@ -176,7 +197,7 @@ public class Headers {
                     createMutableList(policy.getReferer()));
         }
     }
-    
+
     void setFromServerPolicy(HTTPServerPolicy policy) {
         if (policy.isSetCacheControl()) {
             headers.put("Cache-Control",
@@ -204,9 +225,8 @@ public class Headers {
         } else if (policy.isSetKeepAliveParameters()) {
             headers.put("Keep-Alive", 
createMutableList(policy.getKeepAliveParameters()));
         }
-        
-    
-        
+
+
     /*
      * TODO - hook up these policies
     <xs:attribute name="SuppressClientSendErrors" type="xs:boolean" 
use="optional" default="false">
@@ -228,8 +248,8 @@ public class Headers {
         headers.put("Proxy-Authorization",
                 createMutableList(authorization));
     }
-    
-    
+
+
     /**
      * While extracting the Message.PROTOCOL_HEADERS property from the Message,
      * this call ensures that the Message.PROTOCOL_HEADERS property is
@@ -241,7 +261,7 @@ public class Headers {
      */
     public static Map<String, List<String>> getSetProtocolHeaders(final 
Message message) {
         Map<String, List<String>> headers =
-            CastUtils.cast((Map<?, ?>)message.get(Message.PROTOCOL_HEADERS));  
      
+            CastUtils.cast((Map<?, ?>)message.get(Message.PROTOCOL_HEADERS));
         if (null == headers) {
             headers = new TreeMap<String, 
List<String>>(String.CASE_INSENSITIVE_ORDER);
         } else if (headers instanceof HashMap) {
@@ -268,26 +288,31 @@ public class Headers {
     private static List<String> createMutableList(String val) {
         return new ArrayList<String>(Arrays.asList(new String[] {val}));
     }
-    
+
     /**
      * This procedure logs the PROTOCOL_HEADERS from the 
      * Message at the specified logging level.
      * 
+     * @param logger     The Logger to log to.
      * @param level   The Logging Level.
      * @param headers The Message protocol headers.
      */
-    void logProtocolHeaders(Level level) {
-        if (LOG.isLoggable(level)) {
-            for (String header : headers.keySet()) {
-                List<?> headerList = headers.get(header);
-                for (Object value : headerList) {
-                    LOG.log(level, header + ": " 
+    static void logProtocolHeaders(Logger logger, Level level, 
+                                   Map<String, List<String>> headersMap,
+                                   boolean logSensitiveHeaders) {
+        if (logger.isLoggable(level)) {
+            for (Map.Entry<String, List<String>> entry : 
headersMap.entrySet()) {
+                String key = entry.getKey();
+                boolean sensitive = !logSensitiveHeaders && 
SENSITIVE_HEADERS.contains(key);
+                List<String> headerList = sensitive ? SENSITIVE_HEADER_MARKER 
: entry.getValue();
+                for (String value : headerList) {
+                    logger.log(level, key + ": " 
                         + (value == null ? "<null>" : value.toString()));
                 }
             }
         }
     }
-    
+
     /**
      * Set content type and protocol headers (Message.PROTOCOL_HEADERS) 
headers into the URL
      * connection. 
@@ -318,7 +343,6 @@ public class Headers {
                     // otherwise if it is GET then just drop it
                     dropContentType = true;
                 }
-                
             }
             if (!dropContentType) {
                 String ct = emptyRequest && !contentTypeSet ? "*/*" : 
determineContentType();
@@ -329,7 +353,7 @@ public class Headers {
         }
          
         transferProtocolHeadersToURLConnection(connection);
-        logProtocolHeaders(Level.FINE);
+        logProtocolHeaders(LOG, Level.FINE, headers, logSensitiveHeaders());
     }
 
     public String determineContentType() {
@@ -356,7 +380,7 @@ public class Headers {
         }
         return ct;
     }
-    
+
     /**
      * This procedure sets the URLConnection request properties
      * from the PROTOCOL_HEADERS in the message.
@@ -364,8 +388,9 @@ public class Headers {
     private void transferProtocolHeadersToURLConnection(URLConnection 
connection) {
         boolean addHeaders = MessageUtils.isTrue(
                 message.getContextualProperty(ADD_HEADERS_PROPERTY));
-        for (String header : headers.keySet()) {
-            List<String> headerList = headers.get(header);
+        for (Map.Entry<String, List<String>> entry : headers.entrySet()) {
+            String header = entry.getKey();
+            List<String> headerList = entry.getValue();
             if (HttpHeaderHelper.CONTENT_TYPE.equalsIgnoreCase(header)) {
                 continue;
             }
@@ -389,7 +414,7 @@ public class Headers {
             connection.addRequestProperty("User-Agent", 
Version.getCompleteVersionString());
         }
     }
-    
+
     /**
      * Copy the request headers into the message.
      * 
@@ -416,14 +441,19 @@ public class Headers {
             headers.put(Message.CONTENT_TYPE, 
Collections.singletonList(req.getContentType()));
         }
         if (LOG.isLoggable(Level.FINE)) {
-            LOG.log(Level.FINE, "Request Headers: " + headers.toString());
+            LOG.log(Level.FINE, "Request Headers: " + toString(headers, 
+                                                               
logSensitiveHeaders()));
         }
     }
 
+    private boolean logSensitiveHeaders() {
+        // Not allowed by default
+        return 
PropertyUtils.isTrue(message.getContextualProperty(ALLOW_LOGGING_SENSITIVE_HEADERS));
+    }
     private String getContentTypeFromMessage() {
         final String ct  = (String)message.get(Message.CONTENT_TYPE);
         final String enc = (String)message.get(Message.ENCODING);
-        
+
         if (null != ct 
             && null != enc
             && ct.indexOf("charset=") == -1
@@ -433,7 +463,7 @@ public class Headers {
             return ct;
         }
     }
-    
+
     // Assumes that response body is not available only
     // if Content-Length is available and set to 0
     private boolean isResponseBodyAvailable() {
@@ -450,7 +480,7 @@ public class Headers {
         }
         return true;
     }
-    
+
     /**
      * Copy the response headers into the response.
      * 
@@ -467,10 +497,10 @@ public class Headers {
 
         boolean addHeaders = MessageUtils.isTrue(
                 message.getContextualProperty(ADD_HEADERS_PROPERTY));
-        for (Iterator<?> iter = headers.keySet().iterator(); iter.hasNext();) {
-            String header = (String)iter.next();
-            List<?> headerList = headers.get(header);
-            
+        for (Map.Entry<String, List<String>> entry : headers.entrySet()) {
+            String header = entry.getKey();
+            List<?> headerList = entry.getValue();
+
             if (addHeaders || HTTP_HEADERS_SINGLE_VALUE_ONLY.contains(header)) 
{
                 for (int i = 0; i < headerList.size(); i++) {
                     Object headerObject = headerList.get(i);
@@ -485,18 +515,16 @@ public class Headers {
                     if (headerObject != null) {
                         sb.append(headerObjectToString(headerObject));
                     }
-                    
+
                     if (i + 1 < headerList.size()) {
                         sb.append(',');
                     }
                 }
                 response.setHeader(header, sb.toString());
             }
-
-            
         }
     }
-    
+
     private String headerObjectToString(Object headerObject) {
         if (headerObject.getClass() == String.class) {
             // Most likely 
@@ -506,7 +534,7 @@ public class Headers {
             // so that the below code may be pushed back to the JAX-RS 
             // front-end where non String header objects are more likely 
             // to be set. Though the below code may be generally useful
-            
+
             String headerString;
             if (headerObject instanceof Date) {
                 headerString = toHttpDate((Date)headerObject);
@@ -518,7 +546,7 @@ public class Headers {
             return headerString;
         }
     }
-    
+
     void removeContentType() {
         headers.remove(PROTOCOL_HEADERS_CONTENT_TYPE);
     }
@@ -536,12 +564,12 @@ public class Headers {
         dateFormat.setTimeZone(TIME_ZONE_GMT);
         return dateFormat;
     }
-    
+
     public static String toHttpDate(Date date) {
         SimpleDateFormat format = getHttpDateFormat();
         return format.format(date);
     }
-    
+
     public static String toHttpLanguage(Locale locale) {
         StringBuilder sb = new StringBuilder();
         sb.append(locale.getLanguage());

http://git-wip-us.apache.org/repos/asf/cxf/blob/da66c89c/rt/transports/http/src/test/java/org/apache/cxf/transport/http/HeadersTest.java
----------------------------------------------------------------------
diff --git 
a/rt/transports/http/src/test/java/org/apache/cxf/transport/http/HeadersTest.java
 
b/rt/transports/http/src/test/java/org/apache/cxf/transport/http/HeadersTest.java
index 25be560..e20a5e0 100755
--- 
a/rt/transports/http/src/test/java/org/apache/cxf/transport/http/HeadersTest.java
+++ 
b/rt/transports/http/src/test/java/org/apache/cxf/transport/http/HeadersTest.java
@@ -23,6 +23,10 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.logging.Handler;
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import java.util.logging.Logger;
 
 import javax.servlet.http.HttpServletRequest;
 
@@ -31,27 +35,28 @@ import org.apache.cxf.message.Message;
 import org.apache.cxf.message.MessageImpl;
 import org.easymock.EasyMock;
 import org.easymock.IMocksControl;
-import org.junit.After;
+
+import org.junit.AfterClass;
 import org.junit.Assert;
-import org.junit.Before;
+import org.junit.BeforeClass;
 import org.junit.Test;
 
 /**
  *
  */
 public class HeadersTest extends Assert {
-    private IMocksControl control;
-    
-    @Before
-    public void setUp() {
+    private static IMocksControl control;
+
+    @BeforeClass
+    public static void setUpClass() {
         control = EasyMock.createNiceControl();
     }
-    
-    @After
-    public void tearDown() {
+
+    @AfterClass
+    public static void tearDownClass() {
         control.verify();
     }
-    
+
     @Test
     public void setHeadersTest() throws Exception {
         String[] headerNames = {"Content-Type", "authorization", "soapAction"};
@@ -60,7 +65,7 @@ public class HeadersTest extends Assert {
         for (int i = 0; i < headerNames.length; i++) {
             inmap.put(headerNames[i], Arrays.asList(headerValues[i]));
         }
-        
+
         HttpServletRequest req = control.createMock(HttpServletRequest.class);
         
EasyMock.expect(req.getHeaderNames()).andReturn(Collections.enumeration(inmap.keySet()));
         for (int i = 0; i < headerNames.length; i++) {
@@ -68,20 +73,20 @@ public class HeadersTest extends Assert {
                 andReturn(Collections.enumeration(inmap.get(headerNames[i])));
         }
         
EasyMock.expect(req.getContentType()).andReturn(headerValues[0]).anyTimes();
-        
+
         control.replay();
 
         Message message = new MessageImpl();
         message.put(AbstractHTTPDestination.HTTP_REQUEST, req);
-        
+
         Headers headers = new Headers(message);
         headers.copyFromRequest(req);
-        
+
         Map<String, List<String>> protocolHeaders = 
             CastUtils.cast((Map<?, ?>)message.get(Message.PROTOCOL_HEADERS));
-        
+
         assertTrue("unexpected size", protocolHeaders.size() == 
headerNames.length);
-        
+
         assertEquals("unexpected header", 
protocolHeaders.get("Content-Type").get(0), headerValues[0]);
         assertEquals("unexpected header", 
protocolHeaders.get("content-type").get(0), headerValues[0]);
         assertEquals("unexpected header", 
protocolHeaders.get("CONTENT-TYPE").get(0), headerValues[0]);
@@ -91,11 +96,82 @@ public class HeadersTest extends Assert {
         assertEquals("unexpected header", 
protocolHeaders.get("authorization").get(0), headerValues[1]);
         assertEquals("unexpected header", 
protocolHeaders.get("AUTHORIZATION").get(0), headerValues[1]);
         assertEquals("unexpected header", 
protocolHeaders.get("authoriZATION").get(0), headerValues[1]);
-        
+
         assertEquals("unexpected header", 
protocolHeaders.get("SOAPAction").get(0), headerValues[2]);
         assertEquals("unexpected header", 
protocolHeaders.get("soapaction").get(0), headerValues[2]);
         assertEquals("unexpected header", 
protocolHeaders.get("SOAPACTION").get(0), headerValues[2]);
         assertEquals("unexpected header", 
protocolHeaders.get("soapAction").get(0), headerValues[2]);
-        
+
+    }
+
+    @Test
+    public void sensitiveHeadersTest() {
+        Map<String, List<String>> headerMap = new HashMap<String, 
List<String>>();
+        headerMap.put("Authorization", Arrays.asList("FAIL"));
+        headerMap.put("Proxy-Authorization", Arrays.asList("FAIL"));
+        headerMap.put("Content-Type", Arrays.asList("application/xml"));
+        headerMap.put("Accept", Arrays.asList("text/plain"));
+
+        String loggedString = Headers.toString(headerMap, false);
+        assertFalse("The value of a sensitive header could be logged: " + 
loggedString, loggedString.contains("FAIL"));
+        assertTrue("The value of a non-sensitive header would not be logged: " 
+ loggedString, 
+                   loggedString.contains("application/xml") && 
loggedString.contains("text/plain"));
+        assertTrue("Expected header keys were not logged: " + loggedString, 
+                   loggedString.contains("Authorization") && 
loggedString.contains("Proxy-Authorization")
+                   && loggedString.contains("Accept") && 
loggedString.contains("Content-Type"));
+    }
+
+    @Test
+    public void logProtocolHeadersTest() {
+        Map<String, List<String>> headerMap = new HashMap<String, 
List<String>>();
+        headerMap.put("Normal-Header", Arrays.asList("normal"));
+        headerMap.put("Multivalue-Header", Arrays.asList("first", "second"));
+        headerMap.put("Authorization", Arrays.asList("myPassword"));
+        headerMap.put("Null-Header", Arrays.asList((String)null));
+
+        //Set up test logger
+        Logger logger = Logger.getAnonymousLogger();
+        // remove all "normal" handlers and just use our custom handler for 
testing
+        logger.setUseParentHandlers(false);
+        logger.setLevel(Level.INFO);
+        for (Handler h : logger.getHandlers()) {
+            logger.removeHandler(h); 
+        }
+        logger.addHandler(new Handler() {
+
+            @Override
+            public void publish(LogRecord record) {
+                String msg = record.getMessage();
+                if (msg.startsWith("Normal-Header")) {
+                    assertTrue("Unexpected output for normal header - expected 
Normal-Header: normal, received " + msg,
+                               "Normal-Header: normal".equals(msg));
+                } else if (msg.startsWith("Multivalue-Header")) {
+                    assertTrue("Unexpected output for multi-value header - 
expected Multivalue-Header: first or " 
+                        + "Multivalue-Header: second, received: " + msg,
+                        "Multivalue-Header: first".equals(msg) || 
"Multivalue-Header: second".equals(msg));
+                } else if (msg.startsWith("Authorization")) {
+                    assertTrue("Unexpected output for sensitive header - 
expected Authorization: ***, received " + msg,
+                               "Authorization: ***".equals(msg));
+                } else if (msg.startsWith("Null-Header")) {
+                    assertTrue("Unexpected output for null header - expected 
Null-Header: <null>, received " + msg,
+                               "Null-Header: <null>".equals(msg));
+                } else {
+                    fail("Unexpected header logged: " + msg);
+                }
+
+            }
+
+            @Override
+            public void flush() {
+                // no-op
+            }
+
+            @Override
+            public void close() throws SecurityException {
+                // no-op
+            } 
+        });
+
+        Headers.logProtocolHeaders(logger, Level.INFO, headerMap, false);
     }
 }

Reply via email to