Skip to content

Commit

Permalink
Hide sensitive AST/config data unless --debug-config argument passed
Browse files Browse the repository at this point in the history
We now hide this because displaying it is dangerous. Generated code
and other AST data may contain plaintext copies of 'password' type fields.
Users can force this to appear with the --debug-config flag.

Fixes elastic#4964
  • Loading branch information
andrewvc committed Apr 1, 2016
1 parent bb30cc7 commit b2e9638
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 12 deletions.
2 changes: 1 addition & 1 deletion logstash-core/lib/logstash/agent.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def initialize(params)
@web_api_http_host = params[:web_api_http_host]
@web_api_http_port = params[:web_api_http_port]

@config_loader = LogStash::Config::Loader.new(@logger)
@config_loader = LogStash::Config::Loader.new(@logger, params[:debug_config])
@reload_interval = params[:reload_interval] || 3 # seconds
@upgrade_mutex = Mutex.new

Expand Down
15 changes: 10 additions & 5 deletions logstash-core/lib/logstash/config/loader.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
require "logstash/config/defaults"

module LogStash; module Config; class Loader
def initialize(logger)
def initialize(logger, debug_config=false)
@logger = logger
@debug_config = debug_config
end

def format_config(config_path, config_string)
Expand Down Expand Up @@ -69,14 +70,18 @@ def local_config(path)
encoding_issue_files << file
end
config << cfg + "\n"
@logger.debug? && @logger.debug("\nThe following is the content of a file", :config_file => file.to_s)
@logger.debug? && @logger.debug("\n" + cfg + "\n\n")
if @debug_config
@logger.debug? && @logger.debug("\nThe following is the content of a file", :config_file => file.to_s)
@logger.debug? && @logger.debug("\n" + cfg + "\n\n")
end
end
if encoding_issue_files.any?
fail("The following config files contains non-ascii characters but are not UTF-8 encoded #{encoding_issue_files}")
end
@logger.debug? && @logger.debug("\nThe following is the merged configuration")
@logger.debug? && @logger.debug("\n" + config + "\n\n")
if @debug_config
@logger.debug? && @logger.debug("\nThe following is the merged configuration")
@logger.debug? && @logger.debug("\n" + config + "\n\n")
end
return config
end # def load_config

Expand Down
4 changes: 2 additions & 2 deletions logstash-core/lib/logstash/instrument/periodic_poller/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ def collect
end

def start
logger.debug("PeriodicPoller: Starting", :poller => self,
logger.debug("PeriodicPoller: Starting",
:polling_interval => @options[:polling_interval],
:polling_timeout => @options[:polling_timeout]) if logger.debug?
@task.execute
end

def stop
logger.debug("PeriodicPoller: Stopping", :poller => self)
logger.debug("PeriodicPoller: Stopping")
@task.shutdown
end

Expand Down
21 changes: 19 additions & 2 deletions logstash-core/lib/logstash/pipeline.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ module LogStash; class Pipeline
:pipeline_batch_size => 125,
:pipeline_batch_delay => 5, # in milliseconds
:flush_interval => 5, # in seconds
:flush_timeout_interval => 60 # in seconds
:flush_timeout_interval => 60, # in seconds
:debug_config => false
}
MAX_INFLIGHT_WARN_THRESHOLD = 10_000

Expand Down Expand Up @@ -90,7 +91,10 @@ def initialize(config_str, settings = {})

# The config code is hard to represent as a log message...
# So just print it.
@logger.debug? && @logger.debug("Compiled pipeline code:\n#{code}")

if @settings[:debug_config] && logger.debug?
logger.debug("Compiled pipeline code", :code => code)
end

begin
eval(code)
Expand Down Expand Up @@ -546,4 +550,17 @@ def non_reloadable_plugins
end
end

# Sometimes we log stuff that will dump the pipeline which may contain
# sensitive information (like the raw syntax tree which can contain passwords)
# We want to hide most of what's in here
def inspect
{
:pipeline_id => @pipeline_id,
:settings => @settings.inspect,
:ready => @ready,
:running => @running,
:flushing => @flushing
}
end

end end
10 changes: 8 additions & 2 deletions logstash-core/lib/logstash/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ class LogStash::Runner < Clamp::Command
option "--verbose", :flag, I18n.t("logstash.runner.flag.verbose")
option "--debug", :flag, I18n.t("logstash.runner.flag.debug")

option ["--debug-config"], :flag,
I18n.t("logstash.runner.flag.debug_config"),
:attribute_name => :debug_config, :default => false

option ["-V", "--version"], :flag,
I18n.t("logstash.runner.flag.version")

Expand Down Expand Up @@ -166,7 +170,7 @@ def execute
end

if config_test?
config_loader = LogStash::Config::Loader.new(@logger)
config_loader = LogStash::Config::Loader.new(@logger, @debug_config)
config_str = config_loader.format_config(config_path, config_string)
begin
LogStash::Pipeline.new(config_str)
Expand All @@ -184,12 +188,14 @@ def execute
:collect_metric => true,
:debug => debug?,
:node_name => node_name,
:debug_config => debug_config?,
:web_api_http_host => @web_api_http_host,
:web_api_http_port => @web_api_http_port)

@agent.register_pipeline("main", @pipeline_settings.merge({
:config_string => config_string,
:config_path => config_path
:config_path => config_path,
:debug_config => debug_config?
}))

# enable sigint/sigterm before starting the agent
Expand Down
4 changes: 4 additions & 0 deletions logstash-core/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -239,3 +239,7 @@ en:
it will default to the current hostname.
agent: |+
Specify an alternate agent plugin name.
debug_config: |+
Print the compiled config ruby code out as a debug log (you must also have --debug enabled).
WARNING: This will include any 'password' options passed to plugin configs as plaintext, and may result
in plaintext passwords appearing in your logs!
19 changes: 19 additions & 0 deletions logstash-core/spec/logstash/pipeline_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,25 @@ class TestPipeline < LogStash::Pipeline
eos
}

describe "debug compiled" do
let(:logger) { double("pipeline logger").as_null_object }

before do
expect(Cabin::Channel).to receive(:get).with(LogStash).and_return(logger).at_least(:once)
allow(logger).to receive(:debug?).and_return(true)
end

it "should not receive a debug message with the compiled code" do
expect(logger).not_to receive(:debug).with(/Compiled pipeline/, anything)
pipeline = TestPipeline.new(test_config_with_filters)
end

it "should print the compiled code if debug_config is set to true" do
expect(logger).to receive(:debug).with(/Compiled pipeline/, anything)
pipeline = TestPipeline.new(test_config_with_filters, :debug_config => true)
end
end

context "when there is no command line -w N set" do
it "starts one filter thread" do
msg = "Defaulting pipeline worker threads to 1 because there are some filters that might not work with multiple worker threads"
Expand Down
16 changes: 16 additions & 0 deletions logstash-core/spec/logstash/runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,5 +145,21 @@ def run(args); end
subject.run("bin/logstash", args)
end
end

describe "debug_config" do
it "should set 'debug_config' to false by default" do
expect(LogStash::Config::Loader).to receive(:new).with(anything, false).and_call_original
expect(LogStash::Pipeline).to receive(:new).with(pipeline_string, hash_including(:debug_config => false)).and_return(pipeline)
args = ["--debug", "-e", pipeline_string]
subject.run("bin/logstash", args)
end

it "should allow overriding debug_config" do
expect(LogStash::Config::Loader).to receive(:new).with(anything, true).and_call_original
expect(LogStash::Pipeline).to receive(:new).with(pipeline_string, hash_including(:debug_config => true)).and_return(pipeline)
args = ["--debug", "--debug-config", "-e", pipeline_string]
subject.run("bin/logstash", args)
end
end
end
end

0 comments on commit b2e9638

Please sign in to comment.