Make etag opt in

Author: C24-AK

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

@@ -40,11 +40,11 @@ def initialize(http_response, path = '/dev/null')
       etag_value = etag.delete('"').strip
       case etag_value
       when /\A[0-9a-f]{64}\z/i
-        @checksums[:sha256] ||= "{sha256}#{etag_value.downcase}"
+        @checksums[:etag] = "{sha256}#{etag_value.downcase}"
       when /\A[0-9a-f]{40}\z/i
-        @checksums[:sha1] ||= "{sha1}#{etag_value.downcase}"
+        @checksums[:etag] = "{sha1}#{etag_value.downcase}"
       when /\A[0-9a-f]{32}\z/i
-        @checksums[:md5] ||= "{md5}#{etag_value.downcase}"
+        @checksums[:etag] = "{md5}#{etag_value.downcase}"
       end
     end
 
@@ -64,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

@@ -115,112 +115,131 @@
     end
 
     context "with an ETag header" do
-      context "containing an MD5 hash" do
+      context "without checksum => etag" do
         let(:md5) { "f5ffec8d8d16b43d5e9ac6ad4330c445" }
 
-        it "uses the ETag as an md5 checksum" do
+        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 :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.collect
-          expect( metadata.checksum ).to eq "{md5}#{md5}"
+          expect( metadata.checksum_type ).to eq :mtime
         end
       end
 
-      context "containing a SHA1 hash" do
-        let(:sha1) { "01e4d15746f4274b84d740a93e04b9fd2882e3ea" }
-
-        it "uses the ETag as a sha1 checksum" do
-          http_response.add_field('ETag', %("#{sha1}"))
-          metadata = described_class.new(http_response)
-          metadata.collect
-          expect( metadata.checksum_type ).to eq :sha1
-          expect( metadata.checksum ).to eq "{sha1}#{sha1}"
+      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
-      end
 
-      context "containing a SHA256 hash" do
-        let(:sha256) { "a3eda98259c30e1e75039c2123670c18105e1c46efb672e42ca0e4cbe77b002a" }
+        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
 
-        it "uses the ETag as a sha256 checksum" do
-          http_response.add_field('ETag', %("#{sha256}"))
-          metadata = described_class.new(http_response)
-          metadata.collect
-          expect( metadata.checksum_type ).to eq :sha256
-          expect( metadata.checksum ).to eq "{sha256}#{sha256}"
+        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
-      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.collect
-          expect( metadata.checksum_type ).to eq :mtime
+        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
-      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.collect
-          expect( metadata.checksum_type ).to eq :mtime
+        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
-      end
 
-      context "when explicit checksum headers are also present" do
-        let(:explicit_md5) { "c58989e9740a748de4f5054286faf99b" }
-        let(:etag_md5) { "f5ffec8d8d16b43d5e9ac6ad4330c445" }
+        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
 
-        it "prefers X-Checksum-Md5 over ETag" do
-          http_response.add_field('X-Checksum-Md5', explicit_md5)
-          http_response.add_field('ETag', %("#{etag_md5}"))
-          metadata = described_class.new(http_response)
-          metadata.collect
-          expect( metadata.checksum_type ).to eq :md5
-          expect( metadata.checksum ).to eq "{md5}#{explicit_md5}"
+        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 "prefers Content-MD5 over ETag" do
-          base64 = [etag_md5].pack("H*").then { |bin| [bin].pack("m0") }
-          http_response.add_field('Content-MD5', base64)
-          http_response.add_field('ETag', '"0000000000000000000000000000dead"')
+        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 ).to start_with "{md5}"
-          expect( metadata.checksum ).not_to include "dead"
+          expect( metadata.checksum_type ).to eq :mtime
         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"))
+        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 :md5
-          expect( metadata.checksum ).to eq "{md5}#{md5}"
+          expect( metadata.checksum_type ).to eq :mtime
         end
       end
-
-      it "skips ETag-derived md5 on FIPS platforms" 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.collect
-        expect( metadata.checksum_type ).to eq :mtime
-      end
     end
   end
 end

spec/unit/indirector/file_metadata/http_spec.rb

@@ -80,17 +80,26 @@
       expect(result.checksum).to eq("{mtime}2020-01-01 08:00:00 UTC")
     end
 
-    it "uses ETag as md5 when it contains a hex digest" do
+    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" do
+    it "prefers explicit X-Checksum-Sha256 over ETag when not using etag checksum" do
       sha256 = "a3eda98259c30e1e75039c2123670c18105e1c46efb672e42ca0e4cbe77b002a"
       etag_md5 = "f5ffec8d8d16b43d5e9ac6ad4330c445"
       stub_request(:head, key)
@@ -104,13 +113,13 @@
       expect(result.checksum).to eq("{sha256}#{sha256}")
     end
 
-    it "ignores weak ETags" do
+    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)
+      result = model.indirection.find(key, checksum_type: :etag)
       expect(result.checksum_type).to eq(:mtime)
     end
 

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