killbill-aplcache

obfuscators: hack to avoid obfuscating the profiling header Signed-off-by:

5/29/2015 5:07:49 AM

Details

diff --git a/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/ConfigMagicObfuscator.java b/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/ConfigMagicObfuscator.java
index 6ecd7ce..e425aa3 100644
--- a/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/ConfigMagicObfuscator.java
+++ b/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/ConfigMagicObfuscator.java
@@ -21,6 +21,7 @@ import java.util.Collection;
 import java.util.LinkedList;
 import java.util.regex.Pattern;
 
+import ch.qos.logback.classic.spi.ILoggingEvent;
 import com.google.common.collect.ImmutableList;
 
 // See ConfigurationObjectFactory
@@ -48,8 +49,8 @@ public class ConfigMagicObfuscator extends Obfuscator {
     }
 
     @Override
-    public String obfuscate(final String originalString) {
-        return obfuscate(originalString, patterns);
+    public String obfuscate(final String originalString, final ILoggingEvent event) {
+        return obfuscate(originalString, patterns, event);
     }
 
     private Pattern buildPattern(final String key) {
diff --git a/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/LuhnMaskingObfuscator.java b/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/LuhnMaskingObfuscator.java
index 88bda32..1edb705 100644
--- a/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/LuhnMaskingObfuscator.java
+++ b/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/LuhnMaskingObfuscator.java
@@ -17,6 +17,7 @@
 
 package org.killbill.billing.server.log.obfuscators;
 
+import ch.qos.logback.classic.spi.ILoggingEvent;
 import com.google.common.annotations.VisibleForTesting;
 
 /**
@@ -38,7 +39,7 @@ public class LuhnMaskingObfuscator extends Obfuscator {
     }
 
     @Override
-    public String obfuscate(final String originalString) {
+    public String obfuscate(final String originalString, final ILoggingEvent event) {
         return mask(originalString);
     }
 
diff --git a/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/Obfuscator.java b/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/Obfuscator.java
index 68675e7..bf37cca 100644
--- a/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/Obfuscator.java
+++ b/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/Obfuscator.java
@@ -22,10 +22,16 @@ import java.util.regex.Pattern;
 
 import javax.annotation.Nullable;
 
+import org.killbill.commons.profiling.ProfilingFeature.ProfilingFeatureType;
+
+import ch.qos.logback.classic.spi.ILoggingEvent;
 import com.google.common.annotations.VisibleForTesting;
 
 public abstract class Obfuscator {
 
+    @VisibleForTesting
+    static final String LOGGING_FILTER_NAME = "com.sun.jersey.api.container.filter.LoggingFilter";
+
     protected static final int DEFAULT_PATTERN_FLAGS = Pattern.CASE_INSENSITIVE | Pattern.MULTILINE | Pattern.DOTALL;
 
     protected static final String MASK_LABEL = "MASKED";
@@ -40,24 +46,28 @@ public abstract class Obfuscator {
         }
     }
 
-    public abstract String obfuscate(final String originalString);
+    public abstract String obfuscate(final String originalString, final ILoggingEvent event);
 
-    protected String obfuscate(final String originalString, final Iterable<Pattern> patterns) {
+    protected String obfuscate(final String originalString, final Iterable<Pattern> patterns, final ILoggingEvent event) {
         final StringBuilder obfuscatedStringBuilder = new StringBuilder(originalString);
 
         for (final Pattern pattern : patterns) {
             int currentOffset = 0;
             // Create a matcher with a copy of the current obfuscated String
-            Matcher matcher = pattern.matcher(obfuscatedStringBuilder.toString());
+            final Matcher matcher = pattern.matcher(obfuscatedStringBuilder.toString());
             while (matcher.find()) {
                 for (int groupNb = 1; groupNb <= matcher.groupCount(); groupNb++) {
                     final String confidentialData = matcher.group(groupNb);
-                    final String obfuscatedConfidentialData = obfuscateConfidentialData(confidentialData);
-                    obfuscatedStringBuilder.replace(currentOffset + matcher.start(groupNb), currentOffset + matcher.end(groupNb), obfuscatedConfidentialData);
 
-                    // The original String is modified in place, which will confuse the Matcher if it becomes bigger
-                    if (obfuscatedConfidentialData.length() > confidentialData.length()) {
-                        currentOffset += obfuscatedConfidentialData.length() - confidentialData.length();
+                    if (shouldObfuscate(confidentialData, event)) {
+                        final String obfuscatedConfidentialData = obfuscateConfidentialData(confidentialData);
+
+                        obfuscatedStringBuilder.replace(currentOffset + matcher.start(groupNb), currentOffset + matcher.end(groupNb), obfuscatedConfidentialData);
+
+                        // The original String is modified in place, which will confuse the Matcher if it becomes bigger
+                        if (obfuscatedConfidentialData.length() > confidentialData.length()) {
+                            currentOffset += obfuscatedConfidentialData.length() - confidentialData.length();
+                        }
                     }
                 }
             }
@@ -66,6 +76,27 @@ public abstract class Obfuscator {
         return obfuscatedStringBuilder.toString();
     }
 
+    private boolean shouldObfuscate(final String confidentialData, final ILoggingEvent event) {
+        return !isProfilingHeader(confidentialData, event);
+    }
+
+    // Huge hack to avoid obfuscating the "name" in the X-Killbill-Profiling-Resp json. Unfortunately, we can't simply
+    // filter-out c.s.j.a.c.filter.LoggingFilter because we do want to obfuscate requests (in case sensitive data is passed as
+    // query parameters, e.g. in plugin properties)
+    private boolean isProfilingHeader(final String confidentialData, final ILoggingEvent event) {
+        if (!LOGGING_FILTER_NAME.equals(event.getLoggerName())) {
+            return false;
+        }
+
+        for (final ProfilingFeatureType profileType : ProfilingFeatureType.values()) {
+            // See ProfilingDataItem#getKey
+            if (confidentialData.startsWith("\"" + profileType.name() + ":")) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     private String obfuscateConfidentialData(final CharSequence confidentialSequence) {
         return obfuscateConfidentialData(confidentialSequence, null);
     }
diff --git a/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/ObfuscatorConverter.java b/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/ObfuscatorConverter.java
index 2b5f9e5..39b9306 100644
--- a/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/ObfuscatorConverter.java
+++ b/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/ObfuscatorConverter.java
@@ -53,7 +53,7 @@ public class ObfuscatorConverter extends ClassicConverter {
         String convertedMessage = event.getFormattedMessage();
         for (final Obfuscator obfuscator : obfuscators) {
             try {
-                convertedMessage = obfuscator.obfuscate(convertedMessage);
+                convertedMessage = obfuscator.obfuscate(convertedMessage, event);
             } catch (final RuntimeException e) {
                 // Ignore? Not sure the impact of importing a logger here
             }
diff --git a/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/PatternObfuscator.java b/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/PatternObfuscator.java
index dfe216d..bed40f9 100644
--- a/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/PatternObfuscator.java
+++ b/profiles/killbill/src/main/java/org/killbill/billing/server/log/obfuscators/PatternObfuscator.java
@@ -21,6 +21,7 @@ import java.util.Collection;
 import java.util.LinkedList;
 import java.util.regex.Pattern;
 
+import ch.qos.logback.classic.spi.ILoggingEvent;
 import com.google.common.collect.ImmutableList;
 
 public class PatternObfuscator extends Obfuscator {
@@ -63,8 +64,8 @@ public class PatternObfuscator extends Obfuscator {
     }
 
     @Override
-    public String obfuscate(final String originalString) {
-        return obfuscate(originalString, patterns);
+    public String obfuscate(final String originalString, final ILoggingEvent event) {
+        return obfuscate(originalString, patterns, event);
     }
 
     private Pattern buildJSONPattern(final String key) {
diff --git a/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestConfigMagicObfuscator.java b/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestConfigMagicObfuscator.java
index 382783a..fb6d67e 100644
--- a/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestConfigMagicObfuscator.java
+++ b/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestConfigMagicObfuscator.java
@@ -18,9 +18,12 @@
 package org.killbill.billing.server.log.obfuscators;
 
 import org.killbill.billing.server.log.ServerTestSuiteNoDB;
+import org.mockito.Mockito;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
+import ch.qos.logback.classic.spi.ILoggingEvent;
+
 public class TestConfigMagicObfuscator extends ServerTestSuiteNoDB {
 
     private final ConfigMagicObfuscator obfuscator = new ConfigMagicObfuscator();
@@ -44,7 +47,7 @@ public class TestConfigMagicObfuscator extends ServerTestSuiteNoDB {
     }
 
     private void verify(final String input, final String output) {
-        final String obfuscated = obfuscator.obfuscate(input);
+        final String obfuscated = obfuscator.obfuscate(input, Mockito.mock(ILoggingEvent.class));
         Assert.assertEquals(obfuscated, output, obfuscated);
     }
 }
diff --git a/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestLuhnMaskingObfuscator.java b/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestLuhnMaskingObfuscator.java
index 5795287..794a96d 100644
--- a/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestLuhnMaskingObfuscator.java
+++ b/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestLuhnMaskingObfuscator.java
@@ -18,9 +18,12 @@
 package org.killbill.billing.server.log.obfuscators;
 
 import org.killbill.billing.server.log.ServerTestSuiteNoDB;
+import org.mockito.Mockito;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
+import ch.qos.logback.classic.spi.ILoggingEvent;
+
 public class TestLuhnMaskingObfuscator extends ServerTestSuiteNoDB {
 
     private final LuhnMaskingObfuscator obfuscator = new LuhnMaskingObfuscator();
@@ -212,8 +215,46 @@ public class TestLuhnMaskingObfuscator extends ServerTestSuiteNoDB {
                + "6331101999990017");
     }
 
+    @Test(groups = "fast")
+    public void testQueryParametersAreObfuscated() throws Exception {
+        final ILoggingEvent event = Mockito.mock(ILoggingEvent.class);
+        Mockito.when(event.getLoggerName()).thenReturn(Obfuscator.LOGGING_FILTER_NAME);
+
+        // Check the query parameters are obfuscated (see TestPatternObfuscator#testProfilingHeaderIsNotObfuscated)
+        // TODO Pierre: Obfuscate the Authorization and X-Killbill-ApiSecret headers?
+        verify("1 * Server in-bound request\n" +
+               "1 > POST http://127.0.0.1:8080/1.0/kb/accounts/2a55045a-ce1d-4344-942d-b825536328f9/payments?pluginProperty=cc_number=4111111111111111\n" +
+               "1 > X-Killbill-ApiSecret: lazar\n" +
+               "1 > Authorization: Basic YWRtaW46cGFzc3dvcmQ=\n" +
+               "1 > X-Killbill-CreatedBy: admin\n" +
+               "1 > Host: 127.0.0.1:8080\n" +
+               "1 > Content-Length: 62\n" +
+               "1 > User-Agent: curl/7.30.0\n" +
+               "1 > X-Killbill-Profiling-Req: DAO\n" +
+               "1 > X-Killbill-ApiKey: bob\n" +
+               "1 > Content-Type: application/json\n" +
+               "1 > Accept: */*",
+               "1 * Server in-bound request\n" +
+               "1 > POST http://127.0.0.1:8080/1.0/kb/accounts/2a55045a-ce1d-4344-942d-b825536328f9/payments?pluginProperty=cc_number=***MASKED***1111\n" +
+               "1 > X-Killbill-ApiSecret: lazar\n" +
+               "1 > Authorization: Basic YWRtaW46cGFzc3dvcmQ=\n" +
+               "1 > X-Killbill-CreatedBy: admin\n" +
+               "1 > Host: 127.0.0.1:8080\n" +
+               "1 > Content-Length: 62\n" +
+               "1 > User-Agent: curl/7.30.0\n" +
+               "1 > X-Killbill-Profiling-Req: DAO\n" +
+               "1 > X-Killbill-ApiKey: bob\n" +
+               "1 > Content-Type: application/json\n" +
+               "1 > Accept: */*",
+               event);
+    }
+
     private void verify(final String input, final String output) {
-        final String obfuscated = obfuscator.obfuscate(input);
+        verify(input, output, Mockito.mock(ILoggingEvent.class));
+    }
+
+    private void verify(final String input, final String output, final ILoggingEvent event) {
+        final String obfuscated = obfuscator.obfuscate(input, event);
         Assert.assertEquals(obfuscated, output, obfuscated);
     }
 }
diff --git a/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestObfuscator.java b/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestObfuscator.java
index 59bd626..5109ad3 100644
--- a/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestObfuscator.java
+++ b/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestObfuscator.java
@@ -20,16 +20,18 @@ package org.killbill.billing.server.log.obfuscators;
 import java.util.regex.Pattern;
 
 import org.killbill.billing.server.log.ServerTestSuiteNoDB;
+import org.mockito.Mockito;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
+import ch.qos.logback.classic.spi.ILoggingEvent;
 import com.google.common.collect.ImmutableList;
 
 public class TestObfuscator extends ServerTestSuiteNoDB {
 
     private final Obfuscator obfuscator = new Obfuscator() {
         @Override
-        public String obfuscate(final String originalString) {
+        public String obfuscate(final String originalString, final ILoggingEvent event) {
             return null;
         }
     };
@@ -38,7 +40,7 @@ public class TestObfuscator extends ServerTestSuiteNoDB {
     public void testObfuscateWithOnePattern() throws Exception {
         final Pattern pattern = Pattern.compile("number=([^;]+)");
         final ImmutableList<Pattern> patterns = ImmutableList.<Pattern>of(pattern);
-        Assert.assertEquals(obfuscator.obfuscate("number=1234;number=12345;number=123456;number=1234567;number=12345678;number=123456789", patterns),
+        Assert.assertEquals(obfuscator.obfuscate("number=1234;number=12345;number=123456;number=1234567;number=12345678;number=123456789", patterns, Mockito.mock(ILoggingEvent.class)),
                             "number=MASKED;number=MASKED;number=MASKED;number=MASKED*;number=*MASKED*;number=*MASKED**");
 
     }
@@ -48,7 +50,7 @@ public class TestObfuscator extends ServerTestSuiteNoDB {
         final Pattern pattern1 = Pattern.compile("number=([^;]+)");
         final Pattern pattern2 = Pattern.compile("numberB=([^;]+)");
         final ImmutableList<Pattern> patterns = ImmutableList.<Pattern>of(pattern1, pattern2);
-        Assert.assertEquals(obfuscator.obfuscate("number=1234;numberB=12345;number=123456;numberB=1234567;number=12345678;numberB=123456789", patterns),
+        Assert.assertEquals(obfuscator.obfuscate("number=1234;numberB=12345;number=123456;numberB=1234567;number=12345678;numberB=123456789", patterns, Mockito.mock(ILoggingEvent.class)),
                             "number=MASKED;numberB=MASKED;number=MASKED;numberB=MASKED*;number=*MASKED*;numberB=*MASKED**");
 
     }
diff --git a/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestPatternObfuscator.java b/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestPatternObfuscator.java
index f2c1fd6..3476f58 100644
--- a/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestPatternObfuscator.java
+++ b/profiles/killbill/src/test/java/org/killbill/billing/server/log/obfuscators/TestPatternObfuscator.java
@@ -18,9 +18,12 @@
 package org.killbill.billing.server.log.obfuscators;
 
 import org.killbill.billing.server.log.ServerTestSuiteNoDB;
+import org.mockito.Mockito;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
+import ch.qos.logback.classic.spi.ILoggingEvent;
+
 public class TestPatternObfuscator extends ServerTestSuiteNoDB {
 
     private final PatternObfuscator obfuscator = new PatternObfuscator();
@@ -263,8 +266,28 @@ public class TestPatternObfuscator extends ServerTestSuiteNoDB {
                "[cybersource-plugin] \"<?xml version=\\\"1.0\\\" encoding=\\\"UTF-8\\\"?><accountNumber>*****MASKED*****</accountNumber>\\n  <expirationMonth>09</expirationMonth>\\n  \"");
     }
 
+    @Test(groups = "fast")
+    public void testProfilingHeaderIsNotObfuscated() throws Exception {
+        final ILoggingEvent event = Mockito.mock(ILoggingEvent.class);
+        Mockito.when(event.getLoggerName()).thenReturn(Obfuscator.LOGGING_FILTER_NAME);
+
+        verify("1 * Server out-bound response\n" +
+               "1 < 500\n" +
+               "1 < Content-Type: application/json\n" +
+               "1 < X-Killbill-Profiling-Resp: {\"rawData\":[{\"name\":\"DAO:AccountSqlDao:getById\",\"durationUsec\":14873},{\"name\":\"DAO:PaymentMethodSqlDao:getById\",\"durationUsec\":10438},{\"name\":\"DAO:PaymentSqlDao:create\",\"durationUsec\":31750},{\"name\":\"DAO:TransactionSqlDao:create\",\"durationUsec\":23121},{\"name\":\"DAO:PaymentSqlDao:getById\",\"durationUsec\":2541},{\"name\":\"DAO:TransactionSqlDao:getByPaymentId\",\"durationUsec\":3574},{\"name\":\"DAO:PaymentMethodSqlDao:getPaymentMethodIncludedDelete\",\"durationUsec\":1763},{\"name\":\"DAO:TransactionSqlDao:updateTransactionStatus\",\"durationUsec\":13994},{\"name\":\"DAO:PaymentSqlDao:updatePaymentStateName\",\"durationUsec\":11929},{\"name\":\"DAO:TransactionSqlDao:getById\",\"durationUsec\":5245}]}",
+               event);
+    }
+
+    private void verify(final String input, final ILoggingEvent event) {
+        verify(input, input, event);
+    }
+
     private void verify(final String input, final String output) {
-        final String obfuscated = obfuscator.obfuscate(input);
+        verify(input, output, Mockito.mock(ILoggingEvent.class));
+    }
+
+    private void verify(final String input, final String output, final ILoggingEvent event) {
+        final String obfuscated = obfuscator.obfuscate(input, event);
         Assert.assertEquals(obfuscated, output, obfuscated);
     }
 }