Skip to content

Commit b558c63

Browse files
smolnar82smolnar82
committed
KNOX-3039 - Add error message sanitization to GatewayServlet
1 parent c7af5b5 commit b558c63

File tree

8 files changed

+298
-12
lines changed

8 files changed

+298
-12
lines changed

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

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,17 @@
4545
import java.net.URISyntaxException;
4646
import java.nio.charset.StandardCharsets;
4747
import java.util.Enumeration;
48+
import java.util.Objects;
49+
50+
import static org.apache.knox.gateway.util.ExceptionSanitizer.from;
4851

4952
public class GatewayServlet implements Servlet, Filter {
5053
public static final String GATEWAY_DESCRIPTOR_LOCATION_DEFAULT = "gateway.xml";
5154
public static final String GATEWAY_DESCRIPTOR_LOCATION_PARAM = "gatewayDescriptorLocation";
5255

56+
private static boolean isErrorMessageSanitizationEnabled = true;
57+
private static String errorMessageSanitizationPattern = "\\b(?:\\d{1,3}\\.){3}\\d{1,3}\\b";
58+
5359
private static final GatewayResources res = ResourcesFactory.get( GatewayResources.class );
5460
private static final GatewayMessages LOG = MessagesFactory.get( GatewayMessages.class );
5561

@@ -83,6 +89,8 @@ public synchronized void setFilter( GatewayFilter filter ) throws ServletExcepti
8389

8490
@Override
8591
public synchronized void init( ServletConfig servletConfig ) throws ServletException {
92+
final GatewayConfig gatewayConfig = (GatewayConfig) servletConfig.getServletContext().getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE);
93+
configureErrorMessageSanitizationConfigs(gatewayConfig);
8694
try {
8795
if( filter == null ) {
8896
filter = createFilter( servletConfig );
@@ -92,23 +100,31 @@ public synchronized void init( ServletConfig servletConfig ) throws ServletExcep
92100
filter.init( filterConfig );
93101
}
94102
} catch( ServletException | RuntimeException e ) {
95-
LOG.failedToInitializeServletInstace( e );
96-
throw e;
103+
throw logAndSanitizeException(e);
104+
}
105+
}
106+
107+
private void configureErrorMessageSanitizationConfigs(final GatewayConfig gatewayConfig) {
108+
if (Objects.nonNull(gatewayConfig)) {
109+
isErrorMessageSanitizationEnabled = gatewayConfig.isErrorMessageSanitizationEnabled();
110+
errorMessageSanitizationPattern = gatewayConfig.getErrorMessageSanitizationPattern();
97111
}
98112
}
99113

100114
@Override
101115
public void init( FilterConfig filterConfig ) throws ServletException {
102116
try {
103-
if( filter == null ) {
117+
final GatewayConfig gatewayConfig = (GatewayConfig) filterConfig.getServletContext().getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE);
118+
configureErrorMessageSanitizationConfigs(gatewayConfig);
119+
120+
if ( filter == null ) {
104121
filter = createFilter( filterConfig );
105122
}
106123
if( filter != null ) {
107124
filter.init( filterConfig );
108125
}
109126
} catch( ServletException | RuntimeException e ) {
110-
LOG.failedToInitializeServletInstace( e );
111-
throw e;
127+
throw logAndSanitizeException(e);
112128
}
113129
}
114130

@@ -126,8 +142,7 @@ public void service( ServletRequest servletRequest, ServletResponse servletRespo
126142
try {
127143
f.doFilter( servletRequest, servletResponse, null );
128144
} catch( IOException | RuntimeException | ServletException e ) {
129-
LOG.failedToExecuteFilter( e );
130-
throw e;
145+
throw logAndSanitizeException(e);
131146
}
132147
} else {
133148
((HttpServletResponse)servletResponse).setStatus( HttpServletResponse.SC_SERVICE_UNAVAILABLE );
@@ -153,10 +168,8 @@ public void doFilter( ServletRequest servletRequest, ServletResponse servletResp
153168
//TODO: This should really happen naturally somehow as part of being a filter. This way will cause problems eventually.
154169
chain.doFilter( servletRequest, servletResponse );
155170
}
156-
157-
} catch( IOException | RuntimeException | ServletException e ) {
158-
LOG.failedToExecuteFilter( e );
159-
throw e;
171+
} catch (Exception e) {
172+
throw logAndSanitizeException(e);
160173
}
161174
} else {
162175
((HttpServletResponse)servletResponse).setStatus( HttpServletResponse.SC_SERVICE_UNAVAILABLE );
@@ -168,7 +181,6 @@ public void doFilter( ServletRequest servletRequest, ServletResponse servletResp
168181
}
169182
}
170183

171-
172184
@Override
173185
public String getServletInfo() {
174186
return res.gatewayServletInfo();
@@ -277,4 +289,9 @@ public Enumeration<String> getInitParameterNames() {
277289
return config.getInitParameterNames();
278290
}
279291
}
292+
293+
private SanitizedException logAndSanitizeException(Exception e) {
294+
LOG.failedToExecuteFilter(e);
295+
return from(e, isErrorMessageSanitizationEnabled, errorMessageSanitizationPattern);
296+
}
280297
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with this
4+
* work for additional information regarding copyright ownership. The ASF
5+
* licenses this file to you under the Apache License, Version 2.0 (the
6+
* "License"); you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
* <p>
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
* <p>
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
13+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
14+
* License for the specific language governing permissions and limitations under
15+
* the License.
16+
*/
17+
package org.apache.knox.gateway;
18+
19+
import javax.servlet.ServletException;
20+
21+
public class SanitizedException extends ServletException {
22+
23+
public SanitizedException() {
24+
super();
25+
}
26+
27+
public SanitizedException(String message) {
28+
super(message);
29+
}
30+
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,10 @@ public class GatewayConfigImpl extends Configuration implements GatewayConfig {
177177
public static final String CLUSTER_CONFIG_MONITOR_INTERVAL_SUFFIX = ".interval";
178178
public static final String CLUSTER_CONFIG_MONITOR_ENABLED_SUFFIX = ".enabled";
179179

180+
private static final String ERROR_MESSAGE_SANITIZATION_ENABLED = GATEWAY_CONFIG_FILE_PREFIX + ".error.sanitization.enabled";
181+
private static final boolean ERROR_MESSAGE_SANITIZATION_ENABLED_DEFAULT = true;
182+
private static final String ERROR_MESSAGE_SANITIZATION_PATTERN = GATEWAY_CONFIG_FILE_PREFIX + ".error.sanitization.pattern";
183+
private static final String ERROR_MESSAGE_SANITIZATION_PATTERN_DEFAULT = "\\b(?:\\d{1,3}\\.){3}\\d{1,3}\\b";
180184

181185
// These config property names are not inline with the convention of using the
182186
// GATEWAY_CONFIG_FILE_PREFIX as is done by those above. These are left for
@@ -968,6 +972,16 @@ public List<String> getGlobalRulesServices() {
968972
return DEFAULT_GLOBAL_RULES_SERVICES;
969973
}
970974

975+
@Override
976+
public boolean isErrorMessageSanitizationEnabled() {
977+
return getBoolean(ERROR_MESSAGE_SANITIZATION_ENABLED, ERROR_MESSAGE_SANITIZATION_ENABLED_DEFAULT);
978+
}
979+
980+
@Override
981+
public String getErrorMessageSanitizationPattern() {
982+
return get(ERROR_MESSAGE_SANITIZATION_PATTERN, ERROR_MESSAGE_SANITIZATION_PATTERN_DEFAULT);
983+
}
984+
971985
@Override
972986
public boolean isMetricsEnabled() {
973987
return Boolean.parseBoolean(get( METRICS_ENABLED, "false" ));

‎gateway-server/src/main/java/org/apache/knox/gateway/util/ExceptionSanitizer.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package org.apache.knox.gateway.util;
2+
3+
import org.apache.knox.gateway.SanitizedException;
4+
5+
import java.util.Objects;
6+
import java.util.regex.Pattern;
7+
8+
public class ExceptionSanitizer {
9+
10+
private ExceptionSanitizer() {
11+
}
12+
13+
public static SanitizedException from(final Exception e, final boolean isSanitizationEnabled, final String pattern) {
14+
if (Objects.isNull(e) || Objects.isNull(e.getMessage())) {
15+
return new SanitizedException();
16+
}
17+
final String message = isSanitizationEnabled ? e.getMessage().replaceAll(pattern, "[hidden]") : e.getMessage();
18+
return new SanitizedException(message);
19+
}
20+
}

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

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.knox.gateway;
19+
20+
import org.apache.knox.gateway.config.GatewayConfig;
21+
import org.easymock.EasyMock;
22+
import org.easymock.IMocksControl;
23+
import org.junit.Before;
24+
import org.junit.Test;
25+
import org.junit.runner.RunWith;
26+
import org.junit.runners.Parameterized;
27+
28+
import javax.servlet.FilterConfig;
29+
import javax.servlet.ServletConfig;
30+
import javax.servlet.ServletContext;
31+
import javax.servlet.ServletException;
32+
import javax.servlet.http.HttpServletRequest;
33+
import javax.servlet.http.HttpServletResponse;
34+
import java.io.IOException;
35+
import java.util.Arrays;
36+
import java.util.Collection;
37+
38+
import static org.junit.Assert.assertEquals;
39+
import static org.junit.Assert.assertNull;
40+
41+
@RunWith(Parameterized.class)
42+
public class GatewayServletTest {
43+
44+
private static final String IP_ADDRESS_PATTERN = "\\b(?:\\d{1,3}\\.){3}\\d{1,3}\\b";
45+
private static final String EMAIL_ADDRESS_PATTERN = "\\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\\.[A-Z|a-z]{2,}\\b";
46+
private static final String CREDIT_CARD_PATTERN = "\\b\\d{4}-\\d{4}-\\d{4}-\\d{4}\\b";
47+
48+
@Parameterized.Parameters(name = "{index}: SanitizationEnabled={2}, Pattern={3}, Exception={0}, ExpectedMessage={1}")
49+
public static Collection<Object[]> data() {
50+
return Arrays.asList(new Object[][] {
51+
{ new IOException("Connection to 192.168.1.1 failed"), "Connection to [hidden] failed", true, IP_ADDRESS_PATTERN },
52+
{ new RuntimeException("Connection to 192.168.1.1 failed"), "Connection to [hidden] failed", true, IP_ADDRESS_PATTERN },
53+
{ new NullPointerException(), null, true, IP_ADDRESS_PATTERN },
54+
{ new IOException("General failure"), "General failure", true, IP_ADDRESS_PATTERN },
55+
{ new IOException("Connection to 192.168.1.1 failed"), "Connection to 192.168.1.1 failed", false, IP_ADDRESS_PATTERN },
56+
{ new RuntimeException("Connection to 192.168.1.1 failed"), "Connection to 192.168.1.1 failed", false, IP_ADDRESS_PATTERN },
57+
{ new NullPointerException(), null, false, IP_ADDRESS_PATTERN },
58+
{ new IOException("General failure"), "General failure", false, IP_ADDRESS_PATTERN },
59+
{ new IOException("User email: user@example.com"), "User email: [hidden]", true, EMAIL_ADDRESS_PATTERN },
60+
{ new RuntimeException("Credit card number: 1234-5678-9101-1121"), "Credit card number: [hidden]", true, CREDIT_CARD_PATTERN },
61+
});
62+
}
63+
64+
private final Exception exception;
65+
private final String expectedMessage;
66+
private final boolean isSanitizationEnabled;
67+
private final String sanitizationPattern;
68+
69+
public GatewayServletTest(Exception exception, String expectedMessage, boolean isSanitizationEnabled, String sanitizationPattern) {
70+
this.exception = exception;
71+
this.expectedMessage = expectedMessage;
72+
this.isSanitizationEnabled = isSanitizationEnabled;
73+
this.sanitizationPattern = sanitizationPattern;
74+
}
75+
76+
private IMocksControl mockControl;
77+
private ServletConfig servletConfig;
78+
private ServletContext servletContext;
79+
private GatewayConfig gatewayConfig;
80+
private HttpServletRequest request;
81+
private HttpServletResponse response;
82+
private GatewayFilter filter;
83+
84+
@Before
85+
public void setUp() throws ServletException {
86+
mockControl = EasyMock.createControl();
87+
servletConfig = mockControl.createMock(ServletConfig.class);
88+
servletContext = mockControl.createMock(ServletContext.class);
89+
gatewayConfig = mockControl.createMock(GatewayConfig.class);
90+
request = mockControl.createMock(HttpServletRequest.class);
91+
response = mockControl.createMock(HttpServletResponse.class);
92+
filter = mockControl.createMock(GatewayFilter.class);
93+
94+
EasyMock.expect(servletConfig.getServletName()).andStubReturn("default");
95+
EasyMock.expect(servletConfig.getServletContext()).andStubReturn(servletContext);
96+
EasyMock.expect(servletContext.getAttribute("org.apache.knox.gateway.config")).andStubReturn(gatewayConfig);
97+
}
98+
99+
@Test
100+
public void testExceptionSanitization() throws ServletException, IOException {
101+
GatewayServlet servlet = initializeServletWithSanitization(isSanitizationEnabled, sanitizationPattern);
102+
103+
try {
104+
servlet.service(request, response);
105+
} catch (Exception e) {
106+
if (expectedMessage != null) {
107+
assertEquals(expectedMessage, e.getMessage());
108+
} else {
109+
assertNull(e.getMessage());
110+
}
111+
}
112+
113+
mockControl.verify();
114+
}
115+
116+
private GatewayServlet initializeServletWithSanitization(boolean isErrorMessageSanitizationEnabled, String sanitizationPattern) throws ServletException, IOException {
117+
EasyMock.expect(gatewayConfig.isErrorMessageSanitizationEnabled()).andStubReturn(isErrorMessageSanitizationEnabled);
118+
EasyMock.expect(gatewayConfig.getErrorMessageSanitizationPattern()).andStubReturn(sanitizationPattern);
119+
120+
filter.init(EasyMock.anyObject(FilterConfig.class));
121+
EasyMock.expectLastCall().once();
122+
filter.doFilter(EasyMock.eq(request), EasyMock.eq(response), EasyMock.isNull());
123+
EasyMock.expectLastCall().andThrow(exception).once();
124+
125+
mockControl.replay();
126+
127+
GatewayServlet servlet = new GatewayServlet(filter);
128+
servlet.init(servletConfig);
129+
return servlet;
130+
}
131+
}

‎gateway-server/src/test/java/org/apache/knox/gateway/util/ExceptionSanitizerTest.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package org.apache.knox.gateway.util;
2+
3+
import org.apache.knox.gateway.SanitizedException;
4+
import org.junit.Test;
5+
6+
import static org.junit.Assert.assertEquals;
7+
import static org.junit.Assert.assertNull;
8+
9+
public class ExceptionSanitizerTest {
10+
11+
private static final String SANITIZATION_PATTERN = "(?i)password=\\w+";
12+
13+
@Test
14+
public void testSanitizationEnabledWithSensitiveMessage() {
15+
Exception input = new Exception("User login failed: password=secret123");
16+
SanitizedException sanitized = ExceptionSanitizer.from(input, true, SANITIZATION_PATTERN);
17+
assertEquals("User login failed: [hidden]", sanitized.getMessage());
18+
}
19+
20+
@Test
21+
public void testSanitizationDisabledWithSensitiveMessage() {
22+
Exception input = new Exception("User login failed: password=secret123");
23+
SanitizedException sanitized = ExceptionSanitizer.from(input, false, SANITIZATION_PATTERN);
24+
assertEquals("User login failed: password=secret123", sanitized.getMessage());
25+
}
26+
27+
@Test
28+
public void testNullException() {
29+
SanitizedException sanitized = ExceptionSanitizer.from(null, true, SANITIZATION_PATTERN);
30+
assertNull(sanitized.getMessage());
31+
}
32+
33+
@Test
34+
public void testNullMessageInException() {
35+
Exception input = new Exception((String) null);
36+
SanitizedException sanitized = ExceptionSanitizer.from(input, true, SANITIZATION_PATTERN);
37+
assertNull(sanitized.getMessage());
38+
}
39+
40+
@Test
41+
public void testSanitizationEnabledWithNoMatch() {
42+
Exception input = new Exception("Something went wrong, but no password present.");
43+
SanitizedException sanitized = ExceptionSanitizer.from(input, true, SANITIZATION_PATTERN);
44+
assertEquals("Something went wrong, but no password present.", sanitized.getMessage());
45+
}
46+
47+
@Test
48+
public void testSanitizationWithMultipleMatches() {
49+
Exception input = new Exception("password=one password=two");
50+
SanitizedException sanitized = ExceptionSanitizer.from(input, true, SANITIZATION_PATTERN);
51+
assertEquals("[hidden] [hidden]", sanitized.getMessage());
52+
}
53+
}

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ public class GatewayTestConfig extends Configuration implements GatewayConfig {
8080
private ConcurrentMap<String, Integer> topologyPortMapping = new ConcurrentHashMap<>();
8181
private int backupVersionLimit = -1;
8282
private long backupAgeLimit = -1;
83+
private String sanitizationPattern = "sanitizationPattern";
8384

8485
public GatewayTestConfig(Properties props) {
8586
super.getProps().putAll(props);
@@ -630,6 +631,16 @@ public int getWebsocketMaxWaitBufferCount() {
630631
return DEFAULT_WEBSOCKET_MAX_WAIT_BUFFER_COUNT;
631632
}
632633

634+
@Override
635+
public boolean isErrorMessageSanitizationEnabled() {
636+
return true;
637+
}
638+
639+
@Override
640+
public String getErrorMessageSanitizationPattern() {
641+
return sanitizationPattern;
642+
}
643+
633644
@Override
634645
public boolean isMetricsEnabled() {
635646
return false;

0 commit comments

Comments
 (0)