From b02f110cd8a6a67ee8f13b31f4cdc9430829fb8c Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 14 Jun 2017 16:38:31 -0700 Subject: [PATCH] (#7855) Introduce more ssh options for machines This commit allows the user to configure two additional options that were previously not configurable: Compression and DSAAuthentication. Each config option is set as a boolean, and if left out of the config will default to its previous behavior which is included and set to "yes". If the user explicitly sets it to false, it will not be included as an ssh option. --- lib/vagrant/machine.rb | 4 + lib/vagrant/util/ssh.rb | 10 +- plugins/kernel_v2/config/ssh_connect.rb | 6 + test/unit/vagrant/util/ssh_test.rb | 148 ++++++++++++++++++ .../docs/vagrantfile/ssh_settings.html.md | 8 + 5 files changed, 174 insertions(+), 2 deletions(-) diff --git a/lib/vagrant/machine.rb b/lib/vagrant/machine.rb index c5e73b415..884af9503 100644 --- a/lib/vagrant/machine.rb +++ b/lib/vagrant/machine.rb @@ -440,6 +440,8 @@ module Vagrant info[:keys_only] ||= @config.ssh.default.keys_only info[:paranoid] ||= @config.ssh.default.paranoid info[:username] ||= @config.ssh.default.username + info[:compression] ||= @config.ssh.default.compression + info[:dsa_authentication] ||= @config.ssh.default.dsa_authentication # We set overrides if they are set. These take precedence over # provider-returned data. @@ -447,6 +449,8 @@ module Vagrant info[:port] = @config.ssh.port if @config.ssh.port info[:keys_only] = @config.ssh.keys_only info[:paranoid] = @config.ssh.paranoid + info[:compression] = @config.ssh.compression + info[:dsa_authentication] = @config.ssh.dsa_authentication info[:username] = @config.ssh.username if @config.ssh.username info[:password] = @config.ssh.password if @config.ssh.password diff --git a/lib/vagrant/util/ssh.rb b/lib/vagrant/util/ssh.rb index 77cc1e911..45efddfbe 100644 --- a/lib/vagrant/util/ssh.rb +++ b/lib/vagrant/util/ssh.rb @@ -107,10 +107,16 @@ module Vagrant # Command line options command_options = [ "-p", options[:port].to_s, - "-o", "Compression=yes", - "-o", "DSAAuthentication=yes", "-o", "LogLevel=#{log_level}"] + if ssh_info[:compression] + command_options += ["-o", "Compression=yes"] + end + + if ssh_info[:dsa_authentication] + command_options += ["-o", "DSAAuthentication=yes"] + end + # Solaris/OpenSolaris/Illumos uses SunSSH which doesn't support the # IdentitiesOnly option. Also, we don't enable it in plain mode or if # if keys_only is false so that SSH and Net::SSH properly search our identities diff --git a/plugins/kernel_v2/config/ssh_connect.rb b/plugins/kernel_v2/config/ssh_connect.rb index d4cdba5c3..ba2f3175d 100644 --- a/plugins/kernel_v2/config/ssh_connect.rb +++ b/plugins/kernel_v2/config/ssh_connect.rb @@ -9,6 +9,8 @@ module VagrantPlugins attr_accessor :insert_key attr_accessor :keys_only attr_accessor :paranoid + attr_accessor :compression + attr_accessor :dsa_authentication def initialize @host = UNSET_VALUE @@ -19,6 +21,8 @@ module VagrantPlugins @insert_key = UNSET_VALUE @keys_only = UNSET_VALUE @paranoid = UNSET_VALUE + @compression = UNSET_VALUE + @dsa_authentication = UNSET_VALUE end def finalize! @@ -30,6 +34,8 @@ module VagrantPlugins @insert_key = true if @insert_key == UNSET_VALUE @keys_only = true if @keys_only == UNSET_VALUE @paranoid = false if @paranoid == UNSET_VALUE + @compression = true if @compression == UNSET_VALUE + @dsa_authentication = true if @dsa_authentication == UNSET_VALUE if @private_key_path && !@private_key_path.is_a?(Array) @private_key_path = [@private_key_path] diff --git a/test/unit/vagrant/util/ssh_test.rb b/test/unit/vagrant/util/ssh_test.rb index b2f2f853a..2015bf4aa 100644 --- a/test/unit/vagrant/util/ssh_test.rb +++ b/test/unit/vagrant/util/ssh_test.rb @@ -27,4 +27,152 @@ describe Vagrant::Util::SSH do expect(key_path.stat.mode).to eq(0100600) end end + + describe "#exec" do + let(:ssh_info) {{ + host: "localhost", + port: 2222, + username: "vagrant", + private_key_path: [temporary_file], + compression: true, + dsa_authentication: true + }} + + it "raises an exception if there is no ssh" do + allow(Vagrant::Util::Which).to receive(:which).and_return(nil) + + expect { described_class.exec(ssh_info) }. + to raise_error Vagrant::Errors::SSHUnavailable + end + + it "raises an exception if there is no ssh and platform is windows" do + allow(Vagrant::Util::Which).to receive(:which).and_return(nil) + allow(Vagrant::Util::Platform).to receive(:windows?).and_return(true) + + expect { described_class.exec(ssh_info) }. + to raise_error Vagrant::Errors::SSHUnavailableWindows + end + + it "raises an exception if the platform is windows and uses PuTTY Link" do + allow(Vagrant::Util::Platform).to receive(:windows?).and_return(true) + allow(Vagrant::Util::Subprocess).to receive(:execute). + and_return(double("output", stdout: 'PuTTY Link')) + + expect { described_class.exec(ssh_info) }. + to raise_error Vagrant::Errors::SSHIsPuttyLink + end + + it "invokes SSH with options if subprocess is not allowed" do + allow(Vagrant::Util::SafeExec).to receive(:exec).and_return(nil) + + 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", "-i", anything) + end + + context "when disabling compression or dsa_authentication flags" do + let(:ssh_info) {{ + host: "localhost", + port: 2222, + username: "vagrant", + private_key_path: [temporary_file], + compression: false, + dsa_authentication: false + }} + + it "does not include compression or dsa_authentication flags if disabled" do + allow(Vagrant::Util::SafeExec).to receive(:exec).and_return(nil) + + 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", "-i", anything) + end + end + + context "when paranoid is true" do + let(:ssh_info) {{ + host: "localhost", + port: 2222, + username: "vagrant", + private_key_path: [temporary_file], + paranoid: true + }} + + it "does not disable StrictHostKeyChecking or set UserKnownHostsFile" do + allow(Vagrant::Util::SafeExec).to receive(:exec).and_return(nil) + + 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", "-i", anything) + end + end + + context "when not on solaris not using plain mode or with keys_only enabled" do + let(:ssh_info) {{ + host: "localhost", + port: 2222, + username: "vagrant", + private_key_path: [temporary_file], + keys_only: true + }} + + it "adds IdentitiesOnly as an option for ssh" do + allow(Vagrant::Util::SafeExec).to receive(:exec).and_return(nil) + allow(Vagrant::Util::Platform).to receive(:solaris?).and_return(false) + + 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") + end + end + + context "when forward_x11 is enabled" do + let(:ssh_info) {{ + host: "localhost", + port: 2222, + username: "vagrant", + private_key_path: [temporary_file], + forward_x11: true + }} + + it "enables ForwardX11 options" do + allow(Vagrant::Util::SafeExec).to receive(:exec).and_return(nil) + + 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", "-i", anything,"-o", "ForwardX11=yes", "-o", "ForwardX11Trusted=yes") + end + end + + context "when forward_agent is enabled" do + let(:ssh_info) {{ + host: "localhost", + port: 2222, + username: "vagrant", + private_key_path: [temporary_file], + forward_agent: true + }} + + it "enables agent forwarding options" do + allow(Vagrant::Util::SafeExec).to receive(:exec).and_return(nil) + + 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", "-i", anything,"-o", "ForwardAgent=yes") + end + end + + context "with subprocess enabled" do + let(:ssh_info) {{ + host: "localhost", + port: 2222, + username: "vagrant", + private_key_path: [temporary_file], + }} + + it "executes SSH in a subprocess with options and returns an exit code Fixnum" do + expect(described_class.exec(ssh_info, {subprocess: true})).to be_an(Fixnum) + end + end + end end diff --git a/website/source/docs/vagrantfile/ssh_settings.html.md b/website/source/docs/vagrantfile/ssh_settings.html.md index a4acbee31..c6122d248 100644 --- a/website/source/docs/vagrantfile/ssh_settings.html.md +++ b/website/source/docs/vagrantfile/ssh_settings.html.md @@ -149,3 +149,11 @@ config.ssh.export_command_template = 'export %ENV_KEY%="%ENV_VALUE%"' `config.ssh.sudo_command` - The command to use when executing a command with `sudo`. This defaults to `sudo -E -H %c`. The `%c` will be replaced by the command that is being executed. + +`config.ssh.compression` - If `false`, this setting will not include the +compression setting when ssh'ing into a machine. If this is not set, it will +default to `true` and `Compression=yes` will be enabled with ssh. + +`config.ssh.dsa_authentication` - If `false`, this setting will not include +`DSAAuthentication` when ssh'ing into a machine. If this is not set, it will +default to `true` and `DSAAuthentication=yes` will be used with ssh.