Skip to content

Commit

Permalink
(MODULES-8358) Add PowerShell Session Manager for pwsh
Browse files Browse the repository at this point in the history
Previously the pwsh provider would save a file to a temporary location and then
execute that. This is considered a legacy method.  The more modern method, which
is used in the Windows PowerShell provider, is to use the PowerShell Manager.

This commit updates the PowerShell Manager for using PowerShell Core on both
Windows and non-Windows platforms:

* The pwsh provider was modified to use the PowerShell manager if it is supported
  otherwise revert to the legacy temporary path

* The pwsh provider changed the scope of the pwsh_args and get_pwsh_command to
  public so that they can be used the spec tests.  As they're readonly properties
  it is safe to surface these methods

* Modified the PowerShell manager to use Unix Domain Sockets when on non-Windows
  platforms.  This is due to the .Net implmentation of the NamedPipesClient class
  using Domain Sockets on non-Windows platforms as opposed to Named Pipes. [1] [2] [3]
  The manager creates the domain sockets in the user's tmp directrory

  This also means the errors returned on bad/failed pipes is different e.g. EOFError
  and EConnReset.  Also many of the integration tests assume a real named pipe, so
  these tests are guarded to only run on Windows. Note that it was not possible to
  replicate the "delete named pipe" using Unix Domain sockets due to how sockets are
  different to named pipes.

* Modified many of the tests to either be platform independent by either removing the
  platform specific things e.g. remove \r\n references or specific exit codes.  Or the
  tests are modified to use specific items on specific platforms e.g. cmd.exe on Windows
  and /bin/sh on non-Windows.

* Removed the use of aliases (e.g. ps and ls) as they were causing test failures as they
  behave differently on different platforms.  Instead prefer commands like Get-Verb or
  $PSVersionTable which are consistent.

* Moved the 'when specifying a working directory' test from unit to integration as
  it actually calls the operating system for information and was causing failures when
  pwsh was not installed

* Modified the PowerShell Manager integration tests to be a shared example group and
  can then simply use 'it_behaves_like' calls to test when appropriate.  This is
  important on non-Windows when pwsh isn't installed.

* Modified the unit tests for the pwsh provider to use a mocked PowerShell Manager when
  invoked.

[1] https://github.com/dotnet/corefx/blob/94e9d02ad70b2224d012ac4a66eaa1f913ae4f29/src/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs#L49-L60
[2] https://github.com/dotnet/corefx/blob/a10890f4ffe0fadf090c922578ba0e606ebdd16c/src/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs#L44
[3] https://github.com/dotnet/corefx/blob/a10890f4ffe0fadf090c922578ba0e606ebdd16c/src/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs#L298-L299
  • Loading branch information
glennsarti committed Feb 15, 2019
1 parent 9f11091 commit cca8fae
Show file tree
Hide file tree
Showing 6 changed files with 317 additions and 232 deletions.
55 changes: 33 additions & 22 deletions lib/puppet/provider/exec/pwsh.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,23 @@
def run(command, check = false)
@pwsh ||= get_pwsh_command
self.fail 'pwsh could not be found' if @pwsh.nil?
write_script(command) do |native_path|
# Ideally, we could keep a handle open on the temp file in this
# process (to prevent TOCTOU attacks), and execute powershell
# with -File <path>. But powershell complains that it can't open
# the file for exclusive access. If we close the handle, then an
# attacker could modify the file before we invoke powershell. So
# we redirect powershell's stdin to read from the file. Current
# versions of Windows use per-user temp directories with strong
# permissions, but I'd rather not make (poor) assumptions.
if Puppet::Util::Platform.windows?
return super("cmd.exe /c \"\"#{native_path(@pwsh)}\" #{legacy_args} -Command - < \"#{native_path}\"\"", check)
else
return super("/bin/sh -c \"#{native_path(@pwsh)} #{legacy_args} -Command - < #{native_path}\"", check)
if PuppetX::PowerShell::PowerShellManager.supported_on_pwsh?
return ps_manager.execute_resource(command, resource)
else
write_script(command) do |native_path|
# Ideally, we could keep a handle open on the temp file in this
# process (to prevent TOCTOU attacks), and execute powershell
# with -File <path>. But powershell complains that it can't open
# the file for exclusive access. If we close the handle, then an
# attacker could modify the file before we invoke powershell. So
# we redirect powershell's stdin to read from the file. Current
# versions of Windows use per-user temp directories with strong
# permissions, but I'd rather not make (poor) assumptions.
if Puppet::Util::Platform.windows?
return super("cmd.exe /c \"\"#{native_path(@pwsh)}\" #{pwsh_args.join(' ')} -Command - < \"#{native_path}\"\"", check)
else
return super("/bin/sh -c \"#{native_path(@pwsh)} #{pwsh_args.join(' ')} -Command - < #{native_path}\"", check)
end
end
end
end
Expand All @@ -41,11 +45,8 @@ def validatecmd(command)
true
end

private

# Retrieves the absolute path to pwsh
#
# @api private
# @return [String] the absolute path to the found pwsh executable. Returns nil when it does not exist
def get_pwsh_command
if Puppet::Util::Platform.windows?
Expand All @@ -59,10 +60,9 @@ def get_pwsh_command
# ok to have 'Path' and 'PATH'
current_env = Puppet::Util.get_environment
end

# If the resource specifies a search path use that. Otherwise use the default
# PATH from the environment.
search_paths = @resource['path'].nil? ?
search_paths = @resource.nil? || @resource['path'].nil? ?
current_env['PATH'] :
resource[:path].join(File::PATH_SEPARATOR)

Expand All @@ -80,6 +80,21 @@ def get_pwsh_command
end
end

def pwsh_args
['-NoProfile', '-NonInteractive', '-NoLogo', '-ExecutionPolicy', 'Bypass']
end

private

# Retrieves the PowerShell manager specific to our pwsh binary in this resource
#
# @api private
# @return [PuppetX::PowerShell::PowerShellManager] The PowerShell manager for this resource
def ps_manager
debug_output = Puppet::Util::Log.level == :debug
PuppetX::PowerShell::PowerShellManager.instance(@pwsh, pwsh_args, debug: debug_output)
end

def write_script(content, &block)
Tempfile.open(['puppet-pwsh', '.ps1']) do |file|
file.puts(content)
Expand All @@ -96,8 +111,4 @@ def native_path(path)
path
end
end

def legacy_args
'-NoProfile -NonInteractive -NoLogo -ExecutionPolicy Bypass'
end
end
37 changes: 27 additions & 10 deletions lib/puppet_x/puppetlabs/powershell/powershell_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,30 @@ def self.supported?
!win32console_enabled?
end

def self.supported_on_pwsh?
!win32console_enabled?
end

def initialize(cmd, args = [], options = {})
@usable = true
@powershell_command = cmd
@powershell_arguments = args

named_pipe_name = "#{SecureRandom.uuid}PuppetPsHost"
if Puppet::Util::Platform.windows?
# Named pipes under Windows will automatically be mounted in \\.\pipe\...
# https://github.com/dotnet/corefx/blob/a10890f4ffe0fadf090c922578ba0e606ebdd16c/src/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Windows.cs#L34
named_pipe_name = "#{SecureRandom.uuid}PuppetPsHost"
# This named pipe path is Windows specific.
pipe_path = "\\\\.\\pipe\\#{named_pipe_name}"
else
# .Net implements named pipes under Linux etc. as Unix Sockets in the filesystem
# Paths that are rooted are not munged within C# Core.
# https://github.com/dotnet/corefx/blob/94e9d02ad70b2224d012ac4a66eaa1f913ae4f29/src/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs#L49-L60
# https://github.com/dotnet/corefx/blob/a10890f4ffe0fadf090c922578ba0e606ebdd16c/src/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs#L44
# https://github.com/dotnet/corefx/blob/a10890f4ffe0fadf090c922578ba0e606ebdd16c/src/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs#L298-L299
named_pipe_name = File.join(Dir.tmpdir, "#{SecureRandom.uuid}PuppetPsHost")
pipe_path = named_pipe_name
end
pipe_timeout = options[:pipe_timeout] || self.class.default_options[:pipe_timeout]
debug = options[:debug] || self.class.default_options[:debug]
native_cmd = Puppet::Util::Platform.windows? ? "\"#{cmd}\"" : cmd
Expand All @@ -67,17 +85,18 @@ def initialize(cmd, args = [], options = {})

Puppet.debug "#{Time.now} #{cmd} is running as pid: #{@ps_process[:pid]}"

pipe_path = "\\\\.\\pipe\\#{named_pipe_name}"
# wait for the pipe server to signal ready, and fail if no response in 10 seconds

# wait up to 30 seconds in 0.2 second intervals to be able to open the pipe
# If the pipe_timeout is ever specified as less than the sleep interval it will
# never try to connect to a pipe and error out as if a timeout occurred.
sleep_interval = 0.2
(pipe_timeout / sleep_interval).to_int.times do
begin
# pipe is opened in binary mode and must always
@pipe = File.open(pipe_path, 'r+b')
if Puppet::Util::Platform.windows?
# pipe is opened in binary mode and must always
@pipe = File.open(pipe_path, 'r+b')
else
@pipe = UNIXSocket.new(pipe_path)
end
break
rescue
sleep sleep_interval
Expand Down Expand Up @@ -112,8 +131,6 @@ def alive?

def execute(powershell_code, timeout_ms = nil, working_dir = nil, environment_variables = [])
code = make_ps_code(powershell_code, timeout_ms, working_dir, environment_variables)


# err is drained stderr pipe (not captured by redirection inside PS)
# or during a failure, a Ruby callstack array
out, native_stdout, err = exec_read_result(code)
Expand Down Expand Up @@ -417,7 +434,8 @@ def exec_read_result(powershell_code)
# if any pipes are broken, the manager is totally hosed
# bad file descriptors mean closed stream handles
# EOFError is a closed pipe (could be as a result of tearing down process)
rescue Errno::EPIPE, Errno::EBADF, EOFError => e
# Errno::ECONNRESET is a closed unix domain socket (could be as a result of tearing down process)
rescue Errno::EPIPE, Errno::EBADF, EOFError, Errno::ECONNRESET => e
@usable = false
return nil, nil, [e.inspect, e.backtrace].flatten
# catch closed stream errors specifically
Expand All @@ -426,7 +444,6 @@ def exec_read_result(powershell_code)
@usable = false
return nil, nil, [ioerror.inspect, ioerror.backtrace].flatten
end

end
end
end
37 changes: 30 additions & 7 deletions spec/integration/provider/exec/pwsh_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,40 @@ def pwsh_exist?
it "runs commands properly that output to multiple streams" do
skip('Could not locate pwsh binary') unless pwsh_exist?
command = Puppet.features.microsoft_windows? ?
# Note that `/bin/sh -c` or `cmd.exe /c` is required to return an exit code.
# Without this the exitcode is always zero.
'echo "foo"; [System.Console]::Error.WriteLine("bar"); cmd.exe /c foo.exe' :
'echo "foo"; [System.Console]::Error.WriteLine("bar"); & foo.exe'
expected = Puppet.features.microsoft_windows? ?
"foo\nbar\n'foo.exe' is not recognized as an internal or external command,\noperable program or batch file.\n" :
"^foo\nbar\n.+The term \'foo\.exe\' is not recognized as the name of a cmdlet, function.+"
'echo "foo"; [System.Console]::Error.WriteLine("bar"); /bin/sh -c "foo.exe"'

if PuppetX::PowerShell::PowerShellManager.supported_on_pwsh?
expected = Puppet.features.microsoft_windows? ? "foo\r\n" : "^foo\n"
else
# when PowerShellManager is not used, the legacy style invocation
# collects all streams inside of a single output string
expected = Puppet.features.microsoft_windows? ?
"foo\nbar\n'foo.exe' is not recognized as an internal or external command,\noperable program or batch file.\n" :
"^foo\nbar\n.+The term \'foo\.exe\' is not recognized as the name of a cmdlet, function.+"
end
output, status = provider.run(command)

# Due to the different behaviour of uname across non-Windows platforms, must use a regex
expect(output).to match(expected)
expect(status.exitstatus).to eq(1)
expect(status.exitstatus).to_not eq(0) # exit codes for missing files differ across platforms.
end

describe 'when specifying a working directory' do
describe 'that does not exist' do
let(:work_dir) {
Puppet.features.microsoft_windows? ?
"#{ENV['SYSTEMROOT']}\\some\\directory\\that\\does\\not\\exist" :
'/some/directory/that/does/not/exist'
}
let(:command) { 'exit 0' }

it 'emits an error when working directory does not exist' do
skip('Could not locate pwsh binary') unless pwsh_exist?
resource[:cwd] = work_dir
expect { provider.run(command) }.to raise_error(/Working directory .+ does not exist/)
end
end
end
end
end
Loading

0 comments on commit cca8fae

Please sign in to comment.