diff --git a/lib/vagrant.rb b/lib/vagrant.rb index ca48f40c8..7690c13f3 100644 --- a/lib/vagrant.rb +++ b/lib/vagrant.rb @@ -65,6 +65,12 @@ require 'i18n' # there are issues with ciphers not being properly loaded. require 'openssl' +# If we are on Windows, load in File helpers +if Vagrant::Util::Platform.windows? + require "win32-ffi-extensions" + require "win32/file/security" +end + # Always make the version available require 'vagrant/version' global_logger = Log4r::Logger.new("vagrant::global") diff --git a/plugins/communicators/ssh/communicator.rb b/plugins/communicators/ssh/communicator.rb index 149566483..56525a40b 100644 --- a/plugins/communicators/ssh/communicator.rb +++ b/plugins/communicators/ssh/communicator.rb @@ -1,3 +1,4 @@ +require 'etc' require 'logger' require 'pathname' require 'stringio' @@ -193,6 +194,14 @@ module VagrantPlugins f.write(priv) end + # Adjust private key file permissions + if Vagrant::Util::Platform.windows? + priv_path = @machine.data_dir.join("private_key").to_s + File.set_permissions(priv_path, Etc.getlogin => File::FULL) + else + @machine.data_dir.join("private_key").chmod(0600) + end + # Remove the old key if it exists @machine.ui.detail(I18n.t("vagrant.inserting_remove_key")) @machine.guest.capability( diff --git a/test/unit/plugins/communicators/ssh/communicator_test.rb b/test/unit/plugins/communicators/ssh/communicator_test.rb index 50510445a..8f8f88806 100644 --- a/test/unit/plugins/communicators/ssh/communicator_test.rb +++ b/test/unit/plugins/communicators/ssh/communicator_test.rb @@ -16,11 +16,13 @@ describe VagrantPlugins::CommunicatorSSH::Communicator do guest_port: 5986, pty: false, keep_alive: false, - insert_key: false, + insert_key: insert_ssh_key, export_command_template: export_command_template, shell: 'bash -l' ) end + # Do not insert public key by default + let(:insert_ssh_key){ false } # Configuration mock let(:config) { double("config", ssh: ssh) } # Provider mock @@ -35,6 +37,8 @@ describe VagrantPlugins::CommunicatorSSH::Communicator do ui: ui ) end + # SSH information of the machine + let(:machine_ssh_info){ {host: '10.1.2.3', port: 22} } # Subject instance to test let(:communicator){ @communicator ||= described_class.new(machine) } # Underlying net-ssh connection mock @@ -60,12 +64,14 @@ describe VagrantPlugins::CommunicatorSSH::Communicator do # Mock for net-ssh scp let(:scp) { double("scp") } + # Setup for commands using the net-ssh connection. This can be reused where needed # by providing to `before` connection_setup = proc do allow(connection).to receive(:closed?).and_return false allow(connection).to receive(:open_channel). and_yield(channel).and_return(channel) + allow(connection).to receive(:close) allow(channel).to receive(:wait).and_return true allow(channel).to receive(:close) allow(command_channel).to receive(:send_data) @@ -74,10 +80,10 @@ describe VagrantPlugins::CommunicatorSSH::Communicator do and_yield(nil, command_stdout_data) allow(command_channel).to receive(:on_extended_data). and_yield(nil, nil, command_stderr_data) - allow(machine).to receive(:ssh_info).and_return(host: '10.1.2.3', port: 22) - expect(channel).to receive(:exec).with(core_shell_cmd). + allow(machine).to receive(:ssh_info).and_return(machine_ssh_info) + allow(channel).to receive(:exec).with(core_shell_cmd). and_yield(command_channel, '').and_return channel - expect(command_channel).to receive(:on_request).with('exit-status'). + allow(command_channel).to receive(:on_request).with('exit-status'). and_yield(nil, exit_data) # Return mocked net-ssh connection during setup allow(communicator).to receive(:retryable).and_return(connection) @@ -121,6 +127,120 @@ describe VagrantPlugins::CommunicatorSSH::Communicator do expect{ communicator.ready? }.to raise_error Vagrant::Errors::SSHInvalidShell end end + + context "with insert key enabled" do + before do + allow(machine).to receive(:guest).and_return(guest) + allow(guest).to receive(:capability?).with(:insert_public_key). + and_return(has_insert_cap) + allow(guest).to receive(:capability?).with(:remove_public_key). + and_return(has_remove_cap) + allow(communicator).to receive(:insecure_key?).with("KEY_PATH").and_return(true) + allow(ui).to receive(:detail) + end + + let(:insert_ssh_key){ true } + let(:has_insert_cap){ false } + let(:has_remove_cap){ false } + let(:machine_ssh_info){ + {host: '10.1.2.3', port: 22, private_key_path: ["KEY_PATH"]} + } + let(:guest){ double("guest") } + + context "without guest insert_ssh_key or remove_ssh_key capabilities" do + it "should raise an error" do + expect{ communicator.ready? }.to raise_error(Vagrant::Errors::SSHInsertKeyUnsupported) + end + end + + context "without guest insert_ssh_key capability" do + let(:has_remove_cap){ true } + + it "should raise an error" do + expect{ communicator.ready? }.to raise_error(Vagrant::Errors::SSHInsertKeyUnsupported) + end + end + + context "without guest remove_ssh_key capability" do + let(:has_insert_cap){ true } + + it "should raise an error" do + expect{ communicator.ready? }.to raise_error(Vagrant::Errors::SSHInsertKeyUnsupported) + end + end + + context "with guest insert_ssh_key capability and remove_ssh_key capability" do + let(:has_insert_cap){ true } + let(:has_remove_cap){ true } + let(:new_public_key){ :new_public_key } + let(:new_private_key){ :new_private_key } + let(:openssh){ :openssh } + let(:private_key_file){ double("private_key_file") } + let(:path_joiner){ double("path_joiner") } + + before do + allow(ui).to receive(:info) + allow(Vagrant::Util::Keypair).to receive(:create). + and_return([new_public_key, new_private_key, openssh]) + allow(private_key_file).to receive(:open).and_yield(private_key_file) + allow(private_key_file).to receive(:write) + allow(private_key_file).to receive(:chmod) + allow(guest).to receive(:capability) + allow(File).to receive(:chmod) + allow(machine).to receive(:data_dir).and_return(path_joiner) + allow(path_joiner).to receive(:join).and_return(private_key_file) + allow(guest).to receive(:capability).with(:insert_public_key) + allow(guest).to receive(:capability).with(:remove_public_key) + end + + after{ communicator.ready? } + + it "should create a new key pair" do + expect(Vagrant::Util::Keypair).to receive(:create). + and_return([new_public_key, new_private_key, openssh]) + end + + it "should call the insert_public_key guest capability" do + expect(guest).to receive(:capability).with(:insert_public_key, openssh) + end + + it "should write the new private key" do + expect(private_key_file).to receive(:write).with(new_private_key) + end + + it "should set private key file as user readable only" do + expect(private_key_file).to receive(:chmod).with(0600) + end + + it "should remove the default public key" do + expect(guest).to receive(:capability).with(:remove_public_key, any_args) + end + + context "on windows platform" do + let(:owner){ "owner" } + + before do + allow(private_key_file).to receive(:to_s).and_return("PRIVATE_KEY_PATH") + allow(File).to receive(:set_permissions) + allow(Vagrant::Util::Platform).to receive(:windows?).and_return(true) + allow(Etc).to receive(:getlogin).and_return(owner) + stub_const('File::FULL', :full) + end + + it "should get set new permissions on private key file" do + expect(File).to receive(:set_permissions).with("PRIVATE_KEY_PATH", any_args) + end + + context "with multiple permissions on file" do + + it "should delete all non-owner permissions" do + expect(File).to receive(:set_permissions).with("PRIVATE_KEY_PATH", + owner => :full) + end + end + end + end + end end describe ".execute" do diff --git a/vagrant.gemspec b/vagrant.gemspec index edc12f032..4096fd684 100644 --- a/vagrant.gemspec +++ b/vagrant.gemspec @@ -28,6 +28,8 @@ Gem::Specification.new do |s| s.add_dependency "rb-kqueue", "~> 0.2.0" s.add_dependency "rest-client", ">= 1.6.0", "< 3.0" s.add_dependency "wdm", "~> 0.1.0" + s.add_dependency "win32-file", "~> 0.8.1" + s.add_dependency "win32-file-security", "~> 1.0.10" s.add_dependency "winrm", "~> 2.1" s.add_dependency "winrm-fs", "~> 1.0" s.add_dependency "winrm-elevated", "~> 1.1"