diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServlet.java b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServlet.java index a1109d850c..181783d885 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServlet.java +++ b/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,8 +98,7 @@ public synchronized void init( ServletConfig servletConfig ) throws ServletExcep filter.init( filterConfig ); } } catch( ServletException | RuntimeException e ) { - LOG.failedToInitializeServletInstace( e ); - throw e; + throw logAndSanitizeException(e); } } @@ -101,14 +106,16 @@ public synchronized void init( ServletConfig servletConfig ) throws ServletExcep 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 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); + } } diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/SanitizedException.java b/gateway-server/src/main/java/org/apache/knox/gateway/SanitizedException.java new file mode 100644 index 0000000000..b03ab989ce --- /dev/null +++ b/gateway-server/src/main/java/org/apache/knox/gateway/SanitizedException.java @@ -0,0 +1,25 @@ +/* + * 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 javax.servlet.ServletException; + +public class SanitizedException extends ServletException { + public SanitizedException(String message) { + super(message); + } +} diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java b/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java index 1b6e967a7f..b7544befd9 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java +++ b/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 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" )); diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/GatewayServletTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/GatewayServletTest.java new file mode 100644 index 0000000000..8edb035e2a --- /dev/null +++ b/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 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; + } +} diff --git a/gateway-spi-common/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java b/gateway-spi-common/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java index e3f2ddab51..7a7a7c1c56 100644 --- a/gateway-spi-common/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java +++ b/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 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; diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java b/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java index 352aabbe5b..f7261d9f9e 100644 --- a/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java +++ b/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 true if the supplied user is allowed to see all tokens * (i.e. not only tokens where userName or createdBy equals to the