From 0eae5c09261158cee929c601d696c70739b4ea51 Mon Sep 17 00:00:00 2001 From: Jef Mathiot Date: Thu, 17 Apr 2014 12:56:56 +0200 Subject: [PATCH] Removed unsecure calls to SudoWrapper#su_c. --- lib/vagrant-lxc/command/sudoers.rb | 6 +++--- lib/vagrant-lxc/driver.rb | 32 ++++++++++++++++++------------ lib/vagrant-lxc/sudo_wrapper.rb | 10 ---------- spec/unit/driver_spec.rb | 6 +++++- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/lib/vagrant-lxc/command/sudoers.rb b/lib/vagrant-lxc/command/sudoers.rb index f61c0f2..8149d10 100644 --- a/lib/vagrant-lxc/command/sudoers.rb +++ b/lib/vagrant-lxc/command/sudoers.rb @@ -126,8 +126,8 @@ Whitelist.add '/bin/cat', base_path # - Shared folders Whitelist.add '/bin/mkdir', '-p', base_path # - Container config customizations and pruning -Whitelist.add '/bin/su', 'root', '-c', %r{\\A"sed -e '.*' -ibak \#{base}/.*/config"\\z} -Whitelist.add '/bin/su', 'root', '-c', %r{\\A"echo '.*' >> \#{base}/.*/config"\\z} +Whitelist.add '/bin/cp', '-f', %r{/tmp/.*}, base_path +Whitelist.add '/bin/chown', 'root:root', base_path # - Template import Whitelist.add '/bin/cp', boxes_path, templates_path Whitelist.add '/bin/cp', gems_path, templates_path @@ -136,7 +136,7 @@ Whitelist.add '/bin/chmod', '+x', templates_path # - Template removal Whitelist.add '/bin/rm', templates_path # - Packaging -Whitelist.add '/bin/su', 'root', '-c', %r{\\A"cd \#{base}/.* && rm -f rootfs\.tar\.gz && tar --numeric-owner -czf /tmp/.*/rootfs\.tar\.gz -C \#{base}/.*/rootfs '\./\.'"\\z} +Whitelist.add '/bin/tar', '--numeric-owner', '-cvzf', %r{/tmp/.*/rootfs.tar.gz}, '-C', base_path, './rootfs' Whitelist.add '/bin/chown', /\\A\\d+:\\d+\\z/, %r{\\A/tmp/.*/rootfs\.tar\.gz\\z} ## diff --git a/lib/vagrant-lxc/driver.rb b/lib/vagrant-lxc/driver.rb index 858b448..3fd073b 100644 --- a/lib/vagrant-lxc/driver.rb +++ b/lib/vagrant-lxc/driver.rb @@ -121,17 +121,8 @@ module Vagrant target_path = "#{Dir.mktmpdir}/rootfs.tar.gz" @logger.info "Compressing '#{rootfs_path}' rootfs to #{target_path}" - # "vagrant package" will copy the existing lxc-template in the new box file - # To keep this function backwards compatible with existing boxes, the path - # included in the tarball needs to have the same amount of path components (2) - # that will be stripped before extraction, hence the './.' - # TODO: This should be reviewed before 1.0 - cmds = [ - "cd #{base_path}", - "rm -f rootfs.tar.gz", - "tar --numeric-owner -czf #{target_path} -C #{rootfs_path} './.'" - ] - @sudo_wrapper.su_c(cmds.join(' && ')) + @sudo_wrapper.run('tar', '--numeric-owner', '-cvzf', target_path, '-C', + rootfs_path.parent.to_s, "./#{rootfs_path.basename.to_s}") @logger.info "Changing rootfs tarball owner" user_details = Etc.getpwnam(Etc.getlogin) @@ -149,7 +140,9 @@ module Vagrant def prune_customizations # Use sed to just strip out the block of code which was inserted by Vagrant @logger.debug 'Prunning vagrant-lxc customizations' - @sudo_wrapper.su_c("sed -e '/^# VAGRANT-BEGIN/,/^# VAGRANT-END/ d' -ibak #{base_path.join('config')}") + contents = config_string + config_string.gsub! /^# VAGRANT-BEGIN(.|\s)*# VAGRANT-END/, '' + write_config(contents) end protected @@ -160,10 +153,23 @@ module Vagrant end customizations.unshift '# VAGRANT-BEGIN' customizations << '# VAGRANT-END' + contents = config_string config_file = base_path.join('config').to_s customizations.each do |line| - @sudo_wrapper.su_c("echo '#{line}' >> #{config_file}") + contents << line + contents << "\n" + end + write_config(contents) + end + + def write_config(contents) + Tempfile.new('lxc-config').tap do |file| + file.chmod 0644 + file.write contents + file.close + @sudo_wrapper.run 'cp', '-f', file.path, base_path.join('config').to_s + @sudo_wrapper.run 'chown', 'root:root', base_path.join('config').to_s end end diff --git a/lib/vagrant-lxc/sudo_wrapper.rb b/lib/vagrant-lxc/sudo_wrapper.rb index 623be05..2891c52 100644 --- a/lib/vagrant-lxc/sudo_wrapper.rb +++ b/lib/vagrant-lxc/sudo_wrapper.rb @@ -15,16 +15,6 @@ module Vagrant execute *(['sudo'] + command) end - def su_c(command, options={}) - su_command = if @wrapper_path && !options[:no_wrapper] - "#{@wrapper_path} su root -c \"\\\"#{command}\\\"\"" - else - "su root -c \"#{command}\"" - end - @logger.debug "Running 'sudo #{su_command}'" - system "sudo #{su_command}" - end - private # TODO: Review code below this line, it was pretty much a copy and diff --git a/spec/unit/driver_spec.rb b/spec/unit/driver_spec.rb index c56fc63..8b8a3e9 100644 --- a/spec/unit/driver_spec.rb +++ b/spec/unit/driver_spec.rb @@ -75,11 +75,15 @@ describe Vagrant::LXC::Driver do let(:customizations) { [['a', '1'], ['b', '2']] } let(:internal_customization) { ['internal', 'customization'] } let(:cli) { double(Vagrant::LXC::Driver::CLI, start: true) } - let(:sudo) { double(Vagrant::LXC::SudoWrapper, su_c: true) } + let(:sudo) { double(Vagrant::LXC::SudoWrapper) } subject { described_class.new('name', sudo, cli) } before do + sudo.should_receive(:run).with('cat', '/var/lib/lxc/name/config').exactly(3).times. + and_return('# CONFIGURATION') + sudo.should_receive(:run).twice.with('cp', '-f', %r{/tmp/.*}, '/var/lib/lxc/name/config') + sudo.should_receive(:run).twice.with('chown', 'root:root', '/var/lib/lxc/name/config') subject.customizations << internal_customization subject.start(customizations) end