From 94e175dc0702dca5ae92f0eaef15f3a38d2afb72 Mon Sep 17 00:00:00 2001 From: Jef Mathiot Date: Tue, 8 Apr 2014 19:58:07 +0200 Subject: [PATCH 1/4] sudoers command now creates a safe wrapper script. Sudoers now creates a safe wrapper script that performs sanity checks on sudo : * wrapper generated in /usr/local/bin (name includes version to allow multiple wrappers on the same system) * sudoers command now generates a one-line file in /etc/sudoers.d * SudoWrapper use the new wrapper * Removed unused Config#validate method --- lib/vagrant-lxc.rb | 4 + lib/vagrant-lxc/command/sudoers.rb | 208 ++++++++++++++++++++++------- lib/vagrant-lxc/config.rb | 21 --- lib/vagrant-lxc/provider.rb | 5 +- lib/vagrant-lxc/sudo_wrapper.rb | 9 +- 5 files changed, 169 insertions(+), 78 deletions(-) diff --git a/lib/vagrant-lxc.rb b/lib/vagrant-lxc.rb index d1ad0f7..4914a3b 100644 --- a/lib/vagrant-lxc.rb +++ b/lib/vagrant-lxc.rb @@ -6,5 +6,9 @@ module Vagrant def self.source_root @source_root ||= Pathname.new(File.dirname(__FILE__)).join('..').expand_path end + + def self.sudo_wrapper_path + "/usr/local/bin/vagrant-lxc-wrapper-#{VERSION}" + end end end diff --git a/lib/vagrant-lxc/command/sudoers.rb b/lib/vagrant-lxc/command/sudoers.rb index 4aab74a..f61c0f2 100644 --- a/lib/vagrant-lxc/command/sudoers.rb +++ b/lib/vagrant-lxc/command/sudoers.rb @@ -4,6 +4,12 @@ module Vagrant module LXC module Command class Sudoers < Vagrant.plugin("2", :command) + + def initialize(argv, env) + super + @env = env + end + def execute options = { user: ENV['USER'] } @@ -18,68 +24,168 @@ module Vagrant argv = parse_options(opts) return unless argv - filename = "vagrant-lxc-#{options[:user]}" - to_sudoers!(create_tempfile!(options[:user], filename), filename) + wrapper_path = Vagrant::LXC.sudo_wrapper_path + wrapper = create_wrapper! + sudoers = create_sudoers!(options[:user], wrapper_path) + + su_copy([ + {source: wrapper, target: wrapper_path, mode: "0555"}, + {source: sudoers, target: sudoers_path, mode: "0440"} + ]) + end + + def sudoers_path + "/etc/sudoers.d/vagrant-lxc-#{Vagrant::LXC::VERSION.gsub( /\./, '-')}" end private + # REFACTOR: Make use ERB rendering after https://github.com/mitchellh/vagrant/issues/3231 + # lands into core + def create_wrapper! + wrapper = Tempfile.new('lxc-wrapper').tap do |file| + file.puts "#!/usr/bin/env ruby" + file.puts "# Automatically created by vagrant-lxc" + file.puts <<-EOF +class Whitelist + class << self + def add(command, *args) + list[command] << args + end + + def list + @list ||= Hash.new do |key, hsh| + key[hsh] = [] + end + end + + def allowed(command) + list[command] || [] + end + + def run!(argv) + begin + command, args = `which \#{argv.shift}`.chomp, argv || [] + check!(command, args) + puts `\#{command} \#{args.join(" ")}` + exit $?.to_i + rescue => e + STDERR.puts e.message + exit 1 + end + end + + private + def check!(command, args) + allowed(command).each do |checks| + return if valid_args?(args, checks) + end + raise_invalid(command, args) + end + + def valid_args?(args, checks) + return false unless valid_length?(args, checks) + check = nil + args.each_with_index do |provided, i| + check = checks[i] unless check == '**' + return false unless match?(provided, check) + end + true + end + + def valid_length?(args, checks) + args.length == checks.length || checks.last == '**' + end + + def match?(arg, check) + check == '**' || check.is_a?(Regexp) && !!check.match(arg) || arg == check + end + + def raise_invalid(command, args) + raise "Invalid arguments for command \#{command}, " << + "provided args: \#{args.inspect}" + end + end +end + +base = "/var/lib/lxc" +base_path = %r{\\A\#{base}/.*\\z} +templates_path = %r{\\A/usr/(share|lib|lib64|local/lib)/lxc/templates/.*\\z} +boxes_path = %r{\\A#{Regexp.escape(@env.boxes_path.to_s)}/.*\\z} +gems_path = %r{\\A#{Regexp.escape(@env.gems_path.to_s)}/.*\\z} +template_src = %r{\\A#{Vagrant::LXC.source_root.join('scripts/lxc-template').to_s}\\z} + +## +# Commands from provider.rb +# - Check lxc is installed +Whitelist.add '/usr/bin/which', /\\Alxc-\\w+\\z/ + +## +# Commands from driver.rb +# - Container config file +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} +# - Template import +Whitelist.add '/bin/cp', boxes_path, templates_path +Whitelist.add '/bin/cp', gems_path, templates_path +Whitelist.add '/bin/cp', template_src, templates_path +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/chown', /\\A\\d+:\\d+\\z/, %r{\\A/tmp/.*/rootfs\.tar\.gz\\z} + +## +# Commands from driver/cli.rb +Whitelist.add '/usr/bin/lxc-version' +Whitelist.add '/usr/bin/lxc-ls' +Whitelist.add '/usr/bin/lxc-info', '--name', /.*/ +Whitelist.add '/usr/bin/lxc-create', '--template', /.*/, '--name', /.*/, '**' +Whitelist.add '/usr/bin/lxc-destroy', '--name', /.*/ +Whitelist.add '/usr/bin/lxc-start', '-d', '--name', /.*/, '**' +Whitelist.add '/usr/bin/lxc-stop', '--name', /.*/ +Whitelist.add '/usr/bin/lxc-shutdown', '--name', /.*/ +Whitelist.add '/usr/bin/lxc-attach', '--name', /.*/, '**' +Whitelist.add '/usr/bin/lxc-attach', '-h' + +## +# Commands from driver/action/remove_temporary_files.rb +Whitelist.add '/bin/rm', '-rf', %r{\\A\#{base}/.*/rootfs/tmp/.*} + +# Watch out for stones +Whitelist.run!(ARGV) + EOF + end + wrapper.close + wrapper.path + end # REFACTOR: Make use ERB rendering after https://github.com/mitchellh/vagrant/issues/3231 # lands into core - def create_tempfile!(user, filename) - sudoers = Tempfile.new(filename).tap do |file| - file.write "# Automatically created by vagrant-lxc\n" - commands.each do |command| - file.write sudoers_policy(user, command[:cmd], command[:args]) - end + def create_sudoers!(user, command) + sudoers = Tempfile.new('vagrant-lxc-sudoers').tap do |file| + file.puts "# Automatically created by vagrant-lxc" + file.puts "Cmnd_Alias LXC = #{command}" + file.puts "#{user} ALL=(root) NOPASSWD: LXC" end sudoers.close - File.chmod(0644, sudoers.path) sudoers.path end - def to_sudoers!(source, destination) - destination = "/etc/sudoers.d/#{destination}" - commands = [ - "rm -f #{destination}", - "cp #{source} #{destination}", - "chmod 440 #{destination}" - ] - `echo "#{commands.join('; ')}" | sudo sh` - end - - def sudoers_policy(user, command, args) - vagrant_home = "#{`echo ~#{user}`.chomp}/.vagrant.d" - args = args.gsub /%\{VAGRANT_D\}/, vagrant_home - args = args.gsub /%\{BOXES\}/, "#{vagrant_home}/boxes" - "#{user} ALL=(root) NOPASSWD: #{command} #{args}\n" - end - - def commands - [ - { cmd: '/usr/bin/lxc-ls', args: '' }, - { cmd: '/usr/bin/lxc-info', args: '' }, - { cmd: '/usr/bin/lxc-attach', args: '' }, - { cmd: '/usr/bin/which', args: 'lxc-*' }, - { cmd: '/bin/cat', args: '/var/lib/lxc/*' }, - { cmd: '/bin/mkdir', args: '-p /var/lib/lxc/*' }, - { cmd: '/bin/su', args: "root -c sed -e '*' -ibak /var/lib/lxc/*" }, - { cmd: '/bin/su', args: "root -c echo '*' >> /var/lib/lxc/*" }, - { cmd: '/usr/bin/lxc-start', args: '-d --name *' }, - { cmd: '/bin/cp', args: '%{BOXES}/*/lxc/lxc-template /usr/lib/lxc/templates/*' }, - { cmd: '/bin/cp', args: '%{VAGRANT_D}/gems/gems/vagrant-lxc-*/scripts/lxc-template /usr/lib/lxc/templates/*' }, - { cmd: '/bin/cp', args: '%{BOXES}/*/lxc/lxc-template /usr/share/lxc/templates/*' }, - { cmd: '/bin/cp', args: '%{VAGRANT_D}/gems/gems/vagrant-lxc-*/scripts/lxc-template /usr/share/lxc/templates/*' }, - { cmd: '/bin/rm', args: '/usr/lib/lxc/templates/*' }, - { cmd: '/bin/rm', args: '/usr/share/lxc/templates/*' }, - { cmd: '/bin/chmod', args: '+x /usr/lib/lxc/*' }, - { cmd: '/bin/chmod', args: '+x /usr/share/lxc/*' }, - { cmd: '/usr/bin/lxc-create', args: '--template * --name * -- --tarball %{BOXES}/*' }, - { cmd: '/bin/rm', args: '-rf /var/lib/lxc/*/rootfs/tmp/*' }, - { cmd: '/usr/bin/lxc-shutdown', args: '--name *' }, - { cmd: '/usr/bin/lxc-stop', args: '--name *' }, - { cmd: '/usr/bin/lxc-destroy', args: '--name *' } - ] + def su_copy(files) + commands = files.map { |file| + [ + "rm -f #{file[:target]}", + "cp #{file[:source]} #{file[:target]}", + "chown root:root #{file[:target]}", + "chmod #{file[:mode]} #{file[:target]}" + ] + }.flatten + system "echo \"#{commands.join("; ")}\" | sudo sh" end end end diff --git a/lib/vagrant-lxc/config.rb b/lib/vagrant-lxc/config.rb index 1a47058..61626ac 100644 --- a/lib/vagrant-lxc/config.rb +++ b/lib/vagrant-lxc/config.rb @@ -6,12 +6,6 @@ module Vagrant # @return [Array] attr_reader :customizations - # A String that points to a file that acts as a wrapper for sudo commands. - # - # This allows us to have a single entry when whitelisting NOPASSWD commands - # on /etc/sudoers - attr_accessor :sudo_wrapper - # A string to explicitly set the container name. To use the vagrant # machine name, set this to :machine attr_accessor :container_name @@ -41,21 +35,6 @@ module Vagrant @sudo_wrapper = nil if @sudo_wrapper == UNSET_VALUE @container_name = nil if @container_name == UNSET_VALUE end - - def validate(machine) - errors = [] - - if @sudo_wrapper - hostpath = Pathname.new(@sudo_wrapper).expand_path(machine.env.root_path) - if ! hostpath.file? - errors << I18n.t('vagrant_lxc.sudo_wrapper_not_found', path: hostpath.to_s) - elsif ! hostpath.executable? - errors << I18n.t('vagrant_lxc.sudo_wrapper_not_executable', path: hostpath.to_s) - end - end - - { "lxc provider" => errors } - end end end end diff --git a/lib/vagrant-lxc/provider.rb b/lib/vagrant-lxc/provider.rb index 0709b10..85ffd52 100644 --- a/lib/vagrant-lxc/provider.rb +++ b/lib/vagrant-lxc/provider.rb @@ -19,8 +19,9 @@ module Vagrant def sudo_wrapper @shell ||= begin - wrapper = @machine.provider_config.sudo_wrapper - wrapper = Pathname(wrapper).expand_path(@machine.env.root_path).to_s if wrapper + wrapper = Pathname.new(LXC.sudo_wrapper_path).exist? && + LXC.sudo_wrapper_path || nil + @logger.debug("Found sudo wrapper : #{wrapper}") if wrapper SudoWrapper.new(wrapper) end end diff --git a/lib/vagrant-lxc/sudo_wrapper.rb b/lib/vagrant-lxc/sudo_wrapper.rb index d906d1e..623be05 100644 --- a/lib/vagrant-lxc/sudo_wrapper.rb +++ b/lib/vagrant-lxc/sudo_wrapper.rb @@ -10,13 +10,14 @@ module Vagrant end def run(*command) - command.unshift @wrapper_path if @wrapper_path + options = command.last.is_a?(Hash) ? command.last : {} + command.unshift @wrapper_path if @wrapper_path && !options[:no_wrapper] execute *(['sudo'] + command) end - def su_c(command) - su_command = if @wrapper_path - "#{@wrapper_path} \"#{command}\"" + 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 From 0eae5c09261158cee929c601d696c70739b4ea51 Mon Sep 17 00:00:00 2001 From: Jef Mathiot Date: Thu, 17 Apr 2014 12:56:56 +0200 Subject: [PATCH 2/4] 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 From 2666f9e38d86d3dea1a60e23fac279e21dbb0d04 Mon Sep 17 00:00:00 2001 From: Eric Hartmann Date: Wed, 23 Apr 2014 16:27:33 +0200 Subject: [PATCH 3/4] Fix argument parsing --- lib/vagrant-lxc/command/sudoers.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/vagrant-lxc/command/sudoers.rb b/lib/vagrant-lxc/command/sudoers.rb index 8149d10..d99ab34 100644 --- a/lib/vagrant-lxc/command/sudoers.rb +++ b/lib/vagrant-lxc/command/sudoers.rb @@ -7,6 +7,7 @@ module Vagrant def initialize(argv, env) super + @argv @env = env end @@ -16,7 +17,7 @@ module Vagrant opts = OptionParser.new do |opts| opts.banner = "Usage: vagrant lxc sudoers" opts.separator "" - opts.on('-u', '--user', "The user for which to create the policy (defaults to '#{options[:user]}')") do |u| + opts.on('-u user', '--user user', String, "The user for which to create the policy (defaults to '#{options[:user]}')") do |u| options[:user] = u end end From 3b311ad8402570a7db3f1c4c89fa7db95fa307ae Mon Sep 17 00:00:00 2001 From: Eric Hartmann Date: Wed, 23 Apr 2014 17:49:04 +0200 Subject: [PATCH 4/4] Fix sudoers for multiple users --- lib/vagrant-lxc/command/sudoers.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/vagrant-lxc/command/sudoers.rb b/lib/vagrant-lxc/command/sudoers.rb index d99ab34..eb648c0 100644 --- a/lib/vagrant-lxc/command/sudoers.rb +++ b/lib/vagrant-lxc/command/sudoers.rb @@ -111,9 +111,6 @@ end base = "/var/lib/lxc" base_path = %r{\\A\#{base}/.*\\z} templates_path = %r{\\A/usr/(share|lib|lib64|local/lib)/lxc/templates/.*\\z} -boxes_path = %r{\\A#{Regexp.escape(@env.boxes_path.to_s)}/.*\\z} -gems_path = %r{\\A#{Regexp.escape(@env.gems_path.to_s)}/.*\\z} -template_src = %r{\\A#{Vagrant::LXC.source_root.join('scripts/lxc-template').to_s}\\z} ## # Commands from provider.rb @@ -130,9 +127,9 @@ Whitelist.add '/bin/mkdir', '-p', base_path 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 -Whitelist.add '/bin/cp', template_src, templates_path +Whitelist.add '/bin/cp', %r{\\A.*\\z}, templates_path +Whitelist.add '/bin/cp', %r{\\A.*\\z}, templates_path +Whitelist.add '/bin/cp', %r{\\A.*\\z}, templates_path Whitelist.add '/bin/chmod', '+x', templates_path # - Template removal Whitelist.add '/bin/rm', templates_path