From 1152b4e1df97fb5f468491954932d4f0c09875b1 Mon Sep 17 00:00:00 2001 From: Shawn Neal Date: Wed, 10 Jun 2015 12:47:26 -0700 Subject: [PATCH 1/2] Fix issue 5790 Leaving around plaintext username and passwords in a script on a box isn't the best from a security standpoint. This change ensures the scheduled task wrapper script for WinRM doesn't leave these around on the box, and instead passes them to the script as arguments. --- plugins/communicators/winrm/communicator.rb | 35 +++++++++---------- .../winrm/scripts/elevated_shell.ps1.erb | 12 +++---- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/plugins/communicators/winrm/communicator.rb b/plugins/communicators/winrm/communicator.rb index 7ce765298..fdb3fe615 100644 --- a/plugins/communicators/winrm/communicator.rb +++ b/plugins/communicators/winrm/communicator.rb @@ -134,12 +134,7 @@ module VagrantPlugins }.merge(opts || {}) opts[:good_exit] = Array(opts[:good_exit]) - - if opts[:elevated] - guest_script_path = create_elevated_shell_script(command) - command = "powershell -executionpolicy bypass -file #{guest_script_path}" - end - + command = wrap_in_scheduled_task(command) if opts[:elevated] output = shell.send(opts[:shell], command, &block) execution_output(output, opts) end @@ -187,19 +182,15 @@ module VagrantPlugins ) end - # Creates and uploads a PowerShell script which wraps the specified - # command in a scheduled task. The scheduled task allows commands to - # run on the guest as a true local admin without any of the restrictions - # that WinRM puts in place. + # Creates and uploads a PowerShell script which wraps a command in a + # scheduled task. The scheduled task allows commands to run on the guest + # as a true local admin without any of the restrictions that WinRM puts + # in place. # - # @return The path to elevated_shell.ps1 on the guest - def create_elevated_shell_script(command) + # @return The wrapper command to execute + def wrap_in_scheduled_task(command) path = File.expand_path("../scripts/elevated_shell.ps1", __FILE__) - script = Vagrant::Util::TemplateRenderer.render(path, options: { - username: shell.username, - password: shell.password, - command: command.gsub("\"", "`\""), - }) + script = Vagrant::Util::TemplateRenderer.render(path) guest_script_path = "c:/tmp/vagrant-elevated-shell.ps1" file = Tempfile.new(["vagrant-elevated-shell", "ps1"]) begin @@ -211,7 +202,15 @@ module VagrantPlugins file.close file.unlink end - guest_script_path + + # convert to double byte unicode string then base64 encode + # just like PowerShell -EncodedCommand expects + wrapped_encoded_command = Base64.strict_encode64( + "#{command}; exit $LASTEXITCODE".encode('UTF-16LE', 'UTF-8')) + + "powershell -executionpolicy bypass -file \"#{guest_script_path}\" " + + "-username \"#{shell.username}\" -password \"#{shell.password}\" " + + "-encoded_command \"#{wrapped_encoded_command}\"" end # Handles the raw WinRM shell result and converts it to a diff --git a/plugins/communicators/winrm/scripts/elevated_shell.ps1.erb b/plugins/communicators/winrm/scripts/elevated_shell.ps1.erb index 923f034bb..17767e436 100644 --- a/plugins/communicators/winrm/scripts/elevated_shell.ps1.erb +++ b/plugins/communicators/winrm/scripts/elevated_shell.ps1.erb @@ -1,6 +1,4 @@ -$command = "<%= options[:command] %>" + '; exit $LASTEXITCODE' -$user = '<%= options[:username] %>' -$password = '<%= options[:password] %>' +param([String]$username, [String]$password, [String]$encoded_command) $task_name = "WinRM_Elevated_Shell" $out_file = "$env:SystemRoot\Temp\WinRM_Elevated_Shell.log" @@ -14,7 +12,7 @@ $task_xml = @' - {user} + {username} Password HighestAvailable @@ -47,19 +45,17 @@ $task_xml = @' '@ -$bytes = [System.Text.Encoding]::Unicode.GetBytes($command) -$encoded_command = [Convert]::ToBase64String($bytes) $arguments = "/c powershell.exe -EncodedCommand $encoded_command > $out_file 2>&1" $task_xml = $task_xml.Replace("{arguments}", $arguments) -$task_xml = $task_xml.Replace("{user}", $user) +$task_xml = $task_xml.Replace("{username}", $username) $schedule = New-Object -ComObject "Schedule.Service" $schedule.Connect() $task = $schedule.NewTask($null) $task.XmlText = $task_xml $folder = $schedule.GetFolder("\") -$folder.RegisterTaskDefinition($task_name, $task, 6, $user, $password, 1, $null) | Out-Null +$folder.RegisterTaskDefinition($task_name, $task, 6, $username, $password, 1, $null) | Out-Null $registered_task = $folder.GetTask("\$task_name") $registered_task.Run($null) | Out-Null From d2671faa56346420baa288d2787b2eb3b86bffd8 Mon Sep 17 00:00:00 2001 From: Shawn Neal Date: Wed, 10 Jun 2015 16:17:30 -0700 Subject: [PATCH 2/2] Fix WinRM elevated shell test Username, password, and encoded command are now passed via PowerShell arguments which the test needs to account for. --- test/unit/plugins/communicators/winrm/communicator_test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit/plugins/communicators/winrm/communicator_test.rb b/test/unit/plugins/communicators/winrm/communicator_test.rb index 6e5b406d7..ceea0985d 100644 --- a/test/unit/plugins/communicators/winrm/communicator_test.rb +++ b/test/unit/plugins/communicators/winrm/communicator_test.rb @@ -55,7 +55,8 @@ describe VagrantPlugins::CommunicatorWinRM::Communicator do it "wraps command in elevated shell script when elevated is true" do expect(shell).to receive(:upload).with(kind_of(String), "c:/tmp/vagrant-elevated-shell.ps1") expect(shell).to receive(:powershell) do |cmd| - expect(cmd).to eq("powershell -executionpolicy bypass -file c:/tmp/vagrant-elevated-shell.ps1") + expect(cmd).to eq("powershell -executionpolicy bypass -file \"c:/tmp/vagrant-elevated-shell.ps1\" " + + "-username \"vagrant\" -password \"password\" -encoded_command \"ZABpAHIAOwAgAGUAeABpAHQAIAAkAEwAQQBTAFQARQBYAEkAVABDAE8ARABFAA==\"") end.and_return({ exitcode: 0 }) expect(subject.execute("dir", { elevated: true })).to eq(0) end