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

Fix/avoid leak secrects in debug log of ifs #13997

Merged

Conversation

andsel
Copy link
Contributor

@andsel andsel commented Apr 14, 2022

Release notes

Fix the leak of secret store secrets when if statements are printed when started with debug log.

What does this PR do?

Updates the ConfigVariableExpander.expand to selectively create SecretVariable instances for SecretStore resolved environment variables.
SecretVariable instances in if statements are decrypted during eq EventCondition compilation; bringing the secret value and using in the comparator.

Why is it important/What is the impact to the user?

Permit the user to avoid leakage into debug log of secret stores's variables, when used in if conditions.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [ ] I have made corresponding change to the default configuration files (and/or docker env variables)
  • I have added tests that prove my fix is effective or that my feature works

Author's Checklist

  • test with a pipeline and debug log enabled. No leak but the condition should work as expected

How to test this PR locally

  • create a local secret store
bin/logstash-keystore create and save into a variable named `SECRET`
bin/logstash-keystore add SECRET
  • run Logstash in debug with a pipeline that uses the secret variable
input { http { } }

filter {
  if [@metadata][input][http][request][headers][auth] != "${SECRET}" {
    mutate {
      add_field => { "a_secre_field" => "${SECRET}" }
      add_tag => "${SECRET}"
    }
    drop {}
  } 
}


output {
  stdout {codec => rubydebug {metadata => true}}
}
  • verify your secret isn't leak into the logs (run bin/logstash -f <pipeline.conf> --debug)
  • verify the pipeline works as expected
curl -v  --header "auth: s3cr3t" "localhost:8080"

an event should be logged to the console.

Related issues

Use cases

A user would like to use secret store's resolved variables and avoid to leak in logs/console when Logstash is run with debug or trace levels.

Logs

Example of secret disclosure launching bin/logstash --debug:

[2022-04-14T15:40:21,845][INFO ][logstash.javapipeline    ][main] Starting pipeline {:pipeline_id=>"main", "pipeline.workers"=>12, "pipeline.batch.size"=>125, "pipeline.batch.delay"=>50, "pipeline.max_inflight"=>1500, "pipeline.sources"=>["/home/andrea/workspace/logstash_andsel/leak_secret_in_debug_pipeline.conf"], :thread=>"#<Thread:0xab4113a run>"}
[2022-04-14T15:40:22,249][DEBUG][org.logstash.config.ir.CompiledPipeline][main] Compiled conditional
 [if (event.getField('[@metadata][input][http][request][headers][auth]')!='s3cr3t')] 
 into 
 org.logstash.config.ir.compiler.ComputeStepSyntaxElement@9fb449bc

@andsel andsel marked this pull request as ready for review April 15, 2022 11:57
@andsel andsel requested review from kaisecheng and jsvd and removed request for kaisecheng April 15, 2022 14:27
@andsel andsel requested a review from jsvd April 19, 2022 12:25
Map<String, Object> args = ImmutableMap.of(key, ((ValueExpression) expression).get());
Map<String, Object> substitutedArgs = CompiledPipeline.expandConfigVariables(cve, args);
return new ValueExpression(expression.getSourceWithMetadata(), substitutedArgs.get(key));
Object expanded = CompiledPipeline.expandConfigVariableKeepingSecrets(cve, ((ValueExpression) expression).get());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much cleaner <3

… SecretStore, instead of use Password, which is mean to cover another use case
@andsel andsel requested a review from jsvd April 20, 2022 14:40
Copy link
Member

@jsvd jsvd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Successfully merging this pull request may close these issues.

secret value show in debug log
3 participants