(maint) Merge 6.25.1 release to 6.x

luchihoratiu · luchihoratiu · commit 3203125c0f20 · 2021-11-10T11:07:55.000+02:00
* tag '6.25.1': (PUP-11328) Get the environment's name without loading the environment (PUP-11328) Fallback to node request in configurer (PUP-11319) Move `DEFAULT_TIMEOUT` constant for Windows services (PUP-9561) Optimize parameter validation for blocks (PUP-11209) Puppet agent silently skips unknown resources. (PUP-11188) Do not add auth and cookie header when redirecting
diff --git a/acceptance/tests/agent/agent_fails_with_unknown_resource.rb b/acceptance/tests/agent/agent_fails_with_unknown_resource.rb
@@ -0,0 +1,77 @@
+test_name "agent run should fail if it finds an unknown resource type" do
+  tag 'audit:high',
+      'audit:integration'
+
+  require 'puppet/acceptance/common_utils'
+
+  require 'puppet/acceptance/environment_utils'
+  extend Puppet::Acceptance::EnvironmentUtils
+
+  require 'puppet/acceptance/temp_file_utils'
+  extend Puppet::Acceptance::TempFileUtils
+  
+  step "agent should fail when it can't find a resource" do
+    vendor_modules_path = master.tmpdir('vendor_modules')
+    tmp_environment = mk_tmp_environment_with_teardown(master, 'tmp')
+
+    site_pp_content = <<-SITEPP
+      define foocreateresource($one) {
+        $msg = 'hello'
+        notify { $name: message => $msg }
+      }
+      class example($x) {
+        if $x == undef or $x == [] or $x == '' {
+          notice 'foo'
+          return()
+        }
+        notice 'bar'
+      }
+      node default {
+        class { example: x => [] }
+        create_resources('foocreateresource', {'blah'=>{'one'=>'two'}})
+        mycustomtype{'foobar':}
+      }
+    SITEPP
+    manifests_path = "/tmp/#{tmp_environment}/manifests"
+    on(master, "mkdir -p '#{manifests_path}'")
+    create_remote_file(master, "#{manifests_path}/site.pp", site_pp_content)
+
+    custom_type_content = <<-CUSTOMTYPE
+      Puppet::Type.newtype(:mycustomtype) do
+        @doc = "Create a new mycustomtype thing."
+
+        newparam(:name, :namevar => true) do
+          desc "Name of mycustomtype instance"
+        end
+
+        def refresh
+        end
+      end
+    CUSTOMTYPE
+    type_path = "#{vendor_modules_path}/foo/lib/puppet/type"
+    on(master, "mkdir -p '#{type_path}'")
+    create_remote_file(master, "#{type_path}/mycustomtype.rb", custom_type_content)
+
+    on(master, "chmod -R 750 '#{vendor_modules_path}' '/tmp/#{tmp_environment}'")
+    on(master, "chown -R #{master.puppet['user']}:#{master.puppet['group']} '#{vendor_modules_path}' '/tmp/#{tmp_environment}'")
+
+    master_opts = {
+      'main' => {
+        'environment' => tmp_environment,
+        'vendormoduledir' => vendor_modules_path
+       }
+    }
+
+    with_puppet_running_on(master, master_opts) do
+      agents.each do |agent|
+        teardown do
+          agent.rm_rf(vendor_modules_path)
+        end
+
+        on(agent, puppet('agent', '-t', '--environment', tmp_environment), acceptable_exit_codes: [1]) do |result|  
+          assert_match(/Error: Failed to apply catalog: Resource type 'Mycustomtype' was not found/, result.stderr)
+        end
+      end
+    end
+  end
+end
diff --git a/api/schemas/catalog.json b/api/schemas/catalog.json
@@ -45,6 +45,9 @@
                     "line": {
                         "type": "integer"
                     },
+                    "kind": {
+                        "type": "string"
+                    },
                     "file": {
                         "type": "string"
                     },
diff --git a/benchmarks/serialization/catalog.json b/benchmarks/serialization/catalog.json
@@ -118,7 +118,7 @@
   "version": 1492108311,
   "code_id": null,
   "catalog_uuid": "c85cdf7e-f56d-4fc7-b513-3a00532cee91",
-  "catalog_format": 1,
+  "catalog_format": 2,
   "environment": "production",
   "resources": [
     {
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
@@ -800,6 +800,12 @@ def self.initialize_default_settings!(settings)
       :owner    => "service",
       :group    => "service",
       :desc    => "The directory where catalog previews per node are generated."
+    },
+    :location_trusted => {
+      :default => false,
+      :type     => :boolean,
+      :desc    => "This will allow sending the name + password and the cookie header to all hosts that puppet may redirect to.
+        This may or may not introduce a security breach if puppet redirects you to a site to which you'll send your authentication info and cookies."
     }
   )
 
diff --git a/lib/puppet/http/client.rb b/lib/puppet/http/client.rb
@@ -303,7 +303,7 @@ def execute_streaming(request, options: {}, &block)
 
     while !done do
       connect(request.uri, options: options) do |http|
-        apply_auth(request, basic_auth)
+        apply_auth(request, basic_auth) if redirects.zero?
 
         # don't call return within the `request` block
         http.request(request) do |nethttp|
diff --git a/lib/puppet/http/redirector.rb b/lib/puppet/http/redirector.rb
@@ -57,6 +57,11 @@ def redirect_to(request, response, redirects)
     new_request = request.class.new(url)
     new_request.body = request.body
     request.each do |header, value|
+      unless Puppet[:location_trusted]
+        # skip adding potentially sensitive header to other hosts
+        next if header.casecmp('Authorization').zero? && request.uri.host.casecmp(location.host) != 0
+        next if header.casecmp('Cookie').zero? && request.uri.host.casecmp(location.host) != 0
+      end
       new_request[header] = value
     end
 
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
@@ -13,7 +13,7 @@ class Puppet::Parser::Resource < Puppet::Resource
 
   attr_accessor :source, :scope, :collector_id
   attr_accessor :virtual, :override, :translated, :catalog, :evaluated
-  attr_accessor :file, :line
+  attr_accessor :file, :line, :kind
 
   attr_reader :exported, :parameters
 
diff --git a/lib/puppet/pops/evaluator/runtime3_resource_support.rb b/lib/puppet/pops/evaluator/runtime3_resource_support.rb
@@ -40,6 +40,7 @@ def self.create_resources(file, line, scope, virtual, exported, type_name, resou
           :parameters => evaluated_parameters,
           :file => file,
           :line => line,
+          :kind => Puppet::Resource.to_kind(resolved_type),
           :exported => exported,
           :virtual => virtual,
           # WTF is this? Which source is this? The file? The name of the context ?
diff --git a/lib/puppet/resource.rb b/lib/puppet/resource.rb
@@ -11,7 +11,7 @@ class Puppet::Resource
   include Puppet::Util::PsychSupport
 
   include Enumerable
-  attr_accessor :file, :line, :catalog, :exported, :virtual, :strict
+  attr_accessor :file, :line, :catalog, :exported, :virtual, :strict, :kind
   attr_reader :type, :title, :parameters
 
   # @!attribute [rw] sensitive_parameters
@@ -29,11 +29,16 @@ class Puppet::Resource
   EMPTY_ARRAY = [].freeze
   EMPTY_HASH = {}.freeze
 
-  ATTRIBUTES = [:file, :line, :exported].freeze
+  ATTRIBUTES = [:file, :line, :exported, :kind].freeze
   TYPE_CLASS = 'Class'.freeze
   TYPE_NODE  = 'Node'.freeze
   TYPE_SITE  = 'Site'.freeze
 
+  CLASS_STRING = 'class'.freeze
+  DEFINED_TYPE_STRING = 'defined_type'.freeze
+  COMPILABLE_TYPE_STRING = 'compilable_type'.freeze
+  UNKNOWN_TYPE_STRING  = 'unknown'.freeze
+
   PCORE_TYPE_KEY = '__ptype'.freeze
   VALUE_KEY = 'value'.freeze
 
@@ -194,6 +199,18 @@ def builtin_type?
     resource_type.is_a?(Puppet::CompilableResourceType)
   end
 
+  def self.to_kind(resource_type)
+    if resource_type == CLASS_STRING
+      CLASS_STRING
+    elsif resource_type.is_a?(Puppet::Resource::Type) && resource_type.type == :definition
+      DEFINED_TYPE_STRING
+    elsif resource_type.is_a?(Puppet::CompilableResourceType)
+      COMPILABLE_TYPE_STRING
+    else
+      UNKNOWN_TYPE_STRING
+    end
+  end
+
   # Iterate over each param/value pair, as required for Enumerable.
   def each
     parameters.each { |p,v| yield p, v }
@@ -248,6 +265,7 @@ def initialize(type, title = nil, attributes = EMPTY_HASH)
       src = type
       self.file = src.file
       self.line = src.line
+      self.kind = src.kind
       self.exported = src.exported
       self.virtual = src.virtual
       self.set_tags(src)
@@ -310,6 +328,7 @@ def initialize(type, title = nil, attributes = EMPTY_HASH)
 
       rt = resource_type
 
+      self.kind = self.class.to_kind(rt) unless kind
       if strict? && rt.nil?
         if self.class?
           raise ArgumentError, _("Could not find declared class %{title}") % { title: title }
@@ -493,10 +512,24 @@ def to_ref
     ref
   end
 
-  # Convert our resource to a RAL resource instance.  Creates component
-  # instances for resource types that don't exist.
+  # Convert our resource to a RAL resource instance. Creates component
+  # instances for resource types that are not of a compilable_type kind. In case
+  # the resource doesn’t exist and it’s compilable_type kind, raise an error.
+  # There are certain cases where a resource won't be in a catalog, such as 
+  # when we create a resource directly by using Puppet::Resource.new(...), so we 
+  # must check its kind before deciding whether the catalog format is of an older
+  # version or not.
   def to_ral
-    typeklass = Puppet::Type.type(self.type) || Puppet::Type.type(:component)
+    if self.kind == COMPILABLE_TYPE_STRING
+      typeklass = Puppet::Type.type(self.type)
+    elsif self.catalog && self.catalog.catalog_format >= 2
+      typeklass = Puppet::Type.type(:component)
+    else
+      typeklass =  Puppet::Type.type(self.type) || Puppet::Type.type(:component)
+    end
+
+    raise(Puppet::Error, "Resource type '#{self.type}' was not found") unless typeklass
+
     typeklass.new(self)
   end
 
diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb
@@ -315,7 +315,7 @@ def initialize(name = nil, environment = Puppet::Node::Environment::NONE, code_i
     super()
     @name = name
     @catalog_uuid = SecureRandom.uuid
-    @catalog_format = 1
+    @catalog_format = 2
     @metadata = {}
     @recursive_metadata = {}
     @classes = []
diff --git a/spec/fixtures/integration/application/agent/cached_deferred_catalog.json b/spec/fixtures/integration/application/agent/cached_deferred_catalog.json
@@ -6,7 +6,7 @@
   "version": 1607629733,
   "code_id": null,
   "catalog_uuid": "afc8472a-306b-4b24-b060-e956dffb79b8",
-  "catalog_format": 1,
+  "catalog_format": 2,
   "environment": "production",
   "resources": [
     {
@@ -50,6 +50,7 @@
       ],
       "file": "",
       "line": 1,
+      "kind": "compilable_type",
       "exported": false,
       "parameters": {
         "message": {
diff --git a/spec/integration/parser/pcore_resource_spec.rb b/spec/integration/parser/pcore_resource_spec.rb
@@ -136,6 +136,16 @@ module Puppet
       expect(catalog.resource(:cap, "c")['message']).to eq('c works')
     end
 
+    it 'considers Pcore types to be builtin ' do
+      genface.types
+      catalog = compile_to_catalog(<<-MANIFEST)
+        test1 { 'a':
+          message => 'a works'
+        }
+      MANIFEST
+      expect(catalog.resource(:test1, "a").kind).to eq('compilable_type')
+    end
+
     it 'the validity of attribute names are checked' do
       genface.types
       expect do
diff --git a/spec/unit/configurer_spec.rb b/spec/unit/configurer_spec.rb
@@ -755,7 +755,7 @@ def expects_neither_new_or_cached_catalog
           expect(configurer.run).to be_nil
         end
 
-        it "should proceed with the cached catalog if its environment matchs the local environment" do
+        it "should proceed with the cached catalog if its environment matches the local environment" do
           expects_cached_catalog_only(catalog)
 
           expect(configurer.run).to eq(0)
diff --git a/spec/unit/http/client_spec.rb b/spec/unit/http/client_spec.rb
@@ -597,11 +597,68 @@ def redirect_to(status: 302, url:)
       expect(response).to be_success
     end
 
-    it "preserves basic authorization" do
+    it "does not preserve basic authorization when redirecting to different hosts" do
+      stub_request(:get, start_url).with(basic_auth: credentials).to_return(redirect_to(url: other_host))
+      stub_request(:get, other_host).to_return(status: 200)
+
+      client.get(start_url, options: {basic_auth: {user: 'user', password: 'pass'}})
+      expect(a_request(:get, other_host).
+      with{ |req| !req.headers.key?('Authorization')}).to have_been_made
+    end
+
+    it "does preserve basic authorization when redirecting to the same hosts" do
+      stub_request(:get, start_url).with(basic_auth: credentials).to_return(redirect_to(url: bar_url))
+      stub_request(:get, bar_url).with(basic_auth: credentials).to_return(status: 200)
+
+      client.get(start_url, options: {basic_auth: {user: 'user', password: 'pass'}})
+      expect(a_request(:get, bar_url).
+      with{ |req| req.headers.key?('Authorization')}).to have_been_made
+    end
+
+    it "does not preserve cookie header when redirecting to different hosts" do
+      headers = { 'Cookie' => 'TEST_COOKIE'}
+
+      stub_request(:get, start_url).with(headers: headers).to_return(redirect_to(url: other_host))
+      stub_request(:get, other_host).to_return(status: 200)
+
+      client.get(start_url, headers: headers)
+      expect(a_request(:get, other_host).
+      with{ |req| !req.headers.key?('Cookie')}).to have_been_made
+    end
+
+    it "does preserve cookie header when redirecting to the same hosts" do
+      headers = { 'Cookie' => 'TEST_COOKIE'}
+
+      stub_request(:get, start_url).with(headers: headers).to_return(redirect_to(url: bar_url))
+      stub_request(:get, bar_url).with(headers: headers).to_return(status: 200)
+
+      client.get(start_url, headers: headers)
+      expect(a_request(:get, bar_url).
+      with{ |req| req.headers.key?('Cookie')}).to have_been_made
+    end
+
+    it "does preserves cookie header and basic authentication when Puppet[:location_trusted] is true redirecting to different hosts" do
+      headers = { 'cookie' => 'TEST_COOKIE'}
+      Puppet[:location_trusted] = true
+
+      stub_request(:get, start_url).with(headers: headers, basic_auth: credentials).to_return(redirect_to(url: other_host))
+      stub_request(:get, other_host).with(headers: headers, basic_auth: credentials).to_return(status: 200)
+
+      client.get(start_url, headers: headers, options: {basic_auth: {user: 'user', password: 'pass'}})
+      expect(a_request(:get, other_host).
+      with{ |req| req.headers.key?('Authorization') && req.headers.key?('Cookie')}).to have_been_made
+    end
+
+    it "treats hosts as case-insensitive" do
+      start_url = URI("https://www.EXAmple.com:8140/Start")
+      bar_url = "https://www.example.com:8140/bar"
+
       stub_request(:get, start_url).with(basic_auth: credentials).to_return(redirect_to(url: bar_url))
       stub_request(:get, bar_url).with(basic_auth: credentials).to_return(status: 200)
 
       client.get(start_url, options: {basic_auth: {user: 'user', password: 'pass'}})
+      expect(a_request(:get, bar_url).
+      with{ |req| req.headers.key?('Authorization')}).to have_been_made
     end
 
     it "redirects given a relative location" do
diff --git a/spec/unit/resource/catalog_spec.rb b/spec/unit/resource/catalog_spec.rb
@@ -104,7 +104,7 @@
 
   it "should include the current catalog_format" do
     catalog = Puppet::Resource::Catalog.new("host")
-    expect(catalog.catalog_format).to eq(1)
+    expect(catalog.catalog_format).to eq(2)
   end
 
   describe "when compiling" do
@@ -178,6 +178,7 @@
       @original.add_edge(@middle, @bottom)
       @original.add_edge(@bottom, @bottomobject)
 
+      @original.catalog_format = 1
       @catalog = @original.to_ral
     end
 
@@ -190,6 +191,18 @@
       end
     end
 
+    it "should raise if an unknown resource is being converted" do
+      @new_res = Puppet::Resource.new "Unknown", "type", :kind => 'compilable_type'
+      @resource_array = [@new_res]
+
+      @original.add_resource(*@resource_array)
+      @original.add_edge(@bottomobject, @new_res)
+
+      @original.catalog_format = 2
+
+      expect { @original.to_ral }.to raise_error(Puppet::Error, "Resource type 'Unknown' was not found")
+    end
+
     it "should copy the tag list to the new catalog" do
       expect(@catalog.tags.sort).to eq(@original.tags.sort)
     end
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -118,7 +118,7 @@`
`118`	`118`	`"version": 1492108311,`
`119`	`119`	`"code_id": null,`
`120`	`120`	`"catalog_uuid": "c85cdf7e-f56d-4fc7-b513-3a00532cee91",`
`121`		`- "catalog_format": 1,`
	`121`	`+ "catalog_format": 2,`
`122`	`122`	`"environment": "production",`
`123`	`123`	`"resources": [`
`124`	`124`	`{`
Original file line number	Diff line number	Diff line change
`@@ -800,6 +800,12 @@ def self.initialize_default_settings!(settings)`
`800`	`800`	`:owner => "service",`
`801`	`801`	`:group => "service",`
`802`	`802`	`:desc => "The directory where catalog previews per node are generated."`
	`803`	`+ },`
	`804`	`+ :location_trusted => {`
	`805`	`+ :default => false,`
	`806`	`+ :type => :boolean,`
	`807`	`+ :desc => "This will allow sending the name + password and the cookie header to all hosts that puppet may redirect to.`
	`808`	`+ This may or may not introduce a security breach if puppet redirects you to a site to which you'll send your authentication info and cookies."`
`803`	`809`	`}`
`804`	`810`	`)`
`805`	`811`