From 984c4f402522e54c67a1a884bb6a060434f02945 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 5 Aug 2012 13:12:53 -0700 Subject: [PATCH] Add Util::SSH which has methods for checking key permissions and exec --- lib/vagrant.rb | 1 - lib/vagrant/ssh.rb | 128 ----------------------------- lib/vagrant/util/ssh.rb | 121 +++++++++++++++++++++++++++ test/unit/vagrant/ssh_test.rb | 30 ------- test/unit/vagrant/util/ssh_test.rb | 30 +++++++ 5 files changed, 151 insertions(+), 159 deletions(-) delete mode 100644 lib/vagrant/ssh.rb create mode 100644 lib/vagrant/util/ssh.rb delete mode 100644 test/unit/vagrant/ssh_test.rb create mode 100644 test/unit/vagrant/util/ssh_test.rb diff --git a/lib/vagrant.rb b/lib/vagrant.rb index bd021743d..24af56514 100644 --- a/lib/vagrant.rb +++ b/lib/vagrant.rb @@ -78,7 +78,6 @@ module Vagrant autoload :Hosts, 'vagrant/hosts' autoload :Machine, 'vagrant/machine' autoload :Plugin, 'vagrant/plugin' - autoload :SSH, 'vagrant/ssh' autoload :TestHelpers, 'vagrant/test_helpers' autoload :UI, 'vagrant/ui' autoload :Util, 'vagrant/util' diff --git a/lib/vagrant/ssh.rb b/lib/vagrant/ssh.rb deleted file mode 100644 index f23b4d0f3..000000000 --- a/lib/vagrant/ssh.rb +++ /dev/null @@ -1,128 +0,0 @@ -require 'log4r' - -require 'vagrant/util/file_mode' -require 'vagrant/util/platform' -require 'vagrant/util/safe_exec' - -module Vagrant - # Manages SSH connection information as well as allows opening an - # SSH connection. - class SSH - include Util::SafeExec - - def initialize(vm) - @vm = vm - @logger = Log4r::Logger.new("vagrant::ssh") - end - - # Returns a hash of information necessary for accessing this - # virtual machine via SSH. - # - # @return [Hash] - def info - results = { - :host => @vm.config.ssh.host, - :port => @vm.config.ssh.port || @vm.driver.ssh_port(@vm.config.ssh.guest_port), - :username => @vm.config.ssh.username, - :forward_agent => @vm.config.ssh.forward_agent, - :forward_x11 => @vm.config.ssh.forward_x11 - } - - # This can happen if no port is set and for some reason Vagrant - # can't detect an SSH port. - raise Errors::SSHPortNotDetected if !results[:port] - - # Determine the private key path, which is either set by the - # configuration or uses just the built-in insecure key. - pk_path = @vm.config.ssh.private_key_path || @vm.env.default_private_key_path - results[:private_key_path] = File.expand_path(pk_path, @vm.env.root_path) - - # We need to check and fix the private key permissions - # to make sure that SSH gets a key with 0600 perms. - check_key_permissions(results[:private_key_path]) - - # Return the results - return results - end - - # Connects to the environment's virtual machine, replacing the ruby - # process with an SSH process. - # - # @param [Hash] opts Options hash - # @options opts [Boolean] :plain_mode If True, doesn't authenticate with - # the machine, only connects, allowing the user to connect. - def exec(opts={}) - # Get the SSH information and cache it here - ssh_info = info - - if Util::Platform.windows? - raise Errors::SSHUnavailableWindows, :host => ssh_info[:host], - :port => ssh_info[:port], - :username => ssh_info[:username], - :key_path => ssh_info[:private_key_path] - end - - raise Errors::SSHUnavailable if !Kernel.system("which ssh > /dev/null 2>&1") - - # If plain mode is enabled then we don't do any authentication (we don't - # set a user or an identity file) - plain_mode = opts[:plain_mode] - - options = {} - options[:host] = ssh_info[:host] - options[:port] = ssh_info[:port] - options[:username] = ssh_info[:username] - options[:private_key_path] = ssh_info[:private_key_path] - - # Command line options - command_options = ["-p", options[:port].to_s, "-o", "UserKnownHostsFile=/dev/null", - "-o", "StrictHostKeyChecking=no", "-o", "LogLevel=FATAL"] - - # Solaris/OpenSolaris/Illumos uses SunSSH which doesn't support the IdentitiesOnly option - # (Also don't use it in plain mode, it'll skip user agents.) - command_options += ["-o", "IdentitiesOnly=yes"] if !(Util::Platform.solaris? || plain_mode) - - command_options += ["-i", options[:private_key_path]] if !plain_mode - command_options += ["-o", "ForwardAgent=yes"] if ssh_info[:forward_agent] - - # If there are extra options, then we append those - command_options.concat(opts[:extra_args]) if opts[:extra_args] - - if ssh_info[:forward_x11] - # Both are required so that no warnings are shown regarding X11 - command_options += ["-o", "ForwardX11=yes"] - command_options += ["-o", "ForwardX11Trusted=yes"] - end - - host_string = options[:host] - host_string = "#{options[:username]}@#{host_string}" if !plain_mode - command_options << host_string - @logger.info("Invoking SSH: #{command_options.inspect}") - safe_exec("ssh", *command_options) - end - - # Checks the file permissions for a private key, resetting them - # if needed. - def check_key_permissions(key_path) - # Windows systems don't have this issue - return if Util::Platform.windows? - - @logger.debug("Checking key permissions: #{key_path}") - stat = File.stat(key_path) - - if stat.owned? && Util::FileMode.from_octal(stat.mode) != "600" - @logger.info("Attempting to correct key permissions to 0600") - File.chmod(0600, key_path) - - stat = File.stat(key_path) - if Util::FileMode.from_octal(stat.mode) != "600" - raise Errors::SSHKeyBadPermissions, :key_path => key_path - end - end - rescue Errno::EPERM - # This shouldn't happen since we verified we own the file, but - # it is possible in theory, so we raise an error. - raise Errors::SSHKeyBadPermissions, :key_path => key_path - end - end -end diff --git a/lib/vagrant/util/ssh.rb b/lib/vagrant/util/ssh.rb new file mode 100644 index 000000000..0bac2a4a7 --- /dev/null +++ b/lib/vagrant/util/ssh.rb @@ -0,0 +1,121 @@ +require "log4r" + +require "vagrant/util/file_mode" +require "vagrant/util/platform" +require "vagrant/util/safe_exec" + +module Vagrant + module Util + # This is a class that has helpers on it for dealing with SSH. These + # helpers don't depend on any part of Vagrant except what is given + # via the parameters. + class SSH + include SafeExec + + LOGGER = Log4r::Logger.new("vagrant::util::ssh") + + # Checks that the permissions for a private key are valid, and fixes + # them if possible. SSH requires that permissions on the private key + # are 0600 on POSIX based systems. This will make a best effort to + # fix these permissions if they are not properly set. + # + # @param [Pathname] key_path The path to the private key. + def self.check_key_permissions(key_path) + # Don't do anything if we're on Windows, since Windows doesn't worry + # about key permissions. + return if Platform.windows? + + LOGGER.debug("Checking key permissions: #{key_path}") + stat = key_path.stat + + if stat.owned? && FileMode.from_octal(stat.mode) != "600" + LOGGER.info("Attempting to correct key permissions to 0600") + key_path.chmod(0600) + + # Re-stat the file to get the new mode, and verify it worked + stat = key_path.stat + if FileMode.from_octal(stat.mode) != "600" + raise Errors::SSHKeyBadPermissions, :key_path => key_path + end + end + rescue Errno::EPERM + # This shouldn't happen since we verify we own the file, but + # it is possible in theory, so we raise an error. + raise Errors::SSHKeyBadPermissions, :key_path => key_path + end + + # Halts the running of this process and replaces it with a full-fledged + # SSH shell into a remote machine. + # + # Note: This method NEVER returns. The process ends after this. + # + # @param [Hash] ssh_info This is the SSH information. For the keys + # required please see the documentation of {Machine#ssh_info}. + # @param [Hash] opts These are additional options that are supported + # by exec. + def exec(ssh_info, opts={}) + # If we're running Windows, raise an exception since we currently + # still don't support exec-ing into SSH. In the future this should + # certainly be possible if we can detect we're in an environment that + # supports it. + if Platform.windows? + raise Errors::SSHUnavailableWindows, + :host => ssh_info[:host], + :port => ssh_info[:port], + :username => ssh_info[:username], + :key_path => ssh_info[:private_key_path] + end + + # Verify that we have SSH available on the system. + raise Errors::SSHUnavailable if !Kernel.system("which ssh > /dev/null 2>&1") + + # If plain mode is enabled then we don't do any authentication (we don't + # set a user or an identity file) + plain_mode = opts[:plain_mode] + + options = {} + options[:host] = ssh_info[:host] + options[:port] = ssh_info[:port] + options[:username] = ssh_info[:username] + options[:private_key_path] = ssh_info[:private_key_path] + + # Command line options + command_options = [ + "-p", options[:port].to_s, + "-o", "LogLevel=FATAL", + "-o", "StrictHostKeyChecking=no", + "-o", "UserKnownHostsFile=/dev/null"] + + # Configurables + command_options += ["-o", "ForwardAgent=yes"] if ssh_info[:forward_agent] + command_options.concat(opts[:extra_args]) if opts[:extra_args] + + # Solaris/OpenSolaris/Illumos uses SunSSH which doesn't support the + # IdentitiesOnly option. Also, we don't enable it in plain mode so + # that SSH properly searches our identities and tries to do it itself. + if !Platform.solaris? && !plain_mode + command_options += ["-o", "IdentitiesOnly=yes"] + end + + # If we're not in plain mode, attach the private key path. + command_options += ["-i", options[:private_key_path]] if !plain_mode + + if ssh_info[:forward_x11] + # Both are required so that no warnings are shown regarding X11 + command_options += [ + "-o", "ForwardX11=yes", + "-o", "ForwardX11Trusted=yes"] + end + + # Build up the host string for connecting + host_string = options[:host] + host_string = "#{options[:username]}@#{host_string}" if !plain_mode + command_options << host_string + + # Invoke SSH with all our options + @logger.info("Invoking SSH: #{command_options.inspect}") + safe_exec("ssh", *command_options) + end + end + end +end diff --git a/test/unit/vagrant/ssh_test.rb b/test/unit/vagrant/ssh_test.rb deleted file mode 100644 index 268222d95..000000000 --- a/test/unit/vagrant/ssh_test.rb +++ /dev/null @@ -1,30 +0,0 @@ -require File.expand_path("../../base", __FILE__) - -describe Vagrant::SSH do - context "check_key_permissions" do - let(:key_path) do - # We create a tempfile to guarantee some level of uniqueness - # then explicitly close/unlink but save the path so we can re-use - temp = Tempfile.new("vagrant") - result = Pathname.new(temp.path) - temp.close - temp.unlink - - result - end - - let(:ssh_instance) { Vagrant::SSH.new(double) } - - it "should not raise an exception if we set a keyfile permission correctly" do - # Write some stuff to our key file and chmod it to some - # incorrect permissions. - key_path.open("w") { |f| f.write("hello!") } - key_path.chmod(0644) - - # This should work! - expect { ssh_instance.check_key_permissions(key_path) }. - to_not raise_error - end - end -end - diff --git a/test/unit/vagrant/util/ssh_test.rb b/test/unit/vagrant/util/ssh_test.rb new file mode 100644 index 000000000..30bfe0d5e --- /dev/null +++ b/test/unit/vagrant/util/ssh_test.rb @@ -0,0 +1,30 @@ +require File.expand_path("../../../base", __FILE__) + +require "vagrant/util/platform" +require "vagrant/util/ssh" + +describe Vagrant::Util::SSH do + include_context "unit" + + describe "checking key permissions" do + let(:key_path) { temporary_file } + + it "should do nothing on Windows" do + Vagrant::Util::Platform.stub(:windows?).and_return(true) + + key_path.chmod(0700) + + # Get the mode now and verify that it is untouched afterwards + mode = key_path.stat.mode + described_class.check_key_permissions(key_path) + key_path.stat.mode.should == mode + end + + it "should fix the permissions" do + key_path.chmod(0644) + + described_class.check_key_permissions(key_path) + key_path.stat.mode.should == 0100600 + end + end +end