KNOX-3039 Add error message sanitization to GatewayServlet

gateway-server/src/main/java/org/apache/knox/gateway/GatewayServlet.java

@@ -50,6 +50,9 @@ public class GatewayServlet implements Servlet, Filter {
   public static final String GATEWAY_DESCRIPTOR_LOCATION_DEFAULT = "gateway.xml";
   public static final String GATEWAY_DESCRIPTOR_LOCATION_PARAM = "gatewayDescriptorLocation";
 
+  private static boolean isErrorMessageSanitizationEnabled = true;
+  private static String errorMessageSanitizationPattern = "\\b(?:\\d{1,3}\\.){3}\\d{1,3}\\b";
+
   private static final GatewayResources res = ResourcesFactory.get( GatewayResources.class );
   private static final GatewayMessages LOG = MessagesFactory.get( GatewayMessages.class );
 
@@ -83,6 +86,9 @@ public synchronized void setFilter( GatewayFilter filter ) throws ServletExcepti
 
   @Override
   public synchronized void init( ServletConfig servletConfig ) throws ServletException {
+    GatewayConfig gatewayConfig = (GatewayConfig) servletConfig.getServletContext().getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE);
+    isErrorMessageSanitizationEnabled = gatewayConfig.isErrorMessageSanitizationEnabled();
+    errorMessageSanitizationPattern = gatewayConfig.getErrorMessageSanitizationPattern();
     try {
       if( filter == null ) {
         filter = createFilter( servletConfig );
@@ -92,23 +98,24 @@ public synchronized void init( ServletConfig servletConfig ) throws ServletExcep
         filter.init( filterConfig );
       }
     } catch( ServletException | RuntimeException e ) {
-      LOG.failedToInitializeServletInstace( e );
-      throw e;
+      throw logAndSanitizeException(e);
     }
   }
 
   @Override
   public void init( FilterConfig filterConfig ) throws ServletException {
     try {
       if( filter == null ) {
+        GatewayConfig gatewayConfig = (GatewayConfig) filterConfig.getServletContext().getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE);
+        isErrorMessageSanitizationEnabled = gatewayConfig.isErrorMessageSanitizationEnabled();
+        errorMessageSanitizationPattern = gatewayConfig.getErrorMessageSanitizationPattern();
         filter = createFilter( filterConfig );
       }
       if( filter != null ) {
         filter.init( filterConfig );
       }
     } catch( ServletException | RuntimeException e ) {
-      LOG.failedToInitializeServletInstace( e );
-      throw e;
+      throw logAndSanitizeException(e);
     }
   }
 
@@ -126,8 +133,7 @@ public void service( ServletRequest servletRequest, ServletResponse servletRespo
         try {
           f.doFilter( servletRequest, servletResponse, null );
         } catch( IOException | RuntimeException | ServletException e ) {
-          LOG.failedToExecuteFilter( e );
-          throw e;
+          throw logAndSanitizeException(e);
         }
       } else {
         ((HttpServletResponse)servletResponse).setStatus( HttpServletResponse.SC_SERVICE_UNAVAILABLE );
@@ -153,10 +159,8 @@ public void doFilter( ServletRequest servletRequest, ServletResponse servletResp
             //TODO: This should really happen naturally somehow as part of being a filter.  This way will cause problems eventually.
             chain.doFilter( servletRequest, servletResponse );
           }
-
-        } catch( IOException | RuntimeException | ServletException e ) {
-          LOG.failedToExecuteFilter( e );
-          throw e;
+        } catch (Exception e) {
+          throw logAndSanitizeException(e);
         }
       } else {
         ((HttpServletResponse)servletResponse).setStatus( HttpServletResponse.SC_SERVICE_UNAVAILABLE );
@@ -168,7 +172,6 @@ public void doFilter( ServletRequest servletRequest, ServletResponse servletResp
     }
   }
 
-
   @Override
   public String getServletInfo() {
     return res.gatewayServletInfo();
@@ -277,4 +280,16 @@ public Enumeration<String> getInitParameterNames() {
       return config.getInitParameterNames();
     }
   }
+
+  private SanitizedException logAndSanitizeException(Exception e) {
+    LOG.failedToExecuteFilter(e);
+    if (e == null || e.getMessage() == null) {
+      return new SanitizedException(e.getMessage());
+    }
+    if (!isErrorMessageSanitizationEnabled || e.getMessage() == null) {
+      return new SanitizedException(e.getMessage());
+    }
+    String sanitizedMessage = e.getMessage().replaceAll(errorMessageSanitizationPattern, "[hidden]");
+    return new SanitizedException(sanitizedMessage);
+  }
 }

gateway-server/src/main/java/org/apache/knox/gateway/SanitizedException.java

@@ -0,0 +1,9 @@
+package org.apache.knox.gateway;
+
+import javax.servlet.ServletException;
+
+public class SanitizedException extends ServletException {
+    public SanitizedException(String message) {
+        super(message);
+    }
+}

gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java

@@ -176,6 +176,10 @@ public class GatewayConfigImpl extends Configuration implements GatewayConfig {
   public static final String CLUSTER_CONFIG_MONITOR_INTERVAL_SUFFIX = ".interval";
   public static final String CLUSTER_CONFIG_MONITOR_ENABLED_SUFFIX = ".enabled";
 
+  private static final String ERROR_MESSAGE_SANITIZATION_ENABLED = GATEWAY_CONFIG_FILE_PREFIX + ".error.sanitization.enabled";
+  private static final boolean ERROR_MESSAGE_SANITIZATION_ENABLED_DEFAULT = true;
+  private static final String ERROR_MESSAGE_SANITIZATION_PATTERN = GATEWAY_CONFIG_FILE_PREFIX + ".error.sanitization.pattern";
+  private static final String ERROR_MESSAGE_SANITIZATION_PATTERN_DEFAULT = "\\b(?:\\d{1,3}\\.){3}\\d{1,3}\\b";
 
   // These config property names are not inline with the convention of using the
   // GATEWAY_CONFIG_FILE_PREFIX as is done by those above. These are left for
@@ -933,6 +937,16 @@ public List<String> getGlobalRulesServices() {
     return DEFAULT_GLOBAL_RULES_SERVICES;
   }
 
+  @Override
+  public boolean isErrorMessageSanitizationEnabled() {
+    return getBoolean(ERROR_MESSAGE_SANITIZATION_ENABLED, ERROR_MESSAGE_SANITIZATION_ENABLED_DEFAULT);
+  }
+
+  @Override
+  public String getErrorMessageSanitizationPattern() {
+    return get(ERROR_MESSAGE_SANITIZATION_PATTERN, ERROR_MESSAGE_SANITIZATION_PATTERN_DEFAULT);
+  }
+
   @Override
   public boolean isMetricsEnabled() {
     return Boolean.parseBoolean(get( METRICS_ENABLED, "false" ));

gateway-server/src/test/java/org/apache/knox/gateway/GatewayServletTest.java

@@ -0,0 +1,131 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway;
+
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.easymock.EasyMock;
+import org.easymock.IMocksControl;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletConfig;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collection;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+@RunWith(Parameterized.class)
+public class GatewayServletTest {
+
+    private static final String IP_ADDRESS_PATTERN = "\\b(?:\\d{1,3}\\.){3}\\d{1,3}\\b";
+    private static final String EMAIL_ADDRESS_PATTERN = "\\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\\.[A-Z|a-z]{2,}\\b";
+    private static final String CREDIT_CARD_PATTERN = "\\b\\d{4}-\\d{4}-\\d{4}-\\d{4}\\b";
+
+    @Parameterized.Parameters(name = "{index}: SanitizationEnabled={2}, Pattern={3}, Exception={0}, ExpectedMessage={1}")
+    public static Collection<Object[]> data() {
+        return Arrays.asList(new Object[][] {
+                { new IOException("Connection to 192.168.1.1 failed"), "Connection to [hidden] failed", true, IP_ADDRESS_PATTERN },
+                { new RuntimeException("Connection to 192.168.1.1 failed"), "Connection to [hidden] failed", true, IP_ADDRESS_PATTERN },
+                { new NullPointerException(), null, true, IP_ADDRESS_PATTERN },
+                { new IOException("General failure"), "General failure", true, IP_ADDRESS_PATTERN },
+                { new IOException("Connection to 192.168.1.1 failed"), "Connection to 192.168.1.1 failed", false, IP_ADDRESS_PATTERN },
+                { new RuntimeException("Connection to 192.168.1.1 failed"), "Connection to 192.168.1.1 failed", false, IP_ADDRESS_PATTERN },
+                { new NullPointerException(), null, false, IP_ADDRESS_PATTERN },
+                { new IOException("General failure"), "General failure", false, IP_ADDRESS_PATTERN },
+                { new IOException("User email: user@example.com"), "User email: [hidden]", true, EMAIL_ADDRESS_PATTERN },
+                { new RuntimeException("Credit card number: 1234-5678-9101-1121"), "Credit card number: [hidden]", true, CREDIT_CARD_PATTERN },
+        });
+    }
+
+    private final Exception exception;
+    private final String expectedMessage;
+    private final boolean isSanitizationEnabled;
+    private final String sanitizationPattern;
+
+    public GatewayServletTest(Exception exception, String expectedMessage, boolean isSanitizationEnabled, String sanitizationPattern) {
+        this.exception = exception;
+        this.expectedMessage = expectedMessage;
+        this.isSanitizationEnabled = isSanitizationEnabled;
+        this.sanitizationPattern = sanitizationPattern;
+    }
+
+    private IMocksControl mockControl;
+    private ServletConfig servletConfig;
+    private ServletContext servletContext;
+    private GatewayConfig gatewayConfig;
+    private HttpServletRequest request;
+    private HttpServletResponse response;
+    private GatewayFilter filter;
+
+    @Before
+    public void setUp() throws ServletException {
+        mockControl = EasyMock.createControl();
+        servletConfig = mockControl.createMock(ServletConfig.class);
+        servletContext = mockControl.createMock(ServletContext.class);
+        gatewayConfig = mockControl.createMock(GatewayConfig.class);
+        request = mockControl.createMock(HttpServletRequest.class);
+        response = mockControl.createMock(HttpServletResponse.class);
+        filter = mockControl.createMock(GatewayFilter.class);
+
+        EasyMock.expect(servletConfig.getServletName()).andStubReturn("default");
+        EasyMock.expect(servletConfig.getServletContext()).andStubReturn(servletContext);
+        EasyMock.expect(servletContext.getAttribute("org.apache.knox.gateway.config")).andStubReturn(gatewayConfig);
+    }
+
+    @Test
+    public void testExceptionSanitization() throws ServletException, IOException {
+        GatewayServlet servlet = initializeServletWithSanitization(isSanitizationEnabled, sanitizationPattern);
+
+        try {
+            servlet.service(request, response);
+        } catch (Exception e) {
+            if (expectedMessage != null) {
+                assertEquals(expectedMessage, e.getMessage());
+            } else {
+                assertNull(e.getMessage());
+            }
+        }
+
+        mockControl.verify();
+    }
+
+    private GatewayServlet initializeServletWithSanitization(boolean isErrorMessageSanitizationEnabled, String sanitizationPattern) throws ServletException, IOException {
+        EasyMock.expect(gatewayConfig.isErrorMessageSanitizationEnabled()).andStubReturn(isErrorMessageSanitizationEnabled);
+        EasyMock.expect(gatewayConfig.getErrorMessageSanitizationPattern()).andStubReturn(sanitizationPattern);
+
+        filter.init(EasyMock.anyObject(FilterConfig.class));
+        EasyMock.expectLastCall().once();
+        filter.doFilter(EasyMock.eq(request), EasyMock.eq(response), EasyMock.isNull());
+        EasyMock.expectLastCall().andThrow(exception).once();
+
+        mockControl.replay();
+
+        GatewayServlet servlet = new GatewayServlet(filter);
+        servlet.init(servletConfig);
+        return servlet;
+    }
+}

gateway-spi-common/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java

@@ -79,6 +79,7 @@ public class GatewayTestConfig extends Configuration implements GatewayConfig {
   private ConcurrentMap<String, Integer> topologyPortMapping = new ConcurrentHashMap<>();
   private int backupVersionLimit = -1;
   private long backupAgeLimit = -1;
+  private String sanitizationPattern = "sanitizationPattern";
 
   public GatewayTestConfig(Properties props) {
    super.getProps().putAll(props);
@@ -619,6 +620,16 @@ public int getWebsocketMaxWaitBufferCount() {
     return DEFAULT_WEBSOCKET_MAX_WAIT_BUFFER_COUNT;
   }
 
+  @Override
+  public boolean isErrorMessageSanitizationEnabled() {
+    return true;
+  }
+
+  @Override
+  public String getErrorMessageSanitizationPattern() {
+    return sanitizationPattern;
+  }
+
   @Override
   public boolean isMetricsEnabled() {
     return false;

gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java

@@ -901,6 +901,16 @@ public interface GatewayConfig {
    */
   boolean isAsyncSupported();
 
+  /**
+   * @return true if error message sanitization is enabled; false otherwise (defaults to true)
+   */
+  boolean isErrorMessageSanitizationEnabled();
+
+  /**
+   * @return the error message sanitization pattern
+   */
+  String getErrorMessageSanitizationPattern();
+
   /**
    * @return <code>true</code> if the supplied user is allowed to see all tokens
    *         (i.e. not only tokens where userName or createdBy equals to the