From df5303912f65c7fb4a40ee73b8f9750a72ac3d1a Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Thu, 7 Apr 2016 17:59:54 -0600 Subject: [PATCH] Add global zuul sensitiveHeaders option. Route specific sensitiveHeaders override global. fixes gh-944 --- .../main/asciidoc/spring-cloud-netflix.adoc | 2 + .../netflix/zuul/filters/ZuulProperties.java | 14 ++++++- .../zuul/filters/pre/PreDecorationFilter.java | 35 +++++++++-------- .../zuul/filters/ZuulPropertiesTests.java | 28 ++++++++++--- .../filters/pre/PreDecorationFilterTests.java | 39 ++++++++++++++++++- 5 files changed, 93 insertions(+), 25 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 6e3ae0f0..2f943e14 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -1267,6 +1267,8 @@ route, e.g. url: https://dowstream ---- +Sensitive headers can also be set globally setting `zuul.sensitiveHeaders`. If `sensitiveHeaders` is set on a route, this will override the global `sensitiveHeaders` setting. + NOTE: this is the default value for `sensitiveHeaders`, so you don't need to set it unless you want it to be different. N.B. this is new in Spring Cloud Netflix 1.1 (in 1.0 the user had no control over headers diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java index 03862c91..8e5049c1 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java @@ -115,6 +115,17 @@ public class ZuulProperties { */ private boolean removeSemicolonContent = true; + /** + * List of sensitive headers that are not passed to downstream requests. Defaults + * to a "safe" set of headers that commonly contain user credentials. It's OK to + * remove those from the list if the downstream service is part of the same system + * as the proxy, so they are sharing authentication data. If using a physical URL + * outside your own domain, then generally it would be a bad idea to leak user + * credentials. + */ + private Set sensitiveHeaders = new LinkedHashSet<>( + Arrays.asList("Cookie", "Set-Cookie", "Authorization")); + public Set getIgnoredHeaders() { Set ignoredHeaders = new LinkedHashSet<>(this.ignoredHeaders); if (ClassUtils.isPresent( @@ -193,8 +204,7 @@ public class ZuulProperties { * outside your own domain, then generally it would be a bad idea to leak user * credentials. */ - private Set sensitiveHeaders = new LinkedHashSet<>( - Arrays.asList("Cookie", "Set-Cookie", "Authorization")); + private Set sensitiveHeaders = new LinkedHashSet<>(); public ZuulRoute(String text) { String location = null; diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java index 5cd16f30..d15178ca 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java @@ -19,6 +19,7 @@ package org.springframework.cloud.netflix.zuul.filters.pre; import java.net.MalformedURLException; import java.net.URL; +import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper; import org.springframework.cloud.netflix.zuul.filters.Route; import org.springframework.cloud.netflix.zuul.filters.RouteLocator; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; @@ -37,21 +38,18 @@ public class PreDecorationFilter extends ZuulFilter { private RouteLocator routeLocator; - private boolean addProxyHeaders; - private String dispatcherServletPath; - private String zuulServletPath; + + private ZuulProperties properties; private UrlPathHelper urlPathHelper = new UrlPathHelper(); public PreDecorationFilter(RouteLocator routeLocator, - String dispatcherServletPath, ZuulProperties zuulProperties) { + String dispatcherServletPath, ZuulProperties properties) { this.routeLocator = routeLocator; - this.addProxyHeaders = zuulProperties.isAddProxyHeaders(); - this.urlPathHelper.setRemoveSemicolonContent(zuulProperties.isRemoveSemicolonContent()); + this.properties = properties; + this.urlPathHelper.setRemoveSemicolonContent(properties.isRemoveSemicolonContent()); this.dispatcherServletPath = dispatcherServletPath; - this.zuulServletPath = zuulProperties.getServletPath(); - } @Override @@ -67,9 +65,8 @@ public class PreDecorationFilter extends ZuulFilter { @Override public boolean shouldFilter() { RequestContext ctx = RequestContext.getCurrentContext(); - return !ctx.containsKey("forward.to") // another filter has already forwarded - && !ctx.containsKey("serviceId"); // another filter has already determined - // serviceId + return !ctx.containsKey("forward.to") // a filter has already forwarded + && !ctx.containsKey("serviceId"); // a filter has already determined serviceId } @Override @@ -83,7 +80,11 @@ public class PreDecorationFilter extends ZuulFilter { if (location != null) { ctx.put("requestURI", route.getPath()); ctx.put("proxy", route.getId()); - ctx.put("ignoredHeaders", route.getSensitiveHeaders()); + if (route.getSensitiveHeaders().isEmpty()) { + ctx.put(ProxyRequestHelper.IGNORED_HEADERS, this.properties.getSensitiveHeaders()); + } else { + ctx.put(ProxyRequestHelper.IGNORED_HEADERS, route.getSensitiveHeaders()); + } if (route.getRetryable() != null) { ctx.put("retryable", route.getRetryable()); @@ -105,7 +106,7 @@ public class PreDecorationFilter extends ZuulFilter { ctx.setRouteHost(null); ctx.addOriginResponseHeader("X-Zuul-ServiceId", location); } - if (this.addProxyHeaders) { + if (this.properties.isAddProxyHeaders()) { ctx.addZuulRequestHeader("X-Forwarded-Host", ctx.getRequest().getServerName()); ctx.addZuulRequestHeader("X-Forwarded-Port", @@ -129,14 +130,14 @@ public class PreDecorationFilter extends ZuulFilter { } else { log.warn("No route found for uri: " + requestURI); - + String fallBackUri = requestURI; String fallbackPrefix = dispatcherServletPath; //default fallback servlet is DispatcherServlet - + if (RequestUtils.isZuulServletRequest()) { //remove the Zuul servletPath from the requestUri - log.debug("zuulServletPath=" + zuulServletPath); - fallBackUri = fallBackUri.replaceFirst(zuulServletPath, ""); + log.debug("zuulServletPath=" + this.properties.getServletPath()); + fallBackUri = fallBackUri.replaceFirst(this.properties.getServletPath(), ""); log.debug("Replaced Zuul servlet path:" + fallBackUri); } else { //remove the DispatcherServlet servletPath from the requestUri diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ZuulPropertiesTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ZuulPropertiesTests.java index a9a4f4ad..5a1270bb 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ZuulPropertiesTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ZuulPropertiesTests.java @@ -16,8 +16,11 @@ package org.springframework.cloud.netflix.zuul.filters; +import java.util.Arrays; import java.util.Collections; +import org.junit.After; +import org.junit.Before; import org.junit.Test; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.ZuulRoute; @@ -29,7 +32,17 @@ import static org.junit.Assert.assertTrue; */ public class ZuulPropertiesTests { - private ZuulProperties zuul = new ZuulProperties(); + private ZuulProperties zuul; + + @Before + public void setup() { + this.zuul = new ZuulProperties(); + } + + @After + public void teardown() { + this.zuul = null; + } @Test public void defaultIgnoredHeaders() { @@ -46,17 +59,22 @@ public class ZuulPropertiesTests { public void defaultSensitiveHeaders() { ZuulRoute route = new ZuulRoute("foo"); this.zuul.getRoutes().put("foo", route); - assertTrue(this.zuul.getRoutes().get("foo").getSensitiveHeaders() - .contains("Cookie")); + assertTrue(this.zuul.getRoutes().get("foo").getSensitiveHeaders().isEmpty()); + assertTrue(this.zuul.getSensitiveHeaders().containsAll( + Arrays.asList("Cookie", "Set-Cookie", "Authorization"))); } @Test public void addSensitiveHeaders() { + this.zuul.setSensitiveHeaders(Collections.singleton("x-bar")); ZuulRoute route = new ZuulRoute("foo"); route.setSensitiveHeaders(Collections.singleton("x-foo")); this.zuul.getRoutes().put("foo", route); - assertFalse(this.zuul.getRoutes().get("foo").getSensitiveHeaders() - .contains("Cookie")); + ZuulRoute foo = this.zuul.getRoutes().get("foo"); + assertTrue(foo.getSensitiveHeaders().contains("x-foo")); + assertFalse(foo.getSensitiveHeaders().contains("Cookie")); + assertTrue(this.zuul.getSensitiveHeaders().contains("x-bar")); + assertFalse(this.zuul.getSensitiveHeaders().contains("Cookie")); } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java index 13349969..cfea9d5d 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java @@ -16,12 +16,15 @@ package org.springframework.cloud.netflix.zuul.filters.pre; +import java.util.Collections; import java.util.List; +import java.util.Set; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; import org.springframework.cloud.client.discovery.DiscoveryClient; +import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.ZuulRoute; import org.springframework.cloud.netflix.zuul.filters.discovery.DiscoveryClientRouteLocator; @@ -31,6 +34,8 @@ import com.netflix.util.Pair; import com.netflix.zuul.context.RequestContext; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.MockitoAnnotations.initMocks; /** @@ -236,7 +241,39 @@ public class PreDecorationFilterTests { this.filter.run(); assertEquals("/special/api/bar/1", ctx.get("forward.to")); - } + } + + @Test + public void sensitiveHeadersOverride() throws Exception { + this.properties.setPrefix("/api"); + this.properties.setStripPrefix(true); + this.properties.setSensitiveHeaders(Collections.singleton("x-bar")); + this.request.setRequestURI("/api/foo/1"); + ZuulRoute route = new ZuulRoute("/foo/**", "foo"); + route.setSensitiveHeaders(Collections.singleton("x-foo")); + this.routeLocator.addRoute(route); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + @SuppressWarnings("unchecked") + Set sensitiveHeaders = (Set) ctx.get(ProxyRequestHelper.IGNORED_HEADERS); + assertTrue("sensitiveHeaders is wrong", sensitiveHeaders.containsAll(Collections.singletonList("x-foo"))); + assertFalse("sensitiveHeaders is wrong", sensitiveHeaders.contains("Cookie")); + } + + @Test + public void sensitiveHeadersDefaults() throws Exception { + this.properties.setPrefix("/api"); + this.properties.setStripPrefix(true); + this.properties.setSensitiveHeaders(Collections.singleton("x-bar")); + this.request.setRequestURI("/api/foo/1"); + this.routeLocator.addRoute("/foo/**", "foo"); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + @SuppressWarnings("unchecked") + Set sensitiveHeaders = (Set) ctx.get(ProxyRequestHelper.IGNORED_HEADERS); + assertTrue("sensitiveHeaders is wrong", sensitiveHeaders.containsAll(Collections.singletonList("x-bar"))); + assertFalse("sensitiveHeaders is wrong", sensitiveHeaders.contains("Cookie")); + } private Object getHeader(List> headers, String key) { String value = null;