Favor system ssh binary over embedded ssh binary

lib/vagrant/util/ssh.rb

@@ -54,6 +54,23 @@ module Vagrant
         raise Errors::SSHKeyBadPermissions, key_path: key_path
       end
 
+      def self.determine_ssh_bin(original_ssh_path)
+        begin
+          original_path_env = ENV['PATH']
+          ENV['PATH'] = ENV['VAGRANT_OLD_ENV_PATH']
+          ssh_path = Which.which("ssh")
+          puts "the path: #{ssh_path}"
+          if !ssh_path.nil?
+            LOGGER.debug("Found default ssh binary. Using that instead...")
+            original_ssh_path = ssh_path
+          end
+        ensure
+          ENV['PATH'] = original_path_env
+        end
+
+        return original_ssh_path
+      end
+
       # Halts the running of this process and replaces it with a full-fledged
       # SSH shell into a remote machine.
       #
@@ -79,9 +96,9 @@ module Vagrant
           raise Errors::SSHUnavailable
         end
 
+        if Platform.windows?
           # On Windows, we need to detect whether SSH is actually "plink"
           # underneath the covers. In this case, we tell the user.
-        if Platform.windows?
           r = Subprocess.execute(ssh_path)
           if r.stdout.include?("PuTTY Link") || r.stdout.include?("Plink: command-line connection utility")
             raise Errors::SSHIsPuttyLink,
@@ -90,6 +107,9 @@ module Vagrant
               username: ssh_info[:username],
               key_path: ssh_info[:private_key_path].join(", ")
           end
+
+          # use system ssh if available
+          ssh_path = determine_ssh_bin(ssh_path)
         end
 
         # If plain mode is enabled then we don't do any authentication (we don't
@@ -185,7 +205,9 @@ module Vagrant
         # we really don't care since both work.
         ENV["nodosfilewarning"] = "1" if Platform.cygwin?
 
-        ssh = ssh_info[:ssh_command] || 'ssh'
+        # If an ssh command is defined, use that. If an ssh binary was
+        # discovered on the path, use that. Otherwise fail to just trying `ssh`
+        ssh = ssh_info[:ssh_command] || ssh_path || 'ssh'
 
         # Invoke SSH with all our options
         if !opts[:subprocess]

test/unit/vagrant/util/ssh_test.rb

@@ -28,6 +28,32 @@ describe Vagrant::Util::SSH do
     end
   end
 
+  describe "#determine_ssh_bin" do
+    let (:original_ssh_path) { "/system/dir/bin/ssh" }
+    let (:new_ssh_path) { "/new/ssh/path/bin/ssh" }
+    let (:old_path) { "/old/path/bin:/usr/local/bin:/usr/bin" }
+
+    it "returns passed in ssh path if not found on original path" do
+      path = ENV["PATH"]
+      allow(ENV).to receive(:[]).with("PATH").and_return(path)
+      allow(ENV).to receive(:[]).with("VAGRANT_OLD_ENV_PATH").and_return(old_path)
+      allow(Vagrant::Util::Which).to receive(:which).and_return(nil)
+
+      expect(described_class.determine_ssh_bin(original_ssh_path)).to eq(original_ssh_path)
+      expect(ENV["PATH"]).to eq(path)
+    end
+
+    it "returns system ssh path if found on original path" do
+      path = ENV["PATH"]
+      allow(ENV).to receive(:[]).with("PATH").and_return(path)
+      allow(ENV).to receive(:[]).with("VAGRANT_OLD_ENV_PATH").and_return(old_path)
+      allow(Vagrant::Util::Which).to receive(:which).and_return(new_ssh_path)
+
+      expect(described_class.determine_ssh_bin(original_ssh_path)).to eq(new_ssh_path)
+      expect(ENV["PATH"]).to eq(path)
+    end
+  end
+
   describe "#exec" do
     let(:ssh_info) {{
       host: "localhost",
@@ -38,6 +64,8 @@ describe Vagrant::Util::SSH do
       dsa_authentication: true
     }}
 
+    let(:ssh_path) { "/usr/bin/ssh" }
+
     it "raises an exception if there is no ssh" do
       allow(Vagrant::Util::Which).to receive(:which).and_return(nil)
 
@@ -67,7 +95,7 @@ describe Vagrant::Util::SSH do
 
       expect(described_class.exec(ssh_info)).to eq(nil)
       expect(Vagrant::Util::SafeExec).to have_received(:exec)
-        .with("ssh", "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL","-o", "Compression=yes", "-o", "DSAAuthentication=yes", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"")
+        .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL","-o", "Compression=yes", "-o", "DSAAuthentication=yes", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"")
     end
 
     context "when disabling compression or dsa_authentication flags" do
@@ -85,7 +113,7 @@ describe Vagrant::Util::SSH do
 
         expect(described_class.exec(ssh_info)).to eq(nil)
         expect(Vagrant::Util::SafeExec).to have_received(:exec)
-          .with("ssh", "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"")
+          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"")
       end
     end
 
@@ -103,7 +131,7 @@ describe Vagrant::Util::SSH do
 
         expect(described_class.exec(ssh_info)).to eq(nil)
         expect(Vagrant::Util::SafeExec).to have_received(:exec)
-          .with("ssh", "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"")
+          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"")
       end
     end
 
@@ -122,7 +150,7 @@ describe Vagrant::Util::SSH do
 
         expect(described_class.exec(ssh_info, {plain_mode: true})).to eq(nil)
         expect(Vagrant::Util::SafeExec).to have_received(:exec)
-          .with("ssh", "localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null")
+          .with(ssh_path, "localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null")
       end
     end
 
@@ -140,7 +168,7 @@ describe Vagrant::Util::SSH do
 
         expect(described_class.exec(ssh_info)).to eq(nil)
         expect(Vagrant::Util::SafeExec).to have_received(:exec)
-          .with("ssh", "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"","-o", "ForwardX11=yes", "-o", "ForwardX11Trusted=yes")
+          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"","-o", "ForwardX11=yes", "-o", "ForwardX11Trusted=yes")
       end
     end
 
@@ -158,7 +186,7 @@ describe Vagrant::Util::SSH do
 
         expect(described_class.exec(ssh_info)).to eq(nil)
         expect(Vagrant::Util::SafeExec).to have_received(:exec)
-          .with("ssh", "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"","-o", "ForwardAgent=yes")
+          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"","-o", "ForwardAgent=yes")
       end
     end
 
@@ -176,7 +204,7 @@ describe Vagrant::Util::SSH do
 
         expect(described_class.exec(ssh_info)).to eq(nil)
         expect(Vagrant::Util::SafeExec).to have_received(:exec)
-          .with("ssh", "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"", "-L", "8008:localhost:80")
+          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"", "-L", "8008:localhost:80")
       end
     end
 
@@ -194,7 +222,7 @@ describe Vagrant::Util::SSH do
 
         expect(described_class.exec(ssh_info)).to eq(nil)
         expect(Vagrant::Util::SafeExec).to have_received(:exec)
-          .with("ssh", "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"", "-6")
+          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"", "-6")
       end
     end