From 775f0fa8613c5360bac2159f4c45089733049587 Mon Sep 17 00:00:00 2001 From: David Good Date: Fri, 11 Sep 2020 11:29:45 +0200 Subject: [PATCH] Improve sanitization for list of URI types Prior to this commit, Actuator would sanitize properties values when serializing them on the dedicated endpoint. Keys like "password" or "secret" are entirely sanitized, but other keys like "uri" or "address" are considered as URI types and only the password part of the user info is sanitized. This commit fixes the sanitization process where lists of such URI types would not match the first entries of the list since they're starting with `'['`. This commit improves the regexp matching process to sanitize all URIs within a collection. The documentation is also updated to better underline the processing difference between complete sanitization and selective sanitization for URIs. Fixes gh-23037 --- .../boot/actuate/endpoint/Sanitizer.java | 3 ++- .../boot/actuate/endpoint/SanitizerTests.java | 17 +++++++++++++++++ .../src/main/asciidoc/howto.adoc | 7 +++++-- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/Sanitizer.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/Sanitizer.java index a0a0d345304..d0c2e735393 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/Sanitizer.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/Sanitizer.java @@ -37,6 +37,7 @@ import org.springframework.util.StringUtils; * @author Stephane Nicoll * @author HaiTao Zhang * @author Chris Bono + * @author David Good * @since 2.0.0 */ public class Sanitizer { @@ -49,7 +50,7 @@ public class Sanitizer { private static final Set URI_USERINFO_KEYS = new LinkedHashSet<>( Arrays.asList("uri", "uris", "address", "addresses")); - private static final Pattern URI_USERINFO_PATTERN = Pattern.compile("[A-Za-z]+://.+:(.*)@.+$"); + private static final Pattern URI_USERINFO_PATTERN = Pattern.compile("\\[?[A-Za-z]+://.+:(.*)@.+$"); private Pattern[] keysToSanitize; diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/SanitizerTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/SanitizerTests.java index 9785411463f..0a8f0b22cf8 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/SanitizerTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/SanitizerTests.java @@ -30,6 +30,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Phillip Webb * @author Stephane Nicoll * @author Chris Bono + * @author David Good */ class SanitizerTests { @@ -105,6 +106,22 @@ class SanitizerTests { .isEqualTo("http://user1:******@localhost:8080,http://user2:******@localhost:8082"); } + @ParameterizedTest(name = "key = {0}") + @MethodSource("matchingUriUserInfoKeys") + void uriKeyWithUserProvidedListLiteralShouldBeSanitized(String key) { + Sanitizer sanitizer = new Sanitizer(); + assertThat(sanitizer.sanitize(key, "[amqp://username:password@host/]")) + .isEqualTo("[amqp://username:******@host/]"); + assertThat(sanitizer.sanitize(key, + "[http://user1:password1@localhost:8080,http://user2@localhost:8082,http://localhost:8083]")).isEqualTo( + "[http://user1:******@localhost:8080,http://user2@localhost:8082,http://localhost:8083]"); + assertThat(sanitizer.sanitize(key, + "[http://user1:password1@localhost:8080,http://user2:password2@localhost:8082]")) + .isEqualTo("[http://user1:******@localhost:8080,http://user2:******@localhost:8082]"); + assertThat(sanitizer.sanitize(key, "[http://user1@localhost:8080,http://user2@localhost:8082]")) + .isEqualTo("[http://user1@localhost:8080,http://user2@localhost:8082]"); + } + private static Stream matchingUriUserInfoKeys() { return Stream.of("uri", "my.uri", "myuri", "uris", "my.uris", "myuris", "address", "my.address", "myaddress", "addresses", "my.addresses", "myaddresses"); diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc index 30af1d34406..25cb500277e 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc @@ -2227,10 +2227,13 @@ Information returned by the `env` and `configprops` endpoints can be somewhat se The patterns to use can be customized using the `management.endpoint.env.keys-to-sanitize` and `management.endpoint.configprops.keys-to-sanitize` respectively. -Spring Boot uses sensible defaults for such keys: any key ending with the word "password", "secret", "key", "token", "vcap_services", "sun.java.command", "uri", "uris", "address" or "addresses" is sanitized. +Spring Boot uses sensible defaults for such keys: any key ending with the word "password", "secret", "key", "token", "vcap_services", "sun.java.command" is entirely sanitized. Additionally, any key that holds the word `credentials` as part of the key is sanitized (configured as a regular expression, i.e. `+*credentials.*+`). -If any of the keys to sanitize are URI format (i.e. `://:@:/`), only the password part is sanitized. +Furthermore, Spring Boot only sanitizes the sensitive portion of URIs for keys which end with "uri", "uris", "address", or "addresses". +The sensitive portion of the URI is identified using the format `://:@:/`. +For example, for the property `myclient.uri=http://user1:password1@localhost:8081`, the resulting sanitized value is +`++http://user1:******@localhost:8081++`.