Skip to content

Commit

Permalink
(#12463) eliminate secure_open in favour of replace_file
Browse files Browse the repository at this point in the history
None of the users of `secure_open` in the codebase actually want to achieve a
result different from `replace_file`, so we might as well eliminate the older
API in favour of one that is easier to use and clearer in intent.

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
  • Loading branch information
Daniel Pittman authored and haus committed Feb 20, 2012
1 parent 0c96703 commit f0c9995
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 33 deletions.
4 changes: 2 additions & 2 deletions lib/puppet/daemon.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ def daemonize
Puppet::Util::Log.reopen
rescue => detail
Puppet.err "Could not start #{Puppet[:name]}: #{detail}"
Puppet::Util::secure_open("/tmp/daemonout", "w") { |f|
Puppet::Util::replace_file("/tmp/daemonout", 0644) do |f|
f.puts "Could not start #{Puppet[:name]}: #{detail}"
}
end
exit(12)
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/network/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def daemonize
$stderr.reopen $stdout
Puppet::Util::Log.reopen
rescue => detail
Puppet::Util.secure_open("/tmp/daemonout", "w") { |f|
Puppet::Util.replace_file("/tmp/daemonout", 0644) { |f|
f.puts "Could not start #{Puppet[:name]}: #{detail}"
}
raise "Could not start #{Puppet[:name]}: #{detail}"
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/rails/benchmark.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,6 @@ def write_benchmarks
data = {}
end
data[branch] = $benchmarks
Puppet::Util.secure_open(file, "w") { |f| f.print YAML.dump(data) }
Puppet::Util.replace_file(file, 0644) { |f| f.print YAML.dump(data) }
end
end
22 changes: 0 additions & 22 deletions lib/puppet/util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -481,28 +481,6 @@ def thinmark

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

# Because IO#binread is only available in 1.9
def binread(file)
File.open(file, 'rb') { |f| f.read }
Expand Down
15 changes: 8 additions & 7 deletions lib/puppet/util/reference.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,15 @@ def self.page(*sections)

def self.pdf(text)
puts "creating pdf"
Puppet::Util.secure_open("/tmp/puppetdoc.txt", "w") do |f|
f.puts text
end
rst2latex = which('rst2latex') || which('rst2latex.py') || raise("Could not find rst2latex")
rst2latex = which('rst2latex') || which('rst2latex.py') ||
raise("Could not find rst2latex")

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
end
Puppet::Util.replace_file("/tmp/puppetdoc.txt") {|f| f.puts text }
# There used to be an attempt to use secure_open / replace_file to secure
# the target, too, but that did nothing: the race was still here. We can
# get exactly the same benefit from running this effort:
File.unlink('/tmp/puppetdoc.tex') rescue nil
output = %x{#{cmd}}
unless $CHILD_STATUS == 0
$stderr.puts "rst2latex failed"
Expand Down

0 comments on commit f0c9995

Please sign in to comment.