From 7a97f0a53e7a4c8c4d78211cb5e4289470f1ad7f Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 8 Jun 2017 16:03:25 -0700 Subject: [PATCH] (#6640) Use default cipher list for ssh communicator Prior to this commit, the ssh communicator would use the default cipher list in Net::SSH to negociate which ciphers it should use between hosts. Due to a bug in Net::SSH and the position of the `none` cipher in its default cipher list, if a host supported the none cipher, but also only supported other ciphers that came after none in the default list, it would accept none and attempt to use that cipher instead of the other supported ciphers. This commit fixes that behavior by copying the default cipher list from Net::SSH and placing none last in the list so that other ciphers can be used in the negotiation before attempting to use the unsecure none cipher. --- plugins/communicators/ssh/communicator.rb | 9 +++++++++ .../plugins/communicators/ssh/communicator_test.rb | 14 ++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/plugins/communicators/ssh/communicator.rb b/plugins/communicators/ssh/communicator.rb index be48252a8..831dcc779 100644 --- a/plugins/communicators/ssh/communicator.rb +++ b/plugins/communicators/ssh/communicator.rb @@ -348,6 +348,14 @@ module VagrantPlugins auth_methods << "publickey" if ssh_info[:private_key_path] auth_methods << "password" if ssh_info[:password] + # yanked directly from ruby's Net::SSH, but with `none` last + # TODO: Remove this once Vagrant has updated its dependency on Net:SSH + # to be > 4.1.0, which should include this fix. + cipher_array = Net::SSH::Transport::Algorithms::ALGORITHMS[:encryption].dup + if cipher_array.delete("none") + cipher_array.push("none") + end + # Build the options we'll use to initiate the connection via Net::SSH common_connect_opts = { auth_methods: auth_methods, @@ -361,6 +369,7 @@ module VagrantPlugins timeout: 15, user_known_hosts_file: [], verbose: :debug, + encryption: cipher_array, } # Connect to SSH, giving it a few tries diff --git a/test/unit/plugins/communicators/ssh/communicator_test.rb b/test/unit/plugins/communicators/ssh/communicator_test.rb index a0998c6dc..0a2f67195 100644 --- a/test/unit/plugins/communicators/ssh/communicator_test.rb +++ b/test/unit/plugins/communicators/ssh/communicator_test.rb @@ -396,6 +396,20 @@ describe VagrantPlugins::CommunicatorSSH::Communicator do ).and_return(true) communicator.send(:connect) end + + it "includes the default cipher array for encryption" do + cipher_array = %w(aes128-cbc 3des-cbc blowfish-cbc cast128-cbc + aes192-cbc aes256-cbc rijndael-cbc@lysator.liu.se + idea-cbc arcfour128 arcfour256 arcfour + aes128-ctr aes192-ctr aes256-ctr + cast128-ctr blowfish-ctr 3des-ctr none) + expect(Net::SSH).to receive(:start).with( + nil, nil, hash_including( + encryption: cipher_array + ) + ).and_return(true) + communicator.send(:connect) + end end context "with keys_only disabled and paranoid enabled" do