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

Config dump on error leaks credentials #137

Open
chrooti opened this issue Jan 2, 2024 · 1 comment
Open

Config dump on error leaks credentials #137

chrooti opened this issue Jan 2, 2024 · 1 comment

Comments

@chrooti
Copy link

chrooti commented Jan 2, 2024

When the config is dumped, which happens when e.g. Kaffe can't connect to the Kafka cluster, the whole producer/consumer keyword list is written to stdout, credentials included.

It's not really a library issue per se, but I thought I'd raise it just in case (maybe a warning in the README is enough).

@foo-1a
Copy link

foo-1a commented Jan 2, 2024

would like to chip in with further detail here - what we're observing is that when part of configuration is not provided to producer, this leads to a Elixir.KeyError being returned from Kaffe:start/1, making it non-conformant to the expected error return value as per behaviour doc of {error, term()}.
This causes the full term being logged as a result of a supervisor start failure, and in particular the partial consumer config containing credentials.

In our case this is being triggered by virtue of having a phoenix config/runtime.exs configuration always configuring kaffe's producer credentials (thus creating config :kaffe {producer: [sasl: #{..creds..}]}, but then not adding the configuration for endpoints in one specific deployment environment where we were not intending to connect to kafka.
This leads kaffe to attempt configuration of a producer, but failure to find the endpoints key from producer: [...] before setting up its own supervisor.

If you think that the case is weird b/c of the way we're configuring the lib that's legit - but I'd still suggest that adapting the error so that Kaffe:start/1 returns {error, term()} and/or redacting the sasl: #{...} term when present, as it is known to contain credentials, could be beneficial.

Observed stacktrace, with some formatting:

Kernel pid terminated (application_controller) (
  {application_start_failure,kaffe,
    {bad_return,
      {
        {'Elixir.Kaffe',start,[normal,[]]},
        {'EXIT',
          {
            #{'__exception__' => true,
              '__struct__' => 'Elixir.KeyError',
              key => endpoints,
              message => nil,
              term => [
                {sasl,
                  #{login => <<"REDACTED">>,mechanism => plain,password => <<"REDACTED">>}
                }
              ]
          },[
            {'Elixir.Keyword','fetch!',2,[{file,"lib/keyword.ex"},{line,595}]},
            {'Elixir.Kaffe.Config.Producer',endpoints,0,[{file,"lib/kaffe/config/producer.ex"},{line,20}]},
            {'Elixir.Kaffe.Config.Producer',configuration,0,[{file,"lib/kaffe/config/producer.ex"},{line,6}]},
            {'Elixir.Kaffe.Producer',start_producer_client,0,[{file,"lib/kaffe/producer.ex"},{line,50}]},
            {'Elixir.Kaffe',start,2,[{file,"lib/kaffe.ex"},{line,19}]},
            {application_master,start_it_old,4,[{file,"application_master.erl"},{line,293}]}]}}}}})

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

No branches or pull requests

2 participants