From b2e96381c8a8b90511053f682b58a44146ebd76b Mon Sep 17 00:00:00 2001 From: Andrew Cholakian Date: Thu, 31 Mar 2016 16:10:32 -0500 Subject: [PATCH] Hide sensitive AST/config data unless --debug-config argument passed 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 https://github.com/elastic/logstash/issues/4964 --- logstash-core/lib/logstash/agent.rb | 2 +- logstash-core/lib/logstash/config/loader.rb | 15 ++++++++----- .../instrument/periodic_poller/base.rb | 4 ++-- logstash-core/lib/logstash/pipeline.rb | 21 +++++++++++++++++-- logstash-core/lib/logstash/runner.rb | 10 +++++++-- logstash-core/locales/en.yml | 4 ++++ logstash-core/spec/logstash/pipeline_spec.rb | 19 +++++++++++++++++ logstash-core/spec/logstash/runner_spec.rb | 16 ++++++++++++++ 8 files changed, 79 insertions(+), 12 deletions(-) diff --git a/logstash-core/lib/logstash/agent.rb b/logstash-core/lib/logstash/agent.rb index a925cc33752..1afd8c775ce 100644 --- a/logstash-core/lib/logstash/agent.rb +++ b/logstash-core/lib/logstash/agent.rb @@ -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 diff --git a/logstash-core/lib/logstash/config/loader.rb b/logstash-core/lib/logstash/config/loader.rb index 37179518ed5..1cd0f3febaa 100644 --- a/logstash-core/lib/logstash/config/loader.rb +++ b/logstash-core/lib/logstash/config/loader.rb @@ -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) @@ -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 diff --git a/logstash-core/lib/logstash/instrument/periodic_poller/base.rb b/logstash-core/lib/logstash/instrument/periodic_poller/base.rb index 32bfd931a9a..1834c3342f5 100644 --- a/logstash-core/lib/logstash/instrument/periodic_poller/base.rb +++ b/logstash-core/lib/logstash/instrument/periodic_poller/base.rb @@ -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 diff --git a/logstash-core/lib/logstash/pipeline.rb b/logstash-core/lib/logstash/pipeline.rb index 2cbedae3bf0..7d67e70a641 100644 --- a/logstash-core/lib/logstash/pipeline.rb +++ b/logstash-core/lib/logstash/pipeline.rb @@ -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 @@ -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) @@ -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 diff --git a/logstash-core/lib/logstash/runner.rb b/logstash-core/lib/logstash/runner.rb index 8d261d49276..1828d5de07d 100644 --- a/logstash-core/lib/logstash/runner.rb +++ b/logstash-core/lib/logstash/runner.rb @@ -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") @@ -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) @@ -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 diff --git a/logstash-core/locales/en.yml b/logstash-core/locales/en.yml index e373ff3c4bc..d5859b14203 100644 --- a/logstash-core/locales/en.yml +++ b/logstash-core/locales/en.yml @@ -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! \ No newline at end of file diff --git a/logstash-core/spec/logstash/pipeline_spec.rb b/logstash-core/spec/logstash/pipeline_spec.rb index 3b94ab4e828..e8a782122f2 100644 --- a/logstash-core/spec/logstash/pipeline_spec.rb +++ b/logstash-core/spec/logstash/pipeline_spec.rb @@ -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" diff --git a/logstash-core/spec/logstash/runner_spec.rb b/logstash-core/spec/logstash/runner_spec.rb index 98802f32d59..7476f75069d 100644 --- a/logstash-core/spec/logstash/runner_spec.rb +++ b/logstash-core/spec/logstash/runner_spec.rb @@ -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