Skip to content

Commit 37575d7

Browse files
committed
Merge pull request #19999 from bono007
* pr/19999: Polish contribution Add 'uris', 'address' and 'addresses' to keys to sanitize. Closes gh-19999
2 parents 45fd603 + acc453d commit 37575d7

File tree

4 files changed

+133
-22
lines changed

4 files changed

+133
-22
lines changed

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/Sanitizer.java

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2019 the original author or authors.
2+
* Copyright 2012-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,8 +16,12 @@
1616

1717
package org.springframework.boot.actuate.endpoint;
1818

19+
import java.util.Arrays;
20+
import java.util.LinkedHashSet;
21+
import java.util.Set;
1922
import java.util.regex.Matcher;
2023
import java.util.regex.Pattern;
24+
import java.util.stream.Collectors;
2125

2226
import org.springframework.util.Assert;
2327
import org.springframework.util.StringUtils;
@@ -32,18 +36,29 @@
3236
* @author Nicolas Lejeune
3337
* @author Stephane Nicoll
3438
* @author HaiTao Zhang
39+
* @author Chris Bono
3540
* @since 2.0.0
3641
*/
3742
public class Sanitizer {
3843

3944
private static final String[] REGEX_PARTS = { "*", "$", "^", "+" };
4045

46+
private static final Set<String> DEFAULT_KEYS_TO_SANITIZE = new LinkedHashSet<>(Arrays.asList("password", "secret",
47+
"key", "token", ".*credentials.*", "vcap_services", "sun.java.command"));
48+
49+
private static final Set<String> URI_USERINFO_KEYS = new LinkedHashSet<>(
50+
Arrays.asList("uri", "uris", "address", "addresses"));
51+
4152
private static final Pattern URI_USERINFO_PATTERN = Pattern.compile("[A-Za-z]+://.+:(.*)@.+$");
4253

4354
private Pattern[] keysToSanitize;
4455

56+
static {
57+
DEFAULT_KEYS_TO_SANITIZE.addAll(URI_USERINFO_KEYS);
58+
}
59+
4560
public Sanitizer() {
46-
this("password", "secret", "key", "token", ".*credentials.*", "vcap_services", "sun.java.command", "uri");
61+
this(DEFAULT_KEYS_TO_SANITIZE.toArray(new String[0]));
4762
}
4863

4964
public Sanitizer(String... keysToSanitize) {
@@ -91,21 +106,33 @@ public Object sanitize(String key, Object value) {
91106
}
92107
for (Pattern pattern : this.keysToSanitize) {
93108
if (pattern.matcher(key).matches()) {
94-
if (pattern.matcher("uri").matches()) {
95-
return sanitizeUri(value);
109+
if (keyIsUriWithUserInfo(pattern)) {
110+
return sanitizeUris(value.toString());
96111
}
97112
return "******";
98113
}
99114
}
100115
return value;
101116
}
102117

103-
private Object sanitizeUri(Object value) {
104-
String uriString = value.toString();
105-
Matcher matcher = URI_USERINFO_PATTERN.matcher(uriString);
118+
private boolean keyIsUriWithUserInfo(Pattern pattern) {
119+
for (String uriKey : URI_USERINFO_KEYS) {
120+
if (pattern.matcher(uriKey).matches()) {
121+
return true;
122+
}
123+
}
124+
return false;
125+
}
126+
127+
private Object sanitizeUris(String value) {
128+
return Arrays.stream(value.split(",")).map(this::sanitizeUri).collect(Collectors.joining(","));
129+
}
130+
131+
private String sanitizeUri(String value) {
132+
Matcher matcher = URI_USERINFO_PATTERN.matcher(value);
106133
String password = matcher.matches() ? matcher.group(1) : null;
107134
if (password != null) {
108-
return StringUtils.replace(uriString, ":" + password + "@", ":******@");
135+
return StringUtils.replace(value, ":" + password + "@", ":******@");
109136
}
110137
return value;
111138
}

spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/context/properties/ConfigurationPropertiesReportEndpointTests.java

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2019 the original author or authors.
2+
* Copyright 2012-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -51,6 +51,7 @@
5151
* @author Andy Wilkinson
5252
* @author Stephane Nicoll
5353
* @author HaiTao Zhang
54+
* @author Chris Bono
5455
*/
5556
class ConfigurationPropertiesReportEndpointTests {
5657

@@ -170,19 +171,26 @@ void sanitizeWithCustomPatternUsingCompositeKeys() {
170171
}
171172

172173
@Test
173-
void sanitizedUriWithSensitiveInfo() {
174+
void sanitizeUriWithSensitiveInfo() {
174175
this.contextRunner.withUserConfiguration(SensiblePropertiesConfiguration.class)
175176
.run(assertProperties("sensible", (properties) -> assertThat(properties.get("sensitiveUri"))
176177
.isEqualTo("http://user:******@localhost:8080")));
177178
}
178179

179180
@Test
180-
void sanitizedUriWithNoPassword() {
181+
void sanitizeUriWithNoPassword() {
181182
this.contextRunner.withUserConfiguration(SensiblePropertiesConfiguration.class)
182183
.run(assertProperties("sensible", (properties) -> assertThat(properties.get("noPasswordUri"))
183184
.isEqualTo("http://user:******@localhost:8080")));
184185
}
185186

187+
@Test
188+
void sanitizeAddressesFieldContainingMultipleRawSensitiveUris() {
189+
this.contextRunner.withUserConfiguration(SensiblePropertiesConfiguration.class)
190+
.run(assertProperties("sensible", (properties) -> assertThat(properties.get("rawSensitiveAddresses"))
191+
.isEqualTo("http://user:******@localhost:8080,http://user2:******@localhost:8082")));
192+
}
193+
186194
@Test
187195
void sanitizeLists() {
188196
this.contextRunner.withUserConfiguration(SensiblePropertiesConfiguration.class)
@@ -574,6 +582,8 @@ public static class SensibleProperties {
574582

575583
private URI noPasswordUri = URI.create("http://user:@localhost:8080");
576584

585+
private String rawSensitiveAddresses = "http://user:password@localhost:8080,http://user2:password2@localhost:8082";
586+
577587
private List<ListItem> listItems = new ArrayList<>();
578588

579589
private List<List<ListItem>> listOfListItems = new ArrayList<>();
@@ -599,6 +609,14 @@ public URI getNoPasswordUri() {
599609
return this.noPasswordUri;
600610
}
601611

612+
public String getRawSensitiveAddresses() {
613+
return this.rawSensitiveAddresses;
614+
}
615+
616+
public void setRawSensitiveAddresses(final String rawSensitiveAddresses) {
617+
this.rawSensitiveAddresses = rawSensitiveAddresses;
618+
}
619+
602620
public List<ListItem> getListItems() {
603621
return this.listItems;
604622
}

spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/SanitizerTests.java

Lines changed: 62 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2019 the original author or authors.
2+
* Copyright 2012-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,7 +16,11 @@
1616

1717
package org.springframework.boot.actuate.endpoint;
1818

19+
import java.util.stream.Stream;
20+
1921
import org.junit.jupiter.api.Test;
22+
import org.junit.jupiter.params.ParameterizedTest;
23+
import org.junit.jupiter.params.provider.MethodSource;
2024

2125
import static org.assertj.core.api.Assertions.assertThat;
2226

@@ -25,11 +29,12 @@
2529
*
2630
* @author Phillip Webb
2731
* @author Stephane Nicoll
32+
* @author Chris Bono
2833
*/
2934
class SanitizerTests {
3035

3136
@Test
32-
void defaults() {
37+
void defaultNonUriKeys() {
3338
Sanitizer sanitizer = new Sanitizer();
3439
assertThat(sanitizer.sanitize("password", "secret")).isEqualTo("******");
3540
assertThat(sanitizer.sanitize("my-password", "secret")).isEqualTo("******");
@@ -40,23 +45,71 @@ void defaults() {
4045
assertThat(sanitizer.sanitize("sometoken", "secret")).isEqualTo("******");
4146
assertThat(sanitizer.sanitize("find", "secret")).isEqualTo("secret");
4247
assertThat(sanitizer.sanitize("sun.java.command", "--spring.redis.password=pa55w0rd")).isEqualTo("******");
43-
assertThat(sanitizer.sanitize("my.uri", "http://user:password@localhost:8080"))
48+
}
49+
50+
@ParameterizedTest(name = "key = {0}")
51+
@MethodSource("matchingUriUserInfoKeys")
52+
void uriWithSingleValueWithPasswordShouldBeSanitized(String key) {
53+
Sanitizer sanitizer = new Sanitizer();
54+
assertThat(sanitizer.sanitize(key, "http://user:password@localhost:8080"))
4455
.isEqualTo("http://user:******@localhost:8080");
4556
}
4657

47-
@Test
48-
void uriWithNoPasswordShouldNotBeSanitized() {
58+
@ParameterizedTest(name = "key = {0}")
59+
@MethodSource("matchingUriUserInfoKeys")
60+
void uriWithSingleValueWithNoPasswordShouldNotBeSanitized(String key) {
4961
Sanitizer sanitizer = new Sanitizer();
50-
assertThat(sanitizer.sanitize("my.uri", "http://localhost:8080")).isEqualTo("http://localhost:8080");
62+
assertThat(sanitizer.sanitize(key, "http://localhost:8080")).isEqualTo("http://localhost:8080");
63+
assertThat(sanitizer.sanitize(key, "http://user@localhost:8080")).isEqualTo("http://user@localhost:8080");
5164
}
5265

53-
@Test
54-
void uriWithPasswordMatchingOtherPartsOfString() {
66+
@ParameterizedTest(name = "key = {0}")
67+
@MethodSource("matchingUriUserInfoKeys")
68+
void uriWithSingleValueWithPasswordMatchingOtherPartsOfStringShouldBeSanitized(String key) {
5569
Sanitizer sanitizer = new Sanitizer();
56-
assertThat(sanitizer.sanitize("my.uri", "http://user://@localhost:8080"))
70+
assertThat(sanitizer.sanitize(key, "http://user://@localhost:8080"))
5771
.isEqualTo("http://user:******@localhost:8080");
5872
}
5973

74+
@ParameterizedTest(name = "key = {0}")
75+
@MethodSource("matchingUriUserInfoKeys")
76+
void uriWithMultipleValuesEachWithPasswordShouldHaveAllSanitized(String key) {
77+
Sanitizer sanitizer = new Sanitizer();
78+
assertThat(
79+
sanitizer.sanitize(key, "http://user1:password1@localhost:8080,http://user2:password2@localhost:8082"))
80+
.isEqualTo("http://user1:******@localhost:8080,http://user2:******@localhost:8082");
81+
}
82+
83+
@ParameterizedTest(name = "key = {0}")
84+
@MethodSource("matchingUriUserInfoKeys")
85+
void uriWithMultipleValuesNoneWithPasswordShouldHaveNoneSanitized(String key) {
86+
Sanitizer sanitizer = new Sanitizer();
87+
assertThat(sanitizer.sanitize(key, "http://user@localhost:8080,http://localhost:8082"))
88+
.isEqualTo("http://user@localhost:8080,http://localhost:8082");
89+
}
90+
91+
@ParameterizedTest(name = "key = {0}")
92+
@MethodSource("matchingUriUserInfoKeys")
93+
void uriWithMultipleValuesSomeWithPasswordShouldHaveThoseSanitized(String key) {
94+
Sanitizer sanitizer = new Sanitizer();
95+
assertThat(sanitizer.sanitize(key,
96+
"http://user1:password1@localhost:8080,http://user2@localhost:8082,http://localhost:8083")).isEqualTo(
97+
"http://user1:******@localhost:8080,http://user2@localhost:8082,http://localhost:8083");
98+
}
99+
100+
@ParameterizedTest(name = "key = {0}")
101+
@MethodSource("matchingUriUserInfoKeys")
102+
void uriWithMultipleValuesWithPasswordMatchingOtherPartsOfStringShouldBeSanitized(String key) {
103+
Sanitizer sanitizer = new Sanitizer();
104+
assertThat(sanitizer.sanitize(key, "http://user1://@localhost:8080,http://user2://@localhost:8082"))
105+
.isEqualTo("http://user1:******@localhost:8080,http://user2:******@localhost:8082");
106+
}
107+
108+
private static Stream<String> matchingUriUserInfoKeys() {
109+
return Stream.of("uri", "my.uri", "myuri", "uris", "my.uris", "myuris", "address", "my.address", "myaddress",
110+
"addresses", "my.addresses", "myaddresses");
111+
}
112+
60113
@Test
61114
void regex() {
62115
Sanitizer sanitizer = new Sanitizer(".*lock.*");

spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/env/EnvironmentEndpointTests.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2019 the original author or authors.
2+
* Copyright 2012-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -50,6 +50,7 @@
5050
* @author Madhura Bhave
5151
* @author Andy Wilkinson
5252
* @author HaiTao Zhang
53+
* @author Chris Bono
5354
*/
5455
class EnvironmentEndpointTests {
5556

@@ -246,13 +247,25 @@ void multipleSourcesWithSameProperty() {
246247
}
247248

248249
@Test
249-
void uriPropertryWithSensitiveInfo() {
250+
void uriPropertyWithSensitiveInfo() {
250251
ConfigurableEnvironment environment = new StandardEnvironment();
251252
TestPropertyValues.of("sensitive.uri=http://user:password@localhost:8080").applyTo(environment);
252253
EnvironmentEntryDescriptor descriptor = new EnvironmentEndpoint(environment).environmentEntry("sensitive.uri");
253254
assertThat(descriptor.getProperty().getValue()).isEqualTo("http://user:******@localhost:8080");
254255
}
255256

257+
@Test
258+
void addressesPropertyWithMultipleEntriesEachWithSensitiveInfo() {
259+
ConfigurableEnvironment environment = new StandardEnvironment();
260+
TestPropertyValues
261+
.of("sensitive.addresses=http://user:password@localhost:8080,http://user2:password2@localhost:8082")
262+
.applyTo(environment);
263+
EnvironmentEntryDescriptor descriptor = new EnvironmentEndpoint(environment)
264+
.environmentEntry("sensitive.addresses");
265+
assertThat(descriptor.getProperty().getValue())
266+
.isEqualTo("http://user:******@localhost:8080,http://user2:******@localhost:8082");
267+
}
268+
256269
private static ConfigurableEnvironment emptyEnvironment() {
257270
StandardEnvironment environment = new StandardEnvironment();
258271
environment.getPropertySources().remove(StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME);

0 commit comments

Comments
 (0)