Skip to content

sanitize by default a key containing uri or url. #6876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

sanitize by default a key containing uri or url. #6876

wants to merge 1 commit into from

Conversation

making
Copy link
Member

@making making commented Sep 13, 2016

Spring Boot apps in Cloud foundry often expose credentials as the property whose key ends with url or uri as follows:

image

I hope those are hidden by default.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 13, 2016
@snicoll
Copy link
Member

snicoll commented Sep 13, 2016

That seems a bit agressive to me. I'd rather have something that parse the url to detect that a password is specified and santize that. It could be either a standard user:pass in an HTTP url or some parameter with a well-defined name (i.e. password).

@making
Copy link
Member Author

making commented Sep 13, 2016

How does .*\\.connection\\..* sounds?

or .*\\.connection\\..*(uri|url)$

@snicoll
Copy link
Member

snicoll commented Sep 13, 2016

I don't like it either.

You don't want to santize the full URL, do you? You want to sanitize the credentials only. Wondering what others think.

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 13, 2016
@joshiste
Copy link
Contributor

You don't want to santize the full URL, do you? You want to sanitize the credentials only. Wondering what others think.

So how about making the Sanitizer exchangable so that custom implementations for the masking can be used?

@making
Copy link
Member Author

making commented Sep 14, 2016

@joshiste it's already customizable https://github.com/spring-projects/spring-boot/blob/v1.4.0.RELEASE/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/EnvironmentEndpoint.java#L51-L53 .
I'd like to mask it by default. Because CF users often make password public without realizing it.
Actually, VCAP_SERVICES and .*credentials.* are hidden by default. It's nice but not enough.

@joshiste
Copy link
Contributor

joshiste commented Sep 14, 2016

it's already customizable

Only the list of the regexes, but the Sanitizer implementation it self can't be exchanged.
And there is a open issue to alter the behaviour for Spring Cloud. Imho it would be reasonable to make the Sanitizer more customizable, so that other implementations can be chosen via (auto-)config (contributed by libs).

@making
Copy link
Member Author

making commented Sep 14, 2016

@joshiste Sounds nice!
I imagine following classes:

class Sanitizer {
  Sanitizer(SanitizePatterns patterns) {
   // ...
  }
}
class SanitizePatterns {
  private final List<SanitizePattern> patterns;
}

and auto-configuration

@Bean
@ConditionOnMissingBean
SanitizePatterns sanitizePatterns(List<SanitizePattern> patterns) {
  SanitizePatterns patterns = new SanitizePatterns("password", "secret", "key", "token", ".*credentials.*", "vcap_services");
  patterns.addAll(patterns);
  return patterns;
}

If a project want to mask other patterns, add

@Bean
SanitizerPattern myPattern() {
  return new SanitizerPattern("url");
}

@snicoll
Copy link
Member

snicoll commented Sep 14, 2016

@joshiste please share that idea in a separate issue, thanks.

@philwebb
Copy link
Member

Similar request in #6587

@snicoll
Copy link
Member

snicoll commented Sep 16, 2016

Another example: spring.data.mongodb.uri

@snicoll
Copy link
Member

snicoll commented May 18, 2018

Closing in favour of #6587 (and #11264). Thanks for the PR anyway @making!

@snicoll snicoll closed this May 18, 2018
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-feedback We need additional information before we can continue labels May 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants