From a12054f2adef76d465dda828a30b0a1ae3cb1ae9 Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Fri, 14 Oct 2022 14:25:18 +0100 Subject: [PATCH] Validate provided synced_folders types for access (#1644) Reject any 9p synced folders that the user does not have read access to the host path where using qemu sessions. This is because the VM will launched with the user permissions instead of system permissions and will fail to come up if trying to add a path that is not readable to be mounted into the guest. Additionally flag that virtiofs may not be supported with qemu sessions, but do not reject in case support is added in the future. Fixes: #1430 --- lib/vagrant-libvirt/config.rb | 17 +++++++ spec/unit/config_spec.rb | 83 ++++++++++++++++++++++++++++++++++- 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/lib/vagrant-libvirt/config.rb b/lib/vagrant-libvirt/config.rb index 329b30a..6263860 100644 --- a/lib/vagrant-libvirt/config.rb +++ b/lib/vagrant-libvirt/config.rb @@ -3,6 +3,7 @@ require 'cgi' require 'vagrant' +require 'vagrant/action/builtin/mixin_synced_folders' require 'vagrant-libvirt/errors' require 'vagrant-libvirt/util/resolvers' @@ -10,6 +11,8 @@ require 'vagrant-libvirt/util/resolvers' module VagrantPlugins module ProviderLibvirt class Config < Vagrant.plugin('2', :config) + include Vagrant::Action::Builtin::MixinSyncedFolders + # manually specify URI # will supersede most other options if provided attr_accessor :uri @@ -1149,6 +1152,20 @@ module VagrantPlugins end end + # if run via a session, then qemu will be run with user permissions, make sure the user + # has permissions to access the host paths otherwise there will be an error triggered + if machine.provider_config.qemu_use_session + synced_folders(machine).fetch(:"9p", []).each do |_, options| + unless File.readable?(options[:hostpath]) + errors << "9p synced_folder cannot mount host path #{options[:hostpath]} into guest #{options[:guestpath]} when using qemu session as executing user does not have permissions to read the directory on the user." + end + end + + unless synced_folders(machine)[:"virtiofs"].nil? + machine.ui.warn("Note: qemu session may not support virtiofs for synced_folders, use 9p or enable use of qemu:///system context unless you know what you are doing") + end + end + errors = validate_sysinfo(machine, errors) { 'Libvirt Provider' => errors } diff --git a/spec/unit/config_spec.rb b/spec/unit/config_spec.rb index 14331c9..273721f 100644 --- a/spec/unit/config_spec.rb +++ b/spec/unit/config_spec.rb @@ -688,7 +688,7 @@ describe VagrantPlugins::ProviderLibvirt::Config do def assert_valid subject.finalize! errors = subject.validate(machine).values.first - expect(errors).to be_empty + expect(errors).to be_empty, lambda { "received errors unexpectedly: #{errors}" } end describe '#validate' do @@ -892,6 +892,87 @@ describe VagrantPlugins::ProviderLibvirt::Config do expect{ subject.finalize! }.to raise_error('Only two floppies may be attached at a time') end end + + context 'with synced_folders' do + let(:vagrantfile) do + <<-EOF + Vagrant.configure('2') do |config| + config.vm.box = "vagrant-libvirt/test" + config.vm.define :test + + config.vm.synced_folder "/path/to/share", "/srv", type: "#{type}" + end + EOF + end + let(:driver) { instance_double(::VagrantPlugins::ProviderLibvirt::Driver) } + + before do + allow(machine.provider).to receive(:driver).and_return(driver) + allow(driver).to receive_message_chain('connection.client.libversion').and_return(6_002_000) + end + + context 'when type is 9p' do + let(:type) { "9p" } + + context 'when using qemu:///session' do + before do + subject.qemu_use_session = true + end + + it 'should validate if user can read host path' do + expect(File).to receive(:readable?).with('/path/to/share').and_return(true) + + assert_valid + end + + it 'should reject if user does not have read access to host path' do + expect(File).to receive(:readable?).with('/path/to/share').and_return(false) + + assert_invalid + end + end + + context 'when using qemu:///system' do + before do + subject.qemu_use_session = false + end + + it 'should validate without checking if user has read access to host path' do + expect(File).to_not receive(:readable?) + + assert_valid + end + end + end + + context 'when type is virtiofs' do + let(:type) { "virtiofs" } + + context 'when using qemu:///session' do + before do + subject.qemu_use_session = true + end + + it 'should warn that it may not be supported' do + expect(ui).to receive(:warn).with(/Note: qemu session may not support virtiofs for synced_folders.*/) + + assert_valid + end + end + + context 'when using qemu:///system' do + before do + subject.qemu_use_session = false + end + + it 'should not emit a warning message' do + expect(ui).to_not receive(:warn) + + assert_valid + end + end + end + end end describe '#merge' do