From 7da70a52fd1f3dcc20e2f7d370aa83f536ab3f7f Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Tue, 7 Mar 2017 16:59:10 -0800 Subject: [PATCH] Mask sensitive placeholders in env endpoint Closes gh-8282 --- .../actuate/endpoint/EnvironmentEndpoint.java | 61 ++++++++++++++++--- .../endpoint/mvc/EnvironmentMvcEndpoint.java | 11 +--- .../endpoint/EnvironmentEndpointTests.java | 61 +++++++++++++++++++ .../mvc/EnvironmentMvcEndpointTests.java | 11 ++++ 4 files changed, 126 insertions(+), 18 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/EnvironmentEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/EnvironmentEndpoint.java index c21442ba03..431a86c2e3 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/EnvironmentEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/EnvironmentEndpoint.java @@ -26,7 +26,10 @@ import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.EnumerablePropertySource; import org.springframework.core.env.Environment; import org.springframework.core.env.MutablePropertySources; +import org.springframework.core.env.PropertyResolver; import org.springframework.core.env.PropertySource; +import org.springframework.core.env.PropertySources; +import org.springframework.core.env.PropertySourcesPropertyResolver; import org.springframework.core.env.StandardEnvironment; /** @@ -35,6 +38,7 @@ import org.springframework.core.env.StandardEnvironment; * @author Dave Syer * @author Phillip Webb * @author Christian Dupuis + * @author Madhura Bhave */ @ConfigurationProperties(prefix = "endpoints.env") public class EnvironmentEndpoint extends AbstractEndpoint> { @@ -56,14 +60,15 @@ public class EnvironmentEndpoint extends AbstractEndpoint> { public Map invoke() { Map result = new LinkedHashMap(); result.put("profiles", getEnvironment().getActiveProfiles()); - for (Entry> entry : getPropertySources().entrySet()) { + PropertyResolver resolver = getResolver(); + for (Entry> entry : getPropertySourcesAsMap().entrySet()) { PropertySource source = entry.getValue(); String sourceName = entry.getKey(); if (source instanceof EnumerablePropertySource) { EnumerablePropertySource enumerable = (EnumerablePropertySource) source; Map properties = new LinkedHashMap(); for (String name : enumerable.getPropertyNames()) { - properties.put(name, sanitize(name, enumerable.getProperty(name))); + properties.put(name, sanitize(name, resolver.getProperty(name))); } properties = postProcessSourceProperties(sourceName, properties); if (properties != null) { @@ -74,9 +79,24 @@ public class EnvironmentEndpoint extends AbstractEndpoint> { return result; } - private Map> getPropertySources() { + public PropertyResolver getResolver() { + PlaceholderSanitizingPropertyResolver resolver = new PlaceholderSanitizingPropertyResolver( + getPropertySources(), this.sanitizer); + resolver.setIgnoreUnresolvableNestedPlaceholders(true); + return resolver; + } + + private Map> getPropertySourcesAsMap() { Map> map = new LinkedHashMap>(); - MutablePropertySources sources = null; + MutablePropertySources sources = getPropertySources(); + for (PropertySource source : sources) { + extract("", map, source); + } + return map; + } + + private MutablePropertySources getPropertySources() { + MutablePropertySources sources; Environment environment = getEnvironment(); if (environment != null && environment instanceof ConfigurableEnvironment) { sources = ((ConfigurableEnvironment) environment).getPropertySources(); @@ -84,10 +104,7 @@ public class EnvironmentEndpoint extends AbstractEndpoint> { else { sources = new StandardEnvironment().getPropertySources(); } - for (PropertySource source : sources) { - extract("", map, source); - } - return map; + return sources; } private void extract(String root, Map> map, @@ -120,4 +137,32 @@ public class EnvironmentEndpoint extends AbstractEndpoint> { return properties; } + /** + * {@link PropertySourcesPropertyResolver} that sanitizes sensitive placeholders + * if present. + * + * @author Madhura Bhave + */ + private class PlaceholderSanitizingPropertyResolver extends PropertySourcesPropertyResolver { + + private final Sanitizer sanitizer; + + /** + * Create a new resolver against the given property sources. + * @param propertySources the set of {@link PropertySource} objects to use + * @param sanitizer the sanitizer used to sanitize sensitive values + */ + PlaceholderSanitizingPropertyResolver(PropertySources + propertySources, Sanitizer sanitizer) { + super(propertySources); + this.sanitizer = sanitizer; + } + + @Override + protected String getPropertyAsRawString(String key) { + String value = super.getPropertyAsRawString(key); + return (String) this.sanitizer.sanitize(key, value); + } + } + } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/EnvironmentMvcEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/EnvironmentMvcEndpoint.java index c55c2abec8..5ac94c7196 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/EnvironmentMvcEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/EnvironmentMvcEndpoint.java @@ -22,10 +22,8 @@ import org.springframework.context.EnvironmentAware; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.EnumerablePropertySource; import org.springframework.core.env.Environment; -import org.springframework.core.env.PropertyResolver; import org.springframework.core.env.PropertySource; import org.springframework.core.env.PropertySources; -import org.springframework.core.env.PropertySourcesPropertyResolver; import org.springframework.http.HttpStatus; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.ResponseBody; @@ -95,14 +93,7 @@ public class EnvironmentMvcEndpoint extends EndpointMvcAdapter @Override protected Object getOptionalValue(Environment source, String name) { - PropertyResolver resolver = source; - if (source instanceof ConfigurableEnvironment) { - resolver = new PropertySourcesPropertyResolver( - ((ConfigurableEnvironment) source).getPropertySources()); - ((PropertySourcesPropertyResolver) resolver) - .setIgnoreUnresolvableNestedPlaceholders(true); - } - Object result = resolver.getProperty(name); + Object result = ((EnvironmentEndpoint) getDelegate()).getResolver().getProperty(name); if (result != null) { result = ((EnvironmentEndpoint) getDelegate()).sanitize(name, result); } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/EnvironmentEndpointTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/EnvironmentEndpointTests.java index 71ce4c33b7..6fc48feb69 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/EnvironmentEndpointTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/EnvironmentEndpointTests.java @@ -39,6 +39,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Christian Dupuis * @author Nicolas Lejeune * @author Stephane Nicoll + * @author Madhura Bhave */ public class EnvironmentEndpointTests extends AbstractEndpointTests { @@ -194,6 +195,66 @@ public class EnvironmentEndpointTests extends AbstractEndpointTests env = report.invoke(); + Map testProperties = (Map) env + .get("test"); + assertThat(testProperties.get("my.foo")).isEqualTo("hello"); + } + + @SuppressWarnings("unchecked") + @Test + public void propertyWithPlaceholderNotResolved() throws Exception { + this.context = new AnnotationConfigApplicationContext(); + EnvironmentTestUtils.addEnvironment(this.context, + "my.foo: ${bar.blah}"); + this.context.register(Config.class); + this.context.refresh(); + EnvironmentEndpoint report = getEndpointBean(); + Map env = report.invoke(); + Map testProperties = (Map) env + .get("test"); + assertThat(testProperties.get("my.foo")).isEqualTo("${bar.blah}"); + } + + @SuppressWarnings("unchecked") + @Test + public void propertyWithSensitivePlaceholderResolved() throws Exception { + this.context = new AnnotationConfigApplicationContext(); + EnvironmentTestUtils.addEnvironment(this.context, + "my.foo: http://${bar.password}://hello", "bar.password: hello"); + this.context.register(Config.class); + this.context.refresh(); + EnvironmentEndpoint report = getEndpointBean(); + Map env = report.invoke(); + Map testProperties = (Map) env + .get("test"); + assertThat(testProperties.get("my.foo")).isEqualTo("http://******://hello"); + } + + @SuppressWarnings("unchecked") + @Test + public void propertyWithSensitivePlaceholderNotResolved() throws Exception { + this.context = new AnnotationConfigApplicationContext(); + EnvironmentTestUtils.addEnvironment(this.context, + "my.foo: http://${bar.password}://hello"); + this.context.register(Config.class); + this.context.refresh(); + EnvironmentEndpoint report = getEndpointBean(); + Map env = report.invoke(); + Map testProperties = (Map) env + .get("test"); + assertThat(testProperties.get("my.foo")).isEqualTo("http://${bar.password}://hello"); + } + @Configuration @EnableConfigurationProperties public static class Config { diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/EnvironmentMvcEndpointTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/EnvironmentMvcEndpointTests.java index ee38d20058..d67ccbd0c0 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/EnvironmentMvcEndpointTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/EnvironmentMvcEndpointTests.java @@ -148,6 +148,17 @@ public class EnvironmentMvcEndpointTests { .andExpect(content().string(containsString("\"my.foo\":\"${my.bar}\""))); } + @Test + public void nestedPathWithSensitivePlaceholderShouldSanitize() throws Exception { + Map map = new HashMap(); + map.put("my.foo", "${my.password}"); + map.put("my.password", "hello"); + ((ConfigurableEnvironment) this.context.getEnvironment()).getPropertySources() + .addFirst(new MapPropertySource("placeholder", map)); + this.mvc.perform(get("/env/my.*")).andExpect(status().isOk()) + .andExpect(content().string(containsString("\"my.foo\":\"******\""))); + } + @Configuration @Import({ JacksonAutoConfiguration.class, HttpMessageConvertersAutoConfiguration.class, WebMvcAutoConfiguration.class,