Merge pull request #10450 from chrisroberts/f-vbox-default-nic-type

Update default_nic_type implementation within VirtualBox provider

plugins/providers/virtualbox/action.rb

@@ -42,6 +42,7 @@ module VagrantPlugins
       autoload :PrepareForwardedPortCollisionParams, File.expand_path("../action/prepare_forwarded_port_collision_params", __FILE__)
       autoload :Resume, File.expand_path("../action/resume", __FILE__)
       autoload :SaneDefaults, File.expand_path("../action/sane_defaults", __FILE__)
+      autoload :SetDefaultNICType, File.expand_path("../action/set_default_nic_type", __FILE__)
       autoload :SetName, File.expand_path("../action/set_name", __FILE__)
       autoload :SnapshotDelete, File.expand_path("../action/snapshot_delete", __FILE__)
       autoload :SnapshotRestore, File.expand_path("../action/snapshot_restore", __FILE__)
@@ -71,6 +72,7 @@ module VagrantPlugins
           b.use SyncedFolderCleanup
           b.use SyncedFolders
           b.use PrepareNFSSettings
+          b.use SetDefaultNICType
           b.use ClearNetworkInterfaces
           b.use Network
           b.use NetworkFixIPv6

plugins/providers/virtualbox/action/network.rb

@@ -37,8 +37,6 @@ module VagrantPlugins
             available_slots.delete(slot)
           end
 
-          default_nic_type = env[:machine].provider_config.default_nic_type
-
           @logger.debug("Available slots for high-level adapters: #{available_slots.inspect}")
           @logger.info("Determining network adapters required for high-level configuration...")
           available_slots = available_slots.to_a.sort
@@ -46,11 +44,6 @@ module VagrantPlugins
             # We only handle private and public networks
             next if type != :private_network && type != :public_network
 
-            if default_nic_type && !options.key?(:nic_type) && !options.key?(:virtualbox__nic_type)
-              @logger.info("Setting default nic type (`#{default_nic_type}`) for `#{type}` - `#{options}`")
-              options[:virtualbox__nic_type] = default_nic_type
-            end
-
             options = scoped_hash_override(options, :virtualbox)
 
             # Figure out the slot that this adapter will go into

plugins/providers/virtualbox/action/set_default_nic_type.rb

@@ -0,0 +1,69 @@
+require "log4r"
+
+module VagrantPlugins
+  module ProviderVirtualBox
+    module Action
+      # This sets the default NIC type used for network adapters created
+      # on the guest. Also includes a check of NIC types in use and VirtualBox
+      # version to determine if E1000 NIC types are vulnerable.
+      #
+      # NOTE: Vulnerability was fixed here: https://www.virtualbox.org/changeset/75330/vbox
+      class SetDefaultNICType
+        # Defines versions of VirtualBox with susceptible implementation
+        # of the E1000 devices.
+        E1000_SUSCEPTIBLE = Gem::Requirement.new("<= 5.2.22").freeze
+
+        def initialize(app, env)
+          @logger = Log4r::Logger.new("vagrant::plugins::virtualbox::set_default_nic_type")
+          @app    = app
+        end
+
+        def call(env)
+          default_nic_type = env[:machine].provider_config.default_nic_type
+
+          e1000_in_use = [
+            # simple check on default_nic_type
+            ->{ default_nic_type.nil? || default_nic_type.to_s.start_with?("8254") },
+            # check provider defined adapters
+            ->{ env[:machine].provider_config.network_adapters.values.detect{ |_, opts|
+                opts[:nic_type].to_s.start_with?("8254") } },
+            # finish with inspecting configured networks
+            ->{ env[:machine].config.vm.networks.detect{ |_, opts|
+                opts.fetch(:virtualbox__nic_type, opts[:nic_type]).to_s.start_with?("8254") } }
+          ]
+
+          # Check if VirtualBox E1000 implementation is vulnerable
+          if E1000_SUSCEPTIBLE.satisfied_by?(Gem::Version.new(env[:machine].provider.driver.version))
+            @logger.info("Detected VirtualBox version with susceptible E1000 implementation (`#{E1000_SUSCEPTIBLE}`)")
+            if e1000_in_use.any?(&:call)
+              env[:ui].warn I18n.t("vagrant.actions.vm.set_default_nic_type.e1000_warning")
+            end
+          end
+
+          if default_nic_type
+            @logger.info("Default NIC type for VirtualBox interfaces `#{default_nic_type}`")
+            # Update network adapters defined in provider configuration
+            env[:machine].provider_config.network_adapters.each do |slot, args|
+              _, opts = args
+              if opts && !opts.key?(:nic_type)
+                @logger.info("Setting default NIC type (`#{default_nic_type}`) adapter `#{slot}` - `#{args}`")
+                opts[:nic_type] = default_nic_type
+              end
+            end
+
+            # Update generally defined networks
+            env[:machine].config.vm.networks.each do |type, options|
+              next if !type.to_s.end_with?("_network")
+              if !options.key?(:nic_type) && !options.key?(:virtualbox__nic_type)
+                @logger.info("Setting default NIC type (`#{default_nic_type}`) for `#{type}` - `#{options}`")
+                options[:virtualbox__nic_type] = default_nic_type
+              end
+            end
+          end
+
+          @app.call(env)
+        end
+      end
+    end
+  end
+end

plugins/providers/virtualbox/config.rb

@@ -20,8 +20,8 @@ module VagrantPlugins
       attr_reader :customizations
 
       # Set the default type of NIC hardware to be used for network
-      # devices. By default this is "virtio". If it is set to `nil`
-      # no type will be set and VirtualBox's default will be used.
+      # devices. By default this is `nil` and VirtualBox's default
+      # will be used.
       #
       # @return [String]
       attr_accessor :default_nic_type
@@ -167,8 +167,7 @@ module VagrantPlugins
         # The default name is just nothing, and we default it
         @name = nil if @name == UNSET_VALUE
 
-        @default_nic_type = "virtio" if @default_nic_type == UNSET_VALUE
-        set_default_nic_type! if @default_nic_type
+        @default_nic_type = nil if @default_nic_type == UNSET_VALUE
       end
 
       def validate(machine)
@@ -201,15 +200,6 @@ module VagrantPlugins
         { "VirtualBox Provider" => errors }
       end
 
-      def set_default_nic_type!
-        network_adapters.each do |_, args|
-          _, opts = args
-          if opts && !opts.key?(:nic_type)
-            opts[:nic_type] = @default_nic_type
-          end
-        end
-      end
-
       def to_s
         "VirtualBox"
       end

templates/locales/en.yml

@@ -2300,6 +2300,17 @@ en:
           mounting: Mounting shared folders...
           mounting_entry: "%{guestpath} => %{hostpath}"
           nomount_entry: "Automounting disabled: %{hostpath}"
+        set_default_nic_type:
+          e1000_warning: |-
+            Vagrant has detected a configuration issue which exposes a
+            vulnerability with the installed version of VirtualBox. The
+            current guest is configured to use an E1000 NIC type for a
+            network adapter which is vulnerable in this version of VirtualBox.
+            Ensure the guest is trusted to use this configuration or update
+            the NIC type using one of the methods below:
+
+              https://www.vagrantup.com/docs/virtualbox/configuration.html#default-nic-type
+              https://www.vagrantup.com/docs/virtualbox/networking.html#virtualbox-nic-type
         set_name:
           setting_name: |-
             Setting the name of the VM: %{name}

test/unit/plugins/providers/virtualbox/action/network_test.rb

@@ -84,42 +84,6 @@ describe VagrantPlugins::ProviderVirtualBox::Action::Network do
     expect{ subject.call(env) }.not_to raise_error(Vagrant::Errors::NetworkAddressInvalid)
   end
 
-  describe "setting nic type" do
-    before do
-      guest = double("guest")
-      allow(driver).to receive(:read_bridged_interfaces) { [] }
-      allow(driver).to receive(:read_host_only_interfaces) { [] }
-      allow(driver).to receive(:create_host_only_network) { {} }
-      allow(driver).to receive(:read_dhcp_servers) { [] }
-      allow(driver).to receive(:create_dhcp_server)
-      allow(machine).to receive(:guest) { guest }
-      allow(guest).to receive(:capability)
-    end
-
-    it "sets default nic type when unset" do
-      machine.config.vm.network 'private_network', { type: 'dhcp' }
-      subject.call(env)
-      _, net_config = machine.config.vm.networks.detect { |type, _| type == :private_network }
-      expect(net_config[:virtualbox__nic_type]).to eq("virtio")
-    end
-
-    it "does not set nic type when already set" do
-      machine.config.vm.network 'private_network', { type: 'dhcp', nic_type: "custom" }
-      subject.call(env)
-      _, net_config = machine.config.vm.networks.detect { |type, _| type == :private_network }
-      expect(net_config[:nic_type]).to eq("custom")
-      expect(net_config[:virtualbox__nic_type]).to be_nil
-    end
-
-    it "does not set nic type when namespaced option is set" do
-      machine.config.vm.network 'private_network', { type: 'dhcp', virtualbox__nic_type: "custom" }
-      subject.call(env)
-      _, net_config = machine.config.vm.networks.detect { |type, _| type == :private_network }
-      expect(net_config[:nic_type]).to be_nil
-      expect(net_config[:virtualbox__nic_type]).to eq("custom")
-    end
-  end
-
   context "with a dhcp private network" do
     let(:bridgedifs)  { [] }
     let(:hostonlyifs) { [] }
@@ -154,7 +118,7 @@ describe VagrantPlugins::ProviderVirtualBox::Action::Network do
         mac: nil,
         name: nil,
         netmask: "255.255.255.0",
-        nic_type: "virtio",
+        nic_type: nil,
         type: :dhcp,
         dhcp_ip: "172.28.128.2",
         dhcp_lower: "172.28.128.3",

test/unit/plugins/providers/virtualbox/action/set_default_nic_type_test.rb

@@ -0,0 +1,148 @@
+require_relative "../base"
+
+describe VagrantPlugins::ProviderVirtualBox::Action::SetDefaultNICType do
+  include_context "unit"
+  include_context "virtualbox"
+
+  let(:iso_env) do
+    # We have to create a Vagrantfile so there is a root path
+    env = isolated_environment
+    env.vagrantfile("")
+    env.create_vagrant_env
+  end
+
+  let(:machine) do
+    iso_env.machine(iso_env.machine_names[0], :virtualbox).tap do |m|
+      allow(m.provider).to receive(:driver).and_return(driver)
+    end
+  end
+
+  let(:env)    {{ machine: machine, ui: machine.ui }}
+  let(:app)    { lambda { |*args| }}
+  let(:driver) { double("driver") }
+
+  subject { described_class.new(app, env) }
+
+  describe "#call" do
+    let(:provider_config) {
+      double("provider_config",
+        default_nic_type: default_nic_type,
+        network_adapters: network_adapters)
+    }
+    let(:default_nic_type) { nil }
+    let(:network_adapters) { {} }
+    let(:virtualbox_version) { "5.2.23" }
+
+    before do
+      allow(driver).to receive(:version).and_return(virtualbox_version)
+      allow(machine).to receive(:provider_config).and_return(provider_config)
+    end
+
+    it "should call the next action" do
+      expect(app).to receive(:call)
+      subject.call(env)
+    end
+
+    context "when default_nic_type is set" do
+      let(:default_nic_type) { "CUSTOM_NIC_TYPE" }
+
+      context "when network adapters are defined" do
+        let(:network_adapters) { {"1" => [:nat, {}], "2" => [:intnet, {nic_type: nil}]} }
+
+        it "should set nic type if not defined" do
+          subject.call(env)
+          expect(network_adapters["1"].last[:nic_type]).to eq(default_nic_type)
+        end
+
+        it "should not set nic type if already defined" do
+          subject.call(env)
+          expect(network_adapters["2"].last[:nic_type]).to be_nil
+        end
+      end
+
+      context "when vm networks are defined" do
+        before do
+          machine.config.vm.network :private_network
+          machine.config.vm.network :public_network, nic_type: nil
+          machine.config.vm.network :private_network, virtualbox__nic_type: "STANDARD"
+        end
+
+        it "should add namespaced nic type when not defined" do
+          subject.call(env)
+          networks = machine.config.vm.networks.map { |type, opts|
+            opts if type.to_s.end_with?("_network") }.compact
+          expect(networks.first[:virtualbox__nic_type]).to eq(default_nic_type)
+        end
+
+        it "should not add namespaced nic type when nic type defined" do
+          subject.call(env)
+          networks = machine.config.vm.networks.map { |type, opts|
+            opts if type.to_s.end_with?("_network") }.compact
+          expect(networks[1][:virtualbox__nic_type]).to be_nil
+        end
+
+        it "should not modify existing namespaced nic type" do
+          subject.call(env)
+          networks = machine.config.vm.networks.map { |type, opts|
+            opts if type.to_s.end_with?("_network") }.compact
+          expect(networks.last[:virtualbox__nic_type]).to eq("STANDARD")
+        end
+      end
+    end
+
+    context "when virtualbox version is has susceptible E1000" do
+      let(:virtualbox_version) { "5.2.22" }
+
+      it "should output a warning" do
+        expect(machine.ui).to receive(:warn)
+        subject.call(env)
+      end
+
+      context "when default_nic_type is set to E1000 type" do
+        let(:default_nic_type) { "82540EM" }
+
+        it "should output a warning" do
+          expect(machine.ui).to receive(:warn)
+          subject.call(env)
+        end
+      end
+
+      context "when default_nic_type is set to non-E1000 type" do
+        let(:default_nic_type) { "virtio" }
+
+        it "should not output a warning" do
+          expect(machine.ui).not_to receive(:warn)
+          subject.call(env)
+        end
+
+        context "when network adapter is configured with E1000 type" do
+          let(:network_adapters) { {"1" => [:nat, {nic_type: "82540EM" }]} }
+
+          it "should output a warning" do
+            expect(machine.ui).to receive(:warn)
+            subject.call(env)
+          end
+        end
+
+        context "when vm network is configured with E1000 type" do
+          before { machine.config.vm.network :private_network, nic_type: "82540EM" }
+
+          it "should output a warning" do
+            expect(machine.ui).to receive(:warn)
+            subject.call(env)
+          end
+        end
+
+        context "when vm network is configured with E1000 type in namespaced argument" do
+          before { machine.config.vm.network :private_network, virtualbox__nic_type: "82540EM" }
+
+          it "should output a warning" do
+            expect(machine.ui).to receive(:warn)
+            subject.call(env)
+          end
+        end
+      end
+    end
+
+  end
+end

test/unit/plugins/providers/virtualbox/config_test.rb

@@ -43,11 +43,11 @@ describe VagrantPlugins::ProviderVirtualBox::Config do
     it { expect(subject.gui).to be(false) }
     it { expect(subject.name).to be_nil }
     it { expect(subject.functional_vboxsf).to be(true) }
-    it { expect(subject.default_nic_type).to eq("virtio") }
+    it { expect(subject.default_nic_type).to be_nil }
 
     it "should have one NAT adapter" do
       expect(subject.network_adapters).to eql({
-        1 => [:nat, {nic_type: "virtio"}],
+        1 => [:nat, {}],
       })
     end
   end
@@ -61,22 +61,6 @@ describe VagrantPlugins::ProviderVirtualBox::Config do
     end
 
     it { expect(subject.default_nic_type).to eq(nic_type) }
-
-    it "should set NAT adapter nic type" do
-      expect(subject.network_adapters.values.first.last[:nic_type]).
-        to eq(nic_type)
-    end
-
-    context "when set to nil" do
-      let(:nic_type) { nil }
-
-      it { expect(subject.default_nic_type).to be_nil }
-
-      it "should not set NAT adapter nic type" do
-        expect(subject.network_adapters.values.first.last[:nic_type]).
-          to be_nil
-      end
-    end
   end
 
   describe "#merge" do

website/source/docs/virtualbox/configuration.html.md

@@ -43,9 +43,10 @@ end
 
 ## Default NIC Type
 
-By default Vagrant will set the NIC type for all network interfaces to
-`"virtio"`. If you would rather a different NIC type be used as the
-default for guests:
+By default Vagrant will not set the NIC type for network interfaces. This
+allows VirtualBox to apply the default NIC type for the guest. If you would
+like to use a specific NIC type by default for guests, set the `default_nic_type`
+option:
 
 ```ruby
 config.vm.provider "virtualbox" do |v|
@@ -53,15 +54,6 @@ config.vm.provider "virtualbox" do |v|
 end
 ```
 
-or if you would like to disable the default type so VirtualBox's default
-is applied you can set the value to `nil`:
-
-```ruby
-config.vm.provider "virtualbox" do |v|
-  v.default_nic_type = nil
-end
-```
-
 ## Linked Clones
 
 By default new machines are created by importing the base box. For large