From 36a086a4d1172dd7e8e0fe57c7aed792427ed6e6 Mon Sep 17 00:00:00 2001 From: sophia Date: Fri, 5 Jun 2020 14:19:47 -0500 Subject: [PATCH 1/2] Validate netmask value from VirtualBox --- lib/vagrant/errors.rb | 8 ++++ lib/vagrant/util/validate_netmask.rb | 14 +++++++ .../virtualbox/driver/version_5_0.rb | 19 +++++++++- templates/locales/en.yml | 4 ++ .../vagrant/util/validate_netmask_test.rb | 37 +++++++++++++++++++ 5 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 lib/vagrant/util/validate_netmask.rb create mode 100644 test/unit/vagrant/util/validate_netmask_test.rb diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 05e821bba..21451037d 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -428,6 +428,10 @@ module Vagrant error_key(:host_explicit_not_detected) end + class InvalidNetMask < VagrantError + error_key(:invalid_netmask) + end + class LinuxMountFailed < VagrantError error_key(:linux_mount_failed) end @@ -932,6 +936,10 @@ module Vagrant error_key(:virtualbox_invalid_version) end + class VirtualBoxNetworkSetting < VagrantError + error_key(:virtualbox_invalid_network_setting) + end + class VirtualBoxNoRoomForHighLevelNetwork < VagrantError error_key(:virtualbox_no_room_for_high_level_network) end diff --git a/lib/vagrant/util/validate_netmask.rb b/lib/vagrant/util/validate_netmask.rb new file mode 100644 index 000000000..25195e383 --- /dev/null +++ b/lib/vagrant/util/validate_netmask.rb @@ -0,0 +1,14 @@ +module Vagrant + module Util + class ValidateNetmask + + MASK_REGEX = /^(((0|128|192|224|240|248|252|254)\.0\.0\.0)|(255\.(0|128|192|224|240|248|252|254)\.0\.0)|(255\.255\.(0|128|192|224|240|248|252|254)\.0)|(255\.255\.255\.(0|128|192|224|240|248|252|254|255)))$/.freeze + + def self.validate(mask) + if ! MASK_REGEX.match(mask) + raise Vagrant::Errors::InvalidNetMask, mask: mask + end + end + end + end +end diff --git a/plugins/providers/virtualbox/driver/version_5_0.rb b/plugins/providers/virtualbox/driver/version_5_0.rb index b08576a8f..8798a4990 100644 --- a/plugins/providers/virtualbox/driver/version_5_0.rb +++ b/plugins/providers/virtualbox/driver/version_5_0.rb @@ -1,6 +1,7 @@ require 'log4r' require "vagrant/util/platform" +require "vagrant/util/validate_netmask" require File.expand_path("../base", __FILE__) @@ -530,7 +531,14 @@ module VagrantPlugins elsif line =~ /^IPAddress:\s+(.+?)$/ info[:ip] = $1.to_s elsif line =~ /^NetworkMask:\s+(.+?)$/ - info[:netmask] = $1.to_s + mask = $1.to_s + begin + Vagrant::Util::ValidateNetmask.validate(mask) + info[:netmask] = mask + rescue Vagrant::Errors::InvalidNetMask + raise Vagrant::Errors::VirtualBoxNetworkSetting, + setting_type: 'mask', setting_value: mask, interface_type: "bridged", interface_name: info[:name] || "unknown" + end elsif line =~ /^Status:\s+(.+?)$/ info[:status] = $1.to_s end @@ -619,7 +627,14 @@ module VagrantPlugins elsif line =~ /^IPAddress:\s+(.+?)$/ info[:ip] = $1.to_s elsif line =~ /^NetworkMask:\s+(.+?)$/ - info[:netmask] = $1.to_s + mask = $1.to_s + begin + Vagrant::Util::ValidateNetmask.validate(mask) + info[:netmask] = mask + rescue Vagrant::Errors::InvalidNetMask + raise Vagrant::Errors::VirtualBoxNetworkSetting, + setting_type: 'mask', setting_value: mask, interface_type: "bridged", interface_name: info[:name] || "unknown" + end elsif line =~ /^IPV6Address:\s+(.+?)$/ info[:ipv6] = $1.to_s.strip elsif line =~ /^IPV6NetworkMaskPrefixLength:\s+(.+?)$/ diff --git a/templates/locales/en.yml b/templates/locales/en.yml index a6f8feea9..1900be9c0 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -920,6 +920,8 @@ en: as mounting shared folders and configuring networks. Please add the ability to detect this guest operating system to Vagrant by creating a plugin or reporting a bug. + invalid_netmask: |- + Invalid mask '%{mask}' home_dir_later_version: |- It appears that a newer version of Vagrant was run on this machine at some point. The current version of Vagrant is unable to read @@ -1682,6 +1684,8 @@ en: A Vagrant update may also be available that adds support for the version you specified. Please check www.vagrantup.com/downloads.html to download the latest version. + virtualbox_invalid_network_setting: |- + Invalid %{setting_type} '%{setting_value}' for %{interface_type} interface '%{interface_name}' virtualbox_kernel_module_not_loaded: |- VirtualBox is complaining that the kernel module is not loaded. Please run `VBoxManage --version` or open the VirtualBox GUI to see the error diff --git a/test/unit/vagrant/util/validate_netmask_test.rb b/test/unit/vagrant/util/validate_netmask_test.rb new file mode 100644 index 000000000..5c402420c --- /dev/null +++ b/test/unit/vagrant/util/validate_netmask_test.rb @@ -0,0 +1,37 @@ +require File.expand_path("../../../base", __FILE__) + +require "vagrant/util/validate_netmask" + +describe Vagrant::Util::ValidateNetmask do + include_context "unit" + subject { described_class } + + describe ".validate" do + it "validates valid netmask" do + masks = [ + "0.0.0.0", + "255.255.255.255", + "255.255.255.0", + "255.255.0.0", + "255.0.0.0", + "255.255.192.0", + "255.252.0.0", + "192.0.0.0" + ] + masks.each do |mask| + subject.validate(mask) + end + end + + it "raises an error for invalid netmask" do + masks = [ + "255.255.192.255", + "1.2.3.4", + "192.0.0.0.0" + ] + masks.each do |mask| + expect { subject.validate(mask) }.to raise_error(Vagrant::Errors::InvalidNetMask) + end + end + end +end From e3e573c133cb21e40f3b020b08e076c4a9b045ef Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 9 Jun 2020 10:22:19 -0500 Subject: [PATCH 2/2] Rescue if an invalid netmask is provided --- lib/vagrant/errors.rb | 8 ---- lib/vagrant/util/network_ip.rb | 12 +++++- lib/vagrant/util/validate_netmask.rb | 14 ------- .../virtualbox/driver/version_5_0.rb | 19 +--------- templates/locales/en.yml | 4 -- test/unit/vagrant/util/network_ip_test.rb | 30 +++++++++++---- .../vagrant/util/validate_netmask_test.rb | 37 ------------------- 7 files changed, 35 insertions(+), 89 deletions(-) delete mode 100644 lib/vagrant/util/validate_netmask.rb delete mode 100644 test/unit/vagrant/util/validate_netmask_test.rb diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 21451037d..05e821bba 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -428,10 +428,6 @@ module Vagrant error_key(:host_explicit_not_detected) end - class InvalidNetMask < VagrantError - error_key(:invalid_netmask) - end - class LinuxMountFailed < VagrantError error_key(:linux_mount_failed) end @@ -936,10 +932,6 @@ module Vagrant error_key(:virtualbox_invalid_version) end - class VirtualBoxNetworkSetting < VagrantError - error_key(:virtualbox_invalid_network_setting) - end - class VirtualBoxNoRoomForHighLevelNetwork < VagrantError error_key(:virtualbox_no_room_for_high_level_network) end diff --git a/lib/vagrant/util/network_ip.rb b/lib/vagrant/util/network_ip.rb index 1a4653656..8930bc36a 100644 --- a/lib/vagrant/util/network_ip.rb +++ b/lib/vagrant/util/network_ip.rb @@ -3,11 +3,21 @@ require "ipaddr" module Vagrant module Util module NetworkIP + + DEFAULT_MASK = "255.255.255.0".freeze + + LOGGER = Log4r::Logger.new("vagrant::util::NetworkIP") + # Returns the network address of the given IP and subnet. # # @return [String] def network_address(ip, subnet) - IPAddr.new(ip).mask(subnet).to_s + begin + IPAddr.new(ip).mask(subnet).to_s + rescue IPAddr::InvalidPrefixError + LOGGER.warn("Provided mask '#{subnet}' is invalid. Falling back to using mask '#{DEFAULT_MASK}'") + IPAddr.new(ip).mask(DEFAULT_MASK).to_s + end end end end diff --git a/lib/vagrant/util/validate_netmask.rb b/lib/vagrant/util/validate_netmask.rb deleted file mode 100644 index 25195e383..000000000 --- a/lib/vagrant/util/validate_netmask.rb +++ /dev/null @@ -1,14 +0,0 @@ -module Vagrant - module Util - class ValidateNetmask - - MASK_REGEX = /^(((0|128|192|224|240|248|252|254)\.0\.0\.0)|(255\.(0|128|192|224|240|248|252|254)\.0\.0)|(255\.255\.(0|128|192|224|240|248|252|254)\.0)|(255\.255\.255\.(0|128|192|224|240|248|252|254|255)))$/.freeze - - def self.validate(mask) - if ! MASK_REGEX.match(mask) - raise Vagrant::Errors::InvalidNetMask, mask: mask - end - end - end - end -end diff --git a/plugins/providers/virtualbox/driver/version_5_0.rb b/plugins/providers/virtualbox/driver/version_5_0.rb index 8798a4990..b08576a8f 100644 --- a/plugins/providers/virtualbox/driver/version_5_0.rb +++ b/plugins/providers/virtualbox/driver/version_5_0.rb @@ -1,7 +1,6 @@ require 'log4r' require "vagrant/util/platform" -require "vagrant/util/validate_netmask" require File.expand_path("../base", __FILE__) @@ -531,14 +530,7 @@ module VagrantPlugins elsif line =~ /^IPAddress:\s+(.+?)$/ info[:ip] = $1.to_s elsif line =~ /^NetworkMask:\s+(.+?)$/ - mask = $1.to_s - begin - Vagrant::Util::ValidateNetmask.validate(mask) - info[:netmask] = mask - rescue Vagrant::Errors::InvalidNetMask - raise Vagrant::Errors::VirtualBoxNetworkSetting, - setting_type: 'mask', setting_value: mask, interface_type: "bridged", interface_name: info[:name] || "unknown" - end + info[:netmask] = $1.to_s elsif line =~ /^Status:\s+(.+?)$/ info[:status] = $1.to_s end @@ -627,14 +619,7 @@ module VagrantPlugins elsif line =~ /^IPAddress:\s+(.+?)$/ info[:ip] = $1.to_s elsif line =~ /^NetworkMask:\s+(.+?)$/ - mask = $1.to_s - begin - Vagrant::Util::ValidateNetmask.validate(mask) - info[:netmask] = mask - rescue Vagrant::Errors::InvalidNetMask - raise Vagrant::Errors::VirtualBoxNetworkSetting, - setting_type: 'mask', setting_value: mask, interface_type: "bridged", interface_name: info[:name] || "unknown" - end + info[:netmask] = $1.to_s elsif line =~ /^IPV6Address:\s+(.+?)$/ info[:ipv6] = $1.to_s.strip elsif line =~ /^IPV6NetworkMaskPrefixLength:\s+(.+?)$/ diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 1900be9c0..a6f8feea9 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -920,8 +920,6 @@ en: as mounting shared folders and configuring networks. Please add the ability to detect this guest operating system to Vagrant by creating a plugin or reporting a bug. - invalid_netmask: |- - Invalid mask '%{mask}' home_dir_later_version: |- It appears that a newer version of Vagrant was run on this machine at some point. The current version of Vagrant is unable to read @@ -1684,8 +1682,6 @@ en: A Vagrant update may also be available that adds support for the version you specified. Please check www.vagrantup.com/downloads.html to download the latest version. - virtualbox_invalid_network_setting: |- - Invalid %{setting_type} '%{setting_value}' for %{interface_type} interface '%{interface_name}' virtualbox_kernel_module_not_loaded: |- VirtualBox is complaining that the kernel module is not loaded. Please run `VBoxManage --version` or open the VirtualBox GUI to see the error diff --git a/test/unit/vagrant/util/network_ip_test.rb b/test/unit/vagrant/util/network_ip_test.rb index 5351f4f9a..181b642cb 100644 --- a/test/unit/vagrant/util/network_ip_test.rb +++ b/test/unit/vagrant/util/network_ip_test.rb @@ -5,33 +5,47 @@ require "vagrant/util/network_ip" describe Vagrant::Util::NetworkIP do let(:klass) do Class.new do - extend Vagrant::Util::NetworkIP + include Vagrant::Util::NetworkIP end end - describe "network address" do + subject { klass.new } + + describe "#network_address" do it "calculates it properly" do - expect(klass.network_address("192.168.2.234", "255.255.255.0")).to eq("192.168.2.0") + expect(subject.network_address("192.168.2.234", "255.255.255.0")).to eq("192.168.2.0") end it "calculates it properly with integer submask" do - expect(klass.network_address("192.168.2.234", "24")).to eq("192.168.2.0") + expect(subject.network_address("192.168.2.234", "24")).to eq("192.168.2.0") end it "calculates it properly with integer submask" do - expect(klass.network_address("192.168.2.234", 24)).to eq("192.168.2.0") + expect(subject.network_address("192.168.2.234", 24)).to eq("192.168.2.0") end it "calculates it properly for IPv6" do - expect(klass.network_address("fde4:8dba:82e1::c4", "64")).to eq("fde4:8dba:82e1::") + expect(subject.network_address("fde4:8dba:82e1::c4", "64")).to eq("fde4:8dba:82e1::") end it "calculates it properly for IPv6" do - expect(klass.network_address("fde4:8dba:82e1::c4", 64)).to eq("fde4:8dba:82e1::") + expect(subject.network_address("fde4:8dba:82e1::c4", 64)).to eq("fde4:8dba:82e1::") end it "calculates it properly for IPv6 for string mask" do - expect(klass.network_address("fde4:8dba:82e1::c4", "ffff:ffff:ffff:ffff::")).to eq("fde4:8dba:82e1::") + expect(subject.network_address("fde4:8dba:82e1::c4", "ffff:ffff:ffff:ffff::")).to eq("fde4:8dba:82e1::") + end + + it "recovers from invalid netmask" do + # The mask function will produce an error for ruby >= 2.5 + # If using a version of ruby that produces and error, then + # test to ensure `subject.network_address` produces expected + # results. + begin + IPAddr.new("192.168.2.234").mask("1.2.3.4") + rescue IPAddr::InvalidPrefixError + expect(subject.network_address("192.168.2.234", "1.2.3.4")).to eq("192.168.2.0") + end end end end diff --git a/test/unit/vagrant/util/validate_netmask_test.rb b/test/unit/vagrant/util/validate_netmask_test.rb deleted file mode 100644 index 5c402420c..000000000 --- a/test/unit/vagrant/util/validate_netmask_test.rb +++ /dev/null @@ -1,37 +0,0 @@ -require File.expand_path("../../../base", __FILE__) - -require "vagrant/util/validate_netmask" - -describe Vagrant::Util::ValidateNetmask do - include_context "unit" - subject { described_class } - - describe ".validate" do - it "validates valid netmask" do - masks = [ - "0.0.0.0", - "255.255.255.255", - "255.255.255.0", - "255.255.0.0", - "255.0.0.0", - "255.255.192.0", - "255.252.0.0", - "192.0.0.0" - ] - masks.each do |mask| - subject.validate(mask) - end - end - - it "raises an error for invalid netmask" do - masks = [ - "255.255.192.255", - "1.2.3.4", - "192.0.0.0.0" - ] - masks.each do |mask| - expect { subject.validate(mask) }.to raise_error(Vagrant::Errors::InvalidNetMask) - end - end - end -end