Update generated ssh private key file permissions on create

This updates the permissions on the automatically generated private
key file to only be readable by the user. Includes support for file
permission modification on Windows platform.

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/file"
+  require "win32/file/security"
+end
+
 # Always make the version available
 require 'vagrant/version'
 global_logger = Log4r::Logger.new("vagrant::global")

plugins/communicators/ssh/communicator.rb

@@ -193,6 +193,21 @@ 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
+            priv_owner = File.owner(priv_path)
+            new_perms = File.get_permissions(priv_path).map do |p_user, perm|
+              if p_user != priv_owner
+                perm = File::DELETE
+              end
+              [p_user, perm]
+            end
+            File.set_permissions(priv_path, Hash[new_perms])
+          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(

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)
+    allow(machine).to receive(:ssh_info).and_return(machine_ssh_info)
-    expect(channel).to receive(:exec).with(core_shell_cmd).
+    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,127 @@ 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" }
+          let(:permissions){ {} }
+
+          before do
+            allow(private_key_file).to receive(:to_s).and_return("PRIVATE_KEY_PATH")
+            allow(File).to receive(:owner).and_return(owner)
+            allow(File).to receive(:get_permissions).and_return(permissions)
+            allow(File).to receive(:set_permissions)
+            allow(Vagrant::Util::Platform).to receive(:windows?).and_return(true)
+            stub_const('File::DELETE', :delete)
+          end
+
+          it "should extract owner username from file" do
+            expect(File).to receive(:owner).with("PRIVATE_KEY_PATH")
+          end
+
+          it "should get set new permissions on private key file" do
+            expect(File).to receive(:set_permissions).with("PRIVATE_KEY_PATH", {})
+          end
+
+          context "with multiple permissions on file" do
+            let(:permissions){ {owner => :permission, "other" => :permission, "all" => :permission} }
+
+            it "should delete all non-owner permissions" do
+              expect(File).to receive(:set_permissions).with("PRIVATE_KEY_PATH",
+                owner => :permission, "other" => :delete, "all" => :delete)
+            end
+          end
+        end
+      end
+    end
   end
 
   describe ".execute" do

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"