(PUP-1840) Use configured digest_algorithm as default checksum_type f…

…or file resources

Without this patch, file resources will always try to checksum
themselves using MD5. On FIPS 140-2 compliant hosts, this will
fail. This patch adds sha256 as a permissible value for the File
resource's checksum parameter, and makes the checksum parameter
default to using the digest_algorithm, as set in the puppet.conf.

lib/puppet/type/file.rb

@@ -613,7 +613,9 @@ def recurse_remote(children)
       end
       children[meta.relative_path] ||= newchild(meta.relative_path)
       children[meta.relative_path][:source] = meta.source
-      children[meta.relative_path][:checksum] = :md5 if meta.ftype == "file"
+      algo = Puppet[:digest_algorithm] || 'md5'
+      algo = algo.intern unless algo.is_a? Symbol
+      children[meta.relative_path][:checksum] = algo if meta.ftype == "file"
 
       children[meta.relative_path].parameter(:source).metadata = meta
     end

lib/puppet/type/file/checksum.rb

@@ -9,23 +9,27 @@
 
     The default checksum type is md5."
 
-  newvalues "md5", "md5lite", "mtime", "ctime", "none"
+  newvalues "md5", "md5lite", "sha256", "sha256lite", "mtime", "ctime", "none"
 
-  defaultto :md5
+  defaultto do
+    algo = Puppet[:digest_algorithm] || 'md5'
+    algo = algo.intern unless algo.is_a? Symbol
+    algo
+  end
 
   def sum(content)
-    type = value || :md5 # because this might be called before defaults are set
+    type = value || Puppet[:digest_algorithm] || :md5 # because this might be called before defaults are set
     "{#{type}}" + send(type, content)
   end
 
   def sum_file(path)
-    type = value || :md5 # because this might be called before defaults are set
+    type = value || Puppet[:digest_algorithm] || :md5 # because this might be called before defaults are set
     method = type.to_s + "_file"
     "{#{type}}" + send(method, path).to_s
   end
 
   def sum_stream(&block)
-    type = value || :md5 # same comment as above
+    type = value || Puppet[:digest_algorithm] || :md5 # same comment as above
     method = type.to_s + "_stream"
     checksum = send(method, &block)
     "{#{type}}#{checksum}"

spec/integration/type/file_spec.rb

@@ -10,7 +10,7 @@ class WindowsSecurity
   end
 end
 
-describe Puppet::Type.type(:file) do
+describe Puppet::Type.type(:file), :uses_checksums => true do
   include PuppetSpec::Files
 
   let(:catalog) { Puppet::Resource::Catalog.new }
@@ -401,7 +401,7 @@ def get_aces_for_path_by_sid(path, sid)
     end
   end
 
-  describe "when writing files" do
+  using_checksums_describe "when writing files" do
     it "should backup files to a filebucket when one is configured" do
       filebucket = Puppet::Type.type(:filebucket).new :path => tmpfile("filebucket"), :name => "mybucket"
       file = described_class.new :path => path, :backup => "mybucket", :content => "foo"
@@ -410,11 +410,11 @@ def get_aces_for_path_by_sid(path, sid)
 
       File.open(file[:path], "w") { |f| f.write("bar") }
 
-      md5 = Digest::MD5.hexdigest("bar")
+      d = digest(IO.binread(file[:path]))
 
       catalog.apply
 
-      filebucket.bucket.getfile(md5).should == "bar"
+      filebucket.bucket.getfile(d).should == "bar"
     end
 
     it "should backup files in the local directory when a backup string is provided" do
@@ -460,7 +460,7 @@ def get_aces_for_path_by_sid(path, sid)
       File.open(dest1, "w") { |f| f.puts "whatever" }
       Puppet::FileSystem.symlink(dest1, link)
 
-      md5 = Digest::MD5.hexdigest(File.read(file[:path]))
+      d = digest(File.read(file[:path]))
 
       catalog.apply
 
@@ -498,13 +498,13 @@ def get_aces_for_path_by_sid(path, sid)
       File.open(barfile, "w") { |f| f.print "baryay" }
 
 
-      foomd5 = Digest::MD5.hexdigest(File.read(foofile))
-      barmd5 = Digest::MD5.hexdigest(File.read(barfile))
+      food = digest(File.read(foofile))
+      bard = digest(File.read(barfile))
 
       catalog.apply
 
-      bucket.bucket.getfile(foomd5).should == "fooyay"
-      bucket.bucket.getfile(barmd5).should == "baryay"
+      bucket.bucket.getfile(food).should == "fooyay"
+      bucket.bucket.getfile(bard).should == "baryay"
     end
   end
 

spec/unit/application/inspect_spec.rb

@@ -33,7 +33,7 @@
     end
   end
 
-  describe "when executing" do
+  describe "when executing", :uses_checksums => true do
     before :each do
       Puppet[:report] = true
       @inspect.options[:setdest] = true
@@ -54,11 +54,11 @@
       @inspect.run_command
     end
 
-    it "should audit the specified properties" do
+    using_checksums_it "should audit the specified properties" do
       catalog = Puppet::Resource::Catalog.new
       file = Tempfile.new("foo")
       file.binmode
-      file.puts("file contents")
+      file.print plaintext
       file.close
       resource = Puppet::Resource.new(:file, file.path, :parameters => {:audit => "all"})
       catalog.add_resource(resource)
@@ -76,7 +76,7 @@
         property_values.merge(event.property => event.previous_value)
       end
       properties["ensure"].should == :file
-      properties["content"].should == "{md5}#{Digest::MD5.hexdigest("file contents\n")}"
+      properties["content"].should == "{#{algo}}#{checksum}"
       properties.has_key?("target").should == false
     end
 

spec/unit/type/file/checksum_spec.rb

@@ -30,6 +30,12 @@
     @checksum.sum("foobar").should == "{md5}#{sum}"
   end
 
+  it "when using digest_algorithm 'sha256' should return the summed contents with a checksum label" do
+    sum = Digest::SHA256.hexdigest("foobar")
+    @resource[:checksum] = :sha256
+    @checksum.sum("foobar").should == "{sha256}#{sum}"
+  end
+
   it "should use :md5 as its default type" do
     @checksum.default.should == :md5
   end

spec/unit/type/file/content_spec.rb

@@ -5,14 +5,20 @@
 require 'puppet/network/resolver'
 
 content = Puppet::Type.type(:file).attrclass(:content)
-describe content do
+
+describe content, :uses_checksums => true do
+using_checksums_describe "" do
   include PuppetSpec::Files
   before do
+    # Construct the @resource after the digest_algorithm Puppet
+    # setting is set by using_checksums_describe, so that the resource
+    # will get the setting.
     @filename = tmpfile('testfile')
     @catalog = Puppet::Resource::Catalog.new
     @resource = Puppet::Type.type(:file).new :path => @filename, :catalog => @catalog
     File.open(@filename, 'w') {|f| f.write "initial file content"}
     content.stubs(:standalone?).returns(false)
+    @algo = algo || 'md5'
   end
 
   describe "when determining the checksum type" do
@@ -30,6 +36,11 @@
       @content = content.new(:resource => @resource)
       @content.checksum_type.should == :md5lite
     end
+
+    it "should use the type specified by digest_algorithm by default" do
+      @content = content.new(:resource => @resource)
+      @content.checksum_type.should == @algo.intern
+    end
   end
 
   describe "when determining the actual content to write" do
@@ -60,12 +71,12 @@
 
     it "should store the checksum as the desired content" do
       @content = content.new(:resource => @resource)
-      digest = Digest::MD5.hexdigest("this is some content")
+      d = digest("this is some content")
 
-      @content.stubs(:checksum_type).returns "md5"
+      @content.stubs(:checksum_type).returns @algo
       @content.should = "this is some content"
 
-      @content.should.must == "{md5}#{digest}"
+      @content.should.must == "{#@algo}#{d}"
     end
 
     it "should not checksum 'absent'" do
@@ -77,9 +88,9 @@
 
     it "should accept a checksum as the desired content" do
       @content = content.new(:resource => @resource)
-      digest = Digest::MD5.hexdigest("this is some content")
+      d = digest("this is some content")
 
-      string = "{md5}#{digest}"
+      string = "{#@algo}#{d}"
       @content.should = string
 
       @content.should.must == string
@@ -136,9 +147,9 @@
       @content = content.new(:resource => @resource)
       stat = mock 'stat', :ftype => "file"
       @resource.expects(:stat).returns stat
-      @resource.parameter(:checksum).expects(:md5_file).with(@resource[:path]).returns "mysum"
+      @resource.parameter(:checksum).expects("#{@algo}_file".intern).with(@resource[:path]).returns "mysum"
 
-      @content.retrieve.should == "{md5}mysum"
+      @content.retrieve.should == "{#@algo}mysum"
     end
   end
 
@@ -188,9 +199,8 @@
       end
 
       it "should return true if the sum for the current contents is the same as the sum for the desired content" do
-        @content.must be_safe_insync("{md5}" + Digest::MD5.hexdigest("some content"))
+        @content.must be_safe_insync("{#@algo}" + digest("some content"))
       end
-
       [true, false].product([true, false]).each do |cfg, param|
         describe "and Puppet[:show_diff] is #{cfg} and show_diff => #{param}" do
           before do
@@ -344,7 +354,7 @@
 
       it "should return the checksum computed" do
         File.open(@filename, 'wb') do |file|
-          @content.write(file).should == "{md5}#{Digest::MD5.hexdigest(@source_content)}"
+          @content.write(file).should == "{#@algo}#{digest(@source_content)}"
         end
       end
     end
@@ -385,7 +395,7 @@
 
       it "should return the checksum computed" do
         File.open(@filename, 'w') do |file|
-          @content.write(file).should == "{md5}#{Digest::MD5.hexdigest(@source_content)}"
+          @content.write(file).should == "{#@algo}#{digest(@source_content)}"
         end
       end
     end
@@ -461,3 +471,4 @@
     end
   end
 end
+end

spec/unit/type/file_spec.rb

@@ -664,7 +664,7 @@
     end
   end
 
-  describe "#recurse_remote" do
+  describe "#recurse_remote", :uses_checksums => true do
     let(:my) { File.expand_path('/my') }
 
     before do
@@ -722,12 +722,14 @@
 
     # LAK:FIXME This is a bug, but I can't think of a fix for it.  Fortunately it's already
     # filed, and when it's fixed, we'll just fix the whole flow.
-    it "should set the checksum type to :md5 if the remote file is a file" do
-      @first.stubs(:ftype).returns "file"
-      file.stubs(:perform_recursion).returns [@first]
-      @resource.stubs(:[]=)
-      @resource.expects(:[]=).with(:checksum, :md5)
-      file.recurse_remote("first" => @resource)
+    using_checksums_describe "it should set the checksum type" do
+      it "to #{(metadata[:digest_algorithm] || 'md5').intern} if the remote file is a file" do
+        @first.stubs(:ftype).returns "file"
+        file.stubs(:perform_recursion).returns [@first]
+        @resource.stubs(:[]=)
+        @resource.expects(:[]=).with(:checksum, (algo || 'md5').intern)
+        file.recurse_remote("first" => @resource)
+      end
     end
 
     it "should store the metadata in the source property for each resource so the source does not have to requery the metadata" do