RabbitProperties addresses field is not sanitized by default and may contain sensitive information

[onobc]

Problem:

The actuator configprops and env endpoints show RabbitProperties#addresses in un-sanitized form.

Workaround:

These endpoints expose the keys-to-sanitize property and I could set that property to the current defaults plus addresses. However, this is not desirable as

1. we have many applications that will use this and will have to do this in every app (or write a starter that does it).
2. copying the current default values in the endpoint code is a risk of getting out of sync w/ that code (defaults in the endpoint).

It feels like this is something that should be handled out-of-box.

Couple of options:

1. Update default keys-to-sanitize to include an expression for addresses.

2. Add an additional-keys-to-sanitize to allow adds to the defaults and set that property in my applications.

3. Add an annotation that can be set on a @ConfigurationProperties field (such as @Sensitive) that would automatically get that field excluded from the endpoint reports.

4. Do nothing. Use the existing keys-to-sanitize property w/ the current coded defaults and add addresses to it. Repeat this in each application. Update each application when/if the defaults happen to change in the endpoint code.

I am happy to submit a merge request for any of these options.

[mbhave]

@Bono007 We sanitize the password in the uri as of #8293. Could you tell us why this doesn't work for you?

[onobc]

I will take a look this evening @mbhave

[onobc]

Ok - this is only an issue if you in fact set the user/pass in the spring.rabbitmq.addresses property such as

spring.rabbitmq.addresses: amqp://user:pass@host1:port,amqp://user:pass@host2:port

I did not realize I could put the user/pass in the top-level properties (which is sanitized) and still have multiple addresses (but they get their creds from the top-level properties) and leave out of the address such as

spring.rabbitmq.username: user
spring.rabbitmq.password: pass
spring.rabbitmq.addresses: amqp://host1:port,amqp://host2:port

Should probably add a warning in docs somewhere (even javadoc of the RabbitProperties.addresses) that its not sanitized and user/pass will be shown if included in those fields directly.

[mbhave]

#8293 doesn't work in this case because the property is addresses and not uri. We could add addresses to the list of of keys to sanitize but that won't cover a property that was called address. Flagging for team attention to see what we should o.

[scottfrederick]

Preventing these types of secrets from being leaked is pretty important. I think the implementation should be updated to support a field named address the same way that uri is handled now, and also updated to support fields named addresses and uris that contain multiple URIs.

[mbhave]

We've decided to add addresses, address and uris to list of keys to sanitize. As part of this issue, we should also do a sweep of all our @ConfigurationProperties to make sure we aren't missing any.

[onobc]

@mbhave sounds good. Is this something the team wants to handle internally? If not, I will have some cycles this weekend to get an MR together. Just lmk. Thx.

[mbhave]

@Bono007 A PR would be most welcome, thank you.

[onobc]

@mbhave I see this is on the 2.1.x milestone but the original fix w/ "uri" sanitize is not in 2.1.x currently - it is in master. Would it be easier for me to fix this in master and then retrofit back into 2.1.x? What is the preference in this case? Thanks

[mbhave]

It was fixed in 2.2.x actually. I've moved the issue to 2.2.x.

[onobc]

@mbhave, PR is in.

[mbhave]

Closing in favor of #19999.