KNOX-3039 Add error message sanitization to GatewayServlet

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

@@ -42,6 +42,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.lang.reflect.Constructor;
 import java.net.URISyntaxException;
 import java.nio.charset.StandardCharsets;
 import java.util.Enumeration;
@@ -50,6 +51,8 @@ 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 final GatewayResources res = ResourcesFactory.get( GatewayResources.class );
   private static final GatewayMessages LOG = MessagesFactory.get( GatewayMessages.class );
 
@@ -83,6 +86,8 @@ 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();
     try {
       if( filter == null ) {
         filter = createFilter( servletConfig );
@@ -92,23 +97,23 @@ public synchronized void init( ServletConfig servletConfig ) throws ServletExcep
         filter.init( filterConfig );
       }
     } catch( ServletException | RuntimeException e ) {
-      LOG.failedToInitializeServletInstace( e );
-      throw e;
+      throw sanitizeAndRethrow(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();
         filter = createFilter( filterConfig );
       }
       if( filter != null ) {
         filter.init( filterConfig );
       }
     } catch( ServletException | RuntimeException e ) {
-      LOG.failedToInitializeServletInstace( e );
-      throw e;
+      throw sanitizeAndRethrow(e);
     }
   }
 
@@ -126,8 +131,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 sanitizeAndRethrow(e);
         }
       } else {
         ((HttpServletResponse)servletResponse).setStatus( HttpServletResponse.SC_SERVICE_UNAVAILABLE );
@@ -153,10 +157,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 sanitizeAndRethrow(e);
         }
       } else {
         ((HttpServletResponse)servletResponse).setStatus( HttpServletResponse.SC_SERVICE_UNAVAILABLE );
@@ -168,7 +170,6 @@ public void doFilter( ServletRequest servletRequest, ServletResponse servletResp
     }
   }
 
-
   @Override
   public String getServletInfo() {
     return res.gatewayServletInfo();
@@ -277,4 +278,34 @@ public Enumeration<String> getInitParameterNames() {
       return config.getInitParameterNames();
     }
   }
+
+  private Exception sanitizeException(Exception e) {
+    if (e == null || e.getMessage() == null) {
+      return e;
+    }
+    if (!isErrorMessageSanitizationEnabled || e.getMessage() == null) {
+      return e;
+    }
+    String sanitizedMessage = e.getMessage().replaceAll("\\b(?:\\d{1,3}\\.){3}\\d{1,3}\\b", "[hidden]");
+    return createNewException(e, sanitizedMessage);
+  }
+
+  private <T extends Exception> T createNewException(T e, String sanitizedMessage) {
+    try {
+      Constructor<? extends Exception> constructor = e.getClass().getConstructor(String.class, Throwable.class);
+      T sanitizedException = (T) constructor.newInstance(sanitizedMessage, e.getCause());
+      sanitizedException.setStackTrace(e.getStackTrace());
+      return sanitizedException;
+    } catch (Exception ex) {
+      Exception genericException = new Exception(sanitizedMessage, e.getCause());
+      genericException.setStackTrace(e.getStackTrace());
+      return (T) genericException;
+    }
+  }
+
+  private <T extends Exception> T sanitizeAndRethrow(Exception e) throws T {
+    Exception sanitizedException = sanitizeException(e);
+    LOG.failedToExecuteFilter(sanitizedException);
+    throw (T) sanitizedException;
+  }
 }

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

@@ -176,6 +176,8 @@ 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 DEFAULT_ERROR_MESSAGE_SANITIZATION_ENABLED = true;
 
   // 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 +935,11 @@ public List<String> getGlobalRulesServices() {
     return DEFAULT_GLOBAL_RULES_SERVICES;
   }
 
+  @Override
+  public boolean isErrorMessageSanitizationEnabled() {
+    return getBoolean(ERROR_MESSAGE_SANITIZATION_ENABLED, DEFAULT_ERROR_MESSAGE_SANITIZATION_ENABLED);
+  }
+
   @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,122 @@
+/*
+ * 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 {
+
+    @Parameterized.Parameters(name = "{index}: SanitizationEnabled={2}, 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 },
+                { new RuntimeException("Connection to 192.168.1.1 failed"), "Connection to [hidden] failed", true },
+                { new NullPointerException(), null, true },
+                { new IOException("General failure"), "General failure", true },
+                { new IOException("Connection to 192.168.1.1 failed"), "Connection to 192.168.1.1 failed", false },
+                { new RuntimeException("Connection to 192.168.1.1 failed"), "Connection to 192.168.1.1 failed", false },
+                { new NullPointerException(), null, false },
+                { new IOException("General failure"), "General failure", false }
+        });
+    }
+
+    private final Exception exception;
+    private final String expectedMessage;
+    private final boolean isSanitizationEnabled;
+
+    public GatewayServletTest(Exception exception, String expectedMessage, boolean isSanitizationEnabled) {
+        this.exception = exception;
+        this.expectedMessage = expectedMessage;
+        this.isSanitizationEnabled = isSanitizationEnabled;
+    }
+
+    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);
+
+        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) throws ServletException, IOException {
+        EasyMock.expect(gatewayConfig.isErrorMessageSanitizationEnabled()).andStubReturn(isErrorMessageSanitizationEnabled);
+
+        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

@@ -619,6 +619,11 @@ public int getWebsocketMaxWaitBufferCount() {
     return DEFAULT_WEBSOCKET_MAX_WAIT_BUFFER_COUNT;
   }
 
+  @Override
+  public boolean isErrorMessageSanitizationEnabled() {
+    return true;
+  }
+
   @Override
   public boolean isMetricsEnabled() {
     return false;

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

@@ -901,6 +901,11 @@ public interface GatewayConfig {
    */
   boolean isAsyncSupported();
 
+  /**
+   * @return true if error message sanitization is enabled; false otherwise (defaults to true)
+   */
+  boolean isErrorMessageSanitizationEnabled();
+
   /**
    * @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