diff options
author | Bjørn Christian Seime <bjorncs@verizonmedia.com> | 2020-11-19 11:36:59 +0100 |
---|---|---|
committer | Bjørn Christian Seime <bjorncs@verizonmedia.com> | 2020-11-19 11:36:59 +0100 |
commit | e8dae5ac60da5fee6d84ddaf055ee4834a178c0d (patch) | |
tree | ea100c409011063459a2ee4a6c67efa90fcb96b5 /jdisc_http_service | |
parent | 301252bba0c1c17d5e962091191ed4187be038c1 (diff) |
Add strict filtering mode to jdisc
Jdisc will reject requests that does not match a request filter (chain) if strict filtering is enabled.
Diffstat (limited to 'jdisc_http_service')
5 files changed, 66 insertions, 9 deletions
diff --git a/jdisc_http_service/abi-spec.json b/jdisc_http_service/abi-spec.json index 8bf7f30964a..8b48631d4aa 100644 --- a/jdisc_http_service/abi-spec.json +++ b/jdisc_http_service/abi-spec.json @@ -742,6 +742,7 @@ "public com.yahoo.jdisc.http.ServerConfig$Builder filter(java.util.List)", "public com.yahoo.jdisc.http.ServerConfig$Builder defaultFilters(com.yahoo.jdisc.http.ServerConfig$DefaultFilters$Builder)", "public com.yahoo.jdisc.http.ServerConfig$Builder defaultFilters(java.util.List)", + "public com.yahoo.jdisc.http.ServerConfig$Builder strictFiltering(boolean)", "public com.yahoo.jdisc.http.ServerConfig$Builder maxWorkerThreads(int)", "public com.yahoo.jdisc.http.ServerConfig$Builder minWorkerThreads(int)", "public com.yahoo.jdisc.http.ServerConfig$Builder stopTimeout(double)", @@ -930,6 +931,7 @@ "public com.yahoo.jdisc.http.ServerConfig$Filter filter(int)", "public java.util.List defaultFilters()", "public com.yahoo.jdisc.http.ServerConfig$DefaultFilters defaultFilters(int)", + "public boolean strictFiltering()", "public int maxWorkerThreads()", "public int minWorkerThreads()", "public double stopTimeout()", diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterResolver.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterResolver.java index b80dd216b04..1e2686aa184 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterResolver.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterResolver.java @@ -2,6 +2,12 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.jdisc.Metric; +import com.yahoo.jdisc.NoopSharedResource; +import com.yahoo.jdisc.Response; +import com.yahoo.jdisc.handler.FastContentWriter; +import com.yahoo.jdisc.handler.ResponseDispatch; +import com.yahoo.jdisc.handler.ResponseHandler; +import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.filter.RequestFilter; import com.yahoo.jdisc.http.filter.ResponseFilter; import com.yahoo.jdisc.http.servlet.ServletRequest; @@ -22,10 +28,12 @@ class FilterResolver { private final FilterBindings bindings; private final Metric metric; + private final boolean strictFiltering; - FilterResolver(FilterBindings bindings, Metric metric) { + FilterResolver(FilterBindings bindings, Metric metric, boolean strictFiltering) { this.bindings = bindings; this.metric = metric; + this.strictFiltering = strictFiltering; } Optional<RequestFilter> resolveRequestFilter(HttpServletRequest servletRequest, URI jdiscUri) { @@ -33,8 +41,13 @@ class FilterResolver { if (maybeFilterId.isPresent()) { metric.add(MetricDefinitions.FILTERING_REQUEST_HANDLED, 1L, createMetricContext(servletRequest, maybeFilterId.get())); servletRequest.setAttribute(ServletRequest.JDISC_REQUEST_CHAIN, maybeFilterId.get()); - } else { + } else if (!strictFiltering) { metric.add(MetricDefinitions.FILTERING_REQUEST_UNHANDLED, 1L, createMetricContext(servletRequest, null)); + } else { + String syntheticFilterId = RejectingRequestFilter.SYNTHETIC_FILTER_CHAIN_ID; + metric.add(MetricDefinitions.FILTERING_REQUEST_HANDLED, 1L, createMetricContext(servletRequest, syntheticFilterId)); + servletRequest.setAttribute(ServletRequest.JDISC_REQUEST_CHAIN, syntheticFilterId); + return Optional.of(RejectingRequestFilter.INSTANCE); } return maybeFilterId.map(bindings::getRequestFilter); } @@ -56,4 +69,20 @@ class FilterResolver { : Map.of(); return JDiscHttpServlet.getConnector(request).createRequestMetricContext(request, extraDimensions); } + + private static class RejectingRequestFilter extends NoopSharedResource implements RequestFilter { + + private static final RejectingRequestFilter INSTANCE = new RejectingRequestFilter(); + private static final String SYNTHETIC_FILTER_CHAIN_ID = "strict-reject"; + + @Override + public void filter(HttpRequest request, ResponseHandler handler) { + Response response = new Response(Response.Status.FORBIDDEN); + response.headers().add("Content-Type", "text/plain"); + try (FastContentWriter writer = ResponseDispatch.newInstance(response).connectFastWriter(handler)) { + writer.write("Request did not match any request filter chain"); + } + } + } + } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscContext.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscContext.java index 66471587bd5..b37a7352dc6 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscContext.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscContext.java @@ -20,7 +20,7 @@ public class JDiscContext { Metric metric, ServerConfig serverConfig) { - this.filterResolver = new FilterResolver(filterBindings, metric); + this.filterResolver = new FilterResolver(filterBindings, metric, serverConfig.strictFiltering()); this.container = container; this.janitor = janitor; this.metric = metric; diff --git a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def index f33dc35ea0b..f75a4aaa441 100644 --- a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def +++ b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def @@ -33,6 +33,9 @@ defaultFilters[].filterId string # The local port which the default filter should be applied to defaultFilters[].localPort int +# Reject all requests not handled by a request filter (chain) +strictFiltering bool default = false + # Max number of threads in underlying Jetty pool maxWorkerThreads int default = 200 diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java index fd929b3e037..9c5c4027ae3 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java @@ -35,6 +35,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.mockito.ArgumentMatchers.any; @@ -495,7 +496,7 @@ public class FilterTestCase { .build(); MetricConsumerMock metricConsumerMock = new MetricConsumerMock(); MyRequestHandler requestHandler = new MyRequestHandler(); - TestDriver testDriver = newDriver(requestHandler, filterBindings, metricConsumerMock); + TestDriver testDriver = newDriver(requestHandler, filterBindings, metricConsumerMock, false); testDriver.client().get("/status.html"); assertThat(requestHandler.awaitInvocation(), is(true)); @@ -510,25 +511,47 @@ public class FilterTestCase { assertThat(testDriver.close(), is(true)); } + @Test + public void requireThatStrictFilteringRejectsRequestsNotMatchingFilterChains() throws IOException { + RequestFilter filter = mock(RequestFilter.class); + FilterBindings filterBindings = new FilterBindings.Builder() + .addRequestFilter("my-request-filter", filter) + .addRequestFilterBinding("my-request-filter", "http://*/filtered/*") + .build(); + MyRequestHandler requestHandler = new MyRequestHandler(); + TestDriver testDriver = newDriver(requestHandler, filterBindings, new MetricConsumerMock(), true); + + testDriver.client().get("/unfiltered/") + .expectStatusCode(is(Response.Status.FORBIDDEN)) + .expectContent(containsString("Request did not match any request filter chain")); + verify(filter, never()).filter(any(), any()); + assertThat(testDriver.close(), is(true)); + } + private static TestDriver newDriver(MyRequestHandler requestHandler, FilterBindings filterBindings) { - return newDriver(requestHandler, filterBindings, new MetricConsumerMock()); + return newDriver(requestHandler, filterBindings, new MetricConsumerMock(), false); } - private static TestDriver newDriver(MyRequestHandler requestHandler, FilterBindings filterBindings, MetricConsumerMock metricConsumer) { + private static TestDriver newDriver( + MyRequestHandler requestHandler, + FilterBindings filterBindings, + MetricConsumerMock metricConsumer, + boolean strictFiltering) { return TestDriver.newInstance( JettyHttpServer.class, requestHandler, - newFilterModule(filterBindings, metricConsumer)); + newFilterModule(filterBindings, metricConsumer, strictFiltering)); } - private static com.google.inject.Module newFilterModule(FilterBindings filterBindings, MetricConsumerMock metricConsumer) { + private static com.google.inject.Module newFilterModule( + FilterBindings filterBindings, MetricConsumerMock metricConsumer, boolean strictFiltering) { return Modules.combine( new AbstractModule() { @Override protected void configure() { bind(FilterBindings.class).toInstance(filterBindings); - bind(ServerConfig.class).toInstance(new ServerConfig(new ServerConfig.Builder())); + bind(ServerConfig.class).toInstance(new ServerConfig(new ServerConfig.Builder().strictFiltering(strictFiltering))); bind(ConnectorConfig.class).toInstance(new ConnectorConfig(new ConnectorConfig.Builder())); bind(ServletPathsConfig.class).toInstance(new ServletPathsConfig(new ServletPathsConfig.Builder())); } |