Rein Henrichs <[email protected]> writes: *nod* Attached is the updated patch from my PoC local git branch, doing that.
> We should definitely be using Tempfile here, it is mktemp like (and avoids > TOCTOU attacks). > > Rein Henrichs > http://reductivelabs.com > > On Thu, Apr 8, 2010 at 11:50 PM, Luke Kanies <[email protected]> wrote: > >> On Apr 8, 2010, at 8:48 PM, Daniel Pittman wrote: >> >>> Luke Kanies <[email protected]> writes: >>> >>>> AFAIK the file shouldn't stay past the existence of the process itself - >>>> it just needs to persist after the user is done editing it. >>>> >>> Really? I though the purpose of 'ralsh --edit' was to let me modify the >>> file and use the content later. If not, then Tempfile alone is absolutely >>> the right answer. >> >> Nope, the purpose is to let you edit that file and then run it right >> away. I think it currently even prints the edited resource on stdout once >> it's done modifying it (which I consider a bug, ftr). -- ✣ Daniel Pittman ✉ [email protected] ☎ +61 401 155 707 ♽ made with 100 percent post-consumer electrons
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
>From 3721a6d4f939df693a4d8335f8a6c3ed89b461f1 Mon Sep 17 00:00:00 2001 From: Daniel Pittman <[email protected]> Date: Thu, 8 Apr 2010 23:44:14 +1000 Subject: [PATCH] replace_file: eliminate the old secure_open API. Since the purpose of the two methods was identical, but replace_file is marginally easier to use, simply migrate all the callers of replace_file. --- lib/puppet/daemon.rb | 2 +- lib/puppet/network/server.rb | 2 +- lib/puppet/rails/benchmark.rb | 2 +- lib/puppet/util.rb | 22 ---------------------- lib/puppet/util/reference.rb | 15 ++++++++------- 5 files changed, 11 insertions(+), 32 deletions(-) diff --git a/lib/puppet/daemon.rb b/lib/puppet/daemon.rb index 0f538fe..6479503 100755 --- a/lib/puppet/daemon.rb +++ b/lib/puppet/daemon.rb @@ -32,7 +32,7 @@ class Puppet::Daemon Puppet::Util::Log.reopen rescue => detail Puppet.err "Could not start %s: %s" % [Puppet[:name], detail] - Puppet::Util::secure_open("/tmp/daemonout", "w") { |f| + Puppet::Util.replace("/tmp/daemonout", 0640) { |f| f.puts "Could not start %s: %s" % [Puppet[:name], detail] } exit(12) diff --git a/lib/puppet/network/server.rb b/lib/puppet/network/server.rb index 01a55df..3a495b9 100644 --- a/lib/puppet/network/server.rb +++ b/lib/puppet/network/server.rb @@ -22,7 +22,7 @@ class Puppet::Network::Server $stderr.reopen $stdout Puppet::Util::Log.reopen rescue => detail - Puppet::Util.secure_open("/tmp/daemonout", "w") { |f| + Puppet::Util.replace("/tmp/daemonout", 0640) { |f| f.puts "Could not start %s: %s" % [Puppet[:name], detail] } raise "Could not start %s: %s" % [Puppet[:name], detail] diff --git a/lib/puppet/rails/benchmark.rb b/lib/puppet/rails/benchmark.rb index 1fbd011..d72518d 100644 --- a/lib/puppet/rails/benchmark.rb +++ b/lib/puppet/rails/benchmark.rb @@ -64,6 +64,6 @@ module Puppet::Rails::Benchmark data = {} end data[branch] = $benchmarks - Puppet::Util.secure_open(file, "w") { |f| f.print YAML.dump(data) } + Puppet::Util.replace(file, 0644) { |f| f.print YAML.dump(data) } end end diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb index 036032e..00b6710 100644 --- a/lib/puppet/util.rb +++ b/lib/puppet/util.rb @@ -424,28 +424,6 @@ module Util module_function :memory, :thinmark - def secure_open(file,must_be_w,&block) - raise Puppet::DevError,"secure_open only works with mode 'w'" unless must_be_w == 'w' - raise Puppet::DevError,"secure_open only requires a block" unless block_given? - Puppet.warning "#{file} was a symlink to #{File.readlink(file)}" if File.symlink?(file) - if File.exists?(file) or File.symlink?(file) - wait = File.symlink?(file) ? 5.0 : 0.1 - File.delete(file) - sleep wait # give it a chance to reappear, just in case someone is actively trying something. - end - begin - File.open(file,File::CREAT|File::EXCL|File::TRUNC|File::WRONLY,&block) - rescue Errno::EEXIST - desc = File.symlink?(file) ? "symlink to #{File.readlink(file)}" : File.stat(file).ftype - puts "Warning: #{file} was apparently created by another process (as" - puts "a #{desc}) as soon as it was deleted by this process. Someone may be trying" - puts "to do something objectionable (such as tricking you into overwriting system" - puts "files if you are running as root)." - raise - end - end - module_function :secure_open - def replace_file (file, mode = 0600, validate = nil, &block) raise Puppet::DevError,"replace_file requires a block" unless block_given? if validate and !validate.is_a?(Lambda) diff --git a/lib/puppet/util/reference.rb b/lib/puppet/util/reference.rb index 93542df..7e63ea3 100644 --- a/lib/puppet/util/reference.rb +++ b/lib/puppet/util/reference.rb @@ -36,7 +36,7 @@ class Puppet::Util::Reference def self.pdf(text) puts "creating pdf" - Puppet::Util.secure_open("/tmp/puppetdoc.txt", "w") do |f| + Puppet::Util.replace("/tmp/puppetdoc.txt", 0644) do |f| f.puts text end rst2latex = %x{which rst2latex} @@ -47,11 +47,12 @@ class Puppet::Util::Reference raise "Could not find rst2latex" end rst2latex.chomp! - cmd = %{#{rst2latex} /tmp/puppetdoc.txt > /tmp/puppetdoc.tex} - Puppet::Util.secure_open("/tmp/puppetdoc.tex","w") do |f| - # If we get here without an error, /tmp/puppetdoc.tex isn't a tricky cracker's symlink + cmd = %{#{rst2latex} /tmp/puppetdoc.txt} + Puppet::Util.replace("/tmp/puppetdoc.tex", 0644) do |f| + # Write the output, making sure that /tmp/puppetdoc.tex isn't a + # tricky cracker's symlink or something. + f.print %x{#{cmd}} end - output = %x{#{cmd}} unless $? == 0 $stderr.puts "rst2latex failed" $stderr.puts output @@ -70,7 +71,7 @@ class Puppet::Util::Reference puts "Creating markdown for #{name} reference." dir = "/tmp/" + Puppet::PUPPETVERSION FileUtils.mkdir(dir) unless File.directory?(dir) - Puppet::Util.secure_open(dir + "/" + "#{name}.rst", "w") do |f| + Puppet::Util.replace(dir + "/" + "#{name}.rst", 0644) do |f| f.puts text end pandoc = %x{which pandoc} @@ -193,7 +194,7 @@ class Puppet::Util::Reference end def trac - Puppet::Util.secure_open("/tmp/puppetdoc.txt", "w") do |f| + Puppet::Util.replace("/tmp/puppetdoc.txt", 0644) do |f| f.puts self.to_trac end -- 1.7.0.4
