diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index d0df2f08f..175789256 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -863,6 +863,10 @@ module Vagrant error_key(:ssh_key_type_not_supported) end + class SSHKeyTypeNotSupportedByServer < VagrantError + error_key(:ssh_key_type_not_supported_by_server) + end + class SSHNoExitStatus < VagrantError error_key(:ssh_no_exit_status) end diff --git a/plugins/communicators/ssh/communicator.rb b/plugins/communicators/ssh/communicator.rb index 1e8060153..7e0e9af37 100644 --- a/plugins/communicators/ssh/communicator.rb +++ b/plugins/communicators/ssh/communicator.rb @@ -113,6 +113,8 @@ module VagrantPlugins raise rescue Vagrant::Errors::SSHKeyTypeNotSupported raise + rescue Vagrant::Errors::SSHKeyTypeNotSupportedByServer + raise rescue Vagrant::Errors::SSHKeyBadOwner raise rescue Vagrant::Errors::SSHKeyBadPermissions @@ -188,25 +190,56 @@ module VagrantPlugins @machine.guest.capability?(:remove_public_key) raise Vagrant::Errors::SSHInsertKeyUnsupported if !cap - # Check for supported key type - key_type = catch(:key_type) do - begin - Vagrant::Util::Keypair::PREFER_KEY_TYPES.each do |type_name, type| - throw :key_type, type if supports_key_type?(type_name) + key_type = machine_config_ssh.key_type + + begin + # If the key type is set to `:auto` check for supported type. Otherwise + # ensure that the key type is supported by the guest + if key_type == :auto + key_type = catch(:key_type) do + begin + Vagrant::Util::Keypair::PREFER_KEY_TYPES.each do |type_name, type| + throw :key_type, type if supports_key_type?(type_name) + end + nil + rescue => err + @logger.warn("Failed to check key types server supports: #{err}") + nil + end + end + + @logger.debug("Detected key type for new private key: #{key_type}") + + # If no key type was discovered, default to rsa + if key_type.nil? + @logger.debug("Failed to detect supported key type in: #{supported_key_types.join(", ")}") + available_types = supported_key_types.map { |t| + next if !Vagrant::Util::Keypair::PREFER_KEY_TYPES.key?(t) + "#{t} (#{Vagrant::Util::Keypair::PREFER_KEY_TYPES[t]})" + }.compact.join(", ") + + raise Vagrant::Errors::SSHKeyTypeNotSupportedByServer, + requested_key_type: ":auto", + available_key_types: available_types + end + else + type_name = Vagrant::Util::Keypair::PREFER_KEY_TYPES.key(key_type) + if !supports_key_type?(type_name) + available_types = supported_key_types.map { |t| + next if !Vagrant::Util::Keypair::PREFER_KEY_TYPES.key?(t) + "#{t} (#{Vagrant::Util::Keypair::PREFER_KEY_TYPES[t]})" + }.compact.join(", ") + raise Vagrant::Errors::SSHKeyTypeNotSupportedByServer, + requested_key_type: "#{type_name} (#{key_type})", + available_key_types: available_types end - nil - rescue => err - @logger.warn("Failed to check key types server supports: #{err}") - nil end - end - - @logger.debug("Detected key type for new private key: #{key_type}") - - # If no key type was discovered, default to rsa - if key_type.nil? - @logger.debug("Failed to detect supported key type, defaulting to rsa") - key_type = :rsa + rescue ServerDataError + @logger.warn("failed to load server data for key type check") + if key_type.nil? || key_type == :auto + @logger.warn("defaulting key type to :rsa due to failed server data loading") + key_type = :rsa + end end @logger.info("Creating new ssh keypair (type: #{key_type.inspect})") @@ -788,6 +821,8 @@ module VagrantPlugins protected + class ServerDataError < StandardError; end + # Check if server supports given key type # # @param [String, Symbol] type Key type @@ -798,21 +833,31 @@ module VagrantPlugins if @connection.nil? raise Vagrant::Errors::SSHNotReady end + + supported_key_types.include?(type.to_s) + end + + def supported_key_types + if @connection.nil? + raise Vagrant::Errors::SSHNotReady + end + server_data = @connection. transport&. algorithms&. instance_variable_get(:@server_data) if server_data.nil? @logger.warn("No server data available for key type support check") - return false + raise ServerDataError, "no data available" end if !server_data.is_a?(Hash) @logger.warn("Server data is not expected type (expecting Hash, got #{server_data.class})") - return false + raise ServerDataError, "unexpected type encountered (expecting Hash, got #{server_data.class})" end - @logger.debug("server data used for host key support check: #{server_data.inspect}") - server_data[:host_key].include?(type.to_s) + @logger.debug("server supported key type list: #{server_data[:host_key]}") + + server_data[:host_key] end end end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 58042eb32..2bb540df4 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1621,6 +1621,14 @@ en: sometimes keys in your ssh-agent can interfere with this as well, so verify the keys are valid there in addition to standard file paths. + ssh_key_type_not_supported_by_server: |- + The private key you are attempting to generate is not supported by + the guest SSH server. Please use one of the available key types defined + below that is supported by the guest SSH server. + + Requested: %{requested_key_type} + Available: %{available_key_types} + ssh_not_ready: |- The provider for this Vagrant-managed machine is reporting that it is not yet ready for SSH. Depending on your provider this can carry diff --git a/test/unit/plugins/communicators/ssh/communicator_test.rb b/test/unit/plugins/communicators/ssh/communicator_test.rb index 7cc161c5d..f77006264 100644 --- a/test/unit/plugins/communicators/ssh/communicator_test.rb +++ b/test/unit/plugins/communicators/ssh/communicator_test.rb @@ -13,6 +13,7 @@ describe VagrantPlugins::CommunicatorSSH::Communicator do # SSH configuration information mock let(:ssh) do double("ssh", + key_type: :auto, timeout: 1, host: nil, port: 5986, @@ -264,46 +265,48 @@ describe VagrantPlugins::CommunicatorSSH::Communicator do allow(guest).to receive(:capability).with(:remove_public_key) allow(connection).to receive(:transport).and_return(transport) allow(algorithms).to receive(:instance_variable_get).with(:@server_data).and_return(server_data) + allow(communicator).to receive(:supported_key_types).and_raise(described_class.const_get(:ServerDataError)) 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]) + communicator.ready? end it "should call the insert_public_key guest capability" do expect(guest).to receive(:capability).with(:insert_public_key, openssh) + communicator.ready? end it "should write the new private key" do expect(private_key_file).to receive(:write).with(new_private_key) + communicator.ready? end it "should call the set_ssh_key_permissions host capability" do expect(host).to receive(:capability?).with(:set_ssh_key_permissions).and_return(true) expect(host).to receive(:capability).with(:set_ssh_key_permissions, private_key_file) + communicator.ready? end it "should remove the default public key" do expect(guest).to receive(:capability).with(:remove_public_key, any_args) + communicator.ready? end context "with server algorithm support data" do - context "when no key type matches are found" do - it "should default to rsa type" do - expect(Vagrant::Util::Keypair).to receive(:create). - with(type: :rsa).and_call_original - end + before do + allow(communicator).to receive(:supported_key_types).and_call_original end context "when rsa is the only match" do - let(:valid_key_types) { ["ssh-edsca", "ssh-rsa"] } + let(:valid_key_types) { ["ssh-ecdsa", "ssh-rsa"] } it "should use rsa type" do expect(Vagrant::Util::Keypair).to receive(:create). with(type: :rsa).and_call_original + communicator.ready? end end @@ -313,27 +316,69 @@ describe VagrantPlugins::CommunicatorSSH::Communicator do it "should use ed25519 type" do expect(Vagrant::Util::Keypair).to receive(:create). with(type: :ed25519).and_call_original + communicator.ready? end end context "when ed25519 is the only match" do - let(:valid_key_types) { ["ssh-edsca", "ssh-ed25519"] } + let(:valid_key_types) { ["ssh-ecdsa", "ssh-ed25519"] } it "should use ed25519 type" do expect(Vagrant::Util::Keypair).to receive(:create). with(type: :ed25519).and_call_original + communicator.ready? + end + end + + context "with key_type set as :auto in configuration" do + let(:valid_key_types) { ["ssh-ed25519", "ssh-rsa"] } + before { allow(ssh).to receive(:key_type).and_return(:auto) } + + it "should use the preferred ed25519 key type" do + expect(Vagrant::Util::Keypair).to receive(:create). + with(type: :ed25519).and_call_original + communicator.ready? + end + + context "when no supported key type is detected" do + let(:valid_key_types) { ["fake-type", "other-fake-type"] } + + it "should raise an error" do + expect { communicator.ready? }.to raise_error(Vagrant::Errors::SSHKeyTypeNotSupportedByServer) + end + end + end + + context "with key_type set as :ecdsa521 in configuration" do + let(:valid_key_types) { ["ssh-ed25519", "ssh-rsa", "ecdsa-sha2-nistp521", "ecdsa-sha2-nistp256"] } + before { allow(ssh).to receive(:key_type).and_return(:ecdsa521) } + + it "should use the requested key type" do + expect(Vagrant::Util::Keypair).to receive(:create). + with(type: :ecdsa521).and_call_original + communicator.ready? + end + + context "when requested key type is not supported" do + let(:valid_key_types) { ["ssh-ed25519", "ssh-rsa", "ecdsa-sha2-nistp256"] } + + it "should raise an error" do + expect { communicator.ready? }.to raise_error(Vagrant::Errors::SSHKeyTypeNotSupportedByServer) + end end end end context "when an error is encountered getting server data" do before do + expect(communicator).to receive(:supported_key_types).and_call_original expect(connection).to receive(:transport).and_raise(StandardError) end it "should default to rsa key" do expect(Vagrant::Util::Keypair).to receive(:create). with(type: :rsa).and_call_original + communicator.ready? end end end