Missing sanitization in lists of URIs properties on actuator endpoints

[crmky]

Problem

I'm using Spring Boot 2.2.9. The change introduced by #19999 considers any keys contains "uri", "uris", "address" or "addresses" are "comma separated URLs". This is not always the right assumption. It will try to remove password from those URLs, however if it's not a URL format, it will return the original content.

Expected Behavior

If that key is not a URL format, it should return as ******. Or at least allow developers to configure whether they want sanitize URLs or not.

Reproducer

Sanitizer sanitizer = new Sanitizer();
System.out.println(sanitizer.sanitize("uris", "[amqp://foo:bar@host/]"));
System.out.println(sanitizer.sanitize("uris", "amqp://foo:bar@host/"));

The output is:

[amqp://foo:bar@host/]
amqp://foo:******@host/

[bclozel]

Hi, this is a first-timers-only issue. This means we've worked to make it more legible to folks who either haven't contributed to our codebase before, or even folks who haven't contributed to open source before.

If that's you, we're interested in helping you take the first step and can answer questions and help you out as you do. Note that we're especially interested in contributions from people from groups underrepresented in free and open source software!

If you have contributed before, consider leaving this one for someone new, and looking through our general ideal-for-contribution issues. Thanks!

Problem

The Actuator org.springframework.boot.actuate.endpoint.Sanitizer takes care of sanitizing sensitive information when exposing information on the env and configprops Actuator endpoints.

When the configuration key matches one of the configured pattern, the entire configuration value is sanitized.
There is a specific case for URI-like keys ("uri", "uris", "address", "addresses"), where the Sanitizer tries to sanitize only the password part of URLs:

something.uri=https://emily:springboot@spring.io/ -> something.uri=https://emily:******@spring.io/
other.uri=https://spring.io/ -> other.uri=https://spring.io/

The first problem is about documentation. The docs imply that all configuration keys are processed the same way.

If any of the keys to sanitize are URI format (i.e. ://:@:/), only the password part is sanitized.

But in fact user info keys ("uri", "uris", "address", "addresses") and others are treated differently:

• user info keys ("uri", "uris", "address", "addresses"): only hide the password part in URLs, or return the value unchanged
• other keys ("password", "secret", "key", "token", "vcap_services", "sun.java.command","credentials") are completely sanitized

The second problem is that the sanitizer is used against serialized configuration values - whenever a List is serialized as JSON, it is serialized as [https://emily:springboot@spring.io/,https://spring.io] and not https://emily:springboot@spring.io/,https://spring.io. The problem is that if the current URI_USERINFO_PATTERN doesn't support this case and the original value is returned, nothing is sanitized.

Solution

First, the reference documentation (located in spring-boot-project/spring-boot-docs/src/docs/asciidoc/howto.adoc) should be improved to better explain the way keys are processed (some entirely sanitized in all cases, others only the password part if present).

Also, the URI_USERINFO_PATTERN pattern in the Sanitizer should be improved to support the list case. Maybe something like "\\[?[A-Za-z]+://.+:(.*)@.+$" would work? All tests in the SanitizerTests should be still green after that change, and we should have a new test case checking that:

"[http://user1:password1@localhost:8080,http://user2@localhost:8082,http://localhost:8083]" -> "[http://user1:******@localhost:8080,http://user2@localhost:8082,http://localhost:8083]";

If possible, the PR should be done against the 2.2.x branch - if master is more convenient for the contributor, project maintainers will rebase the PR.

Steps to Fix

• Claim this issue with a comment below and ask any clarifying questions you need
• Set up a repository locally following the Contributing Guidelines
• Try to fix the issue following the steps above
• Commit your changes and start a pull request.

[helloworldless]

This looks reasonable for my first contribution! Taking a look...