Merge pull request #329 from C24-AK/feature/file-etag-support

Feature: file etag support

Author: bastelfreak

lib/puppet/defaults.rb

@@ -26,8 +26,8 @@ def self.default_file_checksum_types
 
   def self.valid_file_checksum_types
     Puppet::Util::Platform.fips_enabled? ?
-      %w[sha256 sha256lite sha384 sha512 sha224 sha1 sha1lite mtime ctime] :
-      %w[sha256 sha256lite sha384 sha512 sha224 sha1 sha1lite md5 md5lite mtime ctime]
+      %w[sha256 sha256lite sha384 sha512 sha224 sha1 sha1lite mtime ctime etag] :
+      %w[sha256 sha256lite sha384 sha512 sha224 sha1 sha1lite md5 md5lite mtime ctime etag]
   end
 
   def self.default_cadir

lib/puppet/file_serving/http_metadata.rb

@@ -35,6 +35,19 @@ def initialize(http_response, path = '/dev/null')
       end
     end
 
+    etag = http_response['etag']
+    if etag && !etag.start_with?('W/')
+      etag_value = etag.delete('"').strip
+      case etag_value
+      when /\A[0-9a-f]{64}\z/i
+        @checksums[:etag] = "{sha256}#{etag_value.downcase}"
+      when /\A[0-9a-f]{40}\z/i
+        @checksums[:etag] = "{sha1}#{etag_value.downcase}"
+      when /\A[0-9a-f]{32}\z/i
+        @checksums[:etag] = "{md5}#{etag_value.downcase}"
+      end
+    end
+
     last_modified = http_response['last-modified']
     if last_modified
       mtime = DateTime.httpdate(last_modified).to_time
@@ -51,6 +64,18 @@ def collect
     # Prefer the checksum_type from the indirector request options
     # but fall back to the alternative otherwise
     [@checksum_type, :sha256, :sha1, :md5, :mtime].each do |type|
+      if type == :etag
+        if @checksums[:etag]
+          @checksum = @checksums[:etag]
+          resolved = sumtype(@checksum).to_sym
+          next if resolved == :md5 && Puppet::Util::Platform.fips_enabled?
+
+          @checksum_type = resolved
+          break
+        end
+        next
+      end
+
       next if type == :md5 && Puppet::Util::Platform.fips_enabled?
 
       @checksum_type = type

lib/puppet/type/file/checksum.rb

@@ -13,7 +13,7 @@
     The default checksum type is sha256."
 
   # The values are defined in Puppet::Util::Checksums.known_checksum_types
-  newvalues(:sha256, :sha256lite, :md5, :md5lite, :sha1, :sha1lite, :sha512, :sha384, :sha224, :mtime, :ctime, :none)
+  newvalues(:sha256, :sha256lite, :md5, :md5lite, :sha1, :sha1lite, :sha512, :sha384, :sha224, :mtime, :ctime, :none, :etag)
 
   defaultto do
     Puppet[:digest_algorithm].to_sym
@@ -47,8 +47,18 @@ def sum_stream(&block)
   private
 
   # Return the appropriate digest algorithm with fallbacks in case puppet defaults have not
-  # been initialized.
+  # been initialized. When the checksum type is :etag, resolve to the actual
+  # hash algorithm that the HTTP server's ETag represents.
   def digest_algorithm
-    value || Puppet[:digest_algorithm].to_sym
+    type = value || Puppet[:digest_algorithm].to_sym
+    return type unless type == :etag
+
+    source = resource.parameter(:source)
+    resolved = source&.metadata&.checksum_type
+    if resolved && ![:etag, :mtime, :ctime, :none].include?(resolved)
+      return resolved
+    end
+
+    :md5
   end
 end

lib/puppet/util/checksums.rb

@@ -17,7 +17,8 @@ module Puppet::Util::Checksums
     :sha512,
     :sha384,
     :sha224,
-    :mtime, :ctime, :none
+    :mtime, :ctime, :none,
+    :etag
   ].freeze
 
   # It's not a good idea to use some of these in some contexts: for example, I
@@ -339,6 +340,25 @@ def none_stream
     ""
   end
 
+  # ETag-based checksum delegates to md5 for local file computation.
+  # The actual algorithm is determined at runtime by HttpMetadata#collect
+  # based on the ETag header length.
+  def etag(content)
+    md5(content)
+  end
+
+  def etag?(string)
+    string =~ /^\h{32,64}$/
+  end
+
+  def etag_file(filename, lite = false)
+    md5_file(filename, lite)
+  end
+
+  def etag_stream(lite = false, &block)
+    md5_stream(lite, &block)
+  end
+
   class DigestLite
     def initialize(digest, lite = false)
       @digest = digest

spec/unit/file_serving/http_metadata_spec.rb

@@ -113,5 +113,133 @@
         expect( metadata.checksum ).to eq "{sha256}#{sha256}"
       end
     end
+
+    context "with an ETag header" do
+      context "without checksum => etag" do
+        let(:md5) { "f5ffec8d8d16b43d5e9ac6ad4330c445" }
+
+        it "does not auto-activate ETag and falls back to mtime" do
+          http_response.add_field('ETag', %("#{md5}"))
+          metadata = described_class.new(http_response)
+          metadata.collect
+          expect( metadata.checksum_type ).to eq :mtime
+        end
+      end
+
+      context "with checksum_type => etag" do
+        context "containing an MD5 hash" do
+          let(:md5) { "f5ffec8d8d16b43d5e9ac6ad4330c445" }
+
+          it "resolves to md5" do
+            http_response.add_field('ETag', %("#{md5}"))
+            metadata = described_class.new(http_response)
+            metadata.checksum_type = :etag
+            metadata.collect
+            expect( metadata.checksum_type ).to eq :md5
+            expect( metadata.checksum ).to eq "{md5}#{md5}"
+          end
+
+          it "normalizes uppercase hex to lowercase" do
+            http_response.add_field('ETag', %("#{md5.upcase}"))
+            metadata = described_class.new(http_response)
+            metadata.checksum_type = :etag
+            metadata.collect
+            expect( metadata.checksum ).to eq "{md5}#{md5}"
+          end
+        end
+
+        context "containing a SHA1 hash" do
+          let(:sha1) { "01e4d15746f4274b84d740a93e04b9fd2882e3ea" }
+
+          it "resolves to sha1" do
+            http_response.add_field('ETag', %("#{sha1}"))
+            metadata = described_class.new(http_response)
+            metadata.checksum_type = :etag
+            metadata.collect
+            expect( metadata.checksum_type ).to eq :sha1
+            expect( metadata.checksum ).to eq "{sha1}#{sha1}"
+          end
+        end
+
+        context "containing a SHA256 hash" do
+          let(:sha256) { "a3eda98259c30e1e75039c2123670c18105e1c46efb672e42ca0e4cbe77b002a" }
+
+          it "resolves to sha256" do
+            http_response.add_field('ETag', %("#{sha256}"))
+            metadata = described_class.new(http_response)
+            metadata.checksum_type = :etag
+            metadata.collect
+            expect( metadata.checksum_type ).to eq :sha256
+            expect( metadata.checksum ).to eq "{sha256}#{sha256}"
+          end
+        end
+
+        context "that is a weak ETag" do
+          it "ignores the ETag and falls back to mtime" do
+            http_response.add_field('ETag', 'W/"f5ffec8d8d16b43d5e9ac6ad4330c445"')
+            metadata = described_class.new(http_response)
+            metadata.checksum_type = :etag
+            metadata.collect
+            expect( metadata.checksum_type ).to eq :mtime
+          end
+        end
+
+        context "that is not a recognizable hash" do
+          it "ignores the ETag and falls back to mtime" do
+            http_response.add_field('ETag', '"5e8c5-27a-3e8b8840"')
+            metadata = described_class.new(http_response)
+            metadata.checksum_type = :etag
+            metadata.collect
+            expect( metadata.checksum_type ).to eq :mtime
+          end
+        end
+
+        context "when explicit checksum headers are also present" do
+          let(:explicit_md5) { "c58989e9740a748de4f5054286faf99b" }
+          let(:etag_md5) { "f5ffec8d8d16b43d5e9ac6ad4330c445" }
+
+          it "prefers the ETag over X-Checksum-Md5" do
+            http_response.add_field('X-Checksum-Md5', explicit_md5)
+            http_response.add_field('ETag', %("#{etag_md5}"))
+            metadata = described_class.new(http_response)
+            metadata.checksum_type = :etag
+            metadata.collect
+            expect( metadata.checksum_type ).to eq :md5
+            expect( metadata.checksum ).to eq "{md5}#{etag_md5}"
+          end
+        end
+
+        context "with ETag and Last-Modified" do
+          let(:md5) { "f5ffec8d8d16b43d5e9ac6ad4330c445" }
+          let(:time) { Time.now.utc }
+
+          it "prefers ETag-derived md5 over mtime" do
+            http_response.add_field('ETag', %("#{md5}"))
+            http_response.add_field('last-modified', time.strftime("%a, %d %b %Y %T GMT"))
+            metadata = described_class.new(http_response)
+            metadata.checksum_type = :etag
+            metadata.collect
+            expect( metadata.checksum_type ).to eq :md5
+            expect( metadata.checksum ).to eq "{md5}#{md5}"
+          end
+        end
+
+        it "skips ETag-derived md5 on FIPS platforms and falls back" do
+          allow(Puppet::Util::Platform).to receive(:fips_enabled?).and_return(true)
+          http_response.add_field('ETag', '"f5ffec8d8d16b43d5e9ac6ad4330c445"')
+          metadata = described_class.new(http_response)
+          metadata.checksum_type = :etag
+          metadata.collect
+          expect( metadata.checksum_type ).to eq :mtime
+        end
+
+        it "falls back to other checksums when no ETag is present" do
+          metadata = described_class.new(http_response)
+          metadata.checksum_type = :etag
+          metadata.collect
+          expect( metadata.checksum_type ).to eq :mtime
+        end
+      end
+    end
   end
 end

spec/unit/indirector/catalog/compiler_spec.rb

@@ -226,7 +226,7 @@ def set_facts(fact_hash)
       allow(node.environment).to receive(:static_catalogs?).and_return(true)
 
       expect { compiler.find(@request) }.to raise_error Puppet::Error,
-        "Unable to find a common checksum type between agent 'atime.md2' and master '[:sha256, :sha256lite, :md5, :md5lite, :sha1, :sha1lite, :sha512, :sha384, :sha224, :mtime, :ctime, :none]'."
+        "Unable to find a common checksum type between agent 'atime.md2' and master '[:sha256, :sha256lite, :md5, :md5lite, :sha1, :sha1lite, :sha512, :sha384, :sha224, :mtime, :ctime, :none, :etag]'."
     end
 
     it "errors if checksum_type contains no shared checksum types" do
@@ -237,7 +237,7 @@ def set_facts(fact_hash)
       allow(node.environment).to receive(:static_catalogs?).and_return(true)
 
       expect { compiler.find(@request) }.to raise_error Puppet::Error,
-        "Unable to find a common checksum type between agent '' and master '[:sha256, :sha256lite, :md5, :md5lite, :sha1, :sha1lite, :sha512, :sha384, :sha224, :mtime, :ctime, :none]'."
+        "Unable to find a common checksum type between agent '' and master '[:sha256, :sha256lite, :md5, :md5lite, :sha1, :sha1lite, :sha512, :sha384, :sha224, :mtime, :ctime, :none, :etag]'."
     end
 
     it "prevents the environment from being evicted during compilation" do

spec/unit/indirector/file_metadata/http_spec.rb

@@ -80,6 +80,49 @@
       expect(result.checksum).to eq("{mtime}2020-01-01 08:00:00 UTC")
     end
 
+    it "does not auto-activate ETag without checksum_type => etag" do
+      etag_md5 = "f5ffec8d8d16b43d5e9ac6ad4330c445"
+      stub_request(:head, key)
+        .to_return(status: 200, headers: DEFAULT_HEADERS.merge("ETag" => %("#{etag_md5}")))
+
+      result = model.indirection.find(key)
+      expect(result.checksum_type).to eq(:mtime)
+    end
+
+    it "uses ETag as md5 when checksum_type is etag" do
+      etag_md5 = "f5ffec8d8d16b43d5e9ac6ad4330c445"
+      stub_request(:head, key)
+        .to_return(status: 200, headers: DEFAULT_HEADERS.merge("ETag" => %("#{etag_md5}")))
+
+      result = model.indirection.find(key, checksum_type: :etag)
+      expect(result.checksum_type).to eq(:md5)
+      expect(result.checksum).to eq("{md5}#{etag_md5}")
+    end
+
+    it "prefers explicit X-Checksum-Sha256 over ETag when not using etag checksum" do
+      sha256 = "a3eda98259c30e1e75039c2123670c18105e1c46efb672e42ca0e4cbe77b002a"
+      etag_md5 = "f5ffec8d8d16b43d5e9ac6ad4330c445"
+      stub_request(:head, key)
+        .to_return(status: 200, headers: DEFAULT_HEADERS.merge(
+          "X-Checksum-Sha256" => sha256,
+          "ETag" => %("#{etag_md5}")
+        ))
+
+      result = model.indirection.find(key)
+      expect(result.checksum_type).to eq(:sha256)
+      expect(result.checksum).to eq("{sha256}#{sha256}")
+    end
+
+    it "ignores weak ETags even with checksum_type => etag" do
+      stub_request(:head, key)
+        .to_return(status: 200, headers: DEFAULT_HEADERS.merge(
+          "ETag" => 'W/"f5ffec8d8d16b43d5e9ac6ad4330c445"'
+        ))
+
+      result = model.indirection.find(key, checksum_type: :etag)
+      expect(result.checksum_type).to eq(:mtime)
+    end
+
     it "leniently parses base64" do
       # Content-MD5 header is missing '==' padding
       stub_request(:head, key)

spec/unit/type/file_spec.rb

@@ -1465,7 +1465,7 @@
       file[:source] = source
     end
 
-    Puppet::Type::File::ParameterChecksum.value_collection.values.reject {|v| v == :none}.each do |checksum_type|
+    Puppet::Type::File::ParameterChecksum.value_collection.values.reject {|v| v == :none || v == :etag}.each do |checksum_type|
       describe "with checksum '#{checksum_type}'" do
         before do
           file[:checksum] = checksum_type
@@ -1591,7 +1591,7 @@
       file[:content] = FILE_CONTENT
     end
 
-    (Puppet::Type::File::ParameterChecksum.value_collection.values - SOURCE_ONLY_CHECKSUMS).each do |checksum_type|
+    (Puppet::Type::File::ParameterChecksum.value_collection.values - SOURCE_ONLY_CHECKSUMS - [:etag]).each do |checksum_type|
       describe "with checksum '#{checksum_type}'" do
         before do
           file[:checksum] = checksum_type

spec/unit/util/checksums_spec.rb

@@ -10,7 +10,7 @@
   end
 
   content_sums = [:md5, :md5lite, :sha1, :sha1lite, :sha256, :sha256lite, :sha512, :sha384, :sha224]
-  file_only = [:ctime, :mtime, :none]
+  file_only = [:ctime, :mtime, :none, :etag]
 
   content_sums.each do |sumtype|
     it "should be able to calculate #{sumtype} sums from strings" do