From e85ef1655bb9c2131bac7175702ab9fe328cb4b8 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 14 Dec 2017 12:31:43 -0800 Subject: [PATCH 01/30] Always remove credentials file after mount attempts --- .../linux/cap/mount_smb_shared_folder.rb | 33 +++++++++++-------- .../linux/cap/mount_smb_shared_folder.rb | 5 +++ 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/plugins/guests/linux/cap/mount_smb_shared_folder.rb b/plugins/guests/linux/cap/mount_smb_shared_folder.rb index c0d1bf3f3..5389437d2 100644 --- a/plugins/guests/linux/cap/mount_smb_shared_folder.rb +++ b/plugins/guests/linux/cap/mount_smb_shared_folder.rb @@ -46,22 +46,27 @@ SCRIPT # Attempt to mount the folder. We retry here a few times because # it can fail early on. - - retryable(on: Vagrant::Errors::LinuxMountFailed, tries: 10, sleep: 2) do - no_such_device = false - stderr = "" - status = machine.communicate.sudo(mount_command, error_check: false) do |type, data| - if type == :stderr - no_such_device = true if data =~ /No such device/i - stderr += data.to_s + begin + retryable(on: Vagrant::Errors::LinuxMountFailed, tries: 10, sleep: 2) do + no_such_device = false + stderr = "" + status = machine.communicate.sudo(mount_command, error_check: false) do |type, data| + if type == :stderr + no_such_device = true if data =~ /No such device/i + stderr += data.to_s + end + end + if status != 0 || no_such_device + clean_command = mount_command.gsub(smb_password, "PASSWORDHIDDEN") + raise Vagrant::Errors::LinuxMountFailed, + command: clean_command, + output: stderr end end - if status != 0 || no_such_device - clean_command = mount_command.gsub(smb_password, "PASSWORDHIDDEN") - raise Vagrant::Errors::LinuxMountFailed, - command: clean_command, - output: stderr - end + ensure + # Always remove credentials file after mounting attempts + # have been completed + machine.communicate.sudo("rm /etc/smb_creds_#{name}") end emit_upstart_notification(machine, expanded_guest_path) diff --git a/test/unit/plugins/guests/linux/cap/mount_smb_shared_folder.rb b/test/unit/plugins/guests/linux/cap/mount_smb_shared_folder.rb index cde67e6a6..62d4745f2 100644 --- a/test/unit/plugins/guests/linux/cap/mount_smb_shared_folder.rb +++ b/test/unit/plugins/guests/linux/cap/mount_smb_shared_folder.rb @@ -63,6 +63,11 @@ describe "VagrantPlugins::GuestLinux::Cap::MountSMBSharedFolder" do cap.mount_smb_shared_folder(machine, mount_name, mount_guest_path, folder_options) end + it "removes the credentials file before completion" do + expect(comm).to receive(:sudo).with(/rm.+smb_creds_.+/) + cap.mount_smb_shared_folder(machine, mount_name, mount_guest_path, folder_options) + end + it "sends upstart notification after mount" do expect(comm).to receive(:sudo).with(/emit/) cap.mount_smb_shared_folder(machine, mount_name, mount_guest_path, folder_options) From 00fa50c296bda4ea708d2791caae09d627494f27 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Fri, 15 Dec 2017 16:31:44 -0800 Subject: [PATCH 02/30] SMB enhancements --- lib/vagrant/util/powershell.rb | 90 +++++++++++++--- .../linux/cap/mount_smb_shared_folder.rb | 6 +- .../darwin/cap/configured_ip_addresses.rb | 18 ++++ plugins/hosts/darwin/cap/smb.rb | 86 +++++++++++++++ plugins/hosts/darwin/plugin.rb | 25 +++++ .../windows/cap/configured_ip_addresses.rb | 29 +++++ plugins/hosts/windows/cap/smb.rb | 102 ++++++++++++++++++ plugins/hosts/windows/plugin.rb | 20 ++++ .../windows}/scripts/host_info.ps1 | 0 plugins/hosts/windows/scripts/set_share.ps1 | 39 +++++++ plugins/hosts/windows/scripts/unset_share.ps1 | 11 ++ plugins/synced_folders/smb/errors.rb | 4 + .../synced_folders/smb/scripts/set_share.ps1 | 44 -------- plugins/synced_folders/smb/synced_folder.rb | 84 ++++----------- templates/locales/synced_folder_smb.yml | 16 +++ 15 files changed, 452 insertions(+), 122 deletions(-) create mode 100644 plugins/hosts/darwin/cap/configured_ip_addresses.rb create mode 100644 plugins/hosts/darwin/cap/smb.rb create mode 100644 plugins/hosts/windows/cap/configured_ip_addresses.rb create mode 100644 plugins/hosts/windows/cap/smb.rb rename plugins/{synced_folders/smb => hosts/windows}/scripts/host_info.ps1 (100%) create mode 100644 plugins/hosts/windows/scripts/set_share.ps1 create mode 100644 plugins/hosts/windows/scripts/unset_share.ps1 delete mode 100644 plugins/synced_folders/smb/scripts/set_share.ps1 diff --git a/lib/vagrant/util/powershell.rb b/lib/vagrant/util/powershell.rb index b5d474cec..fefb60571 100644 --- a/lib/vagrant/util/powershell.rb +++ b/lib/vagrant/util/powershell.rb @@ -1,3 +1,4 @@ +require "tmpdir" require_relative "subprocess" require_relative "which" @@ -25,21 +26,26 @@ module Vagrant # @return [Subprocess::Result] def self.execute(path, *args, **opts, &block) validate_install! - command = [ - "powershell", - "-NoLogo", - "-NoProfile", - "-NonInteractive", - "-ExecutionPolicy", "Bypass", - "&('#{path}')", - args - ].flatten - # Append on the options hash since Subprocess doesn't use - # Ruby 2.0 style options yet. - command << opts + if opts.delete(:sudo) || opts.delete(:runas) + powerup_command(path, args, opts) + else + command = [ + "powershell", + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-ExecutionPolicy", "Bypass", + "&('#{path}')", + args + ].flatten - Subprocess.execute(*command, &block) + # Append on the options hash since Subprocess doesn't use + # Ruby 2.0 style options yet. + command << opts + + Subprocess.execute(*command, &block) + end end # Execute a powershell command. @@ -56,7 +62,7 @@ module Vagrant "-ExecutionPolicy", "Bypass", "-Command", command - ].flatten + ].flatten.compact r = Subprocess.execute(*c) return nil if r.exit_code != 0 @@ -75,7 +81,7 @@ module Vagrant "-NonInteractive", "-ExecutionPolicy", "Bypass", "-Command", - "$PSVersionTable.PSVersion.Major" + "Write-Output $PSVersionTable.PSVersion.Major" ].flatten r = Subprocess.execute(*command) @@ -101,6 +107,60 @@ module Vagrant end @_powershell_validation end + + # Powerup the given command to perform privileged operations. + # + # @param [String] path + # @param [Array] args + # @return [Array] + def self.powerup_command(path, args, opts) + Dir.mktmpdir("vagrant") do |dpath| + all_args = ["-NoProfile", "-NonInteractive", "-ExecutionPolicy", "Bypass", path] + args + arg_list = "@('" + all_args.join("', '") + "')" + stdout = File.join(dpath, "stdout.txt") + stderr = File.join(dpath, "stderr.txt") + exitcode = File.join(dpath, "exitcode.txt") + + script = "$sp = Start-Process -FilePath powershell -ArgumentList #{arg_list} " \ + "-PassThru -Wait -RedirectStandardOutput '#{stdout}' -RedirectStandardError '#{stderr}' -WindowStyle Hidden; " \ + "if($sp){ Set-Content -Path '#{exitcode}' -Value $sp.ExitCode;exit $sp.ExitCode; }else{ exit 1 }" + + # escape quotes so we can nest our script within a start-process + script.gsub!("'", "''") + + cmd = [ + "powershell", + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-ExecutionPolicy", "Bypass", + "-Command", "$p = Start-Process -FilePath powershell -ArgumentList " \ + "@('-NoLogo', '-NoProfile', '-NonInteractive', '-ExecutionPolicy', 'Bypass', '-Command', '#{script}') " \ + "-PassThru -Wait -WindowStyle Hidden -Verb RunAs; if($p){ exit $p.ExitCode; }else{ exit 1 }" + ] + + result = Subprocess.execute(*cmd.push(opts)) + if File.exist?(stdout) + r_stdout = File.read(stdout) + else + r_stdout = result.stdout + end + if File.exist?(stderr) + r_stderr = File.read(stderr) + else + r_stderr = result.stderr + end + + code = 1 + if File.exist?(exitcode) + code_txt = File.read(exitcode).strip + if code_txt.match(/^\d+$/) + code = code_txt.to_i + end + end + Subprocess::Result.new(code, r_stdout, r_stderr) + end + end end end end diff --git a/plugins/guests/linux/cap/mount_smb_shared_folder.rb b/plugins/guests/linux/cap/mount_smb_shared_folder.rb index 5389437d2..f2a2cb7c3 100644 --- a/plugins/guests/linux/cap/mount_smb_shared_folder.rb +++ b/plugins/guests/linux/cap/mount_smb_shared_folder.rb @@ -24,7 +24,11 @@ module VagrantPlugins smb_password = options[:smb_password] options[:mount_options] ||= [] - options[:mount_options] << "sec=ntlm" + if machine.env.host.capability?(:smb_mount_options) + options[:mount_options] += machine.env.host.capability(:smb_mount_options) + else + options[:mount_options] << "sec=ntlm" + end options[:mount_options] << "credentials=/etc/smb_creds_#{name}" mount_options = "-o uid=#{mount_uid},gid=#{mount_gid}" diff --git a/plugins/hosts/darwin/cap/configured_ip_addresses.rb b/plugins/hosts/darwin/cap/configured_ip_addresses.rb new file mode 100644 index 000000000..063295ea1 --- /dev/null +++ b/plugins/hosts/darwin/cap/configured_ip_addresses.rb @@ -0,0 +1,18 @@ +require "socket" + +module VagrantPlugins + module HostDarwin + module Cap + class ConfiguredIPAddresses + + def self.configured_ip_addresses(env) + Socket.getifaddrs.map do |interface| + if interface.addr.ipv4? && !interface.addr.ipv4_loopback? + interface.addr.ip_address + end + end.compact + end + end + end + end +end diff --git a/plugins/hosts/darwin/cap/smb.rb b/plugins/hosts/darwin/cap/smb.rb new file mode 100644 index 000000000..ba0ba1e94 --- /dev/null +++ b/plugins/hosts/darwin/cap/smb.rb @@ -0,0 +1,86 @@ +module VagrantPlugins + module HostDarwin + module Cap + class SMB + + @@logger = Log4r::Logger.new("vagrant::host::darwin::smb") + + # If we have the sharing binary available, smb is installed + def self.smb_installed(env) + File.exist?("/usr/sbin/sharing") + end + + # Required options for mounting a share hosted + # on macos. + def self.smb_mount_options(env) + ["ver=3", "sec=ntlmssp", "nounix", "noperm"] + end + + def self.smb_cleanup(env, machine, opts) + m_id = machine_id(machine) + result = Vagrant::Util::Subprocess.execute("/bin/sh", "-c", + "/usr/sbin/sharing -l | grep -E \"^name:.+\\svgt-#{m_id}-\" | awk '{print $2}'") + if result.exit_code != 0 + @@logger.warn("failed to locate any shares for cleanup") + end + shares = result.stdout.split(/\s/).map(&:strip) + @@logger.debug("shares to be removed: #{shares}") + shares.each do |share_name| + @@logger.info("removing share name=#{share_name}") + share_name.strip! + result = Vagrant::Util::Subprocess.execute("/usr/bin/sudo", + "/usr/sbin/sharing", "-r", share_name) + if result.exit_code != 0 + # Removing always returns 0 even if there are currently + # guests attached so if we get a non-zero value just + # log it as unexpected + @@logger.warn("removing share `#{share_name}` returned non-zero") + end + end + end + + def self.smb_prepare(env, machine, folders, opts) + folders.each do |id, data| + hostpath = data[:hostpath] + + chksum_id = Digest::MD5.hexdigest(id) + name = "vgt-#{machine_id(machine)}-#{chksum_id}" + data[:smb_id] ||= name + + @@logger.info("creating new share name=#{name} id=#{data[:smb_id]}") + + cmd = [ + "/usr/bin/sudo", + "/usr/sbin/sharing", + "-a", hostpath, + "-S", data[:smb_id], + "-s", "001", + "-g", "000", + "-n", name + ] + + r = Vagrant::Util::Subprocess.execute(*cmd) + + if r.exit_code != 0 + raise Errors::DefineShareFailed, + host: hostpath.to_s, + stderr: r.stderr, + stdout: r.stdout + end + end + end + + # Generates a unique identifier for the given machine + # based on the name, provider name, and working directory + # of the environment. + # + # @param [Vagrant::Machine] machine + # @return [String] + def self.machine_id(machine) + @@logger.debug("generating machine ID name=#{machine.name} cwd=#{machine.env.cwd}") + Digest::MD5.hexdigest("#{machine.name}-#{machine.provider_name}-#{machine.env.cwd}") + end + end + end + end +end diff --git a/plugins/hosts/darwin/plugin.rb b/plugins/hosts/darwin/plugin.rb index f37bcd9dd..92cb51787 100644 --- a/plugins/hosts/darwin/plugin.rb +++ b/plugins/hosts/darwin/plugin.rb @@ -20,6 +20,31 @@ module VagrantPlugins require_relative "cap/rdp" Cap::RDP end + + host_capability("darwin", "smb_installed") do + require_relative "cap/smb" + Cap::SMB + end + + host_capability("darwin", "smb_prepare") do + require_relative "cap/smb" + Cap::SMB + end + + host_capability("darwin", "smb_mount_options") do + require_relative "cap/smb" + Cap::SMB + end + + host_capability("darwin", "smb_cleanup") do + require_relative "cap/smb" + Cap::SMB + end + + host_capability("darwin", "configured_ip_addresses") do + require_relative "cap/configured_ip_addresses" + Cap::ConfiguredIPAddresses + end end end end diff --git a/plugins/hosts/windows/cap/configured_ip_addresses.rb b/plugins/hosts/windows/cap/configured_ip_addresses.rb new file mode 100644 index 000000000..43820cb5a --- /dev/null +++ b/plugins/hosts/windows/cap/configured_ip_addresses.rb @@ -0,0 +1,29 @@ +require "pathname" +require "tempfile" + +require "vagrant/util/downloader" +require "vagrant/util/file_checksum" +require "vagrant/util/powershell" +require "vagrant/util/subprocess" + +module VagrantPlugins + module HostWindows + module Cap + class ConfiguredIPAddresses + + def self.configured_ip_addresses(env) + script_path = File.expand_path("../../scripts/host_info.ps1", __FILE__) + r = Vagrant::Util::PowerShell.execute(script_path) + if r.exit_code != 0 + raise Errors::PowershellError, + script: script_path, + stderr: r.stderr + end + + res = JSON.parse(r.stdout)["ip_addresses"] + Array(res) + end + end + end + end +end diff --git a/plugins/hosts/windows/cap/smb.rb b/plugins/hosts/windows/cap/smb.rb new file mode 100644 index 000000000..b990abba3 --- /dev/null +++ b/plugins/hosts/windows/cap/smb.rb @@ -0,0 +1,102 @@ +module VagrantPlugins + module HostWindows + module Cap + class SMB + + # Number of seconds to display UAC warning to user + UAC_PROMPT_WAIT = 4 + + @@logger = Log4r::Logger.new("vagrant::host::windows::smb") + + def self.smb_installed(env) + psv = Vagrant::Util::PowerShell.version.to_i + if psv < 3 + if raise_error + raise SyncedFolderSMB::Errors::PowershellVersion, + version: psv.to_s + end + return false + end + + true + end + + def self.smb_cleanup(env, machine, opts) + script_path = File.expand_path("../../scripts/unset_share.ps1", __FILE__) + + m_id = machine_id(machine) + result = Vagrant::Util::PowerShell.execute_cmd("net share") + if result.nil? + @@logger.warn("failed to get current share list") + return + end + prune_shares = result.split("\n").map do |line| + sections = line.split(/\s/) + if sections.first.to_s.start_with?("vgt-#{m_id}") + sections.first + end + end.compact + @@logger.debug("shares to be removed: #{prune_shares}") + + if prune_shares.size > 0 + machine.env.ui.warn("\n" + I18n.t("vagrant_sf_smb.uac.prune_warning") + "\n") + sleep UAC_PROMPT_WAIT + @@logger.info("remove shares: #{prune_shares}") + result = Vagrant::Util::PowerShell.execute(script_path, *prune_shares, sudo: true) + if result.exit_code != 0 + failed_name = result.stdout.to_s.sub("share name: ", "") + raise SyncedFolderSMB::Errors::PruneShareFailed, + name: failed_name, + stderr: result.stderr, + stdout: result.stdout + end + end + end + + def self.smb_prepare(env, machine, folders, opts) + script_path = File.expand_path("../../scripts/set_share.ps1", __FILE__) + + shares = [] + folders.each do |id, data| + hostpath = data[:hostpath] + + chksum_id = Digest::MD5.hexdigest(id) + name = "vgt-#{machine_id(machine)}-#{chksum_id}" + data[:smb_id] ||= name + + @@logger.info("creating new share name=#{name} id=#{data[:smb_id]}") + + shares << [ + "\"#{hostpath.gsub("/", "\\")}\"", + name, + data[:smb_id] + ] + end + if !shares.empty? + machine.env.ui.warn("\n" + I18n.t("vagrant_sf_smb.uac.create_warning") + "\n") + sleep(UAC_PROMPT_WAIT) + result = Vagrant::Util::PowerShell.execute(script_path, *shares, sudo: true) + if result.exit_code != 0 + share_path = result.stdout.to_s.sub("share path: ", "") + raise SyncedFolderSMB::Errors::DefineShareFailed, + host: share_path, + stderr: result.stderr, + stdout: result.stdout + end + end + end + + # Generates a unique identifier for the given machine + # based on the name, provider name, and working directory + # of the environment. + # + # @param [Vagrant::Machine] machine + # @return [String] + def self.machine_id(machine) + @@logger.debug("generating machine ID name=#{machine.name} cwd=#{machine.env.cwd}") + Digest::MD5.hexdigest("#{machine.name}-#{machine.provider_name}-#{machine.env.cwd}") + end + end + end + end +end diff --git a/plugins/hosts/windows/plugin.rb b/plugins/hosts/windows/plugin.rb index 84a38836b..38691d0a8 100644 --- a/plugins/hosts/windows/plugin.rb +++ b/plugins/hosts/windows/plugin.rb @@ -30,6 +30,26 @@ module VagrantPlugins require_relative "cap/ps" Cap::PS end + + host_capability("windows", "smb_installed") do + require_relative "cap/smb" + Cap::SMB + end + + host_capability("windows", "smb_prepare") do + require_relative "cap/smb" + Cap::SMB + end + + host_capability("windows", "smb_cleanup") do + require_relative "cap/smb" + Cap::SMB + end + + host_capability("windows", "configured_ip_addresses") do + require_relative "cap/configured_ip_addresses" + Cap::ConfiguredIPAddresses + end end end end diff --git a/plugins/synced_folders/smb/scripts/host_info.ps1 b/plugins/hosts/windows/scripts/host_info.ps1 similarity index 100% rename from plugins/synced_folders/smb/scripts/host_info.ps1 rename to plugins/hosts/windows/scripts/host_info.ps1 diff --git a/plugins/hosts/windows/scripts/set_share.ps1 b/plugins/hosts/windows/scripts/set_share.ps1 new file mode 100644 index 000000000..a102e08c2 --- /dev/null +++ b/plugins/hosts/windows/scripts/set_share.ps1 @@ -0,0 +1,39 @@ +# The names of the user are language dependent! +$objSID = New-Object System.Security.Principal.SecurityIdentifier("S-1-1-0") +$objUser = $objSID.Translate([System.Security.Principal.NTAccount]) + +$grant = "$objUser,Full" + +# First we split the defs string by commas to get +# each group of parameters +for ($i=0; $i -le $args.length; $i = $i + 3) { + $path = $args[$i] + $share_name = $args[$i+1] + $share_id = $args[$i+2] + + + if ($path -eq $null) { + Write-Warning "empty path argument encountered - complete" + exit 0 + } + + if ($share_name -eq $null) { + Write-Output "share path: ${path}" + Write-Error "error - no share name provided" + exit 1 + } + + if ($share_id -eq $null) { + Write-Output "share path: ${path}" + Write-Error "error - no share ID provided" + exit 1 + } + + $result = net share $share_name=$path /unlimited /GRANT:$grant /REMARK:"${share_id}" + if ($LastExitCode -ne 0) { + $host.ui.WriteLine("share path: ${path}") + $host.ui.WriteErrorLine("error ${result}") + exit 1 + } +} +exit 0 diff --git a/plugins/hosts/windows/scripts/unset_share.ps1 b/plugins/hosts/windows/scripts/unset_share.ps1 new file mode 100644 index 000000000..ebbfda59b --- /dev/null +++ b/plugins/hosts/windows/scripts/unset_share.ps1 @@ -0,0 +1,11 @@ +# Share names are comma delimited +ForEach ($share_name in $args) { + $result = net share $share_name /DELETE + if ($LastExitCode -ne 0) { + Write-Output "share name: ${share_name}" + Write-Error "error - ${result}" + exit 1 + } +} +Write-Output "share removal completed" +exit 0 diff --git a/plugins/synced_folders/smb/errors.rb b/plugins/synced_folders/smb/errors.rb index 4a5d2503e..f660b46e5 100644 --- a/plugins/synced_folders/smb/errors.rb +++ b/plugins/synced_folders/smb/errors.rb @@ -10,6 +10,10 @@ module VagrantPlugins error_key(:define_share_failed) end + class PruneShareFailed < SMBError + error_key(:prune_share_failed) + end + class NoHostIPAddr < SMBError error_key(:no_routable_host_addr) end diff --git a/plugins/synced_folders/smb/scripts/set_share.ps1 b/plugins/synced_folders/smb/scripts/set_share.ps1 deleted file mode 100644 index 469483f38..000000000 --- a/plugins/synced_folders/smb/scripts/set_share.ps1 +++ /dev/null @@ -1,44 +0,0 @@ -Param( - [Parameter(Mandatory=$true)] - [string]$path, - [Parameter(Mandatory=$true)] - [string]$share_name, - [string]$host_share_username = $null -) - -$ErrorAction = "Stop" - -if (net share | Select-String $share_name) { - net share $share_name /delete /y -} - -# The names of the user are language dependent! -$objSID = New-Object System.Security.Principal.SecurityIdentifier("S-1-1-0") -$objUser = $objSID.Translate([System.Security.Principal.NTAccount]) - -$grant = "$objUser,Full" - -if (![string]::IsNullOrEmpty($host_share_username)) { - $computer_name = $(Get-WmiObject Win32_Computersystem).name - $grant = "$computer_name\$host_share_username,Full" - - # Here we need to set the proper ACL for this folder. This lets full - # recursive access to this folder. - <# - Get-ChildItem $path -recurse -Force |% { - $current_acl = Get-ACL $_.fullname - $permission = "$computer_name\$host_share_username","FullControl","ContainerInherit,ObjectInherit","None","Allow" - $acl_access_rule = New-Object System.Security.AccessControl.FileSystemAccessRule $permission - $current_acl.SetAccessRule($acl_access_rule) - $current_acl | Set-Acl $_.fullname - } - #> -} - -$result = net share $share_name=$path /unlimited /GRANT:$grant -if ($LastExitCode -eq 0) { - exit 0 -} - -$host.ui.WriteErrorLine("Error: $result") -exit 1 diff --git a/plugins/synced_folders/smb/synced_folder.rb b/plugins/synced_folders/smb/synced_folder.rb index acdcc802e..dcbb903c6 100644 --- a/plugins/synced_folders/smb/synced_folder.rb +++ b/plugins/synced_folders/smb/synced_folder.rb @@ -17,69 +17,46 @@ module VagrantPlugins end def usable?(machine, raise_error=false) - if !Vagrant::Util::Platform.windows? - raise Errors::WindowsHostRequired if raise_error - return false - end - - if !Vagrant::Util::Platform.windows_admin? - raise Errors::WindowsAdminRequired if raise_error - return false - end - - psv = Vagrant::Util::PowerShell.version.to_i - if psv < 3 - if raise_error - raise Errors::PowershellVersion, - version: psv.to_s - end - return false - end - - true + # If the machine explicitly states SMB is not supported, then + # believe it + return false if !machine.config.smb.functional + return true if machine.env.host.capability?(:smb_installed) && + machine.env.host.capability(:smb_installed) + return false if !raise_error + raise Vagrant::Errors::SMBNotSupported end def prepare(machine, folders, opts) machine.ui.output(I18n.t("vagrant_sf_smb.preparing")) - script_path = File.expand_path("../scripts/set_share.ps1", __FILE__) + smb_username = smb_password = nil # If we need auth information, then ask the user. have_auth = false folders.each do |id, data| if data[:smb_username] && data[:smb_password] - @creds[:username] = data[:smb_username] - @creds[:password] = data[:smb_password] + smb_username = data[:smb_username] + smb_password = data[:smb_password] have_auth = true break end end if !have_auth - machine.ui.detail(I18n.t("vagrant_sf_smb.warning_password") + "\n ") - @creds[:username] = machine.ui.ask("Username: ") - @creds[:password] = machine.ui.ask("Password (will be hidden): ", echo: false) + machine.env.ui.detail(I18n.t("vagrant_sf_smb.warning_password") + "\n ") + smb_username = machine.env.ui.ask("Username: ") + smb_password = machine.env.ui.ask("Password (will be hidden): ", echo: false) end folders.each do |id, data| - hostpath = data[:hostpath] + data[:smb_username] ||= smb_username + data[:smb_password] ||= smb_password - data[:smb_id] ||= Digest::MD5.hexdigest( - "#{machine.id}-#{id.gsub("/", "-")}") - - args = [] - args << "-path" << "\"#{hostpath.gsub("/", "\\")}\"" - args << "-share_name" << data[:smb_id] - #args << "-host_share_username" << @creds[:username] - - r = Vagrant::Util::PowerShell.execute(script_path, *args) - if r.exit_code != 0 - raise Errors::DefineShareFailed, - host: hostpath.to_s, - stderr: r.stderr, - stdout: r.stdout - end + # Register password as sensitive + Vagrant::Util::CredentialScrubber.sensitive(smb_password) end + + machine.env.host.capability(:smb_prepare, machine, folders, opts) end def enable(machine, folders, nfsopts) @@ -109,7 +86,7 @@ module VagrantPlugins end if need_host_ip - candidate_ips = load_host_ips + candidate_ips = machine.env.host.capability(:configured_ip_addresses) @logger.debug("Potential host IPs: #{candidate_ips.inspect}") host_ip = machine.guest.capability( :choose_addressable_ip_addr, candidate_ips) @@ -141,25 +118,8 @@ module VagrantPlugins end def cleanup(machine, opts) - - end - - protected - - def load_host_ips - script_path = File.expand_path("../scripts/host_info.ps1", __FILE__) - r = Vagrant::Util::PowerShell.execute(script_path) - if r.exit_code != 0 - raise Errors::PowershellError, - script: script_path, - stderr: r.stderr - end - - res = JSON.parse(r.stdout)["ip_addresses"] - if res.instance_of? String - [ res ] - else - res + if machine.env.host.capability?(:smb_cleanup) + machine.env.host.capability(:smb_cleanup, machine, opts) end end end diff --git a/templates/locales/synced_folder_smb.yml b/templates/locales/synced_folder_smb.yml index 442c99405..736640054 100644 --- a/templates/locales/synced_folder_smb.yml +++ b/templates/locales/synced_folder_smb.yml @@ -11,6 +11,13 @@ en: folders shortly. Please use the proper username/password of your Windows account. + uac: + prune_warning: |- + Vagrant requires administator access for pruning SMB shares and + may request access to complete removal of stale shares. + create_warning: |- + Vagrant requires administator access to create SMB shares and + may request access to complete setup of configured shares. errors: define_share_failed: |- Exporting an SMB share failed! Details about the failure are shown @@ -20,6 +27,15 @@ en: Stderr: %{stderr} + Stdout: %{stdout} + prune_share_failed: |- + Pruning an SMB share failed! Details about the failure are shown + below. Please inspect the error message and correct any problems. + + Share name: %{name} + + Stderr: %{stderr} + Stdout: %{stdout} no_routable_host_addr: |- We couldn't detect an IP address that was routable to this From 5a607d924856d62d9edd0c28b843706c95f58ea2 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 20 Dec 2017 16:26:27 -0800 Subject: [PATCH 03/30] Clean up SMB related errors --- plugins/synced_folders/smb/errors.rb | 4 ++++ plugins/synced_folders/smb/synced_folder.rb | 12 +++++------- templates/locales/synced_folder_smb.yml | 4 ++++ 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/plugins/synced_folders/smb/errors.rb b/plugins/synced_folders/smb/errors.rb index f660b46e5..14704b9bb 100644 --- a/plugins/synced_folders/smb/errors.rb +++ b/plugins/synced_folders/smb/errors.rb @@ -6,6 +6,10 @@ module VagrantPlugins error_namespace("vagrant_sf_smb.errors") end + class SMBNotSupported < SMBError + error_key(:not_supported) + end + class DefineShareFailed < SMBError error_key(:define_share_failed) end diff --git a/plugins/synced_folders/smb/synced_folder.rb b/plugins/synced_folders/smb/synced_folder.rb index dcbb903c6..29a60982a 100644 --- a/plugins/synced_folders/smb/synced_folder.rb +++ b/plugins/synced_folders/smb/synced_folder.rb @@ -6,6 +6,8 @@ require "log4r" require "vagrant/util/platform" require "vagrant/util/powershell" +require_relative "errors" + module VagrantPlugins module SyncedFolderSMB class SyncedFolder < Vagrant.plugin("2", :synced_folder) @@ -13,7 +15,6 @@ module VagrantPlugins super @logger = Log4r::Logger.new("vagrant::synced_folders::smb") - @creds = {} end def usable?(machine, raise_error=false) @@ -23,7 +24,7 @@ module VagrantPlugins return true if machine.env.host.capability?(:smb_installed) && machine.env.host.capability(:smb_installed) return false if !raise_error - raise Vagrant::Errors::SMBNotSupported + raise Errors::SMBNotSupported end def prepare(machine, folders, opts) @@ -53,13 +54,13 @@ module VagrantPlugins data[:smb_password] ||= smb_password # Register password as sensitive - Vagrant::Util::CredentialScrubber.sensitive(smb_password) + Vagrant::Util::CredentialScrubber.sensitive(data[:smb_password]) end machine.env.host.capability(:smb_prepare, machine, folders, opts) end - def enable(machine, folders, nfsopts) + def enable(machine, folders, opts) machine.ui.output(I18n.t("vagrant_sf_smb.mounting")) # Make sure that this machine knows this dance @@ -99,10 +100,7 @@ module VagrantPlugins ssh_info = machine.ssh_info folders.each do |id, data| - data = data.dup data[:smb_host] ||= host_ip - data[:smb_username] ||= @creds[:username] - data[:smb_password] ||= @creds[:password] # Default the owner/group of the folder to the SSH user data[:owner] ||= ssh_info[:username] diff --git a/templates/locales/synced_folder_smb.yml b/templates/locales/synced_folder_smb.yml index 736640054..65c82de35 100644 --- a/templates/locales/synced_folder_smb.yml +++ b/templates/locales/synced_folder_smb.yml @@ -1,5 +1,9 @@ en: vagrant_sf_smb: + not_supported: |- + It appears your machine doesn't support SMB, or there is not an + adapter to enable SMB on this machine for Vagrant. Ensure SMB + host functionality is available on this machine and try again. mounting: |- Mounting SMB shared folders... mounting_single: |- From 98ce718e63ab9b3f14e94f3193c828d4079f308a Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 20 Dec 2017 16:26:51 -0800 Subject: [PATCH 04/30] Add test coverage on SMB synced folder --- .../synced_folders/smb/synced_folder_test.rb | 217 ++++++++++++++++++ 1 file changed, 217 insertions(+) create mode 100644 test/unit/plugins/synced_folders/smb/synced_folder_test.rb diff --git a/test/unit/plugins/synced_folders/smb/synced_folder_test.rb b/test/unit/plugins/synced_folders/smb/synced_folder_test.rb new file mode 100644 index 000000000..a1d40d9ef --- /dev/null +++ b/test/unit/plugins/synced_folders/smb/synced_folder_test.rb @@ -0,0 +1,217 @@ +require_relative "../../../base" + +require Vagrant.source_root.join("plugins/synced_folders/smb/synced_folder") + +describe VagrantPlugins::SyncedFolderSMB::SyncedFolder do + include_context "unit" + + let(:iso_env) do + env = isolated_environment + env.vagrantfile("") + env.create_vagrant_env + end + + let(:guest){ double("guest") } + let(:host){ double("host") } + let(:machine) { iso_env.machine(iso_env.machine_names[0], :dummy) } + let(:host_caps){ [] } + let(:guest_caps){ [] } + let(:folders){ {"/first/path" => {}, "/second/path" => {}} } + let(:options){ {} } + + before do + allow(machine.env).to receive(:host).and_return(host) + allow(machine).to receive(:guest).and_return(guest) + allow(machine).to receive(:ssh_info).and_return(username: 'sshuser') + allow(guest).to receive(:name).and_return("guest_name") + allow(host).to receive(:capability?).and_return(false) + host_caps.each do |cap| + allow(host).to receive(:capability?).with(cap).and_return(true) + allow(host).to receive(:capability).with(cap, any_args).and_return(true) + end + allow(guest).to receive(:capability?).and_return(false) + guest_caps.each do |cap| + allow(guest).to receive(:capability?).with(cap).and_return(true) + allow(guest).to receive(:capability).with(cap, any_args).and_return(true) + end + end + + describe ".usable?" do + context "without supporting capabilities" do + it "is not usable" do + expect(subject.usable?(machine)).to be(false) + end + + it "raises exception when raise_error enabled" do + expect{subject.usable?(machine, true)}.to raise_error( + VagrantPlugins::SyncedFolderSMB::Errors::SMBNotSupported) + end + end + + context "with smb not installed" do + let(:host_caps){ [:smb_installed] } + + it "is not usable" do + expect(host).to receive(:capability).with(:smb_installed).and_return(false) + expect(subject.usable?(machine)).to be(false) + end + end + + context "with smb installed" do + let(:host_caps){ [:smb_installed] } + + it "is usable" do + expect(subject.usable?(machine)).to be(true) + end + end + end + + describe ".prepare" do + let(:host_caps){ [:smb_prepare] } + + context "without credentials provided" do + before do + expect(machine.env.ui).to receive(:ask).and_return('username') + expect(machine.env.ui).to receive(:ask).and_return('password') + end + + it "should prompt for credentials" do + subject.prepare(machine, folders, options) + end + + it "should set credential information into all folder options" do + subject.prepare(machine, folders, options) + expect(folders['/first/path'][:smb_username]).to eq('username') + expect(folders['/first/path'][:smb_password]).to eq('password') + expect(folders['/second/path'][:smb_username]).to eq('username') + expect(folders['/second/path'][:smb_password]).to eq('password') + end + end + + context "with credentials provided" do + context "in single share entry" do + let(:folders){ {'/first/path' => {}, '/second/path' => {smb_username: 'smbuser', smb_password: 'smbpass'}} } + + it "should not prompt for credentials" do + expect(machine.env.ui).not_to receive(:ask) + subject.prepare(machine, folders, options) + end + + it "should add existing credentials to folder options without" do + subject.prepare(machine, folders, options) + expect(folders['/first/path'][:smb_username]).to eq('smbuser') + expect(folders['/first/path'][:smb_password]).to eq('smbpass') + end + end + + context "in both entries" do + let(:folders){ {'/first/path' => {smb_username: 'user', smb_password: 'pass'}, + '/second/path' => {smb_username: 'smbuser', smb_password: 'smbpass'}} } + + it "should not modify existing credentials" do + subject.prepare(machine, folders, options) + expect(folders['/first/path'][:smb_username]).to eq('user') + expect(folders['/first/path'][:smb_password]).to eq('pass') + expect(folders['/second/path'][:smb_username]).to eq('smbuser') + expect(folders['/second/path'][:smb_password]).to eq('smbpass') + end + + it "should register passwords with scrubber" do + expect(Vagrant::Util::CredentialScrubber).to receive(:sensitive).with('pass') + expect(Vagrant::Util::CredentialScrubber).to receive(:sensitive).with('smbpass') + subject.prepare(machine, folders, options) + end + end + end + end + + describe ".enable" do + it "fails when guest does not support capability" do + expect{ + subject.enable(machine, folders, options) + }.to raise_error(Vagrant::Errors::GuestCapabilityNotFound) + end + + context "with guest capability supported" do + let(:guest_caps){ [:mount_smb_shared_folder, :choose_addressable_ip_addr] } + let(:host_caps){ [:configured_ip_addresses] } + + it "should attempt to install smb on guest" do + expect(guest).to receive(:capability?).with(:smb_install).and_return(true) + expect(guest).to receive(:capability).with(:smb_install, any_args) + subject.enable(machine, folders, options) + end + + it "should request host IP addresses" do + expect(host).to receive(:capability).with(:configured_ip_addresses) + subject.enable(machine, folders, options) + end + + it "should determine guest accessible address" do + expect(guest).to receive(:capability).with(:choose_addressable_ip_addr, any_args) + subject.enable(machine, folders, options) + end + + it "should error if no guest accessible address is available" do + expect(guest).to receive(:capability).with(:choose_addressable_ip_addr, any_args).and_return(nil) + expect{ subject.enable(machine, folders, options) }.to raise_error( + VagrantPlugins::SyncedFolderSMB::Errors::NoHostIPAddr) + end + + it "should default owner and group to ssh username" do + subject.enable(machine, folders, options) + expect(folders["/first/path"][:owner]).to eq("sshuser") + expect(folders["/first/path"][:group]).to eq("sshuser") + expect(folders["/second/path"][:owner]).to eq("sshuser") + expect(folders["/second/path"][:group]).to eq("sshuser") + end + + it "should set the host address in folder options" do + expect(guest).to receive(:capability).with(:choose_addressable_ip_addr, any_args).and_return("ADDR") + subject.enable(machine, folders, options) + expect(folders["/first/path"][:smb_host]).to eq("ADDR") + expect(folders["/second/path"][:smb_host]).to eq("ADDR") + end + + context "with smb_host option set" do + let(:folders){ {"/first/path" => {smb_host: "ADDR"}, "/second/path" => {}} } + + it "should not update the value" do + expect(guest).to receive(:capability).with(:choose_addressable_ip_addr, any_args).and_return("OTHER") + subject.enable(machine, folders, options) + expect(folders["/first/path"][:smb_host]).to eq("ADDR") + expect(folders["/second/path"][:smb_host]).to eq("OTHER") + end + end + + context "with owner and group set" do + let(:folders){ {"/first/path" => {owner: "smbowner"}, "/second/path" => {group: "smbgroup"}} } + + it "should not update set owner or group" do + subject.enable(machine, folders, options) + expect(folders["/first/path"][:owner]).to eq("smbowner") + expect(folders["/first/path"][:group]).to eq("sshuser") + expect(folders["/second/path"][:owner]).to eq("sshuser") + expect(folders["/second/path"][:group]).to eq("smbgroup") + end + end + end + end + + describe ".cleanup" do + context "without supporting capability" do + it "does nothing" do + subject.cleanup(machine, options) + end + end + + context "with supporting capability" do + let(:host_caps){ [:smb_cleanup] } + + it "runs cleanup" do + expect(host).to receive(:capability).with(:smb_cleanup, any_args) + subject.cleanup(machine, options) + end + end + end +end From e12e2d53743aad09fc57d786713e28411933820b Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 20 Dec 2017 16:52:54 -0800 Subject: [PATCH 05/30] Add test coverage for darwin host configured addresses capability --- .../cap/configured_ip_addresses_test.rb | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 test/unit/plugins/hosts/darwin/cap/configured_ip_addresses_test.rb diff --git a/test/unit/plugins/hosts/darwin/cap/configured_ip_addresses_test.rb b/test/unit/plugins/hosts/darwin/cap/configured_ip_addresses_test.rb new file mode 100644 index 000000000..71d79ed24 --- /dev/null +++ b/test/unit/plugins/hosts/darwin/cap/configured_ip_addresses_test.rb @@ -0,0 +1,31 @@ +require_relative "../../../../base" + +require_relative "../../../../../../plugins/hosts/darwin/cap/configured_ip_addresses" + +describe VagrantPlugins::HostDarwin::Cap::ConfiguredIPAddresses do + + let(:subject){ VagrantPlugins::HostDarwin::Cap::ConfiguredIPAddresses } + let(:interfaces){ ["192.168.1.2"] } + before{ allow(Socket).to receive(:getifaddrs).and_return( + interfaces.map{|i| double(:socket, addr: Addrinfo.ip(i))}) } + + it "should get list of available addresses" do + expect(subject.configured_ip_addresses(nil)).to eq(["192.168.1.2"]) + end + + context "with loopback address" do + let(:interfaces){ ["192.168.1.2", "127.0.0.1"] } + + it "should not include loopback address" do + expect(subject.configured_ip_addresses(nil)).not_to include(["127.0.0.1"]) + end + end + + context "with IPv6 address" do + let(:interfaces){ ["192.168.1.2", "2001:200:dff:fff1:216:3eff:feb1:44d7"] } + + it "should not include IPv6 address" do + expect(subject.configured_ip_addresses(nil)).not_to include(["2001:200:dff:fff1:216:3eff:feb1:44d7"]) + end + end +end From f89c6a37f952321b11a31ba794fe49c831186466 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 21 Dec 2017 07:48:41 -0800 Subject: [PATCH 06/30] Provide namespace to error class --- plugins/hosts/darwin/cap/smb.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/hosts/darwin/cap/smb.rb b/plugins/hosts/darwin/cap/smb.rb index ba0ba1e94..dfbe4c88d 100644 --- a/plugins/hosts/darwin/cap/smb.rb +++ b/plugins/hosts/darwin/cap/smb.rb @@ -62,7 +62,7 @@ module VagrantPlugins r = Vagrant::Util::Subprocess.execute(*cmd) if r.exit_code != 0 - raise Errors::DefineShareFailed, + raise VagrantPlugins::SyncedFolderSMB::Errors::DefineShareFailed, host: hostpath.to_s, stderr: r.stderr, stdout: r.stdout From f330f81fbf578e269eac2d280f07c11b07c3f6c3 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 21 Dec 2017 07:49:01 -0800 Subject: [PATCH 07/30] Add test coverage on darwin host SMB capability --- .../unit/plugins/hosts/darwin/cap/smb_test.rb | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 test/unit/plugins/hosts/darwin/cap/smb_test.rb diff --git a/test/unit/plugins/hosts/darwin/cap/smb_test.rb b/test/unit/plugins/hosts/darwin/cap/smb_test.rb new file mode 100644 index 000000000..dbe653978 --- /dev/null +++ b/test/unit/plugins/hosts/darwin/cap/smb_test.rb @@ -0,0 +1,68 @@ +require_relative "../../../../base" + +require_relative "../../../../../../plugins/hosts/darwin/cap/smb" + +describe VagrantPlugins::HostDarwin::Cap::SMB do + let(:subject){ VagrantPlugins::HostDarwin::Cap::SMB } + let(:machine){ double(:machine) } + let(:env){ double(:env) } + let(:options){ {} } + let(:result){ Vagrant::Util::Subprocess::Result } + + before{ allow(subject).to receive(:machine_id).and_return("CUSTOM_ID") } + + describe ".smb_installed" do + it "is installed if sharing binary exists" do + expect(File).to receive(:exist?).with("/usr/sbin/sharing").and_return(true) + expect(subject.smb_installed(nil)).to be(true) + end + + it "is not installed if sharing binary does not exist" do + expect(File).to receive(:exist?).with("/usr/sbin/sharing").and_return(false) + expect(subject.smb_installed(nil)).to be(false) + end + end + + describe ".smb_cleanup" do + after{ subject.smb_cleanup(env, machine, options) } + + it "should search for shares with generated machine ID" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with( + anything, anything, /.*CUSTOM_ID.*/).and_return(result.new(0, "", "")) + end + + it "should remove shares individually" do + expect(Vagrant::Util::Subprocess).to receive(:execute). + with(anything, anything, /.*CUSTOM_ID.*/). + and_return(result.new(0, "vgt-CUSTOM_ID-1\nvgt-CUSTOM_ID-2\n", "")) + expect(Vagrant::Util::Subprocess).to receive(:execute).with(/sudo/, /sharing/, anything, /CUSTOM_ID/). + twice.and_return(result.new(0, "", "")) + end + end + + describe ".smb_prepare" do + let(:folders){ {"/first/path" => {hostpath: "/first/host", smb_id: "ID1"}, + "/second/path" => {hostpath: "/second/host"}} } + before{ allow(Vagrant::Util::Subprocess).to receive(:execute).and_return(result.new(0, "", "")) } + it "should provide ID value if not set" do + subject.smb_prepare(env, machine, folders, options) + expect(folders["/second/path"][:smb_id]).to start_with("vgt-") + end + + it "should not modify ID if already set" do + subject.smb_prepare(env, machine, folders, options) + expect(folders["/first/path"][:smb_id]).to eq("ID1") + end + + it "should raise error when sharing command fails" do + expect(Vagrant::Util::Subprocess).to receive(:execute).and_return(result.new(1, "", "")) + expect{ subject.smb_prepare(env, machine, folders, options) }.to raise_error( + VagrantPlugins::SyncedFolderSMB::Errors::DefineShareFailed) + end + + it "should add shares individually" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with(/sudo/, any_args).twice.and_return(result.new(0, "", "")) + subject.smb_prepare(env, machine, folders, options) + end + end +end From 81cbdae62a953195785eee98fe913f37e96f8efa Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 21 Dec 2017 08:06:51 -0800 Subject: [PATCH 08/30] Make powershell error generalized --- lib/vagrant/errors.rb | 4 ++++ plugins/hosts/windows/cap/configured_ip_addresses.rb | 2 +- templates/locales/synced_folder_smb.yml | 10 ---------- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index a6928bc12..544536a39 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -552,6 +552,10 @@ module Vagrant error_key(:powershell_invalid_version) end + class PowerShellError < VagrantError + error_key(:powershell_error, "vagrant_ps.errors.powershell_error") + end + class ProviderCantInstall < VagrantError error_key(:provider_cant_install) end diff --git a/plugins/hosts/windows/cap/configured_ip_addresses.rb b/plugins/hosts/windows/cap/configured_ip_addresses.rb index 43820cb5a..cc95775b5 100644 --- a/plugins/hosts/windows/cap/configured_ip_addresses.rb +++ b/plugins/hosts/windows/cap/configured_ip_addresses.rb @@ -15,7 +15,7 @@ module VagrantPlugins script_path = File.expand_path("../../scripts/host_info.ps1", __FILE__) r = Vagrant::Util::PowerShell.execute(script_path) if r.exit_code != 0 - raise Errors::PowershellError, + raise Vagrant::Errors::PowerShellError, script: script_path, stderr: r.stderr end diff --git a/templates/locales/synced_folder_smb.yml b/templates/locales/synced_folder_smb.yml index 65c82de35..3754bc408 100644 --- a/templates/locales/synced_folder_smb.yml +++ b/templates/locales/synced_folder_smb.yml @@ -49,16 +49,6 @@ en: As another option, you can manually specify an IP for the machine to mount from using the `smb_host` option to the synced folder. - powershell_error: |- - An error occurred while executing a PowerShell script. This error - is shown below. Please read the error message and see if this is - a configuration error with your system. If it is not, then please - report a bug. - - Script: %{script} - Error: - - %{stderr} powershell_version: |- PowerShell version 3 or later is required for SMB synced folders to work on Windows. You have version: '%{version}'. Please update From 63d1b5e33d027da366fafefed7de43da210cf9af Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 21 Dec 2017 08:07:24 -0800 Subject: [PATCH 09/30] Add test coverage for windows host configured IP addresses capability --- .../cap/configure_ip_addresses_test.rb | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 test/unit/plugins/hosts/windows/cap/configure_ip_addresses_test.rb diff --git a/test/unit/plugins/hosts/windows/cap/configure_ip_addresses_test.rb b/test/unit/plugins/hosts/windows/cap/configure_ip_addresses_test.rb new file mode 100644 index 000000000..b9f298c1e --- /dev/null +++ b/test/unit/plugins/hosts/windows/cap/configure_ip_addresses_test.rb @@ -0,0 +1,43 @@ +require_relative "../../../../base" + +require_relative "../../../../../../plugins/hosts/windows/cap/configured_ip_addresses" + +describe VagrantPlugins::HostWindows::Cap::ConfiguredIPAddresses do + + let(:subject){ VagrantPlugins::HostWindows::Cap::ConfiguredIPAddresses } + let(:result){ Vagrant::Util::Subprocess::Result } + let(:addresses){ [] } + let(:execute_result){ result.new(0, {ip_addresses: addresses}.to_json, "") } + + before{ allow(Vagrant::Util::PowerShell).to receive(:execute). + and_return(execute_result) } + + it "should return an array" do + expect(subject.configured_ip_addresses(nil)).to be_kind_of(Array) + end + + context "with single address returned" do + let(:addresses){ "ADDRESS" } + + it "should return an array" do + expect(subject.configured_ip_addresses(nil)).to eq([addresses]) + end + end + + context "with multiple addresses returned" do + let(:addresses){ ["ADDRESS1", "ADDRESS2"] } + + it "should return an array" do + expect(subject.configured_ip_addresses(nil)).to eq(addresses) + end + end + + context "with failed script execution" do + let(:execute_result){ result.new(1, "", "") } + + it "should raise error" do + expect{ subject.configured_ip_addresses(nil) }.to raise_error( + Vagrant::Errors::PowerShellError) + end + end +end From 61bfbade485a6b621c39c96000dc0e8a4d7a1291 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 21 Dec 2017 09:53:51 -0800 Subject: [PATCH 10/30] Remove raise error logic in windows smb capability --- plugins/hosts/windows/cap/smb.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/plugins/hosts/windows/cap/smb.rb b/plugins/hosts/windows/cap/smb.rb index b990abba3..983f3ef73 100644 --- a/plugins/hosts/windows/cap/smb.rb +++ b/plugins/hosts/windows/cap/smb.rb @@ -11,10 +11,6 @@ module VagrantPlugins def self.smb_installed(env) psv = Vagrant::Util::PowerShell.version.to_i if psv < 3 - if raise_error - raise SyncedFolderSMB::Errors::PowershellVersion, - version: psv.to_s - end return false end From 3fd9f44921528fbe279824c26ffae42e2f338a0c Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 21 Dec 2017 09:54:24 -0800 Subject: [PATCH 11/30] Add test coverage for windows host smb capability --- .../plugins/hosts/windows/cap/smb_test.rb | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 test/unit/plugins/hosts/windows/cap/smb_test.rb diff --git a/test/unit/plugins/hosts/windows/cap/smb_test.rb b/test/unit/plugins/hosts/windows/cap/smb_test.rb new file mode 100644 index 000000000..20c09d1ef --- /dev/null +++ b/test/unit/plugins/hosts/windows/cap/smb_test.rb @@ -0,0 +1,108 @@ +require_relative "../../../../base" + +require_relative "../../../../../../plugins/hosts/windows/cap/smb" + +describe VagrantPlugins::HostWindows::Cap::SMB do + let(:subject){ VagrantPlugins::HostWindows::Cap::SMB } + let(:machine){ double(:machine, env: double(:machine_env, ui: double(:ui))) } + let(:env){ double(:env) } + let(:options){ {} } + let(:result){ Vagrant::Util::Subprocess::Result } + let(:powershell_version){ "3" } + + before do + allow(subject).to receive(:machine_id).and_return("CUSTOM_ID") + allow(Vagrant::Util::PowerShell).to receive(:version).and_return(powershell_version) + allow(machine.env.ui).to receive(:warn) + allow(subject).to receive(:sleep) + end + + describe ".smb_installed" do + context "when powershell version is greater than 2" do + it "is valid installation" do + expect(subject.smb_installed(nil)).to eq(true) + end + end + + context "when powershell version is less than 3" do + let(:powershell_version){ "2" } + + it "is not a valid installation" do + expect(subject.smb_installed(nil)).to eq(false) + end + end + end + + describe ".smb_cleanup" do + before do + allow(Vagrant::Util::PowerShell).to receive(:execute_cmd).with("net share"). + and_return("vgt-CUSTOM_ID-1\nvgt-CUSTOM_ID-2\n") + allow(Vagrant::Util::PowerShell).to receive(:execute).and_return(result.new(0, "", "")) + end + after{ subject.smb_cleanup(env, machine, options) } + + it "should pause after warning user" do + expect(machine.env.ui).to receive(:warn) + expect(subject).to receive(:sleep) + end + + it "should remove all shares in single call" do + expect(Vagrant::Util::PowerShell).to receive(:execute).with(any_args, sudo: true).once + end + + context "when no shares are defined" do + before do + expect(Vagrant::Util::PowerShell).to receive(:execute_cmd).with("net share"). + and_return("") + end + + it "should not attempt to remove shares" do + expect(Vagrant::Util::PowerShell).not_to receive(:execute).with(any_args, sudo: true) + end + + it "should not warn user" do + expect(machine.env.ui).not_to receive(:warn) + end + end + end + + describe ".smb_prepare" do + let(:folders){ {"/first/path" => {hostpath: "/host/1"}, "/second/path" => {hostpath: "/host/2", smb_id: "ID1"}} } + let(:options){ {} } + + before{ allow(Vagrant::Util::PowerShell).to receive(:execute).and_return(result.new(0, "", "")) } + + it "should add ID when not defined" do + subject.smb_prepare(env, machine, folders, options) + expect(folders["/first/path"][:smb_id]).to start_with("vgt-") + end + + it "should not modify ID when defined" do + subject.smb_prepare(env, machine, folders, options) + expect(folders["/second/path"][:smb_id]).to eq("ID1") + end + + it "should pause after warning user" do + expect(machine.env.ui).to receive(:warn) + expect(subject).to receive(:sleep) + subject.smb_prepare(env, machine, folders, options) + end + + it "should add all shares in single call" do + expect(Vagrant::Util::PowerShell).to receive(:execute).with(any_args, sudo: true).once + subject.smb_prepare(env, machine, folders, options) + end + + context "when no shared are defined" do + after{ subject.smb_prepare(env, machine, {}, options) } + + it "should not attempt to add shares" do + expect(Vagrant::Util::PowerShell).not_to receive(:execute).with(any_args, sudo: true) + end + + it "should not warn user" do + expect(machine.env.ui).not_to receive(:warn) + end + end + end +end From aabf4d689cc8d5e3e1be751e0572a5db93629b9e Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 21 Dec 2017 10:05:50 -0800 Subject: [PATCH 12/30] Fix up NFS test to ignore smb capability requests --- .../providers/virtualbox/action/prepare_nfs_settings_test.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unit/plugins/providers/virtualbox/action/prepare_nfs_settings_test.rb b/test/unit/plugins/providers/virtualbox/action/prepare_nfs_settings_test.rb index 9345b4011..2a854671b 100644 --- a/test/unit/plugins/providers/virtualbox/action/prepare_nfs_settings_test.rb +++ b/test/unit/plugins/providers/virtualbox/action/prepare_nfs_settings_test.rb @@ -30,6 +30,8 @@ describe VagrantPlugins::ProviderVirtualBox::Action::PrepareNFSSettings do env[:test] = true allow(machine.env).to receive(:host) { host } allow(host).to receive(:capability).with(:nfs_installed) { true } + # We don't care about smb support so return not installed + allow(host).to receive(:capability?).with(:smb_installed).and_return(false) end it "calls the next action in the chain" do From 88d2b3676fd6e6556e7d9ab67d2c36ebab7fbd85 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 21 Dec 2017 10:19:28 -0800 Subject: [PATCH 13/30] Remove stale comments from helper scripts --- plugins/hosts/windows/scripts/set_share.ps1 | 2 -- plugins/hosts/windows/scripts/unset_share.ps1 | 1 - 2 files changed, 3 deletions(-) diff --git a/plugins/hosts/windows/scripts/set_share.ps1 b/plugins/hosts/windows/scripts/set_share.ps1 index a102e08c2..8c4e077b5 100644 --- a/plugins/hosts/windows/scripts/set_share.ps1 +++ b/plugins/hosts/windows/scripts/set_share.ps1 @@ -4,8 +4,6 @@ $objUser = $objSID.Translate([System.Security.Principal.NTAccount]) $grant = "$objUser,Full" -# First we split the defs string by commas to get -# each group of parameters for ($i=0; $i -le $args.length; $i = $i + 3) { $path = $args[$i] $share_name = $args[$i+1] diff --git a/plugins/hosts/windows/scripts/unset_share.ps1 b/plugins/hosts/windows/scripts/unset_share.ps1 index ebbfda59b..4fec74d30 100644 --- a/plugins/hosts/windows/scripts/unset_share.ps1 +++ b/plugins/hosts/windows/scripts/unset_share.ps1 @@ -1,4 +1,3 @@ -# Share names are comma delimited ForEach ($share_name in $args) { $result = net share $share_name /DELETE if ($LastExitCode -ne 0) { From 2caf109a030fe0ec11d09903f44195bf8e179f4b Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 21 Dec 2017 12:49:34 -0800 Subject: [PATCH 14/30] Mark passwords as sensitive within guest capabilities --- plugins/guests/darwin/cap/mount_smb_shared_folder.rb | 3 +++ plugins/guests/linux/cap/mount_smb_shared_folder.rb | 5 +++-- plugins/guests/windows/cap/mount_shared_folder.rb | 4 ++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/plugins/guests/darwin/cap/mount_smb_shared_folder.rb b/plugins/guests/darwin/cap/mount_smb_shared_folder.rb index 5f1cdaec4..d2bab0806 100644 --- a/plugins/guests/darwin/cap/mount_smb_shared_folder.rb +++ b/plugins/guests/darwin/cap/mount_smb_shared_folder.rb @@ -20,6 +20,9 @@ module VagrantPlugins end smb_password = Shellwords.shellescape(options[:smb_password]) + # Ensure password is scrubbed + Vagrant::Util::CredentialScrubber.sensitive(smb_password) + mount_options = options[:mount_options]; mount_command = "mount -t smbfs " + (mount_options ? "-o '#{mount_options.join(",")}' " : "") + diff --git a/plugins/guests/linux/cap/mount_smb_shared_folder.rb b/plugins/guests/linux/cap/mount_smb_shared_folder.rb index f2a2cb7c3..dfc592639 100644 --- a/plugins/guests/linux/cap/mount_smb_shared_folder.rb +++ b/plugins/guests/linux/cap/mount_smb_shared_folder.rb @@ -22,6 +22,8 @@ module VagrantPlugins # If a domain is provided in the username, separate it username, domain = (options[:smb_username] || '').split('@', 2) smb_password = options[:smb_password] + # Ensure password is scrubbed + Vagrant::Util::CredentialScrubber.sensitive(smb_password) options[:mount_options] ||= [] if machine.env.host.capability?(:smb_mount_options) @@ -61,9 +63,8 @@ SCRIPT end end if status != 0 || no_such_device - clean_command = mount_command.gsub(smb_password, "PASSWORDHIDDEN") raise Vagrant::Errors::LinuxMountFailed, - command: clean_command, + command: mount_command, output: stderr end end diff --git a/plugins/guests/windows/cap/mount_shared_folder.rb b/plugins/guests/windows/cap/mount_shared_folder.rb index 4a313849b..f1088f6b8 100644 --- a/plugins/guests/windows/cap/mount_shared_folder.rb +++ b/plugins/guests/windows/cap/mount_shared_folder.rb @@ -18,6 +18,10 @@ module VagrantPlugins end def self.mount_smb_shared_folder(machine, name, guestpath, options) + if !options[:smb_password].to_s.empty? + # Ensure password is scrubbed + Vagrant::Util::CredentialScrubber.sensitive(options[:smb_password]) + end machine.communicate.execute("cmdkey /add:#{options[:smb_host]} /user:#{options[:smb_username]} /pass:#{options[:smb_password]}", {shell: :powershell, elevated: true}) mount_shared_folder(machine, name, guestpath, "\\\\#{options[:smb_host]}\\") end From 00d250b994b23e7d0d7094e02d16db1811ae9be2 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 21 Dec 2017 12:50:06 -0800 Subject: [PATCH 15/30] Add SMB entries to OSX sudoers file --- contrib/sudoers/osx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/contrib/sudoers/osx b/contrib/sudoers/osx index 96f7f4978..5e218db16 100644 --- a/contrib/sudoers/osx +++ b/contrib/sudoers/osx @@ -1,5 +1,6 @@ Cmnd_Alias VAGRANT_EXPORTS_ADD = /usr/bin/tee -a /etc/exports Cmnd_Alias VAGRANT_NFSD = /sbin/nfsd restart Cmnd_Alias VAGRANT_EXPORTS_REMOVE = /usr/bin/sed -E -e /*/ d -ibak /etc/exports -%admin ALL=(root) NOPASSWD: VAGRANT_EXPORTS_ADD, VAGRANT_NFSD, VAGRANT_EXPORTS_REMOVE - +Cmnd_Alias VAGRANT_SMB_ADD = /usr/sbin/sharing -a * -S * -s * -g * -n * +Cmnd_Alias VAGRANT_SMB_REMOVE = /usr/sbin/sharing -r * +%admin ALL=(root) NOPASSWD: VAGRANT_EXPORTS_ADD, VAGRANT_NFSD, VAGRANT_EXPORTS_REMOVE, VAGRANT_SMB_ADD, VAGRANT_SMB_REMOVE From c9e4a400f2cf8a5c03a98613fab01e6df38b5d4f Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 21 Dec 2017 12:51:13 -0800 Subject: [PATCH 16/30] Update SMB synced folder documentation --- .../source/docs/synced-folders/smb.html.md | 55 ++++++++++++------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/website/source/docs/synced-folders/smb.html.md b/website/source/docs/synced-folders/smb.html.md index 6b88857e4..a927f3207 100644 --- a/website/source/docs/synced-folders/smb.html.md +++ b/website/source/docs/synced-folders/smb.html.md @@ -19,17 +19,46 @@ SMB is built-in to Windows machines and provides a higher performance alternative to some other mechanisms such as VirtualBox shared folders.
- Windows only! SMB is currently only supported - when the host machine is Windows. The guest machine can be Windows - or Linux. + SMB is currently only supported when the host machine is Windows or + macOS. The guest machine can be Windows, Linux, or macOS.
## Prerequisites -To use the SMB synced folder type, the machine running Vagrant must be -a Windows machine with PowerShell version 3 or later installed. In addition to this, the command prompt executing Vagrant -must have administrative privileges. Vagrant requires these privileges in -order to create new network folder shares. +### Windows Host + +To use the SMB synced folder type on a Windows host, the machine must have +PowerShell version 3 or later installed. In addition, when Vagrant attempts +to create new SMB shares, or remove existing SMB shares, Administrator +privileges will be required. Vagrant will request these privileges using UAC. + +### macOS Host + +To use the SMB synced folder type on a macOS host, file sharing must be enabled +for the local account. Enable SMB file sharing by following the instructions +below: + +* Open "System Preferences" +* Click "Sharing" +* Check the "On" checkbox next to "File Sharing" +* Click "Options" +* Check "Share files and folders using SMB" +* Check the "On" checkbox next to your username within "Windows File Sharing" +* Click "Done" + +When Vagrant attempts to create new SMB shares, or remove existing SMB shares, +root access will be required. Vagrant will request these privileges using +`sudo` to run the `/usr/sbin/sharing` command. Adding the following to +the system's `sudoers` configuration will allow Vagrant to manage SMB shares +without requiring a password each time: + +``` +Cmnd_Alias VAGRANT_SMB_ADD = /usr/sbin/sharing -a * -S * -s * -g * -n * +Cmnd_Alias VAGRANT_SMB_REMOVE = /usr/sbin/sharing -r * +%admin ALL=(root) NOPASSWD: VAGRANT_SMB_ADD, VAGRANT_SMB_REMOVE +``` + +### Guests The destination machine must be able to mount SMB filesystems. On Linux the package to do this is usually called `smbfs` or `cifs`. Vagrant knows @@ -75,18 +104,6 @@ shell. Note that you should research if this is the right option for you. net config server /autodisconnect:-1 ``` -## Limitations - -Because SMB is a relatively new synced folder type in Vagrant, it still -has some rough edges. Hopefully, future versions of Vagrant will address -these. - -The primary limitation of SMB synced folders at the moment is that they are -never pruned or cleaned up. Once the folder share is defined, Vagrant never -removes it. To clean up SMB synced folder shares, periodically run -`net share` in a command prompt, find the shares you do not want, then -run `net share NAME /delete` for each, where NAME is the name of the share. - ## Common Issues ### "wrong fs type" Error From 68439f6bacc0fc88de4cbf17b726e080740d2938 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 21 Dec 2017 14:53:55 -0800 Subject: [PATCH 17/30] Use sudo for sharing lists on darwin platform On systems prior to high sierra the sharing binary requires root user for access, so use sudo to get full list output and inspect output. --- contrib/sudoers/osx | 3 ++- plugins/hosts/darwin/cap/smb.rb | 10 +++++++--- templates/locales/synced_folder_smb.yml | 2 +- website/source/docs/synced-folders/smb.html.md | 3 ++- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/contrib/sudoers/osx b/contrib/sudoers/osx index 5e218db16..d1e1d6606 100644 --- a/contrib/sudoers/osx +++ b/contrib/sudoers/osx @@ -3,4 +3,5 @@ Cmnd_Alias VAGRANT_NFSD = /sbin/nfsd restart Cmnd_Alias VAGRANT_EXPORTS_REMOVE = /usr/bin/sed -E -e /*/ d -ibak /etc/exports Cmnd_Alias VAGRANT_SMB_ADD = /usr/sbin/sharing -a * -S * -s * -g * -n * Cmnd_Alias VAGRANT_SMB_REMOVE = /usr/sbin/sharing -r * -%admin ALL=(root) NOPASSWD: VAGRANT_EXPORTS_ADD, VAGRANT_NFSD, VAGRANT_EXPORTS_REMOVE, VAGRANT_SMB_ADD, VAGRANT_SMB_REMOVE +Cmnd_Alias VAGRANT_SMB_LIST = /usr/sbin/sharing -l +%admin ALL=(root) NOPASSWD: VAGRANT_EXPORTS_ADD, VAGRANT_NFSD, VAGRANT_EXPORTS_REMOVE, VAGRANT_SMB_ADD, VAGRANT_SMB_REMOVE, VAGRANT_SMB_LIST diff --git a/plugins/hosts/darwin/cap/smb.rb b/plugins/hosts/darwin/cap/smb.rb index dfbe4c88d..263d2e4c8 100644 --- a/plugins/hosts/darwin/cap/smb.rb +++ b/plugins/hosts/darwin/cap/smb.rb @@ -18,12 +18,16 @@ module VagrantPlugins def self.smb_cleanup(env, machine, opts) m_id = machine_id(machine) - result = Vagrant::Util::Subprocess.execute("/bin/sh", "-c", - "/usr/sbin/sharing -l | grep -E \"^name:.+\\svgt-#{m_id}-\" | awk '{print $2}'") + result = Vagrant::Util::Subprocess.execute("/usr/bin/sudo", "/usr/sbin/sharing", "-l") if result.exit_code != 0 @@logger.warn("failed to locate any shares for cleanup") end - shares = result.stdout.split(/\s/).map(&:strip) + shares = result.stdout.split("\n").map do |line| + if line.start_with?("name:") + share_name = line.sub("name:", "").strip + share_name if share_name.start_with?("vgt-#{m_id}") + end + end.compact @@logger.debug("shares to be removed: #{shares}") shares.each do |share_name| @@logger.info("removing share name=#{share_name}") diff --git a/templates/locales/synced_folder_smb.yml b/templates/locales/synced_folder_smb.yml index 3754bc408..143a4885f 100644 --- a/templates/locales/synced_folder_smb.yml +++ b/templates/locales/synced_folder_smb.yml @@ -13,7 +13,7 @@ en: warning_password: |- You will be asked for the username and password to use for the SMB folders shortly. Please use the proper username/password of your - Windows account. + account. uac: prune_warning: |- diff --git a/website/source/docs/synced-folders/smb.html.md b/website/source/docs/synced-folders/smb.html.md index a927f3207..1b977db87 100644 --- a/website/source/docs/synced-folders/smb.html.md +++ b/website/source/docs/synced-folders/smb.html.md @@ -55,7 +55,8 @@ without requiring a password each time: ``` Cmnd_Alias VAGRANT_SMB_ADD = /usr/sbin/sharing -a * -S * -s * -g * -n * Cmnd_Alias VAGRANT_SMB_REMOVE = /usr/sbin/sharing -r * -%admin ALL=(root) NOPASSWD: VAGRANT_SMB_ADD, VAGRANT_SMB_REMOVE +Cmnd_Alias VAGRANT_SMB_LIST = /usr/sbin/sharing -l +%admin ALL=(root) NOPASSWD: VAGRANT_SMB_ADD, VAGRANT_SMB_REMOVE, VAGRANT_SMB_LIST ``` ### Guests From 98ec1af30e5f767f87c30290b08cc8cc1a177582 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 21 Dec 2017 15:57:57 -0800 Subject: [PATCH 18/30] Add smb_start capability for darwin --- contrib/sudoers/osx | 4 +- plugins/hosts/darwin/cap/smb.rb | 34 +++++++++++++ plugins/hosts/darwin/plugin.rb | 5 ++ plugins/synced_folders/smb/errors.rb | 8 +++ plugins/synced_folders/smb/synced_folder.rb | 5 ++ templates/locales/synced_folder_smb.yml | 20 ++++++-- .../unit/plugins/hosts/darwin/cap/smb_test.rb | 51 +++++++++++++++++-- .../synced_folders/smb/synced_folder_test.rb | 7 ++- .../source/docs/synced-folders/smb.html.md | 4 +- 9 files changed, 129 insertions(+), 9 deletions(-) diff --git a/contrib/sudoers/osx b/contrib/sudoers/osx index d1e1d6606..fc0c12c0d 100644 --- a/contrib/sudoers/osx +++ b/contrib/sudoers/osx @@ -4,4 +4,6 @@ Cmnd_Alias VAGRANT_EXPORTS_REMOVE = /usr/bin/sed -E -e /*/ d -ibak /etc/exports Cmnd_Alias VAGRANT_SMB_ADD = /usr/sbin/sharing -a * -S * -s * -g * -n * Cmnd_Alias VAGRANT_SMB_REMOVE = /usr/sbin/sharing -r * Cmnd_Alias VAGRANT_SMB_LIST = /usr/sbin/sharing -l -%admin ALL=(root) NOPASSWD: VAGRANT_EXPORTS_ADD, VAGRANT_NFSD, VAGRANT_EXPORTS_REMOVE, VAGRANT_SMB_ADD, VAGRANT_SMB_REMOVE, VAGRANT_SMB_LIST +Cmnd_Alias VAGRANT_SMB_PSTART = /bin/launchctl load -w /System/Library/LaunchDaemons/com.apple.smb.preferences.plist +Cmnd_Alias VAGRANT_SMB_DSTART = /bin/launchctl load -w /System/Library/LaunchDaemons/com.apple.smb.preferences.plist +%admin ALL=(root) NOPASSWD: VAGRANT_EXPORTS_ADD, VAGRANT_NFSD, VAGRANT_EXPORTS_REMOVE, VAGRANT_SMB_ADD, VAGRANT_SMB_REMOVE, VAGRANT_SMB_LIST, VAGRANT_SMB_PSTART, VAGRANT_SMB_DSTART \ No newline at end of file diff --git a/plugins/hosts/darwin/cap/smb.rb b/plugins/hosts/darwin/cap/smb.rb index 263d2e4c8..8581921f4 100644 --- a/plugins/hosts/darwin/cap/smb.rb +++ b/plugins/hosts/darwin/cap/smb.rb @@ -10,6 +10,40 @@ module VagrantPlugins File.exist?("/usr/sbin/sharing") end + # Check if the required SMB services are loaded and enabled. If they are + # not, then start them up + def self.smb_start(env) + result = Vagrant::Util::Subprocess.execute("pwpolicy", "gethashtypes") + if result.exit_code == 0 && !result.stdout.include?("SMB-NT") + @@logger.error("SMB compatible password has not been stored") + raise SyncedFolderSMB::Errors::SMBCredentialsMissing + end + result = Vagrant::Util::Subprocess.execute("launchctl", "list", "com.apple.smb.preferences") + if result.exit_code != 0 + @@logger.warn("smb preferences service not enabled. enabling and starting...") + cmd = ["/bin/launchctl", "load", "-w", "/System/Library/LaunchDaemons/com.apple.smb.preferences.plist"] + result = Vagrant::Util::Subprocess.execute("/usr/bin/sudo", *cmd) + if result.exit_code != 0 + raise SyncedFolderSMB::Errors::SMBStartFailed, + command: cmd.join(" "), + stderr: result.stderr, + stdout: result.stdout + end + end + result = Vagrant::Util::Subprocess.execute("launchctl", "list", "com.apple.smbd") + if result.exit_code != 0 + @@logger.warn("smbd service not enabled. enabling and starting...") + cmd = ["/bin/launchctl", "load", "-w", "/System/Library/LaunchDaemons/com.apple.smbd.plist"] + result = Vagrant::Util::Subprocess.execute("/usr/bin/sudo", *cmd) + if result.exit_code != 0 + raise SyncedFolderSMB::Errors::SMBStartFailed, + command: cmd.join(" "), + stderr: result.stderr, + stdout: result.stdout + end + end + end + # Required options for mounting a share hosted # on macos. def self.smb_mount_options(env) diff --git a/plugins/hosts/darwin/plugin.rb b/plugins/hosts/darwin/plugin.rb index 92cb51787..7b08af3f1 100644 --- a/plugins/hosts/darwin/plugin.rb +++ b/plugins/hosts/darwin/plugin.rb @@ -41,6 +41,11 @@ module VagrantPlugins Cap::SMB end + host_capability("darwin", "smb_start") do + require_relative "cap/smb" + Cap::SMB + end + host_capability("darwin", "configured_ip_addresses") do require_relative "cap/configured_ip_addresses" Cap::ConfiguredIPAddresses diff --git a/plugins/synced_folders/smb/errors.rb b/plugins/synced_folders/smb/errors.rb index 14704b9bb..ebb9bbef3 100644 --- a/plugins/synced_folders/smb/errors.rb +++ b/plugins/synced_folders/smb/errors.rb @@ -10,6 +10,14 @@ module VagrantPlugins error_key(:not_supported) end + class SMBStartFailed < SMBError + error_key(:start_failed) + end + + class SMBCredentialsMissing < SMBError + error_key(:credentials_missing) + end + class DefineShareFailed < SMBError error_key(:define_share_failed) end diff --git a/plugins/synced_folders/smb/synced_folder.rb b/plugins/synced_folders/smb/synced_folder.rb index 29a60982a..426def425 100644 --- a/plugins/synced_folders/smb/synced_folder.rb +++ b/plugins/synced_folders/smb/synced_folder.rb @@ -30,6 +30,11 @@ module VagrantPlugins def prepare(machine, folders, opts) machine.ui.output(I18n.t("vagrant_sf_smb.preparing")) + # Check if this host can start and SMB service + if machine.env.host.capability?(:smb_start) + machine.env.host.capability(:smb_start) + end + smb_username = smb_password = nil # If we need auth information, then ask the user. diff --git a/templates/locales/synced_folder_smb.yml b/templates/locales/synced_folder_smb.yml index 143a4885f..79eb93aef 100644 --- a/templates/locales/synced_folder_smb.yml +++ b/templates/locales/synced_folder_smb.yml @@ -1,9 +1,10 @@ en: vagrant_sf_smb: not_supported: |- - It appears your machine doesn't support SMB, or there is not an - adapter to enable SMB on this machine for Vagrant. Ensure SMB - host functionality is available on this machine and try again. + It appears your machine doesn't support SMB, has not been + properly configured for SMB, or there is not an adapter to + enable SMB on this machine for Vagrant. Ensure SMB host + functionality is available on this machine and try again. mounting: |- Mounting SMB shared folders... mounting_single: |- @@ -23,6 +24,19 @@ en: Vagrant requires administator access to create SMB shares and may request access to complete setup of configured shares. errors: + start_failed: |- + Vagrant failed to automatically start the SMB service. Ensure the + required services can be started and try again. + + Command: %{command} + + Stderr: %{stderr} + + Stdout: %{stdout} + credentials_missing: |- + Vagrant SMB synced folders require the account password to be stored + in an NT compatible format. Please update your sharing settings to + enable a Windows compatible password and try again. define_share_failed: |- Exporting an SMB share failed! Details about the failure are shown below. Please inspect the error message and correct any problems. diff --git a/test/unit/plugins/hosts/darwin/cap/smb_test.rb b/test/unit/plugins/hosts/darwin/cap/smb_test.rb index dbe653978..a0ea44bcd 100644 --- a/test/unit/plugins/hosts/darwin/cap/smb_test.rb +++ b/test/unit/plugins/hosts/darwin/cap/smb_test.rb @@ -3,6 +3,8 @@ require_relative "../../../../base" require_relative "../../../../../../plugins/hosts/darwin/cap/smb" describe VagrantPlugins::HostDarwin::Cap::SMB do + include_context "unit" + let(:subject){ VagrantPlugins::HostDarwin::Cap::SMB } let(:machine){ double(:machine) } let(:env){ double(:env) } @@ -23,18 +25,61 @@ describe VagrantPlugins::HostDarwin::Cap::SMB do end end + describe ".smb_start" do + before{ allow(Vagrant::Util::Subprocess).to receive(:execute) + .and_return(result.new(0, "SMB-NT", "")) } + + it "should check for NT compatible password" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with("pwpolicy", "gethashtypes"). + and_return(result.new(0, "SMB-NT", "")) + subject.smb_start(env) + end + + it "should raise error if NT compatible password is not set" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with("pwpolicy", "gethashtypes"). + and_return(result.new(0, "", "")) + expect{ subject.smb_start(env) }.to raise_error(VagrantPlugins::SyncedFolderSMB::Errors::SMBCredentialsMissing) + end + + it "should ignore if the command returns non-zero" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with("pwpolicy", "gethashtypes"). + and_return(result.new(1, "", "")) + subject.smb_start(env) + end + + it "should not load smb preferences if it is already loaded" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with("launchctl", "list", /preferences/).and_return(result.new(0, "", "")) + expect(Vagrant::Util::Subprocess).not_to receive(:execute).with(/sudo/, /launchctl/, "load", "-w", /preferences/) + subject.smb_start(env) + end + + it "should load smb preferences if it is not already loaded" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with("launchctl", "list", /preferences/).and_return(result.new(1, "", "")) + expect(Vagrant::Util::Subprocess).to receive(:execute).with(/sudo/, /launchctl/, "load", "-w", /preferences/).and_return(result.new(0, "", "")) + subject.smb_start(env) + end + + it "should raise error if load smb preferences fails" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with("launchctl", "list", /preferences/).and_return(result.new(1, "", "")) + expect(Vagrant::Util::Subprocess).to receive(:execute).with(/sudo/, /launchctl/, "load", "-w", /preferences/).and_return(result.new(1, "", "")) + expect{ subject.smb_start(env) }.to raise_error(VagrantPlugins::SyncedFolderSMB::Errors::SMBStartFailed) + end + + # TODO Finish out last start coverage + end + describe ".smb_cleanup" do after{ subject.smb_cleanup(env, machine, options) } it "should search for shares with generated machine ID" do expect(Vagrant::Util::Subprocess).to receive(:execute).with( - anything, anything, /.*CUSTOM_ID.*/).and_return(result.new(0, "", "")) + "/usr/bin/sudo", /sharing/, "-l").and_return(result.new(0, "", "")) end it "should remove shares individually" do expect(Vagrant::Util::Subprocess).to receive(:execute). - with(anything, anything, /.*CUSTOM_ID.*/). - and_return(result.new(0, "vgt-CUSTOM_ID-1\nvgt-CUSTOM_ID-2\n", "")) + with("/usr/bin/sudo", /sharing/, "-l"). + and_return(result.new(0, "name: vgt-CUSTOM_ID-1\nname: vgt-CUSTOM_ID-2\n", "")) expect(Vagrant::Util::Subprocess).to receive(:execute).with(/sudo/, /sharing/, anything, /CUSTOM_ID/). twice.and_return(result.new(0, "", "")) end diff --git a/test/unit/plugins/synced_folders/smb/synced_folder_test.rb b/test/unit/plugins/synced_folders/smb/synced_folder_test.rb index a1d40d9ef..b01902df8 100644 --- a/test/unit/plugins/synced_folders/smb/synced_folder_test.rb +++ b/test/unit/plugins/synced_folders/smb/synced_folder_test.rb @@ -67,7 +67,7 @@ describe VagrantPlugins::SyncedFolderSMB::SyncedFolder do end describe ".prepare" do - let(:host_caps){ [:smb_prepare] } + let(:host_caps){ [:smb_start, :smb_prepare] } context "without credentials provided" do before do @@ -86,6 +86,11 @@ describe VagrantPlugins::SyncedFolderSMB::SyncedFolder do expect(folders['/second/path'][:smb_username]).to eq('username') expect(folders['/second/path'][:smb_password]).to eq('password') end + + it "should start the SMB service if capability is available" do + expect(host).to receive(:capability).with(:smb_install, any_args) + subject.prepare(machine, folders, options) + end end context "with credentials provided" do diff --git a/website/source/docs/synced-folders/smb.html.md b/website/source/docs/synced-folders/smb.html.md index 1b977db87..c85cf674d 100644 --- a/website/source/docs/synced-folders/smb.html.md +++ b/website/source/docs/synced-folders/smb.html.md @@ -56,7 +56,9 @@ without requiring a password each time: Cmnd_Alias VAGRANT_SMB_ADD = /usr/sbin/sharing -a * -S * -s * -g * -n * Cmnd_Alias VAGRANT_SMB_REMOVE = /usr/sbin/sharing -r * Cmnd_Alias VAGRANT_SMB_LIST = /usr/sbin/sharing -l -%admin ALL=(root) NOPASSWD: VAGRANT_SMB_ADD, VAGRANT_SMB_REMOVE, VAGRANT_SMB_LIST +Cmnd_Alias VAGRANT_SMB_PSTART = /bin/launchctl load -w /System/Library/LaunchDaemons/com.apple.smb.preferences.plist +Cmnd_Alias VAGRANT_SMB_DSTART = /bin/launchctl load -w /System/Library/LaunchDaemons/com.apple.smb.preferences.plist +%admin ALL=(root) NOPASSWD: VAGRANT_SMB_ADD, VAGRANT_SMB_REMOVE, VAGRANT_SMB_LIST, VAGRANT_SMB_PSTART, VAGRANT_SMB_DSTART ``` ### Guests From 0dbd8538a055fb701aef05ea9145b18468525200 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 2 Jan 2018 14:03:06 -0800 Subject: [PATCH 19/30] Update return value documentation to actual behavior --- lib/vagrant/util/powershell.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/vagrant/util/powershell.rb b/lib/vagrant/util/powershell.rb index fefb60571..915ae4bb5 100644 --- a/lib/vagrant/util/powershell.rb +++ b/lib/vagrant/util/powershell.rb @@ -51,7 +51,8 @@ module Vagrant # Execute a powershell command. # # @param [String] command PowerShell command to execute. - # @return [Subprocess::Result] + # @return [nil, String] Returns nil if exit code is non-zero. + # Returns stdout string if exit code is zero. def self.execute_cmd(command) validate_install! c = [ From 24d962eb7288952847eeaeeed4691ba88b05a574 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 2 Jan 2018 14:03:34 -0800 Subject: [PATCH 20/30] Swap usage of share name and id for consistent behavior --- plugins/hosts/windows/scripts/set_share.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/hosts/windows/scripts/set_share.ps1 b/plugins/hosts/windows/scripts/set_share.ps1 index 8c4e077b5..b483088af 100644 --- a/plugins/hosts/windows/scripts/set_share.ps1 +++ b/plugins/hosts/windows/scripts/set_share.ps1 @@ -27,7 +27,7 @@ for ($i=0; $i -le $args.length; $i = $i + 3) { exit 1 } - $result = net share $share_name=$path /unlimited /GRANT:$grant /REMARK:"${share_id}" + $result = net share $share_id=$path /unlimited /GRANT:$grant /REMARK:"${share_name}" if ($LastExitCode -ne 0) { $host.ui.WriteLine("share path: ${path}") $host.ui.WriteErrorLine("error ${result}") From 30dcd9a7e5f843916a417e80047c5db8f317976e Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 2 Jan 2018 14:03:54 -0800 Subject: [PATCH 21/30] Only add/remove shares on windows when needed --- plugins/hosts/windows/cap/smb.rb | 53 ++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/plugins/hosts/windows/cap/smb.rb b/plugins/hosts/windows/cap/smb.rb index 983f3ef73..9287957bb 100644 --- a/plugins/hosts/windows/cap/smb.rb +++ b/plugins/hosts/windows/cap/smb.rb @@ -21,17 +21,17 @@ module VagrantPlugins script_path = File.expand_path("../../scripts/unset_share.ps1", __FILE__) m_id = machine_id(machine) - result = Vagrant::Util::PowerShell.execute_cmd("net share") - if result.nil? - @@logger.warn("failed to get current share list") - return - end - prune_shares = result.split("\n").map do |line| - sections = line.split(/\s/) - if sections.first.to_s.start_with?("vgt-#{m_id}") - sections.first + prune_shares = existing_shares.map do |share_name, share_info| + if share_info["Description"].start_with?("vgt-#{m_id}-") + @@logger.info("removing smb share name=#{share_name} id=#{m_id}") + share_name + else + @@logger.info("skipping smb share removal, not owned name=#{share_name}") + @@logger.debug("smb share ID not present name=#{share_name} id=#{m_id} description=#{share_info["Description"]}") + nil end end.compact + @@logger.debug("shares to be removed: #{prune_shares}") if prune_shares.size > 0 @@ -53,13 +53,23 @@ module VagrantPlugins script_path = File.expand_path("../../scripts/set_share.ps1", __FILE__) shares = [] + current_shares = existing_shares folders.each do |id, data| - hostpath = data[:hostpath] + hostpath = data[:hostpath].to_s chksum_id = Digest::MD5.hexdigest(id) name = "vgt-#{machine_id(machine)}-#{chksum_id}" data[:smb_id] ||= name + # Check if this name is already in use + if share_info = current_shares[data[:smb_id]] + if !hostpath.empty? && share_info["Path"].downcase != hostpath.downcase + raise "share in use with different path" + end + @@logger.info("skip creation of existing share name=#{name} id=#{data[:smb_id]}") + next + end + @@logger.info("creating new share name=#{name} id=#{data[:smb_id]}") shares << [ @@ -82,6 +92,29 @@ module VagrantPlugins end end + # Generate a list of existing local smb shares + # + # @return [Hash] + def self.existing_shares + result = Vagrant::Util::PowerShell.execute_cmd("Get-SmbShare|Format-List") + if result.nil? + raise "failed to get existing shares" + end + shares = {} + name = nil + result.lines.each do |line| + key, value = line.split(":", 2).map(&:strip) + if key == "Name" + name = value + shares[name] = {} + end + next if name.nil? || key.to_s.empty? + shares[name][key] = value + end + @@logger.debug("local share listing: #{shares}") + shares + end + # Generates a unique identifier for the given machine # based on the name, provider name, and working directory # of the environment. From 3eeff5932906eafd40867b4aa8c0863634bee436 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 2 Jan 2018 14:36:28 -0800 Subject: [PATCH 22/30] Use custom types and messages for errors --- plugins/hosts/windows/cap/smb.rb | 7 +++++-- plugins/synced_folders/smb/errors.rb | 8 ++++++++ templates/locales/synced_folder_smb.yml | 12 ++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/plugins/hosts/windows/cap/smb.rb b/plugins/hosts/windows/cap/smb.rb index 9287957bb..d224e06a3 100644 --- a/plugins/hosts/windows/cap/smb.rb +++ b/plugins/hosts/windows/cap/smb.rb @@ -64,7 +64,10 @@ module VagrantPlugins # Check if this name is already in use if share_info = current_shares[data[:smb_id]] if !hostpath.empty? && share_info["Path"].downcase != hostpath.downcase - raise "share in use with different path" + raise SyncedFolderSMB::Errors::SMBNameError, + path: hostpath, + existing_path: share_info["Path"], + name: data[:smb_id] end @@logger.info("skip creation of existing share name=#{name} id=#{data[:smb_id]}") next @@ -98,7 +101,7 @@ module VagrantPlugins def self.existing_shares result = Vagrant::Util::PowerShell.execute_cmd("Get-SmbShare|Format-List") if result.nil? - raise "failed to get existing shares" + raise SyncedFolderSMB::Errors::SMBListFailed end shares = {} name = nil diff --git a/plugins/synced_folders/smb/errors.rb b/plugins/synced_folders/smb/errors.rb index ebb9bbef3..761924207 100644 --- a/plugins/synced_folders/smb/errors.rb +++ b/plugins/synced_folders/smb/errors.rb @@ -18,6 +18,14 @@ module VagrantPlugins error_key(:credentials_missing) end + class SMBListFailed < SMBError + error_key(:list_failed) + end + + class SMBNameError < SMBError + error_key(:name_error) + end + class DefineShareFailed < SMBError error_key(:define_share_failed) end diff --git a/templates/locales/synced_folder_smb.yml b/templates/locales/synced_folder_smb.yml index 79eb93aef..3521ff4ff 100644 --- a/templates/locales/synced_folder_smb.yml +++ b/templates/locales/synced_folder_smb.yml @@ -55,6 +55,18 @@ en: Stderr: %{stderr} Stdout: %{stdout} + name_error: |- + Vagrant is unable to setup a requested SMB share. An SMB share already + exists with the given name. + + Share name: %{name} + + Current path: %{existing_path} + + Requested path: %{path} + list_failed: |- + Vagrant failed to generate a list of local SMB shares. Please try + running the command again. no_routable_host_addr: |- We couldn't detect an IP address that was routable to this machine from the guest machine! Please verify networking is properly From 3e4c81f6b1ef21732fec72b55956ae0107677208 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 2 Jan 2018 14:36:56 -0800 Subject: [PATCH 23/30] Clean up tests --- .../unit/plugins/hosts/darwin/cap/smb_test.rb | 18 ++++++++++++++- .../plugins/hosts/windows/cap/smb_test.rb | 23 ++++++++++++++++--- .../synced_folders/smb/synced_folder_test.rb | 2 +- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/test/unit/plugins/hosts/darwin/cap/smb_test.rb b/test/unit/plugins/hosts/darwin/cap/smb_test.rb index a0ea44bcd..5d5884bc9 100644 --- a/test/unit/plugins/hosts/darwin/cap/smb_test.rb +++ b/test/unit/plugins/hosts/darwin/cap/smb_test.rb @@ -65,7 +65,23 @@ describe VagrantPlugins::HostDarwin::Cap::SMB do expect{ subject.smb_start(env) }.to raise_error(VagrantPlugins::SyncedFolderSMB::Errors::SMBStartFailed) end - # TODO Finish out last start coverage + it "should not load smbd if it is already loaded" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with("launchctl", "list", /smbd/).and_return(result.new(0, "", "")) + expect(Vagrant::Util::Subprocess).not_to receive(:execute).with(/sudo/, /launchctl/, "load", "-w", /smbd/) + subject.smb_start(env) + end + + it "should load smbd if it is not already loaded" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with("launchctl", "list", /smbd/).and_return(result.new(1, "", "")) + expect(Vagrant::Util::Subprocess).to receive(:execute).with(/sudo/, /launchctl/, "load", "-w", /smbd/).and_return(result.new(0, "", "")) + subject.smb_start(env) + end + + it "should raise error if load smbd fails" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with("launchctl", "list", /smbd/).and_return(result.new(1, "", "")) + expect(Vagrant::Util::Subprocess).to receive(:execute).with(/sudo/, /launchctl/, "load", "-w", /smbd/).and_return(result.new(1, "", "")) + expect{ subject.smb_start(env) }.to raise_error(VagrantPlugins::SyncedFolderSMB::Errors::SMBStartFailed) + end end describe ".smb_cleanup" do diff --git a/test/unit/plugins/hosts/windows/cap/smb_test.rb b/test/unit/plugins/hosts/windows/cap/smb_test.rb index 20c09d1ef..216be6397 100644 --- a/test/unit/plugins/hosts/windows/cap/smb_test.rb +++ b/test/unit/plugins/hosts/windows/cap/smb_test.rb @@ -9,10 +9,27 @@ describe VagrantPlugins::HostWindows::Cap::SMB do let(:options){ {} } let(:result){ Vagrant::Util::Subprocess::Result } let(:powershell_version){ "3" } + let(:smblist){ <<-EOF +Name : vgt-CUSTOM_ID-1 +Path : /a/path +Description : vgt-CUSTOM_ID-1 + +Name : vgt-CUSTOM_ID-2 +Path : /other/path +Description : vgt-CUSTOM_ID-2 + +Name : my-share +Path : /my/path +Description : Not Vagrant Owned + + EOF + } + before do allow(subject).to receive(:machine_id).and_return("CUSTOM_ID") allow(Vagrant::Util::PowerShell).to receive(:version).and_return(powershell_version) + allow(Vagrant::Util::PowerShell).to receive(:execute_cmd).and_return("") allow(machine.env.ui).to receive(:warn) allow(subject).to receive(:sleep) end @@ -35,8 +52,8 @@ describe VagrantPlugins::HostWindows::Cap::SMB do describe ".smb_cleanup" do before do - allow(Vagrant::Util::PowerShell).to receive(:execute_cmd).with("net share"). - and_return("vgt-CUSTOM_ID-1\nvgt-CUSTOM_ID-2\n") + allow(Vagrant::Util::PowerShell).to receive(:execute_cmd).with(/Get-SmbShare/). + and_return(smblist) allow(Vagrant::Util::PowerShell).to receive(:execute).and_return(result.new(0, "", "")) end after{ subject.smb_cleanup(env, machine, options) } @@ -52,7 +69,7 @@ describe VagrantPlugins::HostWindows::Cap::SMB do context "when no shares are defined" do before do - expect(Vagrant::Util::PowerShell).to receive(:execute_cmd).with("net share"). + expect(Vagrant::Util::PowerShell).to receive(:execute_cmd).with(/Get-SmbShare/). and_return("") end diff --git a/test/unit/plugins/synced_folders/smb/synced_folder_test.rb b/test/unit/plugins/synced_folders/smb/synced_folder_test.rb index b01902df8..a80d45a3a 100644 --- a/test/unit/plugins/synced_folders/smb/synced_folder_test.rb +++ b/test/unit/plugins/synced_folders/smb/synced_folder_test.rb @@ -88,7 +88,7 @@ describe VagrantPlugins::SyncedFolderSMB::SyncedFolder do end it "should start the SMB service if capability is available" do - expect(host).to receive(:capability).with(:smb_install, any_args) + expect(host).to receive(:capability).with(:smb_start, any_args) subject.prepare(machine, folders, options) end end From 5b7714f542668126b1bbe95154db113ad1b31467 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 3 Jan 2018 09:30:44 -0800 Subject: [PATCH 24/30] Include macOS as valid host platform --- plugins/synced_folders/smb/plugin.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/synced_folders/smb/plugin.rb b/plugins/synced_folders/smb/plugin.rb index 02aca938b..0e6370e29 100644 --- a/plugins/synced_folders/smb/plugin.rb +++ b/plugins/synced_folders/smb/plugin.rb @@ -9,7 +9,7 @@ module VagrantPlugins name "SMB synced folders" description <<-EOF The SMB synced folders plugin enables you to use SMB folders on - Windows and share them to guest machines. + Windows or macOS and share them to guest machines. EOF synced_folder("smb", 7) do From 6e4139acd3991b962d88dfa597c9a2a69147a156 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 3 Jan 2018 09:31:01 -0800 Subject: [PATCH 25/30] Include configuration for smb synced folders --- plugins/synced_folders/smb/config.rb | 23 +++++++++++++++++++++++ plugins/synced_folders/smb/plugin.rb | 5 +++++ 2 files changed, 28 insertions(+) create mode 100644 plugins/synced_folders/smb/config.rb diff --git a/plugins/synced_folders/smb/config.rb b/plugins/synced_folders/smb/config.rb new file mode 100644 index 000000000..d853092ac --- /dev/null +++ b/plugins/synced_folders/smb/config.rb @@ -0,0 +1,23 @@ +require "vagrant" + +module VagrantPlugins + module SyncedFolderSMB + class Config < Vagrant.plugin("2", :config) + attr_accessor :functional + + def initialize + super + + @functional = UNSET_VALUE + end + + def finalize! + @functional = true if @functional == UNSET_VALUE + end + + def to_s + "SMB" + end + end + end +end diff --git a/plugins/synced_folders/smb/plugin.rb b/plugins/synced_folders/smb/plugin.rb index 0e6370e29..bfb58a5b2 100644 --- a/plugins/synced_folders/smb/plugin.rb +++ b/plugins/synced_folders/smb/plugin.rb @@ -12,6 +12,11 @@ module VagrantPlugins Windows or macOS and share them to guest machines. EOF + config("smb") do + require_relative "config" + Config + end + synced_folder("smb", 7) do require_relative "synced_folder" init! From 257441ed78a32bdeef5435e27dae0cb939466250 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 3 Jan 2018 09:41:45 -0800 Subject: [PATCH 26/30] Log location of caller for missing configuration key requests --- lib/vagrant/config/v2/root.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/vagrant/config/v2/root.rb b/lib/vagrant/config/v2/root.rb index f4a18c10b..b41e5c031 100644 --- a/lib/vagrant/config/v2/root.rb +++ b/lib/vagrant/config/v2/root.rb @@ -16,6 +16,7 @@ module Vagrant @keys = keys || {} @config_map = config_map @missing_key_calls = Set.new + @logger = Log4r::Logger.new("vagrant::config") end # We use method_missing as a way to get the configuration that is @@ -30,6 +31,7 @@ module Vagrant @keys[name] = config_klass.new return @keys[name] else + @logger.debug("missing key request name=#{name} loc=#{caller.first}") # Record access to a missing key as an error @missing_key_calls.add(name.to_s) return DummyConfig.new From abf74e3757b9dc8ec36dfc5d074658f034ffc720 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 3 Jan 2018 10:05:05 -0800 Subject: [PATCH 27/30] Expand existing and requested paths prior to comparison --- plugins/hosts/windows/cap/smb.rb | 4 +++- .../plugins/hosts/windows/cap/smb_test.rb | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/plugins/hosts/windows/cap/smb.rb b/plugins/hosts/windows/cap/smb.rb index d224e06a3..457807f9c 100644 --- a/plugins/hosts/windows/cap/smb.rb +++ b/plugins/hosts/windows/cap/smb.rb @@ -63,7 +63,9 @@ module VagrantPlugins # Check if this name is already in use if share_info = current_shares[data[:smb_id]] - if !hostpath.empty? && share_info["Path"].downcase != hostpath.downcase + exist_path = File.expand_path(share_info["Path"]).downcase + request_path = File.expand_path(hostpath).downcase + if !hostpath.empty? && exist_path != request_path raise SyncedFolderSMB::Errors::SMBNameError, path: hostpath, existing_path: share_info["Path"], diff --git a/test/unit/plugins/hosts/windows/cap/smb_test.rb b/test/unit/plugins/hosts/windows/cap/smb_test.rb index 216be6397..c904bc205 100644 --- a/test/unit/plugins/hosts/windows/cap/smb_test.rb +++ b/test/unit/plugins/hosts/windows/cap/smb_test.rb @@ -110,6 +110,29 @@ Description : Not Vagrant Owned subject.smb_prepare(env, machine, folders, options) end + context "when share already exists" do + let(:shares){ {"ID1" => {"Path" => "/host/2"}} } + before do + allow(File).to receive(:expand_path).and_call_original + expect(subject).to receive(:existing_shares).and_return(shares) + end + + it "should expand paths when comparing existing to requested" do + expect(File).to receive(:expand_path).at_least(2).with("/host/2").and_return("expanded_path") + subject.smb_prepare(env, machine, folders, options) + end + + context "with different path" do + let(:shares){ {"ID1" => {"Path" => "/host/3"}} } + + it "should raise an error" do + expect{ + subject.smb_prepare(env, machine, folders, options) + }.to raise_error(VagrantPlugins::SyncedFolderSMB::Errors::SMBNameError) + end + end + end + context "when no shared are defined" do after{ subject.smb_prepare(env, machine, {}, options) } From 572f34169711059c0fcf71e8fd07b62c2c36ab78 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 3 Jan 2018 10:29:45 -0800 Subject: [PATCH 28/30] Include synced folder cleanup action within hyperv provider --- plugins/providers/hyperv/action.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/providers/hyperv/action.rb b/plugins/providers/hyperv/action.rb index 867191396..f5e9b25c6 100644 --- a/plugins/providers/hyperv/action.rb +++ b/plugins/providers/hyperv/action.rb @@ -41,6 +41,7 @@ module VagrantPlugins b2.use ProvisionerCleanup, :before b2.use StopInstance b2.use DeleteVM + b2.use SyncedFolderCleanup end end end @@ -144,6 +145,7 @@ module VagrantPlugins b2.use StartInstance b2.use WaitForIPAddress b2.use WaitForCommunicator, [:running] + b2.use SyncedFolderCleanup b2.use SyncedFolders b2.use SetHostname end From 2f3d16507982bf8c638b62ca6f7e719cce81abe7 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 3 Jan 2018 14:41:53 -0800 Subject: [PATCH 29/30] Remove version from darwin host custom mount options --- plugins/hosts/darwin/cap/smb.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/hosts/darwin/cap/smb.rb b/plugins/hosts/darwin/cap/smb.rb index 8581921f4..84c0a4922 100644 --- a/plugins/hosts/darwin/cap/smb.rb +++ b/plugins/hosts/darwin/cap/smb.rb @@ -47,7 +47,7 @@ module VagrantPlugins # Required options for mounting a share hosted # on macos. def self.smb_mount_options(env) - ["ver=3", "sec=ntlmssp", "nounix", "noperm"] + ["sec=ntlmssp", "nounix", "noperm"] end def self.smb_cleanup(env, machine, opts) From 0ffec9cd6f36a40f67d31a1b266fe3d2421f1929 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Fri, 12 Jan 2018 14:25:37 -0800 Subject: [PATCH 30/30] Include explicit start to ensure start --- contrib/sudoers/osx | 7 ++++--- plugins/hosts/darwin/cap/smb.rb | 1 + website/source/docs/synced-folders/smb.html.md | 7 ++++--- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/contrib/sudoers/osx b/contrib/sudoers/osx index fc0c12c0d..ff1a28575 100644 --- a/contrib/sudoers/osx +++ b/contrib/sudoers/osx @@ -4,6 +4,7 @@ Cmnd_Alias VAGRANT_EXPORTS_REMOVE = /usr/bin/sed -E -e /*/ d -ibak /etc/exports Cmnd_Alias VAGRANT_SMB_ADD = /usr/sbin/sharing -a * -S * -s * -g * -n * Cmnd_Alias VAGRANT_SMB_REMOVE = /usr/sbin/sharing -r * Cmnd_Alias VAGRANT_SMB_LIST = /usr/sbin/sharing -l -Cmnd_Alias VAGRANT_SMB_PSTART = /bin/launchctl load -w /System/Library/LaunchDaemons/com.apple.smb.preferences.plist -Cmnd_Alias VAGRANT_SMB_DSTART = /bin/launchctl load -w /System/Library/LaunchDaemons/com.apple.smb.preferences.plist -%admin ALL=(root) NOPASSWD: VAGRANT_EXPORTS_ADD, VAGRANT_NFSD, VAGRANT_EXPORTS_REMOVE, VAGRANT_SMB_ADD, VAGRANT_SMB_REMOVE, VAGRANT_SMB_LIST, VAGRANT_SMB_PSTART, VAGRANT_SMB_DSTART \ No newline at end of file +Cmnd_Alias VAGRANT_SMB_PLOAD = /bin/launchctl load -w /System/Library/LaunchDaemons/com.apple.smb.preferences.plist +Cmnd_Alias VAGRANT_SMB_DLOAD = /bin/launchctl load -w /System/Library/LaunchDaemons/com.apple.smbd.plist +Cmnd_Alias VAGRANT_SMB_DSTART = /bin/launchctl start com.apple.smbd +%admin ALL=(root) NOPASSWD: VAGRANT_EXPORTS_ADD, VAGRANT_NFSD, VAGRANT_SMB_ADD, VAGRANT_SMB_REMOVE, VAGRANT_SMB_LIST, VAGRANT_SMB_PLOAD, VAGRANT_SMB_DLOAD VAGRANT_SMB_DSTART \ No newline at end of file diff --git a/plugins/hosts/darwin/cap/smb.rb b/plugins/hosts/darwin/cap/smb.rb index 84c0a4922..2547ca74e 100644 --- a/plugins/hosts/darwin/cap/smb.rb +++ b/plugins/hosts/darwin/cap/smb.rb @@ -41,6 +41,7 @@ module VagrantPlugins stderr: result.stderr, stdout: result.stdout end + Vagrant::Util::Subprocess.execute("/usr/bin/sudo", "/bin/launchctl", "start", "com.apple.smbd") end end diff --git a/website/source/docs/synced-folders/smb.html.md b/website/source/docs/synced-folders/smb.html.md index c85cf674d..58206ad9a 100644 --- a/website/source/docs/synced-folders/smb.html.md +++ b/website/source/docs/synced-folders/smb.html.md @@ -56,9 +56,10 @@ without requiring a password each time: Cmnd_Alias VAGRANT_SMB_ADD = /usr/sbin/sharing -a * -S * -s * -g * -n * Cmnd_Alias VAGRANT_SMB_REMOVE = /usr/sbin/sharing -r * Cmnd_Alias VAGRANT_SMB_LIST = /usr/sbin/sharing -l -Cmnd_Alias VAGRANT_SMB_PSTART = /bin/launchctl load -w /System/Library/LaunchDaemons/com.apple.smb.preferences.plist -Cmnd_Alias VAGRANT_SMB_DSTART = /bin/launchctl load -w /System/Library/LaunchDaemons/com.apple.smb.preferences.plist -%admin ALL=(root) NOPASSWD: VAGRANT_SMB_ADD, VAGRANT_SMB_REMOVE, VAGRANT_SMB_LIST, VAGRANT_SMB_PSTART, VAGRANT_SMB_DSTART +Cmnd_Alias VAGRANT_SMB_PLOAD = /bin/launchctl load -w /System/Library/LaunchDaemons/com.apple.smb.preferences.plist +Cmnd_Alias VAGRANT_SMB_DLOAD = /bin/launchctl load -w /System/Library/LaunchDaemons/com.apple.smbd.plist +Cmnd_Alias VAGRANT_SMB_DSTART = /bin/launchctl start com.apple.smbd +%admin ALL=(root) NOPASSWD: VAGRANT_SMB_ADD, VAGRANT_SMB_REMOVE, VAGRANT_SMB_LIST, VAGRANT_SMB_PLOAD, VAGRANT_SMB_DLOAD VAGRANT_SMB_DSTART ``` ### Guests