From 16ce2f30de42b1fcec55a58d24b3de346e1f9353 Mon Sep 17 00:00:00 2001 From: Tobias Urdin Date: Tue, 21 Sep 2021 14:13:32 +0200 Subject: [PATCH] Prevent --password from leaking in failed command output There is cases when a command times out or when it fails that we and Puppet [1] will output the raw command that was executed. For a user create command that output contains the --password argument passed down to openstack CLI which causes sensitive passwords to be leaked into log files of the system executing Puppet, these can then be shipped of from the system into a remote syslog and still be in plain text. This tries to use Ruby gsub with a regular expression matching the two cases and instead output [redacted secret] the same way we do with config provider. [1] https://github.com/puppetlabs/puppet/blob/main/lib/puppet/util/execution.rb#L286 Change-Id: I4cad8f88fc7b67bb7aa4330832fc47bac41ae9df --- lib/puppet/provider/openstack.rb | 11 +++++++++- spec/unit/provider/openstack_spec.rb | 31 ++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/lib/puppet/provider/openstack.rb b/lib/puppet/provider/openstack.rb index 3d80d683..5c261e3d 100644 --- a/lib/puppet/provider/openstack.rb +++ b/lib/puppet/provider/openstack.rb @@ -40,6 +40,12 @@ class Puppet::Provider::Openstack < Puppet::Provider self.class_variable_get("@@command_timeout") end + # redact sensitive information in exception and raise + def self.redact_and_raise(e) + new_message = e.message.gsub(/\-\-password\ [\w]+/, "--password [redacted secret]") + raise e.class, new_message + end + # with command_timeout def self.openstack(*args) begin @@ -48,7 +54,10 @@ class Puppet::Provider::Openstack < Puppet::Provider execute([command(:openstack_command)] + args, override_locale: false, failonfail: true, combine: true) end rescue Timeout::Error - raise Puppet::ExecutionFailure, "Command: 'openstack #{args.inspect}' has been running for more than #{command_timeout(action)} seconds" + e = Puppet::ExecutionFailure.new "Command: 'openstack #{args.inspect}' has been running for more than #{command_timeout(action)} seconds" + redact_and_raise(e) + rescue Puppet::ExecutionFailure => e + redact_and_raise(e) end end diff --git a/spec/unit/provider/openstack_spec.rb b/spec/unit/provider/openstack_spec.rb index 375df177..145bd1f2 100644 --- a/spec/unit/provider/openstack_spec.rb +++ b/spec/unit/provider/openstack_spec.rb @@ -84,6 +84,25 @@ name="test" Puppet::Provider::Openstack.request('project', 'list', ['--long'], credentials) end + it 'redacts sensitive data from an exception message' do + e1 = Puppet::ExecutionFailure.new "Execution of 'openstack user create --format shell hello --password world --enable --email foo@example.com --domain Default' returned 1: command failed" + expect do + Puppet::Provider::Openstack.redact_and_raise(e1) + end.to raise_error(Puppet::ExecutionFailure, /Execution of \'openstack user create --format shell hello --password \[redacted secret\] --enable --email foo@example.com --domain Default/) + e2 = Puppet::ExecutionFailure.new "Execution of 'openstack user create --format shell hello --password world' returned 1: command failed" + expect do + Puppet::Provider::Openstack.redact_and_raise(e2) + end.to raise_error(Puppet::ExecutionFailure, /Execution of \'openstack user create --format shell hello --password \[redacted secret\]\' returned/) + end + + it 'redacts password in execution output on exception' do + provider.class.stubs(:execute) + .raises(Puppet::ExecutionFailure, "Execution of '/usr/bin/openstack user create --format shell hello --password world --enable --email foo@example.com --domain Default' returned 1: command failed") + expect do + Puppet::Provider::Openstack.request('user', 'create', ['hello', '--password', 'world', '--enable', '--email', 'foo@example.com', '--domain', 'Default']) + end.to raise_error Puppet::ExecutionFailure, "Execution of '/usr/bin/openstack user create --format shell hello --password [redacted secret] --enable --email foo@example.com --domain Default' returned 1: command failed" + end + context 'on connection errors' do it 'retries the failed command' do provider.class.stubs(:openstack) @@ -96,6 +115,18 @@ name="test" expect(response.first[:description]).to eq 'Test tenant' end + it 'fails after the timeout and redacts' do + provider.class.expects(:execute) + .raises(Puppet::ExecutionFailure, "Execution of 'openstack user create foo --password secret' returned 1: command failed") + .times(3) + provider.class.stubs(:sleep) + provider.class.stubs(:current_time) + .returns(0, 10, 10, 20, 20, 200, 200) + expect do + Puppet::Provider::Openstack.request('project', 'list', ['--long']) + end.to raise_error Puppet::ExecutionFailure, /Execution of \'openstack user create foo --password \[redacted secret\]\' returned 1/ + end + it 'fails after the timeout' do provider.class.expects(:openstack) .with('project', 'list', '--quiet', '--format', 'csv', ['--long'])