From bcc09e10e6e865f0e742548db8fa088e1484968e Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Fri, 7 Jul 2017 09:36:42 -0700 Subject: [PATCH 1/3] Make upload directory for winssh communicator configurable Fixes #8731 --- plugins/communicators/winssh/communicator.rb | 2 +- plugins/communicators/winssh/config.rb | 8 ++++++++ .../plugins/communicators/winssh/communicator_test.rb | 3 ++- website/source/docs/vagrantfile/winssh_settings.html.md | 5 +++++ 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/plugins/communicators/winssh/communicator.rb b/plugins/communicators/winssh/communicator.rb index 37fe347da..e7de70675 100644 --- a/plugins/communicators/winssh/communicator.rb +++ b/plugins/communicators/winssh/communicator.rb @@ -33,7 +33,7 @@ module VagrantPlugins tfile = Tempfile.new('vagrant-ssh') remote_ext = shell == "powershell" ? "ps1" : "bat" - remote_name = "C:\\Windows\\Temp\\#{File.basename(tfile.path)}.#{remote_ext}" + remote_name = "#{machine_config_ssh.upload_directory}\\#{File.basename(tfile.path)}.#{remote_ext}" if shell == "powershell" base_cmd = "powershell -File #{remote_name}" diff --git a/plugins/communicators/winssh/config.rb b/plugins/communicators/winssh/config.rb index 4ad03d19f..e6cc9aec7 100644 --- a/plugins/communicators/winssh/config.rb +++ b/plugins/communicators/winssh/config.rb @@ -4,9 +4,17 @@ module VagrantPlugins module CommunicatorWinSSH class Config < VagrantPlugins::Kernel_V2::SSHConfig + attr_accessor :upload_directory + + def initialize + super + @upload_directory = UNSET_VALUE + end + def finalize! @shell = "cmd" if @shell == UNSET_VALUE @sudo_command = "%c" if @sudo_command == UNSET_VALUE + @upload_directory = "C:\\Windows\\Temp" if @upload_directory == UNSET_VALUE if @export_command_template == UNSET_VALUE if @shell == "cmd" @export_command_template = 'set %ENV_KEY%="%ENV_VALUE%"' diff --git a/test/unit/plugins/communicators/winssh/communicator_test.rb b/test/unit/plugins/communicators/winssh/communicator_test.rb index 459cac6d4..34812ac72 100644 --- a/test/unit/plugins/communicators/winssh/communicator_test.rb +++ b/test/unit/plugins/communicators/winssh/communicator_test.rb @@ -22,7 +22,8 @@ describe VagrantPlugins::CommunicatorWinSSH::Communicator do double("winssh", insert_key: false, export_command_template: export_command_template, - shell: 'cmd' + shell: 'cmd', + upload_directory: "C:\\Windows\\Temp" ) end # Configuration mock diff --git a/website/source/docs/vagrantfile/winssh_settings.html.md b/website/source/docs/vagrantfile/winssh_settings.html.md index 4c0da9753..6722100eb 100644 --- a/website/source/docs/vagrantfile/winssh_settings.html.md +++ b/website/source/docs/vagrantfile/winssh_settings.html.md @@ -161,3 +161,8 @@ config.winssh.export_command_template = '$env:%ENV_KEY%="%ENV_VALUE%"' with `sudo`. This defaults to `%c` (assumes vagrant user is an administator and needs no escalation). The `%c` will be replaced by the command that is being executed. + +
+ +`config.winssh.upload_directory` - The upload directory used on the guest +to store scripts for execute. This is set to `C:\Windows\Temp` by default. From cef38eefd0137e7fa0bec818921304807e35da4d Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Fri, 7 Jul 2017 09:38:11 -0700 Subject: [PATCH 2/3] Add public key capability to Windows guests for winssh communicator --- plugins/guests/windows/cap/public_key.rb | 106 ++++++++++++++++++ plugins/guests/windows/errors.rb | 4 + plugins/guests/windows/plugin.rb | 10 ++ templates/locales/guest_windows.yml | 4 + .../windows/cap/insert_public_key_test.rb | 75 +++++++++++++ .../windows/cap/remove_public_key_test.rb | 66 +++++++++++ 6 files changed, 265 insertions(+) create mode 100644 plugins/guests/windows/cap/public_key.rb create mode 100644 test/unit/plugins/guests/windows/cap/insert_public_key_test.rb create mode 100644 test/unit/plugins/guests/windows/cap/remove_public_key_test.rb diff --git a/plugins/guests/windows/cap/public_key.rb b/plugins/guests/windows/cap/public_key.rb new file mode 100644 index 000000000..c5ede693a --- /dev/null +++ b/plugins/guests/windows/cap/public_key.rb @@ -0,0 +1,106 @@ +require "tempfile" + +module VagrantPlugins + module GuestWindows + module Cap + class PublicKey + def self.insert_public_key(machine, contents) + if machine.communicate.is_a?(CommunicatorWinSSH::Communicator) + winssh_insert_public_key(machine, contents) + else + raise Vagrant::Errors::SSHInsertKeyUnsupported + end + end + + def self.remove_public_key(machine, contents) + if machine.communicate.is_a?(CommunicatorWinSSH::Communicator) + winssh_remove_public_key(machine, contents) + else + raise Vagrant::Errors::SSHInsertKeyUnsupported + end + end + + def self.winssh_insert_public_key(machine, contents) + comm = machine.communicate + contents = contents.strip + + directories = fetch_guest_paths(comm) + home_dir = directories[:home] + temp_dir = directories[:temp] + + remote_ssh_dir = "#{home_dir}\\.ssh" + remote_upload_path = "#{temp_dir}\\vagrant-insert-pubkey-#{Time.now.to_i}" + remote_authkeys_path = "#{remote_ssh_dir}\authorized_keys" + + # Ensure the user's ssh directory exists + comm.execute("dir \"#{remote_ssh_dir}\"\n if errorlevel 1 (mkdir \"#{remote_ssh_dir}\")", shell: "cmd") + remote_upload_path = "#{temp_dir}\\vagrant-insert-pubkey-#{Time.now.to_i}" + remote_authkeys_path = "#{remote_ssh_dir}\\authorized_keys" + + keys_file = Tempfile.new("vagrant-windows-insert-public-key") + # Check if an authorized_keys file already exists + result = comm.execute("dir \"#{remote_authkeys_path}\"", shell: "cmd", error_check: false) + if result == 0 + keys_file.close + comm.download(remote_authkeys_path, keys_file.path) + current_content = File.read(keys_file.path).split(/[\r\n]+/) + if !current_content.include?(contents) + current_content << contents + end + File.write(keys_file.path, current_content.join("\r\n") + "\r\n") + else + keys_file.puts(contents) + keys_file.close + end + keys_file.delete + comm.upload(keys_file.path, remote_upload_path) + comm.execute("move /y \"#{remote_upload_path}\" \"#{remote_authkeys_path}\"", shell: "cmd") + end + + def self.winssh_remove_public_key(machine, contents) + comm = machine.communicate + + directories = fetch_guest_paths(comm) + home_dir = directories[:home] + temp_dir = directories[:temp] + + remote_ssh_dir = "#{home_dir}\\.ssh" + remote_upload_path = "#{temp_dir}\\vagrant-remove-pubkey-#{Time.now.to_i}" + remote_authkeys_path = "#{remote_ssh_dir}\\authorized_keys" + + # Check if an authorized_keys file already exists + result = comm.execute("dir \"#{remote_authkeys_path}\"", shell: "cmd", error_check: false) + if result == 0 + keys_file = Tempfile.new("vagrant-windows-remove-public-key") + keys_file.close + comm.download(remote_authkeys_path, keys_file.path) + current_content = File.read(keys_file.path).split(/[\r\n]+/) + current_content.delete(contents) + File.write(keys_file.path, current_content.join("\r\n") + "\r\n") + comm.upload(keys_file.path, remote_upload_path) + keys_file.delete + comm.execute("move /y \"#{remote_upload_path}\" \"#{remote_authkeys_path}\"", shell: "cmd") + end + end + + # Fetch user's temporary and home directory paths from the Windows guest + # + # @param [Communicator] + # @return [Hash] {:temp, :home} + def self.fetch_guest_paths(communicator) + output = "" + communicator.execute("echo %TEMP%\necho %USERPROFILE%", shell: "cmd") do |type, data| + if type == :stdout + output << data + end + end + temp_dir, home_dir = output.strip.split(/[\r\n]+/) + if temp_dir.nil? || home_dir.nil? + raise Errors::PublicKeyDirectoryFailure + end + {temp: temp_dir, home: home_dir} + end + end + end + end +end diff --git a/plugins/guests/windows/errors.rb b/plugins/guests/windows/errors.rb index e76753646..0b0536ed4 100644 --- a/plugins/guests/windows/errors.rb +++ b/plugins/guests/windows/errors.rb @@ -13,6 +13,10 @@ module VagrantPlugins class RenameComputerFailed < WindowsError error_key(:rename_computer_failed) end + + class PublicKeyDirectoryFailure < WindowsError + error_key(:public_key_directory_failure) + end end end end diff --git a/plugins/guests/windows/plugin.rb b/plugins/guests/windows/plugin.rb index d4c5af98b..f7a71e959 100644 --- a/plugins/guests/windows/plugin.rb +++ b/plugins/guests/windows/plugin.rb @@ -74,6 +74,16 @@ module VagrantPlugins Cap::RSync end + guest_capability(:windows, :insert_public_key) do + require_relative "cap/public_key" + Cap::PublicKey + end + + guest_capability(:windows, :remove_public_key) do + require_relative "cap/public_key" + Cap::PublicKey + end + protected def self.init! diff --git a/templates/locales/guest_windows.yml b/templates/locales/guest_windows.yml index f9a3bf997..0e91a4ca0 100644 --- a/templates/locales/guest_windows.yml +++ b/templates/locales/guest_windows.yml @@ -1,6 +1,10 @@ en: vagrant_windows: errors: + public_key_directory_failure: |- + Vagrant failed to properly discover the correct paths for the + temporary directory and user profile directory on the Windows + guest. Please ensure the guest is properly configured. network_winrm_required: |- Configuring networks on Windows requires the communicator to be set to WinRM. To do this, add the following to your Vagrantfile: diff --git a/test/unit/plugins/guests/windows/cap/insert_public_key_test.rb b/test/unit/plugins/guests/windows/cap/insert_public_key_test.rb new file mode 100644 index 000000000..37d5a64f8 --- /dev/null +++ b/test/unit/plugins/guests/windows/cap/insert_public_key_test.rb @@ -0,0 +1,75 @@ +require "tempfile" +require_relative "../../../../base" +require_relative "../../../../../../plugins/communicators/winssh/communicator" + +describe "VagrantPlugins::GuestWindows::Cap::InsertPublicKey" do + let(:caps) do + VagrantPlugins::GuestWindows::Plugin + .components + .guest_capabilities[:windows] + end + + let(:machine) { double("machine") } + let(:comm) { VagrantTests::DummyCommunicator::Communicator.new(machine) } + let(:auth_keys_check_result){ 1 } + + before do + @tempfile = Tempfile.new("vagrant-test") + allow(Tempfile).to receive(:new).and_return(@tempfile) + allow(comm).to receive(:is_a?).and_return(true) + allow(machine).to receive(:communicate).and_return(comm) + + allow(comm).to receive(:execute).with(/echo .+/, shell: "cmd").and_yield(:stdout, "TEMP\r\nHOME\r\n") + allow(comm).to receive(:execute).with(/dir .+\.ssh/, shell: "cmd") + allow(comm).to receive(:execute).with(/dir .+authorized_keys/, shell: "cmd", error_check: false).and_return(auth_keys_check_result) + end + + after do + @tempfile.delete + end + + describe ".insert_public_key" do + let(:cap) { caps.get(:insert_public_key) } + + context "when authorized_keys exists on guest" do + let(:auth_keys_check_result){ 0 } + before do + expect(@tempfile).to receive(:delete).and_return(true) + expect(@tempfile).to receive(:delete).and_call_original + end + + it "inserts the public key" do + expect(comm).to receive(:download) + expect(comm).to receive(:upload) + expect(comm).to receive(:execute).with(/move .*/, shell: "cmd") + cap.insert_public_key(machine, "ssh-rsa ...") + expect(File.read(@tempfile.path)).to include("ssh-rsa ...") + end + end + + context "when authorized_keys does not exist on guest" do + before do + expect(@tempfile).to receive(:delete).and_return(true) + expect(@tempfile).to receive(:delete).and_call_original + end + + it "inserts the public key" do + expect(comm).to_not receive(:download) + expect(comm).to receive(:upload) + expect(comm).to receive(:execute).with(/move .*/, shell: "cmd") + cap.insert_public_key(machine, "ssh-rsa ...") + expect(File.read(@tempfile.path)).to include("ssh-rsa ...") + end + end + + context "when required directories cannot be fetched from the guest" do + before do + expect(comm).to receive(:execute).with(/echo .+/, shell: "cmd").and_yield(:stdout, "TEMP\r\n") + end + + it "should raise an error" do + expect{ cap.insert_public_key(machine, "ssh-rsa ...") }.to raise_error(VagrantPlugins::GuestWindows::Errors::PublicKeyDirectoryFailure) + end + end + end +end diff --git a/test/unit/plugins/guests/windows/cap/remove_public_key_test.rb b/test/unit/plugins/guests/windows/cap/remove_public_key_test.rb new file mode 100644 index 000000000..9fb2903dd --- /dev/null +++ b/test/unit/plugins/guests/windows/cap/remove_public_key_test.rb @@ -0,0 +1,66 @@ +require "tempfile" +require_relative "../../../../base" +require_relative "../../../../../../plugins/communicators/winssh/communicator" + +describe "VagrantPlugins::GuestWindows::Cap::RemovePublicKey" do + let(:caps) do + VagrantPlugins::GuestWindows::Plugin + .components + .guest_capabilities[:windows] + end + + let(:machine) { double("machine") } + let(:comm) { VagrantTests::DummyCommunicator::Communicator.new(machine) } + let(:public_key_insecure){ "ssh-rsa...insecure" } + let(:public_key_other){ "ssh-rsa...other" } + + let(:auth_keys_check_result){ 1 } + + before do + @tempfile = Tempfile.new("vagrant-test") + @tempfile.puts(public_key_insecure) + @tempfile.puts(public_key_other) + @tempfile.flush + @tempfile.rewind + allow(Tempfile).to receive(:new).and_return(@tempfile) + allow(comm).to receive(:is_a?).and_return(true) + allow(machine).to receive(:communicate).and_return(comm) + + allow(comm).to receive(:execute).with(/echo .+/, shell: "cmd").and_yield(:stdout, "TEMP\r\nHOME\r\n") + allow(comm).to receive(:execute).with(/dir .+authorized_keys/, shell: "cmd", error_check: false).and_return(auth_keys_check_result) + end + + after do + @tempfile.delete + end + + describe ".remove_public_key" do + let(:cap) { caps.get(:remove_public_key) } + + context "when authorized_keys exists on guest" do + let(:auth_keys_check_result){ 0 } + before do + expect(@tempfile).to receive(:delete).and_return(true) + expect(@tempfile).to receive(:delete).and_call_original + end + + it "removes the public key" do + expect(comm).to receive(:download) + expect(comm).to receive(:upload) + expect(comm).to receive(:execute).with(/move .*/, shell: "cmd") + cap.remove_public_key(machine, public_key_insecure) + expect(File.read(@tempfile.path)).to include(public_key_other) + expect(File.read(@tempfile.path)).to_not include(public_key_insecure) + end + end + + context "when authorized_keys does not exist on guest" do + it "does nothing" do + expect(comm).to_not receive(:download) + expect(comm).to_not receive(:upload) + expect(comm).to_not receive(:execute).with(/move .*/, shell: "cmd") + cap.remove_public_key(machine, public_key_insecure) + end + end + end +end From 36a50f4d69e263cca150ed5d79a72b9394667fbd Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Fri, 7 Jul 2017 11:11:10 -0700 Subject: [PATCH 3/3] Allow a bit more time for process to get started --- test/unit/vagrant/util/subprocess_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/vagrant/util/subprocess_test.rb b/test/unit/vagrant/util/subprocess_test.rb index d20a7fffd..94ba4225f 100644 --- a/test/unit/vagrant/util/subprocess_test.rb +++ b/test/unit/vagrant/util/subprocess_test.rb @@ -66,7 +66,7 @@ describe Vagrant::Util::Subprocess do it "should return true" do sp = described_class.new("sleep", "5") thread = Thread.new{ sp.execute } - sleep(0.1) + sleep(0.3) expect(sp.running?).to be_true sp.stop thread.join