From bcf61d001bcaee59adb41551969df2967ac4efc3 Mon Sep 17 00:00:00 2001 From: Timur Alperovich Date: Mon, 23 Nov 2015 14:58:32 -0800 Subject: [PATCH 1/3] Only consider the VM interfaces in the IPv6 fixup. Vagrant should only consider the host-only interfaces used by the virtual machine in the IPv6 fixup code. There may be other interfaces present on the system with IPv6 addresses that for various reasons would fail the routing check (for example, an interface with no machines attached). The patch changes the behavior to not scan all of the host-only interfaces and adds a unit test for the behavior (that the correct IP is validated). Lastly, there is a small fix here that may not be an issue for most people where the IPv6 prefix was asummed to be a multiple of 16 for the purposes of constructing the UDP probe datagram. This assumption has been removed. Fixes #6586 --- .../virtualbox/action/network_fix_ipv6.rb | 21 +-- .../action/network_fix_ipv6_test.rb | 134 +++++++++++++++++- 2 files changed, 144 insertions(+), 11 deletions(-) diff --git a/plugins/providers/virtualbox/action/network_fix_ipv6.rb b/plugins/providers/virtualbox/action/network_fix_ipv6.rb index b8b12be58..dcfa2632e 100644 --- a/plugins/providers/virtualbox/action/network_fix_ipv6.rb +++ b/plugins/providers/virtualbox/action/network_fix_ipv6.rb @@ -41,21 +41,22 @@ module VagrantPlugins # If we have no IPv6, forget it return if !has_v6 - # We do, so fix them if we must - env[:machine].provider.driver.read_host_only_interfaces.each do |interface| - # Ignore interfaces without an IPv6 address - next if interface[:ipv6] == "" - - # Make the test IP. This is just the highest value IP - ip = IPAddr.new(interface[:ipv6]) - ip |= IPAddr.new(":#{":FFFF" * (interface[:ipv6_prefix].to_i / 16)}") - + ifaces = env[:machine].provider.driver.read_network_interfaces + ifaces.select! { |id, iface| iface[:type] == :hostonly } + iface_names = ifaces.values.map { |iface| iface[:hostonly] } + networks = env[:machine].provider.driver.read_host_only_interfaces + networks.select! { |network| iface_names.include?(network[:name]) } + networks.each do |network| + next if network[:ipv6] == "" + next if network[:status] != 'Up' + ip = IPAddr.new(network[:ipv6]) | + ('1' * (128 - network[:ipv6_prefix].to_i)).to_i(2) @logger.info("testing IPv6: #{ip}") begin UDPSocket.new(Socket::AF_INET6).connect(ip.to_s, 80) rescue Errno::EHOSTUNREACH @logger.info("IPv6 host unreachable. Fixing: #{ip}") - env[:machine].provider.driver.reconfig_host_only(interface) + env[:machine].provider.driver.reconfig_host_only(network) end end end diff --git a/test/unit/plugins/providers/virtualbox/action/network_fix_ipv6_test.rb b/test/unit/plugins/providers/virtualbox/action/network_fix_ipv6_test.rb index d4b985a63..2f99b954b 100644 --- a/test/unit/plugins/providers/virtualbox/action/network_fix_ipv6_test.rb +++ b/test/unit/plugins/providers/virtualbox/action/network_fix_ipv6_test.rb @@ -1,4 +1,5 @@ require_relative "../base" +require 'socket' describe VagrantPlugins::ProviderVirtualBox::Action::NetworkFixIPv6 do include_context "unit" @@ -11,11 +12,14 @@ describe VagrantPlugins::ProviderVirtualBox::Action::NetworkFixIPv6 do end let(:machine) do - iso_env.machine(iso_env.machine_names[0], :dummy) + iso_env.machine(iso_env.machine_names[0], :dummy).tap do |m| + m.provider.stub(driver: driver) + end end let(:env) {{ machine: machine }} let(:app) { lambda { |*args| }} + let(:driver) { double("driver") } subject { described_class.new(app, env) } @@ -30,4 +34,132 @@ describe VagrantPlugins::ProviderVirtualBox::Action::NetworkFixIPv6 do .and_return(private_network: { ip: "" }) expect { subject.call(env) }.to_not raise_error end + + context "with IPv6 interfaces" do + let(:socket) { double("socket") } + + before do + # This address is only used to trigger the fixup code. It doesn't matter + # what it is. + allow(machine.config.vm).to receive(:networks) + .and_return(private_network: { ip: 'fe:80::' }) + allow(UDPSocket).to receive(:new).with(Socket::AF_INET6) + .and_return(socket) + socket.stub(:connect) + end + + it "only checks the interfaces associated with the VM" do + all_networks = [{name: "vboxnet0", + ipv6: "dead:beef::", + ipv6_prefix: 64, + status: 'Up' + }, + {name: "vboxnet1", + ipv6: "badd:badd::", + ipv6_prefix: 64, + status: 'Up' + } + ] + ifaces = { 1 => {type: :hostonly, hostonly: "vboxnet0"} + } + allow(machine.provider.driver).to receive(:read_network_interfaces) + .and_return(ifaces) + allow(machine.provider.driver).to receive(:read_host_only_interfaces) + .and_return(all_networks) + subject.call(env) + expect(socket).to have_received(:connect) + .with(all_networks[0][:ipv6] + (['ffff']*4).join(':'), 80) + end + + it "correctly uses the netmask to figure out the probe address" do + all_networks = [{name: "vboxnet0", + ipv6: "dead:beef::", + ipv6_prefix: 113, + status: 'Up' + } + ] + ifaces = { 1 => {type: :hostonly, hostonly: "vboxnet0"} + } + allow(machine.provider.driver).to receive(:read_network_interfaces) + .and_return(ifaces) + allow(machine.provider.driver).to receive(:read_host_only_interfaces) + .and_return(all_networks) + subject.call(env) + expect(socket).to have_received(:connect) + .with(all_networks[0][:ipv6] + '7fff', 80) + end + + it "should ignore interfaces that are down" do + all_networks = [{name: "vboxnet0", + ipv6: "dead:beef::", + ipv6_prefix: 64, + status: 'Down' + } + ] + ifaces = { 1 => {type: :hostonly, hostonly: "vboxnet0"} + } + allow(machine.provider.driver).to receive(:read_network_interfaces) + .and_return(ifaces) + allow(machine.provider.driver).to receive(:read_host_only_interfaces) + .and_return(all_networks) + subject.call(env) + expect(socket).to_not have_received(:connect) + end + + it "should ignore interfaces without an IPv6 address" do + all_networks = [{name: "vboxnet0", + ipv6: "", + ipv6_prefix: 0, + status: 'Up' + } + ] + ifaces = { 1 => {type: :hostonly, hostonly: "vboxnet0"} + } + allow(machine.provider.driver).to receive(:read_network_interfaces) + .and_return(ifaces) + allow(machine.provider.driver).to receive(:read_host_only_interfaces) + .and_return(all_networks) + subject.call(env) + expect(socket).to_not have_received(:connect) + end + + it "should ignore nat interfaces" do + all_networks = [{name: "vboxnet0", + ipv6: "", + ipv6_prefix: 0, + status: 'Up' + } + ] + ifaces = { 1 => {type: :nat} + } + allow(machine.provider.driver).to receive(:read_network_interfaces) + .and_return(ifaces) + allow(machine.provider.driver).to receive(:read_host_only_interfaces) + .and_return(all_networks) + subject.call(env) + expect(socket).to_not have_received(:connect) + end + + it "should reconfigure an interface if unreachable" do + all_networks = [{name: "vboxnet0", + ipv6: "dead:beef::", + ipv6_prefix: 64, + status: 'Up' + } + ] + ifaces = { 1 => {type: :hostonly, hostonly: "vboxnet0"} + } + allow(machine.provider.driver).to receive(:read_network_interfaces) + .and_return(ifaces) + allow(machine.provider.driver).to receive(:read_host_only_interfaces) + .and_return(all_networks) + allow(socket).to receive(:connect) + .with(all_networks[0][:ipv6] + (['ffff']*4).join(':'), 80) + .and_raise Errno::EHOSTUNREACH + allow(machine.provider.driver).to receive(:reconfig_host_only) + subject.call(env) + expect(machine.provider.driver).to have_received(:reconfig_host_only) + .with(all_networks[0]) + end + end end From 9ba8cfcd507da14d5d2a975be53f5452a3676ab6 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Tue, 8 Dec 2015 10:52:54 -0500 Subject: [PATCH 2/3] Separate logic into other functions for readability --- .../virtualbox/action/network_fix_ipv6.rb | 35 +++++++++++++------ 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/plugins/providers/virtualbox/action/network_fix_ipv6.rb b/plugins/providers/virtualbox/action/network_fix_ipv6.rb index dcfa2632e..853890441 100644 --- a/plugins/providers/virtualbox/action/network_fix_ipv6.rb +++ b/plugins/providers/virtualbox/action/network_fix_ipv6.rb @@ -3,6 +3,7 @@ require "socket" require "log4r" +require "vagrant/util/presence" require "vagrant/util/scoped_hash_override" module VagrantPlugins @@ -12,6 +13,7 @@ module VagrantPlugins # a VM with an IPv6 host-only network will someties lose the # route to that machine. class NetworkFixIPv6 + include Vagrant::Util::Presence include Vagrant::Util::ScopedHashOverride def initialize(app, env) @@ -41,17 +43,15 @@ module VagrantPlugins # If we have no IPv6, forget it return if !has_v6 - ifaces = env[:machine].provider.driver.read_network_interfaces - ifaces.select! { |id, iface| iface[:type] == :hostonly } - iface_names = ifaces.values.map { |iface| iface[:hostonly] } - networks = env[:machine].provider.driver.read_host_only_interfaces - networks.select! { |network| iface_names.include?(network[:name]) } - networks.each do |network| - next if network[:ipv6] == "" - next if network[:status] != 'Up' - ip = IPAddr.new(network[:ipv6]) | - ('1' * (128 - network[:ipv6_prefix].to_i)).to_i(2) + networks(env).each do |network| + next if !present?(network[:ipv6]) + next if network[:status] != "Up" + + ip = IPAddr.new(network[:ipv6]) + ip |= ("1" * (128 - network[:ipv6_prefix].to_i)).to_i(2) + @logger.info("testing IPv6: #{ip}") + begin UDPSocket.new(Socket::AF_INET6).connect(ip.to_s, 80) rescue Errno::EHOSTUNREACH @@ -60,6 +60,21 @@ module VagrantPlugins end end end + + # The list of interface names for host-only adapters. + # @return [Array] + def host_only_interface_names(env) + env[:machine].provider.driver.read_network_interfaces + .map { |_, i| i[:hostonly] if i[:type] == :hostonly }.compact + end + + # The list of networks that are tied to a host-only adapter. + # @return [Array] + def networks(env) + iface_names = self.host_only_interface_names(env) + env[:machine].provider.driver.read_host_only_interfaces + .select { |network| iface_names.include?(network[:name]) } + end end end end From aca1e041f6d12f6e157474d4e56f2145d7565a2b Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Tue, 8 Dec 2015 11:05:13 -0500 Subject: [PATCH 3/3] Rename network to interface --- .../virtualbox/action/network_fix_ipv6.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/plugins/providers/virtualbox/action/network_fix_ipv6.rb b/plugins/providers/virtualbox/action/network_fix_ipv6.rb index 853890441..7aec8d110 100644 --- a/plugins/providers/virtualbox/action/network_fix_ipv6.rb +++ b/plugins/providers/virtualbox/action/network_fix_ipv6.rb @@ -43,12 +43,12 @@ module VagrantPlugins # If we have no IPv6, forget it return if !has_v6 - networks(env).each do |network| - next if !present?(network[:ipv6]) - next if network[:status] != "Up" + host_only_interfaces(env).each do |interface| + next if !present?(interface[:ipv6]) + next if interface[:status] != "Up" - ip = IPAddr.new(network[:ipv6]) - ip |= ("1" * (128 - network[:ipv6_prefix].to_i)).to_i(2) + ip = IPAddr.new(interface[:ipv6]) + ip |= ("1" * (128 - interface[:ipv6_prefix].to_i)).to_i(2) @logger.info("testing IPv6: #{ip}") @@ -56,7 +56,7 @@ module VagrantPlugins UDPSocket.new(Socket::AF_INET6).connect(ip.to_s, 80) rescue Errno::EHOSTUNREACH @logger.info("IPv6 host unreachable. Fixing: #{ip}") - env[:machine].provider.driver.reconfig_host_only(network) + env[:machine].provider.driver.reconfig_host_only(interface) end end end @@ -68,12 +68,12 @@ module VagrantPlugins .map { |_, i| i[:hostonly] if i[:type] == :hostonly }.compact end - # The list of networks that are tied to a host-only adapter. + # The list of host_only_interfaces that are tied to a host-only adapter. # @return [Array] - def networks(env) + def host_only_interfaces(env) iface_names = self.host_only_interface_names(env) env[:machine].provider.driver.read_host_only_interfaces - .select { |network| iface_names.include?(network[:name]) } + .select { |interface| iface_names.include?(interface[:name]) } end end end