From d9190ba4211f03851f20cd78869dc80479e0d2ff Mon Sep 17 00:00:00 2001 From: Sandor Molnar Date: Wed, 16 Jul 2025 17:10:17 +0200 Subject: [PATCH] KNOX-3039 - Add error message sanitization to GatewayServlet --- .../apache/knox/gateway/GatewayServlet.java | 41 ++++-- .../knox/gateway/SanitizedException.java | 30 ++++ .../config/impl/GatewayConfigImpl.java | 14 ++ .../knox/gateway/util/ExceptionSanitizer.java | 36 +++++ .../knox/gateway/GatewayServletTest.java | 131 ++++++++++++++++++ .../gateway/util/ExceptionSanitizerTest.java | 70 ++++++++++ .../knox/gateway/GatewayTestConfig.java | 11 ++ .../knox/gateway/config/GatewayConfig.java | 10 ++ 8 files changed, 331 insertions(+), 12 deletions(-) create mode 100644 gateway-server/src/main/java/org/apache/knox/gateway/SanitizedException.java create mode 100644 gateway-server/src/main/java/org/apache/knox/gateway/util/ExceptionSanitizer.java create mode 100644 gateway-server/src/test/java/org/apache/knox/gateway/GatewayServletTest.java create mode 100644 gateway-server/src/test/java/org/apache/knox/gateway/util/ExceptionSanitizerTest.java 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..5cfc3764d3 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 @@ -45,11 +45,17 @@ import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import java.util.Enumeration; +import java.util.Objects; + +import static org.apache.knox.gateway.util.ExceptionSanitizer.from; 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 +89,8 @@ public synchronized void setFilter( GatewayFilter filter ) throws ServletExcepti @Override public synchronized void init( ServletConfig servletConfig ) throws ServletException { + final GatewayConfig gatewayConfig = (GatewayConfig) servletConfig.getServletContext().getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE); + configureErrorMessageSanitizationConfigs(gatewayConfig); try { if( filter == null ) { filter = createFilter( servletConfig ); @@ -92,23 +100,31 @@ public synchronized void init( ServletConfig servletConfig ) throws ServletExcep filter.init( filterConfig ); } } catch( ServletException | RuntimeException e ) { - LOG.failedToInitializeServletInstace( e ); - throw e; + throw logAndSanitizeException(e); + } + } + + private void configureErrorMessageSanitizationConfigs(final GatewayConfig gatewayConfig) { + if (Objects.nonNull(gatewayConfig)) { + isErrorMessageSanitizationEnabled = gatewayConfig.isErrorMessageSanitizationEnabled(); + errorMessageSanitizationPattern = gatewayConfig.getErrorMessageSanitizationPattern(); } } @Override public void init( FilterConfig filterConfig ) throws ServletException { try { - if( filter == null ) { + final GatewayConfig gatewayConfig = (GatewayConfig) filterConfig.getServletContext().getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE); + configureErrorMessageSanitizationConfigs(gatewayConfig); + + if ( filter == null ) { filter = createFilter( filterConfig ); } if( filter != null ) { filter.init( filterConfig ); } } catch( ServletException | RuntimeException e ) { - LOG.failedToInitializeServletInstace( e ); - throw e; + throw logAndSanitizeException(e); } } @@ -126,8 +142,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 +168,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 +181,6 @@ public void doFilter( ServletRequest servletRequest, ServletResponse servletResp } } - @Override public String getServletInfo() { return res.gatewayServletInfo(); @@ -277,4 +289,9 @@ public Enumeration getInitParameterNames() { return config.getInitParameterNames(); } } + + private SanitizedException logAndSanitizeException(Exception e) { + LOG.failedToExecuteFilter(e); + return from(e, isErrorMessageSanitizationEnabled, errorMessageSanitizationPattern); + } } 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..4a769dc88d --- /dev/null +++ b/gateway-server/src/main/java/org/apache/knox/gateway/SanitizedException.java @@ -0,0 +1,30 @@ +/* + * 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() { + super(); + } + + 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 8b9f8d5ce5..cc91915420 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 @@ -177,6 +177,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 @@ -968,6 +972,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/main/java/org/apache/knox/gateway/util/ExceptionSanitizer.java b/gateway-server/src/main/java/org/apache/knox/gateway/util/ExceptionSanitizer.java new file mode 100644 index 0000000000..b8d24fa127 --- /dev/null +++ b/gateway-server/src/main/java/org/apache/knox/gateway/util/ExceptionSanitizer.java @@ -0,0 +1,36 @@ +/* + * 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.util; + +import org.apache.knox.gateway.SanitizedException; + +import java.util.Objects; + +public class ExceptionSanitizer { + + private ExceptionSanitizer() { + } + + public static SanitizedException from(final Exception e, final boolean isSanitizationEnabled, final String pattern) { + if (Objects.isNull(e) || Objects.isNull(e.getMessage())) { + return new SanitizedException(); + } + final String message = isSanitizationEnabled ? e.getMessage().replaceAll(pattern, "[hidden]") : e.getMessage(); + return new SanitizedException(message); + } +} 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-server/src/test/java/org/apache/knox/gateway/util/ExceptionSanitizerTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/util/ExceptionSanitizerTest.java new file mode 100644 index 0000000000..4fce807ff0 --- /dev/null +++ b/gateway-server/src/test/java/org/apache/knox/gateway/util/ExceptionSanitizerTest.java @@ -0,0 +1,70 @@ +/* + * 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.util; + +import org.apache.knox.gateway.SanitizedException; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +public class ExceptionSanitizerTest { + + private static final String SANITIZATION_PATTERN = "(?i)password=\\w+"; + + @Test + public void testSanitizationEnabledWithSensitiveMessage() { + Exception input = new Exception("User login failed: password=secret123"); + SanitizedException sanitized = ExceptionSanitizer.from(input, true, SANITIZATION_PATTERN); + assertEquals("User login failed: [hidden]", sanitized.getMessage()); + } + + @Test + public void testSanitizationDisabledWithSensitiveMessage() { + Exception input = new Exception("User login failed: password=secret123"); + SanitizedException sanitized = ExceptionSanitizer.from(input, false, SANITIZATION_PATTERN); + assertEquals("User login failed: password=secret123", sanitized.getMessage()); + } + + @Test + public void testNullException() { + SanitizedException sanitized = ExceptionSanitizer.from(null, true, SANITIZATION_PATTERN); + assertNull(sanitized.getMessage()); + } + + @Test + public void testNullMessageInException() { + Exception input = new Exception((String) null); + SanitizedException sanitized = ExceptionSanitizer.from(input, true, SANITIZATION_PATTERN); + assertNull(sanitized.getMessage()); + } + + @Test + public void testSanitizationEnabledWithNoMatch() { + Exception input = new Exception("Something went wrong, but no password present."); + SanitizedException sanitized = ExceptionSanitizer.from(input, true, SANITIZATION_PATTERN); + assertEquals("Something went wrong, but no password present.", sanitized.getMessage()); + } + + @Test + public void testSanitizationWithMultipleMatches() { + Exception input = new Exception("password=one password=two"); + SanitizedException sanitized = ExceptionSanitizer.from(input, true, SANITIZATION_PATTERN); + assertEquals("[hidden] [hidden]", sanitized.getMessage()); + } +} 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 c1d2f80ccc..4c4e3a1b4d 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 @@ -80,6 +80,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); @@ -630,6 +631,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 f92b8daec3..d2a2cdaf7c 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 @@ -910,6 +910,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