Skip to content
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

When using DEBUG log level, the provided Kafka config is logged in its entirety which can leak secrets #61

Open
NoxHarmonium opened this issue Nov 12, 2024 · 3 comments · May be fixed by #62

Comments

@NoxHarmonium
Copy link

NoxHarmonium commented Nov 12, 2024

Hi,

We are using bluesky-kafka in a service with a secure Kafka instance, so we provide some secrets via the consumer/producer config (e.g. sasl.password).

We just noticed that when we set the service's log level to DEBUG, that bluesky-kafka is logging the entire config and leaking the password into our logs.

This is preventing us from using debug logs at the moment.

I can think of two possible solutions:

  1. We just remove the relevant log calls and make the consumer of the library responsible for logging the config that they're passing in if they want to.
  2. We could redact sensitive config variables. I could go through the confluent kafka documentation (e.g. https://docs.confluent.io/platform/current/installation/configuration/consumer-configs.html#) and find any sensitive keys and write a function to redact them before logging.
  3. We just log important/interesting keys from the config

I would recommend solution 1 because its simple and I think its easy enough for consumers of the library to log the config themselves. Solution 2 does have the risk that the config schema changes in the future with sensitive keys renamed/added that aren't covered by the redactor and we would need to maintain it.

If you're happy with one of those solutions (or have another suggestion) I'd be happy to raise a PR!

Thanks!

Relevant lines:


logger.debug("producer configuration: %s", self._producer_config)

@NoxHarmonium
Copy link
Author

I was thinking something like this:
AustralianSynchrotron@bb607e8

@mrakitin
Copy link
Member

Thanks for the suggestions, @NoxHarmonium!
While solution (1) is straightforward, users may want to see the configuration of the servers, etc. in the logs.
I share your concerns in the proposed solution (2) regarding redacting the config appearing in the logs, but we could add enough tests to capture the keys if they are not in a "white list" of keys we define.
Solution (3) seems a bit safer if we forget to update the list.

Why not proceeding with (3)?

@NoxHarmonium
Copy link
Author

Sure, sounds good. I should be able to get a PR up for that by the end of the week for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants