Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate VirtualBox hostonly network range #12564

Merged
merged 6 commits into from
Nov 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
runs-on: ubuntu-18.04
strategy:
matrix:
ruby: [ '2.5', '2.6', '2.7', '3.0' ]
ruby: [ '2.6', '2.7', '3.0' ]
name: Vagrant unit tests on Ruby ${{ matrix.ruby }}
steps:
- name: Code Checkout
Expand Down
4 changes: 4 additions & 0 deletions lib/vagrant/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1024,6 +1024,10 @@ class VirtualBoxVersionEmpty < VagrantError
error_key(:virtualbox_version_empty)
end

class VirtualBoxInvalidHostSubnet < VagrantError
error_key(:virtualbox_invalid_host_subnet)
end

class VMBaseMacNotSpecified < VagrantError
error_key(:no_base_mac, "vagrant.actions.vm.match_mac")
end
Expand Down
41 changes: 39 additions & 2 deletions plugins/providers/virtualbox/action/network.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ module Action
#
# This handles all the `config.vm.network` configurations.
class Network

# Location of the VirtualBox networks configuration file
VBOX_NET_CONF = "/etc/vbox/networks.conf".freeze
# Version of VirtualBox that introduced hostonly network range restrictions
HOSTONLY_VALIDATE_VERSION = Gem::Version.new("6.1.28")
# Default valid range for hostonly networks
HOSTONLY_DEFAULT_RANGE = [IPAddr.new("192.168.56.0/21").freeze].freeze

include Vagrant::Util::NetworkIP
include Vagrant::Util::ScopedHashOverride

Expand Down Expand Up @@ -255,15 +263,15 @@ def hostonly_config(options)

# Make sure the type is a symbol
options[:type] = options[:type].to_sym

if options[:type] == :dhcp && !options[:ip]
# Try to find a matching device to set the config ip to
matching_device = hostonly_find_matching_network(options)
if matching_device
options[:ip] = matching_device[:ip]
else
# Default IP is in the 20-bit private network block for DHCP based networks
options[:ip] = "172.28.128.1"
options[:ip] = "192.168.56.1"
end
end

Expand All @@ -288,6 +296,8 @@ def hostonly_config(options)
error: e.message
end

validate_hostonly_ip!(options[:ip])

if ip.ipv4?
# Verify that a host-only network subnet would not collide
# with a bridged networking interface.
Expand Down Expand Up @@ -501,6 +511,33 @@ def hostonly_find_matching_network(config)
nil
end

# Validates the IP used to configure the network is within the allowed
# ranges. It only validates if the network configuration file exists.
# This was introduced in 6.1.28 so previous version won't have restrictions
# placed on the valid ranges
def validate_hostonly_ip!(ip)
return if Gem::Version.new(Driver::Meta.version) < HOSTONLY_VALIDATE_VERSION ||
Vagrant::Util::Platform.windows?

ip = IPAddr.new(ip.to_s) if !ip.is_a?(IPAddr)
valid_ranges = load_net_conf
return if valid_ranges.any?{ |range| range.include?(ip) }
raise Vagrant::Errors::VirtualBoxInvalidHostSubnet,
address: ip,
ranges: valid_ranges.map{ |r| "#{r}/#{r.prefix}" }.join(", ")
end

def load_net_conf
return HOSTONLY_DEFAULT_RANGE if !File.exist?(VBOX_NET_CONF)
File.readlines(VBOX_NET_CONF).map do |line|
line = line.strip
next if !line.start_with?("*")
line[1,line.length].strip.split(" ").map do |entry|
IPAddr.new(entry)
end
end.flatten.compact
end

#-----------------------------------------------------------------
# DHCP Server Helper Functions
#-----------------------------------------------------------------
Expand Down
12 changes: 12 additions & 0 deletions templates/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1842,6 +1842,18 @@ en:
outputted:

%{vboxmanage} --version
virtualbox_invalid_host_subnet: |-
The IP address configured for the host-only network is not within the
allowed ranges. Please update the address used to be within the allowed
ranges and run the command again.

Address: %{address}
Ranges: %{ranges}

Valid ranges can be modified in the /etc/vbox/networks.conf file. For
more information including valid format see:

https://www.virtualbox.org/manual/ch06.html#network_hostonly
vm_creation_required: |-
VM must be created before running this command. Run `vagrant up` first.
vm_inaccessible: |-
Expand Down
202 changes: 192 additions & 10 deletions test/unit/plugins/providers/virtualbox/action/network_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,188 @@
before do
allow(driver).to receive(:enable_adapters)
allow(driver).to receive(:read_network_interfaces) { nics }
allow(VagrantPlugins::ProviderVirtualBox::Driver::Meta).to receive(:version).
and_return("6.1.0")
end

describe "#hostonly_config" do
before do
allow(subject).to receive(:hostonly_find_matching_network)
allow(driver).to receive(:read_bridged_interfaces).and_return([])
subject.instance_eval do
def env=(e)
@env = e
end
end

subject.env = env
end

let(:options) {
{
type: type,
ip: address,
}
}
let(:type) { :dhcp }
let(:address) { nil }

it "should validate the IP" do
expect(subject).to receive(:validate_hostonly_ip!)
subject.hostonly_config(options)
end
end

describe "#validate_hostonly_ip!" do
let(:address) { "192.168.1.2" }
let(:net_conf) { [IPAddr.new(address + "/24")]}

before do
allow(VagrantPlugins::ProviderVirtualBox::Driver::Meta).to receive(:version).
and_return("6.1.28")

allow(subject).to receive(:load_net_conf).and_return(net_conf)
expect(subject).to receive(:validate_hostonly_ip!).and_call_original
end

it "should load net configuration" do
expect(subject).to receive(:load_net_conf).and_return(net_conf)
subject.validate_hostonly_ip!(address)
end

context "when address is within ranges" do
it "should not error" do
subject.validate_hostonly_ip!(address)
end
end

context "when address is not found within ranges" do
let(:net_conf) { [IPAddr.new("127.0.0.1/20")] }

it "should raise an error" do
expect {
subject.validate_hostonly_ip!(address)
}.to raise_error(Vagrant::Errors::VirtualBoxInvalidHostSubnet)
end
end

context "when virtualbox version does not restrict range" do
before do
allow(VagrantPlugins::ProviderVirtualBox::Driver::Meta).to receive(:version).
and_return("6.1.20")
end

it "should not error" do
subject.validate_hostonly_ip!(address)
end

it "should not attempt to load network configuration" do
expect(subject).not_to receive(:load_net_conf)
subject.validate_hostonly_ip!(address)
end
end

context "when platform is windows" do
before do
allow(Vagrant::Util::Platform).to receive(:windows?).and_return(true)
end

it "should not error" do
subject.validate_hostonly_ip!(address)
end

it "should not attempt to load network configuration" do
expect(subject).not_to receive(:load_net_conf)
subject.validate_hostonly_ip!(address)
end
end
end

describe "#load_net_conf" do
let(:file_contents) { [""] }

before do
allow(File).to receive(:exist?).and_call_original
allow(File).to receive(:exist?).
with(described_class.const_get(:VBOX_NET_CONF)).
and_return(true)
allow(File).to receive(:readlines).
with(described_class.const_get(:VBOX_NET_CONF)).
and_return(file_contents)
end

it "should read the configuration file" do
expect(File).to receive(:readlines).
with(described_class.const_get(:VBOX_NET_CONF)).
and_return(file_contents)

subject.load_net_conf
end

context "when file has comments only" do
let(:file_contents) {
[
"# A comment",
"# Another comment",
]
}

it "should return an empty array" do
expect(subject.load_net_conf).to eq([])
end
end

context "when file has valid range entries" do
let(:file_contents) {
[
"* 127.0.0.1/24",
"* 192.168.1.1/24",
]
}

it "should return an array with content" do
expect(subject.load_net_conf).not_to be_empty
end

it "should include IPAddr instances" do
subject.load_net_conf.each do |entry|
expect(entry).to be_a(IPAddr)
end
end
end

context "when file has valid range entries and comments" do
let(:file_contents) {
[
"# Comment in file",
"* 127.0.0.0/8",
"random text",
" * 192.168.2.0/28",
]
}

it "should contain two entries" do
expect(subject.load_net_conf.size).to eq(2)
end
end

context "when file has multiple entries on single line" do
let(:file_contents) {
[
"* 0.0.0.0/0 ::/0"
]
}

it "should contain two entries" do
expect(subject.load_net_conf.size).to eq(2)
end

it "should contain an ipv4 and ipv6 range" do
result = subject.load_net_conf
expect(result.first).to be_ipv4
expect(result.last).to be_ipv6
end
end
end

it "calls the next action in the chain" do
Expand Down Expand Up @@ -133,29 +315,29 @@
subject.call(env)

expect(driver).to have_received(:create_host_only_network).with({
adapter_ip: '172.28.128.1',
adapter_ip: '192.168.56.1',
netmask: '255.255.255.0',
})

expect(driver).to have_received(:create_dhcp_server).with('vboxnet0', {
adapter_ip: "172.28.128.1",
adapter_ip: "192.168.56.1",
auto_config: true,
ip: "172.28.128.1",
ip: "192.168.56.1",
mac: nil,
name: nil,
netmask: "255.255.255.0",
nic_type: nil,
type: :dhcp,
dhcp_ip: "172.28.128.2",
dhcp_lower: "172.28.128.3",
dhcp_upper: "172.28.128.254",
dhcp_ip: "192.168.56.2",
dhcp_lower: "192.168.56.3",
dhcp_upper: "192.168.56.254",
adapter: 2
})

expect(guest).to have_received(:capability).with(:configure_networks, [{
type: :dhcp,
adapter_ip: "172.28.128.1",
ip: "172.28.128.1",
adapter_ip: "192.168.56.1",
ip: "192.168.56.1",
netmask: "255.255.255.0",
auto_config: true,
interface: nil
Expand Down Expand Up @@ -213,8 +395,8 @@
{ ip: 'foo'},
{ ip: '1.2.3'},
{ ip: 'dead::beef::'},
{ ip: '172.28.128.3', netmask: 64},
{ ip: '172.28.128.3', netmask: 'ffff:ffff::'},
{ ip: '192.168.56.3', netmask: 64},
{ ip: '192.168.56.3', netmask: 'ffff:ffff::'},
{ ip: 'dead:beef::', netmask: 'foo:bar::'},
{ ip: 'dead:beef::', netmask: '255.255.255.0'}
].each do |args|
Expand Down
2 changes: 1 addition & 1 deletion vagrant.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Gem::Specification.new do |s|
s.summary = "Build and distribute virtualized development environments."
s.description = "Vagrant is a tool for building and distributing virtualized development environments."

s.required_ruby_version = ">= 2.5", "< 3.1"
s.required_ruby_version = ">= 2.6", "< 3.1"
s.required_rubygems_version = ">= 1.3.6"

s.add_dependency "bcrypt_pbkdf", "~> 1.1"
Expand Down