Commit d2e004dc by Dave Syer

Add support for "sensitiveHeaders" in zuul.routes

By default we now discard all Set-Cookie and Cookie headers. User can manipulate it per route via zuul.routes.*.sensitiveHeaders, or globally via zuul.ignoredHeaders.
parent 40361f1b
......@@ -1227,14 +1227,59 @@ span all services and supersede any other route specification.
This means that all calls such as "/myusers/101" will be forwarded to "/101" on the "users" service.
But calls including "/admin/" will not resolve.
=== Sensitive Headers
=== Cookies and Sensitive Headers
It's OK to share headers between services in the same system, but you
probably don't want sensitive headers leaking downstream into external
servers. Thus if you use an explicit URL in a route configuration (as
opposed to a service id), then you can also specify a list of
sensitive headers and a whitelist of host patterns to not receive
those headers.
servers. You can specify a list of ignored headers as part of the
route configuration. Cookies play a special role because they have
well-defined semantics in browsers, and they are always to be treated
as sensitive. If the consumer of your proxy is a browser, then cookies
for downstream services also cause problems for the user because they
all get jumbled up (all downstream services look like they come from
the same place).
If you are careful with the design of your services, for example if
only one of the downstream services sets cookies, then you might be
able to let them flow from the backend all the way up to the
caller. Also, if your proxy sets cookies and all your back end
services are part of the same system, it can be natural to simply
share them (and for instance us Spring Session to link them up to some
shared state). Other than that, any cookies that get set by downstream
services are likely to be not very useful to the caller, so it is
recommended that you make (at least) "Set-Cookie" and "Cookie" into
sensitive headers for routes that are not part of your domain. Even
for routes that *are* part of your domain, try to think carefully
about what it means before allowing cookies to flow between them and
the proxy.
The sensitive headers can be configured as a comma-separate list per
route, e.g.
.application.yml
[source,yaml]
----
zuul:
routes:
users:
path: /myusers/**
sensitiveHeaders: Cookie,Set-Cookie
url: https://dowstream
----
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
and all cookies flow in both directions).
In addition to the per-route sensitive headers, you can set a global
value for `zuul.ignoredHeaders` for values that should be discarded
(both request and response) during interactions with downstream
services. By default these are empty, if Spring Security is not on the
classpath, and otherwise they are initialized to a set of well-known
"security" headers (e.g. involving caching) as specified by Spring
Security. The assumption in this case is that the downstream services
might add these headers too, and we want the values from the proxy.
=== Strangulation Patterns and Local Forwards
......
......@@ -19,10 +19,8 @@ package org.springframework.cloud.netflix.zuul.filters;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.net.URL;
import java.nio.charset.Charset;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.HashSet;
......@@ -39,7 +37,6 @@ import org.springframework.boot.actuate.trace.TraceRepository;
import org.springframework.http.HttpHeaders;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.util.PatternMatchUtils;
import org.springframework.web.util.UriTemplate;
import org.springframework.web.util.UriUtils;
import org.springframework.web.util.WebUtils;
......@@ -197,28 +194,11 @@ public class ProxyRequestHelper {
for (String name : this.ignoredHeaders) {
set.add(name.toLowerCase());
}
for (String name : getSensitiveHeaders(ctx)) {
set.add(name.toLowerCase());
}
for (String name : names) {
set.add(name.toLowerCase());
}
}
private Collection<String> getSensitiveHeaders(RequestContext ctx) {
URL uri = ctx.getRouteHost();
if (uri == null) {
return Collections.emptySet();
}
String host;
host = uri.getHost();
if (PatternMatchUtils.simpleMatch(this.whitelistHosts.toArray(new String[0]),
host)) {
return this.sensitiveHeaders;
}
return Collections.emptySet();
}
public boolean isIncludedHeader(String headerName) {
String name = headerName.toLowerCase();
RequestContext ctx = RequestContext.getCurrentContext();
......
......@@ -16,6 +16,9 @@
package org.springframework.cloud.netflix.zuul.filters;
import java.util.LinkedHashSet;
import java.util.Set;
import org.springframework.util.StringUtils;
import lombok.Data;
......@@ -24,14 +27,19 @@ import lombok.Data;
public class Route {
public Route(String id, String path, String location, String prefix,
Boolean retryable) {
Boolean retryable, Set<String> ignoredHeaders) {
this.id = id;
this.prefix = StringUtils.hasText(prefix) ? prefix : "";
this.path = path;
this.fullPath = prefix + path;
this.location = location;
this.retryable = retryable;
this.sensitiveHeaders = new LinkedHashSet<>();
if (ignoredHeaders != null) {
for (String header : ignoredHeaders) {
this.sensitiveHeaders.add(header.toLowerCase());
}
}
}
private String id;
......@@ -46,4 +54,6 @@ public class Route {
private Boolean retryable;
private Set<String> sensitiveHeaders = new LinkedHashSet<>();
}
\ No newline at end of file
......@@ -24,15 +24,13 @@ import java.util.Map;
import java.util.Map.Entry;
import java.util.concurrent.atomic.AtomicReference;
import lombok.extern.apachecommons.CommonsLog;
import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.ZuulRoute;
import org.springframework.cloud.netflix.zuul.util.RequestUtils;
import org.springframework.util.AntPathMatcher;
import org.springframework.util.PathMatcher;
import org.springframework.util.StringUtils;
import com.netflix.zuul.http.ZuulServlet;
import lombok.extern.apachecommons.CommonsLog;
/**
* Simple {@link RouteLocator} based on configuration data held in {@link ZuulProperties}.
......@@ -134,7 +132,7 @@ public class SimpleRouteLocator implements RouteLocator {
retryable = route.getRetryable();
}
return new Route(route.getId(), targetPath, route.getLocation(), prefix,
retryable);
retryable, route.getSensitiveHeaders());
}
/**
......@@ -172,22 +170,23 @@ public class SimpleRouteLocator implements RouteLocator {
String adjustedPath = path;
if (RequestUtils.isDispatcherServletRequest()
&& StringUtils.hasText(dispatcherServletPath)) {
if (!dispatcherServletPath.equals("/")) {
&& StringUtils.hasText(this.dispatcherServletPath)) {
if (!this.dispatcherServletPath.equals("/")) {
adjustedPath = path.substring(this.dispatcherServletPath.length());
}
} else if (RequestUtils.isZuulServletRequest()){
if (StringUtils.hasText(zuulServletPath)
&& !zuulServletPath.equals("/")) {
}
else if (RequestUtils.isZuulServletRequest()) {
if (StringUtils.hasText(this.zuulServletPath)
&& !this.zuulServletPath.equals("/")) {
adjustedPath = path.substring(this.zuulServletPath.length());
}
} else {
//do nothing
}
else {
// do nothing
}
log.debug("adjustedPath=" + path);
return adjustedPath;
}
}
......@@ -66,8 +66,6 @@ public class ZuulProperties {
private Set<String> ignoredHeaders = new LinkedHashSet<>();
private Sensitive sensitive = new Sensitive();
private String servletPath = "/zuul";
private boolean ignoreLocalService = true;
......@@ -108,21 +106,6 @@ public class ZuulProperties {
@Data
@AllArgsConstructor
@NoArgsConstructor
public static class Sensitive {
/**
* Headers that are considered sensitive, and not passed on through a proxy.
*/
private Set<String> headers = new LinkedHashSet<>(Arrays.asList("Cookie"));
/**
* Hostname (patterns) that are considered safe and can receive sensitive headers
* when a route is specified as a URL.
*/
private Set<String> whitelist = new LinkedHashSet<>(Arrays.asList("localhost"));
}
@Data
@AllArgsConstructor
@NoArgsConstructor
public static class ZuulRoute {
private String id;
......@@ -137,6 +120,9 @@ public class ZuulProperties {
private Boolean retryable;
private Set<String> sensitiveHeaders = new LinkedHashSet<>(
Arrays.asList("Cookie", "Set-Cookie"));
public ZuulRoute(String text) {
String location = null;
String path = text;
......@@ -184,7 +170,8 @@ public class ZuulProperties {
}
public Route getRoute(String prefix) {
return new Route(this.id, this.path, getLocation(), prefix, this.retryable);
return new Route(this.id, this.path, getLocation(), prefix, this.retryable,
this.sensitiveHeaders);
}
}
......
......@@ -71,6 +71,7 @@ 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.getRetryable() != null) {
ctx.put("retryable", route.getRetryable());
......@@ -81,9 +82,8 @@ public class PreDecorationFilter extends ZuulFilter {
ctx.addOriginResponseHeader("X-Zuul-Service", location);
}
else if (location.startsWith("forward:")) {
ctx.set("forward.to",
StringUtils.cleanPath(location.substring("forward:".length())
+ route.getPath()));
ctx.set("forward.to", StringUtils.cleanPath(
location.substring("forward:".length()) + route.getPath()));
ctx.setRouteHost(null);
return null;
}
......
......@@ -81,7 +81,7 @@ public class ContextPathZuulProxyApplicationTests {
@Test
public void stripPrefixFalseAppendsPath() {
this.routes.addRoute(new ZuulRoute("strip", "/strip/**", "strip",
"http://localhost:" + this.port + "/app/local", false, false));
"http://localhost:" + this.port + "/app/local", false, false, null));
this.endpoint.reset();
ResponseEntity<String> result = new TestRestTemplate().exchange(
"http://localhost:" + this.port + "/app/strip", HttpMethod.GET,
......
......@@ -81,7 +81,7 @@ public class ServletPathZuulProxyApplicationTests {
@Test
public void stripPrefixFalseAppendsPath() {
this.routes.addRoute(new ZuulRoute("strip", "/strip/**", "strip",
"http://localhost:" + this.port + "/app/local", false, false));
"http://localhost:" + this.port + "/app/local", false, false, null));
this.endpoint.reset();
ResponseEntity<String> result = new TestRestTemplate().exchange(
"http://localhost:" + this.port + "/app/strip", HttpMethod.GET,
......
......@@ -100,7 +100,7 @@ public abstract class ZuulProxyTestBase {
@Test
public void stripPrefixFalseAppendsPath() {
this.routes.addRoute(new ZuulProperties.ZuulRoute("strip", "/strip/**", "strip",
"http://localhost:" + this.port + "/local", false, false));
"http://localhost:" + this.port + "/local", false, false, null));
this.endpoint.reset();
ResponseEntity<String> result = new TestRestTemplate().exchange(
"http://localhost:" + this.port + "/strip", HttpMethod.GET,
......
......@@ -17,8 +17,6 @@
package org.springframework.cloud.netflix.zuul.filters;
import java.io.IOException;
import java.net.URL;
import java.util.Collections;
import java.util.List;
import org.junit.Before;
......@@ -181,35 +179,4 @@ public class ProxyRequestHelperTests {
assertThat(queryString, is("?wsdl"));
}
@Test
public void ignoreSensitiveHeadersMatchingHost() throws Exception {
ProxyRequestHelper helper = new ProxyRequestHelper();
helper.setSensitiveHeaders(Collections.singleton("Cookie"));
helper.setWhitelistHosts(Collections.singleton("*"));
RequestContext context = RequestContext.getCurrentContext();
context.setRouteHost(new URL("http://example.com"));
helper.addIgnoredHeaders();
assertThat(helper.isIncludedHeader("Cookie"), is(false));
}
@Test
public void ignoreSensitiveHeadersNotMatching() throws Exception {
ProxyRequestHelper helper = new ProxyRequestHelper();
helper.setSensitiveHeaders(Collections.singleton("Cookie"));
helper.setWhitelistHosts(Collections.singleton("foo.com"));
RequestContext context = RequestContext.getCurrentContext();
context.setRouteHost(new URL("http://example.com"));
helper.addIgnoredHeaders();
assertThat(helper.isIncludedHeader("Cookie"), is(true));
}
@Test
public void ignoreSensitiveHeadersWhenNoRoute() throws Exception {
ProxyRequestHelper helper = new ProxyRequestHelper();
helper.setSensitiveHeaders(Collections.singleton("Cookie"));
helper.setWhitelistHosts(Collections.singleton("foo.com"));
RequestContext context = RequestContext.getCurrentContext();
helper.addIgnoredHeaders();
assertThat(helper.isIncludedHeader("Cookie"), is(true));
}
}
......@@ -19,7 +19,9 @@ package org.springframework.cloud.netflix.zuul.filters;
import java.util.Collections;
import org.junit.Test;
import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.ZuulRoute;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
/**
......@@ -35,9 +37,26 @@ public class ZuulPropertiesTests {
}
@Test
public void addtIgnoredHeaders() {
public void addIgnoredHeaders() {
this.zuul.setIgnoredHeaders(Collections.singleton("x-foo"));
assertTrue(this.zuul.getIgnoredHeaders().contains("x-foo"));
}
@Test
public void defaultSensitiveHeaders() {
ZuulRoute route = new ZuulRoute("foo");
this.zuul.getRoutes().put("foo", route);
assertTrue(this.zuul.getRoutes().get("foo").getSensitiveHeaders()
.contains("Cookie"));
}
@Test
public void addSensitiveHeaders() {
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"));
}
}
......@@ -145,7 +145,7 @@ public class DiscoveryClientRouteLocatorTests {
DiscoveryClientRouteLocator routeLocator = new DiscoveryClientRouteLocator("/",
this.discovery, this.properties);
this.properties.getRoutes().put("foo",
new ZuulRoute("foo", "/foo/**", "foo", null, false, null));
new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null));
this.properties.setStripPrefix(false);
this.properties.setPrefix("/proxy");
routeLocator.getRoutes(); // force refresh
......@@ -172,7 +172,7 @@ public class DiscoveryClientRouteLocatorTests {
DiscoveryClientRouteLocator routeLocator = new DiscoveryClientRouteLocator("/",
this.discovery, this.properties);
this.properties.getRoutes().put("foo",
new ZuulRoute("foo", "/foo/**", "foo", null, false, null));
new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null));
this.properties.setPrefix("/proxy");
routeLocator.getRoutes(); // force refresh
Route route = routeLocator.getMatchingRoute("/proxy/foo/1");
......@@ -188,7 +188,7 @@ public class DiscoveryClientRouteLocatorTests {
DiscoveryClientRouteLocator routeLocator = new DiscoveryClientRouteLocator("/app",
this.discovery, this.properties);
this.properties.getRoutes().put("foo",
new ZuulRoute("foo", "/foo/**", "foo", null, false, null));
new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null));
this.properties.setPrefix("/proxy");
routeLocator.getRoutes(); // force refresh
Route route = routeLocator.getMatchingRoute("/app/proxy/foo/1");
......@@ -203,7 +203,7 @@ public class DiscoveryClientRouteLocatorTests {
DiscoveryClientRouteLocator routeLocator = new DiscoveryClientRouteLocator("/",
this.discovery, this.properties);
this.properties.getRoutes().put("foo",
new ZuulRoute("foo", "/foo/**", "foo", null, false, null));
new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null));
this.properties.setPrefix("/proxy");
routeLocator.getRoutes(); // force refresh
Route route = routeLocator.getMatchingRoute("/zuul/proxy/foo/1");
......@@ -285,7 +285,7 @@ public class DiscoveryClientRouteLocatorTests {
this.discovery, this.properties);
this.properties.setIgnoredPatterns(Collections.singleton(IGNOREDPATTERN));
this.properties.getRoutes().put("foo",
new ZuulRoute("foo", "/foo/**", "foo", null, false, null));
new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null));
this.properties.setStripPrefix(false);
this.properties.setPrefix("/proxy");
routeLocator.getRoutes(); // force refresh
......@@ -302,7 +302,7 @@ public class DiscoveryClientRouteLocatorTests {
this.properties
.setIgnoredPatterns(Collections.singleton("/proxy" + IGNOREDPATTERN));
this.properties.getRoutes().put("foo",
new ZuulRoute("foo", "/foo/**", "foo", null, false, null));
new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null));
this.properties.setStripPrefix(false);
this.properties.setPrefix("/proxy");
routeLocator.getRoutes(); // force refresh
......@@ -347,7 +347,7 @@ public class DiscoveryClientRouteLocatorTests {
this.discovery, this.properties);
this.properties.setIgnoredPatterns(Collections.singleton(IGNOREDPATTERN));
this.properties.getRoutes().put("foo",
new ZuulRoute("foo", "/foo/**", "foo", null, false, null));
new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null));
this.properties.setPrefix("/proxy");
routeLocator.getRoutes(); // force refresh
Route route = routeLocator.getMatchingRoute("/proxy/foo/1");
......@@ -363,7 +363,7 @@ public class DiscoveryClientRouteLocatorTests {
this.properties
.setIgnoredPatterns(Collections.singleton("/proxy" + IGNOREDPATTERN));
this.properties.getRoutes().put("foo",
new ZuulRoute("foo", "/foo/**", "foo", null, false, null));
new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null));
this.properties.setPrefix("/proxy");
routeLocator.getRoutes(); // force refresh
Route route = routeLocator.getMatchingRoute("/proxy/foo/1");
......
......@@ -16,9 +16,6 @@
package org.springframework.cloud.netflix.zuul.filters.pre;
import static org.junit.Assert.assertEquals;
import static org.mockito.MockitoAnnotations.initMocks;
import java.util.List;
import org.junit.Before;
......@@ -33,6 +30,9 @@ import org.springframework.mock.web.MockHttpServletRequest;
import com.netflix.util.Pair;
import com.netflix.zuul.context.RequestContext;
import static org.junit.Assert.assertEquals;
import static org.mockito.MockitoAnnotations.initMocks;
/**
* @author Dave Syer
*/
......@@ -72,15 +72,16 @@ public class PreDecorationFilterTests {
this.properties.setPrefix("/api");
this.properties.setStripPrefix(true);
this.request.setRequestURI("/api/foo/1");
this.routeLocator.addRoute(new ZuulRoute("foo", "/foo/**", "foo", null, false,
null));
this.routeLocator.addRoute(
new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null));
this.filter.run();
RequestContext ctx = RequestContext.getCurrentContext();
assertEquals("/foo/1", ctx.get("requestURI"));
assertEquals("localhost:80", ctx.getZuulRequestHeaders().get("x-forwarded-host"));
assertEquals("http", ctx.getZuulRequestHeaders().get("x-forwarded-proto"));
assertEquals("/api", ctx.getZuulRequestHeaders().get("x-forwarded-prefix"));
assertEquals("foo", getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid"));
assertEquals("foo",
getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid"));
}
@Test
......@@ -88,8 +89,8 @@ public class PreDecorationFilterTests {
this.properties.setPrefix("/api");
this.properties.setStripPrefix(true);
this.request.setRequestURI("/api/foo/1");
this.routeLocator.addRoute(new ZuulRoute("foo", "/foo/**", null, "forward:/foo", true,
null));
this.routeLocator.addRoute(
new ZuulRoute("foo", "/foo/**", null, "forward:/foo", true, null, null));
this.filter.run();
RequestContext ctx = RequestContext.getCurrentContext();
assertEquals("/foo/1", ctx.get("forward.to"));
......@@ -98,8 +99,8 @@ public class PreDecorationFilterTests {
@Test
public void forwardWithoutStripPrefixAppendsPath() throws Exception {
this.request.setRequestURI("/foo/1");
this.routeLocator.addRoute(new ZuulRoute("foo", "/foo/**", null, "forward:/bar", false,
null));
this.routeLocator.addRoute(
new ZuulRoute("foo", "/foo/**", null, "forward:/bar", false, null, null));
this.filter.run();
RequestContext ctx = RequestContext.getCurrentContext();
assertEquals("/bar/foo/1", ctx.get("forward.to"));
......@@ -117,7 +118,8 @@ public class PreDecorationFilterTests {
assertEquals("localhost:80", ctx.getZuulRequestHeaders().get("x-forwarded-host"));
assertEquals("http", ctx.getZuulRequestHeaders().get("x-forwarded-proto"));
assertEquals("/api/foo", ctx.getZuulRequestHeaders().get("x-forwarded-prefix"));
assertEquals("foo", getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid"));
assertEquals("foo",
getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid"));
}
private Object getHeader(List<Pair<String, String>> headers, String key) {
......
......@@ -54,8 +54,8 @@ public class ZuulHandlerMappingTests {
@Test
public void mappedPath() throws Exception {
Mockito.when(this.locator.getRoutes()).thenReturn(
Collections.singletonList(new Route("foo", "/foo/**", "foo", "", null)));
Mockito.when(this.locator.getRoutes()).thenReturn(Collections
.singletonList(new Route("foo", "/foo/**", "foo", "", null, null)));
this.request.setServletPath("/foo/");
this.mapping.setDirty(true);
assertNotNull(this.mapping.getHandler(this.request));
......@@ -63,8 +63,8 @@ public class ZuulHandlerMappingTests {
@Test
public void defaultPath() throws Exception {
Mockito.when(this.locator.getRoutes()).thenReturn(
Collections.singletonList(new Route("default", "/**", "foo", "", null)));
Mockito.when(this.locator.getRoutes()).thenReturn(Collections
.singletonList(new Route("default", "/**", "foo", "", null, null)));
;
this.request.setServletPath("/");
this.mapping.setDirty(true);
......@@ -73,8 +73,8 @@ public class ZuulHandlerMappingTests {
@Test
public void errorPath() throws Exception {
Mockito.when(this.locator.getRoutes()).thenReturn(
Collections.singletonList(new Route("default", "/**", "foo", "", null)));
Mockito.when(this.locator.getRoutes()).thenReturn(Collections
.singletonList(new Route("default", "/**", "foo", "", null, null)));
this.request.setServletPath("/error");
this.mapping.setDirty(true);
assertNull(this.mapping.getHandler(this.request));
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment