Attachment 710427 Details for Bug 919783 – puppet-2.7.11-CVE-Rollup.patch

[patch] puppet-2.7.11-CVE-Rollup.patch

puppet-2.7.11-CVE-Rollup.patch (text/plain), 107.82 KB, created by Kurt Seifried on 2013-03-15 04:55:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kurt Seifried

Created: 2013-03-15 04:55:22 UTC

Size: 107.82 KB

patch

obsolete

>From 8c6878172a9235c94d0f30095e0f891c403377e6 Mon Sep 17 00:00:00 2001
>From: Nick Lewis <nick@puppetlabs.com>
>Date: Wed, 20 Feb 2013 17:28:14 -0800
>Subject: [PATCH] 2.7.11 - CVE Patch Rollup
>
>This contains patches for CVE-2013-1640, CVE-2013-1652, CVE-2013-1653, CVE-2013-1654, CVE-2013-1655, CVE-2013-2275
>
>(#19393) Safely load YAML from the network
>
>When using Psych (only available in Ruby 1.9), YAML.load may in certain
>cases (!map: and !ruby/hash: tags) use the initialize and []= methods to
>create objects. These methods could potentially allow arbitrary code to
>be executed. We now add a safely_load method which parses the input
>first, checks it for dangerous tags, and fails if they are found. If the
>input is clean, we transform the result (as YAML.load would have done)
>and go on our merry way.
>
>This change is only applied to YAML coming in either HTTP requests or
>responses. YAML from the local system is trusted, as anyone who can
>write that YAML could do damage some other way.
>
>(#14093) Remove unsafe attributes from TemplateWrapper
>
>The attribute accessor for the :string attribute in the TemplateWrapper
>conflicted with a variable being in scope that is named string. The
>variable would take precedence and because of the way the
>TemplateWrapper accessed the value of the attribute, the variable's
>value would be used as the template.
>
>This changes the TemplateWrapper to completely remove :string as an
>attribute and only use local variables. This also removes the :file
>attribute in favor of a "private"-style naming of the instance variable.
>There are no collisions with @__file__ that would cause problems, but
>this should reduce the likelihood of them happening in the first place.
>
>This commit is modified from the original version to exclude changes to
>the specs that were not related to issue #14903, due to the scope code
>being so different in 2.7.x than 3.x.
>
>(#14093) Restore access to the filename in the template
>
>The change to fix certain variables interfering with template evaluation
>also removed the accessor method for file. It appears that this is a
>mechanism that had been used by others for identifying the name of the
>template being evaluated so that they can trace back from a file on a
>node to the module and template that created it. This restores that
>functionality.
>
>(#19391) (CVE-2013-1652) Disallow use_node compiler parameter for remote requests
>
>Without this it is possible to bypass catalog access restrictions by passing
>in a node object for a different host. We also validate that facts provided
>match the node requested.
>
>(#19392) (CVE-2013-1653) Validate instances passed to indirector
>
>This adds a general validation method to check that only valid instances can
>be passed into the indirector. Since access control is based on the URI but
>many operations directly use the serialized instance passed in, it was
>possible to bypass restrictions by passing in a custom object. Specifically it
>was possible to cause the puppet kick indirection to execute arbitrary code by
>passing in an instance of the wrong class. This validates that the instance is
>of the correct type and that the name matches the key that was used to
>authorize the request.
>
>Conflicts:
>	lib/puppet/indirector/file_bucket_file/selector.rb
>
>(#19151) Reject SSLv2 SSL handshakes and ciphers
>
>Without this patch, SSL connections on older versions of Ruby will
>negotiate down to insecure modes of operation, specifically SSLv2.  This
>is a problem because SSLv2 needs to be rejected outright to meet
>security policies.
>
>This patch addresses the problem by changing the behavior of the
>OpenSSL::SSL::SSLContext class.  With this patch applied, all SSLContext
>objects will be initialized with a default cipher rule set that always
>contains the '!SSLv2' substring.  This has the effect of removing SSLv2
>ciphers from the cipher list and prohibiting them from being re-added by
>later elements in the cipher spec.
>
>Details regarding how OpenSSL behaves with this cipher string are
>available at: http://www.openssl.org/docs/apps/ciphers.html
>
>In order to see which ciphers are enabled for a specific version of the
>OpenSSL library, please see the output of the command:
>
>    $ openssl ciphers $CIPHERS
>
>This command will display an ordered list of the ciphers enabled for use
>during the SSL handshake.
>
>This change is a monkey patch to MRI Core and will affect all SSL socket
>clients and servers.  The options and cipher list may still be
>explicitly set by passing an options hash with the :options and :ciphers
>keys to the SSLContext#set_params method, or by calling the
>SSLContext#options= instance method as WEBrick does.  We cannot monkey
>patch SSLContext#options= because this method is implemented as a C
>extension to MRI and as such cannot be easily redefined.  Segfaults
>abound in MRI 1.8.5.
>
>Conflicts:
>	lib/puppet/util/monkey_patches.rb
>	spec/unit/network/http_pool_spec.rb
>	spec/unit/util/monkey_patches_spec.rb
>
>(#19391) Backport Request#remote? method
>
>Commit ccf2e4c relies on the Request#remote? method, which only exists
>in 2.7, added in commit fe1f4a20.
>
>This commit backports the change and specs.
>
>(#8858) Refactor tests to use real HTTP objects
>
>This is a backport of commit cd4bee82:
>
>Test SSL setup code, not our stubbing of it.
>
>This rewrites the tests to actually test code - previously, almost every
>single object involved was a stub.
>
>There shouldn't be any functional changes, other than tests that might
>actually fail at some point, falling out of this.
>
>The results are not the most awesome of tests, but they are markedly
>better than before we started.
>
>Conflicts:
>	spec/unit/network/http_pool_spec.rb
>
>(#8858) Explicitly set SSL peer verification mode.
>
>In Ruby 1.8 the Net::HTTP library defaults to skipping peer verification when
>no mode is explicitly set.  Ruby 1.9, on the other hand, does not: it defaults
>to verification of the peer certificate - leading to failure when we depended
>on the default value in our HTTP setup.
>
>This changes to explicitly set the verification mode, ensuring we get
>consistent results across all Ruby versions.
>
>This also updates the http_pool spec to no longer expect the cert_store to be
>nil by default.
>
>Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
>
>(#19531) (CVE-2013-2275) Only allow report save from the node matching the certname
>
>Without this patch applied any authenticated client is able to save a
>report for any node by default.  This is a problem because the
>compliance feature of Puppet Enterprise expects reports to be submitted
>only from the node the report is associated with.
>
>This patch addresses the problem by restricting the access control rules
>in a similar manner to the catalog.  With this patch applied, the
>default behavior of the Puppet master will only allow reports to be
>saved when the node name matches the cert name.
>
>Acceptance tests for CVEs 2013 (1640, 1652, 1653, 1654, 2274, 2275)
>
>(#14093) (CVE-2013-1640) Facts override template scope variables
>(#19391) (CVE-2013-1652) Validate improper query parameters
>(#19392) (CVE-2013-1653) Puppet kick
>(#19151) (CVE-2013-1654) Do not downgrade to SSLv2
>(#19393) (CVE-2013-1655) Add acceptance test for safe YAML de-serialization
>(#19456) (CVE-2013-2274) All your terminii belog to us!
>(#19531) (CVE-2013-2275) Add acceptance test for report save ACL
>
>Always read request body when using Rack
>
>In certain versions of Passenger (all of them), leaving the request body
>unread will cause an EPIPE exception inside Passenger, resulting in a
>500 response. This is because Passenger makes a blocking write of the
>request body to the application (Puppet). If we respond without reading
>the body, this write is interrupted, which isn't handled properly by
>Passenger. So when processing requests, we make sure to have always read
>at least 1 character, which unblocks the write and lets Passenger
>continue.
>
>This issue is not present in other HTTP handlers.
>
>https://code.google.com/p/phusion-passenger/issues/detail?id=471
>
>Paired-With: Josh Cooper <josh@puppetlabs.com>
>
>Fix order-dependent test failure in rest_authconfig_spec
>
>This test was assuming the RestAuthConfig singleton hadn't been created.
>A previous test does create one, so this test was failing. Now, we make
>sure the singleton instance is nil before testing.
>
>Separate tests for same CVEs into separate files
>
>fail tests instead of erroring when we know its a failure
>
>Run openssl from windows when trying to downgrade master
>
>The test assumed the `agents` array always contained a non-Windows host,
>which isn't always true. It also didn't take into account that the path
>for cert, key, etc contain spaces.
>
>Remove unnecessary rubygems require
>
>Previously, the test was failing on our rhel6 box which serves as the
>master for Windows acceptance tests due rubygems not being
>installed. Since we don't actually need rubygems to load openssl, it's
>safe to remove that line from the rogue sslserver script.
>
>Don't assume puppetbindir is defined
>
>The host variable `puppetbindir` is undefined on FOSS windows (unlike
>Unix), so this test was failing on Windows when trying to execute
>`/ruby`. This is really a bug in the harness as the eventlog
>acceptance test open-codes this logic as well.
>
>This commit changes it to fallback to `ruby`. It also simplifies the
>logic for detecting whether an agent supports SSLv2.
>
>Display SSL messages so we can match our regex
>
>On rhel6, `openssl s_client` doesn't display the SSL messages that we
>are trying to match, e.g. SERVER-HELLO. The `-msg` option must be used
>for them to be displayed.
>
>Don't require openssl client to return 0 on failure
>
>Previously, the test assumed openssl s_client would return 0 when the
>master rejected the SSLv2 connection, which is a bit odd.
>
>This commit ignores the exit code when determining if the passed passed
>or failed, and instead requires that a CLIENT-HELLO message be present,
>and a SERVER-HELLO message is absent.
>
>Don't assume master supports SSLv2
>
>Previously, the test tried to create an SSLv2 rogue master, and then
>check each agent's suitability (as to whether it's ruby supports SSLv2).
>But this fails on precise, as its ruby does not support SSLv2.
>Specifically the call to ssl_version = 'SSLv2' raises an exception.
>
>Since there is nothing special about the master, we just need to run a
>ruby process, this commit moves the rogue ssl server to the agent
>itself, and only those agents where SSLv2 is available.
>
>Since the agent may also be a master, it changes the rogue server's
>accept port from 8140 to 8150.
>
>(#19392) Don't validate key for certificate_status
>
>The certificate_status file terminus does not use the name attribute of the
>passed in instance. In order to preserve the existing API which doesn't
>require a name on the instance (and so gets the default of Puppet[:certname])
>we skip validation for this terminus.
>
>(#14093) Stub missing expectations
>
>The test was backported in commit e611b95b to 2.7.11, but did not work
>due to the spec test overly stubbing everything. Rather than redo the
>test, this commit simply stubs out the missing compiler, environment,
>and known_resource_types.
>---
> .../tests/security/cve-2013-1640_facter_string.rb  |  14 +
> .../cve-2013-1652_improper_query_params.rb         |  39 +++
> .../cve-2013-1652_poison_other_node_cache.rb       |  40 +++
> .../tests/security/cve-2013-1653_puppet_kick.rb    | 109 ++++++
> .../cve-2013-1654_sslv2_downgrade_agent.rb         |  93 ++++++
> .../cve-2013-1654_sslv2_downgrade_master.rb        |  32 ++
> .../cve-2013-1655_safe_yaml_deserialization.rb     |  42 +++
> ...13-2274_all_your_agent_terminii_belong_to_us.rb | 104 ++++++
> ...3-2274_all_your_master_terminii_belong_to_us.rb |  79 +++++
> .../tests/security/cve-2013-2275_report_acl.rb     |  30 ++
> conf/auth.conf                                     |   6 +-
> lib/puppet/indirector/catalog/compiler.rb          |  15 +-
> lib/puppet/indirector/certificate_status/file.rb   |   5 +
> lib/puppet/indirector/errors.rb                    |   5 +
> lib/puppet/indirector/file_bucket_file/file.rb     |   4 +
> lib/puppet/indirector/indirection.rb               |   1 +
> lib/puppet/indirector/request.rb                   |   4 +
> lib/puppet/indirector/resource/active_record.rb    |   3 +
> lib/puppet/indirector/resource/ral.rb              |   4 +
> lib/puppet/indirector/resource/store_configs.rb    |   3 +
> lib/puppet/indirector/resource/validator.rb        |   8 +
> lib/puppet/indirector/rest.rb                      |   8 +
> lib/puppet/indirector/run/local.rb                 |   4 +
> lib/puppet/indirector/terminus.rb                  |  20 ++
> lib/puppet/network/formats.rb                      |   6 +-
> lib/puppet/network/handler/master.rb               |   2 +-
> lib/puppet/network/handler/report.rb               |   2 +-
> lib/puppet/network/http/handler.rb                 |   8 +-
> lib/puppet/network/http/rack/rest.rb               |   9 +-
> lib/puppet/network/http/webrick.rb                 |   1 +
> lib/puppet/network/http_pool.rb                    |  25 +-
> lib/puppet/network/rest_authconfig.rb              |   2 +-
> lib/puppet/parser/templatewrapper.rb               |  34 +-
> lib/puppet/util/monkey_patches.rb                  | 103 ++++++
> .../indirector/catalog/compiler_spec.rb            |   1 +
> spec/integration/indirector/catalog/queue_spec.rb  |   2 +-
> spec/integration/resource/catalog_spec.rb          |   1 +
> spec/unit/indirector/catalog/compiler_spec.rb      |  31 +-
> spec/unit/indirector/indirection_spec.rb           |  19 +-
> spec/unit/indirector/request_spec.rb               |  22 ++
> spec/unit/indirector/terminus_spec.rb              | 368 +++++++++++----------
> spec/unit/network/formats_spec.rb                  |  12 +-
> spec/unit/network/http/handler_spec.rb             |  25 ++
> spec/unit/network/http/rack/rest_spec.rb           |  17 +
> spec/unit/network/http/webrick_spec.rb             |   4 +
> spec/unit/network/http_pool_spec.rb                | 145 ++++----
> spec/unit/network/rest_authconfig_spec.rb          |  17 +-
> spec/unit/parser/functions/inline_template_spec.rb |  13 +
> spec/unit/parser/functions/template_spec.rb        |  21 ++
> spec/unit/parser/templatewrapper_spec.rb           |  23 +-
> spec/unit/ssl/certificate_request_spec.rb          |   2 +
> spec/unit/ssl/host_spec.rb                         |   1 +
> spec/unit/util/monkey_patches_spec.rb              |  12 +
> 53 files changed, 1295 insertions(+), 305 deletions(-)
> create mode 100644 acceptance/tests/security/cve-2013-1640_facter_string.rb
> create mode 100644 acceptance/tests/security/cve-2013-1652_improper_query_params.rb
> create mode 100644 acceptance/tests/security/cve-2013-1652_poison_other_node_cache.rb
> create mode 100644 acceptance/tests/security/cve-2013-1653_puppet_kick.rb
> create mode 100644 acceptance/tests/security/cve-2013-1654_sslv2_downgrade_agent.rb
> create mode 100644 acceptance/tests/security/cve-2013-1654_sslv2_downgrade_master.rb
> create mode 100644 acceptance/tests/security/cve-2013-1655_safe_yaml_deserialization.rb
> create mode 100644 acceptance/tests/security/cve-2013-2274_all_your_agent_terminii_belong_to_us.rb
> create mode 100644 acceptance/tests/security/cve-2013-2274_all_your_master_terminii_belong_to_us.rb
> create mode 100644 acceptance/tests/security/cve-2013-2275_report_acl.rb
> create mode 100644 lib/puppet/indirector/errors.rb
> create mode 100644 lib/puppet/indirector/resource/validator.rb
>
>diff --git a/acceptance/tests/security/cve-2013-1640_facter_string.rb b/acceptance/tests/security/cve-2013-1640_facter_string.rb
>new file mode 100644
>index 0000000..ac05226
>--- /dev/null
>+++ b/acceptance/tests/security/cve-2013-1640_facter_string.rb
>@@ -0,0 +1,14 @@
>+# Setting a custom fact to "string" will overwrite a local variable during
>+# template compilation on the master allowing remote code execution by
>+# any authenticated client.
>+test_name "CVE 2013-1640 Remote Code Execution" do
>+  confine :except, :platform => 'windows'
>+
>+  on agents, %q[ FACTER_string="<%= %x{ /bin/echo hax0rd }  %>" ] +
>+             %q[ puppet apply -e ] +
>+             %q[ 'notice(inline_template("<%= \"I am Safe\" %>"))' ] do |test|
>+
>+    assert_match /I am Safe/, test.stdout
>+    assert_no_match /hax0rd/, test.stdout
>+  end
>+end
>diff --git a/acceptance/tests/security/cve-2013-1652_improper_query_params.rb b/acceptance/tests/security/cve-2013-1652_improper_query_params.rb
>new file mode 100644
>index 0000000..76551d5
>--- /dev/null
>+++ b/acceptance/tests/security/cve-2013-1652_improper_query_params.rb
>@@ -0,0 +1,39 @@
>+require 'json'
>+
>+test_name "CVE 2013-1652 Improper query parameter validation" do
>+  confine :except, :platform => 'windows'
>+
>+  with_master_running_on( master, '--autosign true' ) do
>+    # Ensure each agent has a signed cert
>+    on agents, puppet_agent( '-t' )
>+
>+    agents.each do |agent|
>+      next if agent['roles'].include?( 'master' )
>+
>+      certname = on(agent, puppet_agent("--configprint certname")).stdout.chomp
>+
>+      payload = "https://#{master}:8140/production/catalog/#{certname}?use_node=" +
>+                "---%20!ruby/object:Puppet::Node%0A%20%20" +
>+                "name:%20#{master}%0A%20%20classes:%20\[\]%0A%20%20" +
>+                "parameters:%20%7B%7D%0A%20%20facts:%20%7B%7D"
>+
>+      cert_path = on(agent, puppet_agent("--configprint hostcert")).stdout.chomp
>+      key_path = on(agent, puppet_agent("--configprint hostprivkey")).stdout.chomp
>+      curl_base = "curl -g --cert \"#{cert_path}\" --key \"#{key_path}\" -k -H 'Accept: pson'"
>+
>+      curl_call =  "#{curl_base} '#{payload}'"
>+
>+      step "Attempt to retrieve another nodes catalog" do
>+        on agent, curl_call do |test|
>+          begin
>+            res = JSON.parse( test.stdout )
>+            fail_test( "Retrieved catalog for #{master} from #{agent}" ) if
>+              res['data']['name'] == master.name
>+          rescue JSON::ParserError
>+            # good, continue
>+          end
>+        end
>+      end
>+    end
>+  end
>+end
>diff --git a/acceptance/tests/security/cve-2013-1652_poison_other_node_cache.rb b/acceptance/tests/security/cve-2013-1652_poison_other_node_cache.rb
>new file mode 100644
>index 0000000..8667fb1
>--- /dev/null
>+++ b/acceptance/tests/security/cve-2013-1652_poison_other_node_cache.rb
>@@ -0,0 +1,40 @@
>+test_name "CVE 2013-1652 Poison node cache" do
>+
>+  step "Determine suitability of the test" do
>+    versions = on( hosts, puppet( '--version' ))
>+    skip_test( "This test will only run on Puppet 3.x" ) if
>+      versions.any? {|r| r.stdout =~ /\A2\./ }
>+  end
>+
>+  with_master_running_on( master, "--autosign true" ) do
>+    # Ensure each agent has a signed cert
>+    on agents, puppet_agent( '-t' )
>+
>+    agents.each do |agent|
>+      next if agent['roles'].include?( 'master' )
>+
>+      certname = on(agent, puppet_agent("--configprint certname")).stdout.chomp
>+      cert_path = on(agent, puppet_agent("--configprint hostcert")).stdout.chomp
>+      key_path = on(agent, puppet_agent("--configprint hostprivkey")).stdout.chomp
>+
>+      curl_base = "curl -g --cert #{cert_path} --key #{key_path} -k -H 'Accept: pson'"
>+
>+      step "Attempt to poison the master's node cache" do
>+        yamldir = on( master, puppet_master( '--configprint yamldir' )).stdout.chomp
>+        exploited = "#{yamldir}/node/you.lose.yaml"
>+        on master, "rm -rf #{exploited}"
>+        on master, "rm -rf #{yamldir}/node/*"
>+        payload2 = "https://#{master}:8140/production/node/#{certname}?instance=" +
>+                   "---+%21ruby%2Fobject%3APuppet%3A%3ANode%0A+classes" +
>+                   "%3A%0A+-+foo%0A+name%3A+you.lose%0A+parameters" +
>+                   "%3A+%7B%7D%0A+time%3A+2013-02-28+15%3A12%3A30.367008+-08%3A00"
>+
>+        on agent, "#{curl_base} '#{payload2}'"
>+
>+        fail_test( "Found exploit file #{exploited}" ) if
>+          on( master, "[ ! -f #{exploited} ]",
>+             :acceptable_exit_codes => [0,1] ).exit_code == 1
>+      end
>+    end
>+  end
>+end
>diff --git a/acceptance/tests/security/cve-2013-1653_puppet_kick.rb b/acceptance/tests/security/cve-2013-1653_puppet_kick.rb
>new file mode 100644
>index 0000000..0ede979
>--- /dev/null
>+++ b/acceptance/tests/security/cve-2013-1653_puppet_kick.rb
>@@ -0,0 +1,109 @@
>+test_name "CVE 2013-1653: Puppet Kick Remote Code Exploit" do
>+
>+  step "Determine suitability of the test" do
>+    confine :except, :platform => 'windows'
>+
>+    versions = on( hosts, puppet( '--version' ))
>+    skip_test( "This test will not run on Puppet 2.6" ) if
>+      versions.any? {|r| r.stdout =~ /\A2\.6\./ }
>+  end
>+
>+  with_master_running_on( master, '--autosign true' ) do
>+    on agents, puppet_agent( '-t' )
>+  end
>+
>+  def exploit_code( exploiter, exploitee, endpoint, port, file_to_create )
>+
>+    certfile = on( exploiter, puppet_agent( '--configprint hostcert' )).stdout.chomp
>+    keyfile = on( exploiter, puppet_agent( '--configprint hostprivkey' )).stdout.chomp
>+
>+    exploit = %Q[#!/usr/bin/env ruby
>+      require 'puppet'
>+      require 'openssl'
>+      require 'net/https'
>+
>+      yaml = <<EOM
>+--- !ruby/object:ERB
>+ safe_level:
>+ src: |-
>+   #coding:US-ASCII
>+   _erbout = ''; _erbout.concat(( File.open( '#{file_to_create}', 'w') ).to_s)
>+ filename:
>+EOM
>+
>+      headers = {'Content-Type' => 'text/yaml', 'Accept' => 'yaml'}
>+      conn = Net::HTTP.new('#{exploitee}', #{port})
>+      conn.use_ssl = true
>+      conn.cert = OpenSSL::X509::Certificate.new(File.read('#{certfile}'))
>+      conn.key = OpenSSL::PKey::RSA.new(File.read('#{keyfile}'))
>+      conn.verify_mode = OpenSSL::SSL::VERIFY_NONE
>+
>+      conn.request_put("/production/#{endpoint}/#{exploiter}", yaml, headers) do |response|
>+       response.read_body do |chunk|
>+         puts chunk
>+       end
>+      end ]
>+
>+    return exploit
>+  end
>+
>+  exploited = '/tmp/cve-2013-1653-has-worked'
>+  restauth_conf = %q[
>+path /run
>+auth yes
>+allow *
>+]
>+
>+  teardown do
>+    agents.each do |agent|
>+      pidfile = on( agent, puppet_agent("--configprint pidfile") ).stdout.chomp
>+      on agent, "[ -f #{pidfile} ] && kill `cat #{pidfile}` || true"
>+      on agent, "rm -rf #{exploited}"
>+    end
>+  end
>+
>+  agents.each do |agent|
>+    atestdir = agent.tmpdir('puppet-kick-auth')
>+    mtestdir = master.tmpdir('puppet-kick-auth')
>+
>+    step "Daemonize the agent" do
>+      # Lay down a tempory auth.conf that will allow the agent to be kicked
>+      create_remote_file(agent, "#{atestdir}/auth.conf", restauth_conf)
>+
>+      # Start the agent
>+      on(agent, puppet_agent("--debug --daemonize --server #{master} --listen --no-client --rest_authconfig #{atestdir}/auth.conf"))
>+
>+      step "Wait for agent to start listening" do
>+        timeout = 15
>+        begin
>+          Timeout.timeout(timeout) do
>+            loop do
>+              # 7 is "Could not connect to host", which will happen before it's running
>+              result = on(agent, "curl -k https://#{agent}:8139", :acceptable_exit_codes => [0,7])
>+              break if result.exit_code == 0
>+              sleep 1
>+            end
>+          end
>+        rescue Timeout::Error
>+          fail_test "Puppet agent #{agent} failed to start after #{timeout} seconds"
>+        end
>+      end
>+    end
>+
>+    step "Attempt to exploit #{agent}" do
>+      # Ensure there's no stale data
>+      on agent, "rm -rf #{exploited}"
>+      on master, "rm -rf #{mtestdir}/exploit.rb"
>+
>+      # Copy over our exploit and execute
>+      create_remote_file( master, "#{mtestdir}/exploit.rb", exploit_code( master, agent, 'run', 8139, exploited ))
>+      on master, "chmod +x #{mtestdir}/exploit.rb"
>+      on master, "#{mtestdir}/exploit.rb"
>+
>+      # Did it work?
>+      fail_test( "Found exploit file #{exploited}" ) if
>+        on( agent, "[ ! -f #{exploited} ]",
>+           :acceptable_exit_codes => [0,1] ).exit_code == 1
>+    end
>+  end
>+end
>diff --git a/acceptance/tests/security/cve-2013-1654_sslv2_downgrade_agent.rb b/acceptance/tests/security/cve-2013-1654_sslv2_downgrade_agent.rb
>new file mode 100644
>index 0000000..51a02ee
>--- /dev/null
>+++ b/acceptance/tests/security/cve-2013-1654_sslv2_downgrade_agent.rb
>@@ -0,0 +1,93 @@
>+test_name "CVE 2013-1654 SSL2 Downgrade of Agent connection" do
>+
>+  def which_ruby(host)
>+    host['puppetbindir'] ? "#{host['puppetbindir']}/ruby" : 'ruby'
>+  end
>+
>+  def suitable?(host)
>+    cmd = <<END
>+#{which_ruby(host)} -ropenssl -e "puts OpenSSL::SSL::SSLContext::METHODS.include?(:SSLv2)"
>+END
>+    on(host, cmd).stdout.chomp == "true"
>+  end
>+
>+  with_master_running_on( master, '--autosign true' ) do
>+    on agents, puppet_agent( '-t' )
>+  end
>+
>+  agents.each do |agent|
>+    if suitable?( agent )
>+      certfile = on(agent, puppet_agent("--configprint hostcert")).stdout.chomp
>+      keyfile = on(agent, puppet_agent("--configprint hostprivkey")).stdout.chomp
>+      cafile = on(agent, puppet_agent("--configprint localcacert")).stdout.chomp
>+      port = 8150
>+
>+      sslserver = <<END
>+#!/usr/bin/env ruby
>+require 'webrick'
>+require 'webrick/https'
>+
>+class Servlet < WEBrick::HTTPServlet::AbstractServlet
>+  def do_GET(request, response)
>+    response.status = 200
>+    response['Content-Type'] = 'text/pson'
>+    response.body = 'FOOBAR'
>+  end
>+end
>+
>+class SSLServer
>+  def run
>+    config = {}
>+    config[:Port] = #{port}
>+    config[:SSLCACertificateFile] = '#{cafile}'
>+    config[:SSLCertificate] =  OpenSSL::X509::Certificate.new(File.read('#{certfile}'))
>+    config[:SSLPrivateKey] = OpenSSL::PKey::RSA.new(File.read('#{keyfile}'))
>+    config[:SSLStartImmediately] = true
>+    config[:SSLEnable] = true
>+    config[:SSLVerifyClient] = OpenSSL::SSL::VERIFY_NONE
>+
>+    server = WEBrick::HTTPServer.new(config)
>+    server.mount('/', Servlet)
>+    server.ssl_context.ssl_version = 'SSLv2'
>+    trap :TERM do
>+      exit!(0)
>+    end
>+    server.start
>+  end
>+end
>+
>+if $0 == __FILE__
>+  SSLServer.new.run
>+end
>+END
>+      testdir = agent.tmpdir('puppet-sslv2')
>+      teardown do
>+        on(agent, "ps -ef | grep -E 'sslserver.rb' | grep -v grep | awk '{ print $2 }' | xargs kill || echo \"ruby sslserver.rb not running\"")
>+        on(agent, "rm -rf #{testdir}")
>+      end
>+
>+      create_remote_file(agent, "#{testdir}/sslserver.rb", sslserver)
>+      on agent, "#{which_ruby(agent)} #{testdir}/sslserver.rb &>/dev/null &"
>+
>+      timeout = 15
>+      begin
>+        Timeout.timeout(timeout) do
>+          loop do
>+            # 7 is "Could not connect to host", which will happen before it's running
>+            result = on(agent, "curl -m1 -k https://#{agent}:#{port}", :acceptable_exit_codes => [0,7,35])
>+            break if result.exit_code == 0 or result.exit_code == 35
>+            sleep 1
>+          end
>+        end
>+      rescue Timeout::Error
>+        fail_test "Insecure Mock Server on #{agent} failed to start after #{timeout} seconds"
>+      end
>+
>+      on(agent, puppet("agent --debug --test --server #{agent} --masterport #{port}"), :acceptable_exit_codes => [1]) do |test|
>+        assert_no_match(/'FOOBAR'/, test.stdout)
>+      end
>+    else
>+      logger.debug( "skipping #{agent} since SSLv2 is not available" )
>+    end
>+  end
>+end
>diff --git a/acceptance/tests/security/cve-2013-1654_sslv2_downgrade_master.rb b/acceptance/tests/security/cve-2013-1654_sslv2_downgrade_master.rb
>new file mode 100644
>index 0000000..38141bc
>--- /dev/null
>+++ b/acceptance/tests/security/cve-2013-1654_sslv2_downgrade_master.rb
>@@ -0,0 +1,32 @@
>+test_name "CVE 2013-1654 SSL2 Downgrade of Master connection" do
>+
>+  def suitable?(host)
>+    ruby = host['puppetbindir'] ? "#{host['puppetbindir']}/ruby" : 'ruby'
>+    cmd = <<END
>+#{ruby} -ropenssl -e "puts OpenSSL::SSL::SSLContext::METHODS.include?(:SSLv2)"
>+END
>+    on(host, cmd).stdout.chomp == "true"
>+  end
>+
>+  if suitable?( master )
>+    with_master_running_on( master, '--autosign true' ) do
>+
>+      agent = agents.first
>+      on agent, puppet_agent( '-t' )
>+
>+      certfile = on(agent, puppet_agent("--configprint hostcert")).stdout.chomp
>+      keyfile = on(agent, puppet_agent("--configprint hostprivkey")).stdout.chomp
>+      cafile = on(agent, puppet_agent("--configprint localcacert")).stdout.chomp
>+
>+      openssl_call = "openssl s_client -connect #{master}:8140 " +
>+                     "-cert \"#{certfile}\" -key \"#{keyfile}\" -CAfile \"#{cafile}\" -ssl2 -msg < /dev/null"
>+
>+      on(agent, openssl_call, :acceptable_exit_codes => (0..255)) do |test|
>+        assert_match /CLIENT-HELLO/, test.stdout
>+        assert_no_match /SERVER-HELLO/, test.stdout
>+      end
>+    end
>+  else
>+    logger.debug( "Not testing master as SSLv2 isn't available to it" )
>+  end
>+end
>diff --git a/acceptance/tests/security/cve-2013-1655_safe_yaml_deserialization.rb b/acceptance/tests/security/cve-2013-1655_safe_yaml_deserialization.rb
>new file mode 100644
>index 0000000..d128123
>--- /dev/null
>+++ b/acceptance/tests/security/cve-2013-1655_safe_yaml_deserialization.rb
>@@ -0,0 +1,42 @@
>+test_name "#19393: Safe YAML deserialization"
>+step "Verify Puppet safely deserializes YAML encoded objects"
>+
>+# Check if the master is running with a Psych YAML engine.  If not, don't test.
>+check_for_psych_cmd = "ruby -ryaml -e 'exit (defined?(YAML::ENGINE) and YAML::ENGINE.yamler == \"psych\") ? 3 : 7'"
>+
>+master_uses_psych = 'unknown'
>+on master, check_for_psych_cmd, :acceptable_exit_codes => [3,7] do |result|
>+  case result.exit_code
>+  when 3
>+    master_uses_psych = true
>+  when 7
>+    master_uses_psych = false
>+  else
>+    raise "Could not determine if the system under test uses the YAML Psych engine."
>+  end
>+end
>+
>+if master_uses_psych
>+  with_master_running_on(master) do
>+    unsafe_data = "--- !ruby/hash:Array {}"
>+
>+    cmd = [
>+      "curl -k -X PUT",
>+      "--cacert \"$(puppet master --configprint cacert)\"",
>+      "--cert \"$(puppet master --configprint hostcert)\"",
>+      "--key \"$(puppet master --configprint hostprivkey)\"",
>+      "-H 'Content-Type: text/yaml'",
>+      "-d '#{unsafe_data}'",
>+      "\"https://#{master}:8140/production/report/$(puppet master --configprint certname)\""
>+    ].join(" ")
>+
>+    on master, cmd, :acceptable_exit_codes => [0] do
>+      msg = "(#19393) (CVE-2013-1655) Puppet master accepted illegal YAML, " +
>+        "expected rejection with message 'Illegal YAML mapping found ... " +
>+        "please use ... instead'"
>+      assert_match(/Illegal YAML mapping found/, stdout, msg)
>+    end
>+  end
>+else
>+  skip_test "Cannot validate CVE-2013-1655 unless the master is running with the Psych YAML engine"
>+end
>diff --git a/acceptance/tests/security/cve-2013-2274_all_your_agent_terminii_belong_to_us.rb b/acceptance/tests/security/cve-2013-2274_all_your_agent_terminii_belong_to_us.rb
>new file mode 100644
>index 0000000..64c9170
>--- /dev/null
>+++ b/acceptance/tests/security/cve-2013-2274_all_your_agent_terminii_belong_to_us.rb
>@@ -0,0 +1,104 @@
>+test_name "CVE 2013-2274 Agent terminii" do
>+
>+  step "Determine suitability of the test" do
>+    confine :except, :platform => 'windows'
>+
>+    versions = on( hosts, puppet( '--version' ))
>+    skip_test( "This test will only run on Puppet 2.6" ) unless
>+      versions.any? {|r| r.stdout =~ /\A2\.6\./ }
>+  end
>+
>+  with_master_running_on( master, '--autosign true' ) do
>+    on agents, puppet_agent( '-t' )
>+  end
>+
>+  def exploit_code( exploiter, exploitee, endpoint, port, file_to_create )
>+
>+    certfile = on( exploiter, puppet_agent( '--configprint hostcert' )).stdout.chomp
>+    keyfile = on( exploiter, puppet_agent( '--configprint hostprivkey' )).stdout.chomp
>+    certname = on( exploiter, puppet_agent( '--configprint certname' )).stdout.chomp
>+
>+    exploit = %Q[#!/usr/bin/env ruby
>+      require 'puppet'
>+      require 'openssl'
>+      require 'net/https'
>+
>+      exec = Puppet::Type.type(:exec).new(:name => '/bin/touch #{file_to_create}', :logoutput => true)
>+      yaml = exec.to_resource.to_yaml
>+
>+      headers = {'Content-Type' => 'text/yaml', 'Accept' => 'yaml'}
>+      conn = Net::HTTP.new('#{exploitee}', #{port})
>+      conn.use_ssl = true
>+      conn.cert = OpenSSL::X509::Certificate.new(File.read('#{certfile}'))
>+      conn.key = OpenSSL::PKey::RSA.new(File.read('#{keyfile}'))
>+      conn.verify_mode = OpenSSL::SSL::VERIFY_NONE
>+
>+      conn.request_put("/production/#{endpoint}/#{certname}", yaml, headers) do |response|
>+       response.read_body do |chunk|
>+         puts chunk
>+       end
>+      end ]
>+
>+    return exploit
>+  end
>+
>+  exploited = '/tmp/cve-2013-2274-has-worked'
>+  restauth_conf = %q[
>+path /run
>+auth yes
>+allow *
>+]
>+
>+  teardown do
>+    agents.each do |agent|
>+      pidfile = on( agent, puppet_agent("--configprint pidfile") ).stdout.chomp
>+      on agent, "[ -f #{pidfile} ] && kill `cat #{pidfile}` || true"
>+      on agent, "rm -rf #{exploited}"
>+    end
>+  end
>+
>+  agents.each do |agent|
>+    atestdir = agent.tmpdir('puppet-kick-auth')
>+    mtestdir = master.tmpdir('puppet-kick-auth')
>+
>+    step "Daemonize the agent" do
>+      # Lay down a tempory auth.conf that will allow the agent to be kicked
>+      create_remote_file(agent, "#{atestdir}/auth.conf", restauth_conf)
>+
>+      # Start the agent
>+      on(agent, puppet_agent("--debug --daemonize --server #{master} --listen --no-client --rest_authconfig #{atestdir}/auth.conf"))
>+
>+      step "Wait for agent to start listening" do
>+        timeout = 15
>+        begin
>+          Timeout.timeout(timeout) do
>+            loop do
>+              # 7 is "Could not connect to host", which will happen before it's running
>+              result = on(agent, "curl -k https://#{agent}:8139", :acceptable_exit_codes => [0,7])
>+              break if result.exit_code == 0
>+              sleep 1
>+            end
>+          end
>+        rescue Timeout::Error
>+          fail_test "Puppet agent #{agent} failed to start after #{timeout} seconds"
>+        end
>+      end
>+    end
>+
>+    step "Attempt to exploit #{agent}" do
>+      # Ensure there's no stale data
>+      on agent, "rm -rf #{exploited}"
>+      on master, "rm -rf #{mtestdir}/exploit.rb"
>+
>+      # Copy over our exploit and execute
>+      create_remote_file( master, "#{mtestdir}/exploit.rb", exploit_code( master, agent, 'run', 8139, exploited ))
>+      on master, "chmod +x #{mtestdir}/exploit.rb"
>+      on master, "#{mtestdir}/exploit.rb"
>+
>+      # Did it work?
>+      fail_test( "Found exploit file #{exploited}" ) if
>+        on( agent, "[ ! -f #{exploited} ]",
>+           :acceptable_exit_codes => [0,1] ).exit_code == 1
>+    end
>+  end
>+end
>diff --git a/acceptance/tests/security/cve-2013-2274_all_your_master_terminii_belong_to_us.rb b/acceptance/tests/security/cve-2013-2274_all_your_master_terminii_belong_to_us.rb
>new file mode 100644
>index 0000000..fa45f7b
>--- /dev/null
>+++ b/acceptance/tests/security/cve-2013-2274_all_your_master_terminii_belong_to_us.rb
>@@ -0,0 +1,79 @@
>+test_name "CVE 2013-2274" do
>+
>+  step "Determine suitability of the test" do
>+    confine :except, :platform => 'windows'
>+
>+    versions = on( hosts, puppet( '--version' ))
>+    skip_test( "This test will only run on Puppet 2.6" ) unless
>+      versions.any? {|r| r.stdout =~ /\A2\.6\./ }
>+  end
>+
>+
>+  def exploit_code( exploiter, exploitee, endpoint, port, file_to_create, key=nil )
>+
>+    certfile = on( exploiter, puppet_agent( '--configprint hostcert' )).stdout.chomp
>+    keyfile = on( exploiter, puppet_agent( '--configprint hostprivkey' )).stdout.chomp
>+    certname = on( exploiter, puppet_agent( '--configprint certname' )).stdout.chomp
>+
>+    exploit = %Q[#!/usr/bin/env ruby
>+      require 'puppet'
>+      require 'openssl'
>+      require 'net/https'
>+
>+      exec = Puppet::Type.type(:exec).new(:name => 'touch #{file_to_create}', :logoutput => true, :path => '/bin')
>+      yaml = exec.to_resource.to_yaml
>+
>+      headers = {'Content-Type' => 'text/yaml', 'Accept' => 'yaml'}
>+      conn = Net::HTTP.new('#{exploitee}', #{port})
>+      conn.use_ssl = true
>+      conn.cert = OpenSSL::X509::Certificate.new(File.read('#{certfile}'))
>+      conn.key = OpenSSL::PKey::RSA.new(File.read('#{keyfile}'))
>+      conn.verify_mode = OpenSSL::SSL::VERIFY_NONE
>+
>+      conn.request_put("/production/#{endpoint}/#{key || certname}", yaml, headers) do |response|
>+       response.read_body do |chunk|
>+         puts chunk
>+       end
>+      end ]
>+
>+    return exploit
>+  end
>+
>+  exploited = '/tmp/cve-2013-2274-has-worked'
>+
>+  teardown do
>+    agents.each do |agent|
>+      pidfile = on( agent, puppet_agent("--configprint pidfile") ).stdout.chomp
>+      on agent, "[ -f #{pidfile} ] && kill `cat #{pidfile}` || true"
>+      on agent, "rm -rf #{exploited}"
>+    end
>+  end
>+
>+  with_master_running_on(master, "--autosign true") do
>+    agents.each do |agent|
>+
>+      testdir = agent.tmpdir('puppet-kick-auth')
>+      step "Prepare the agent:#{agent} to exploit the master #{master}" do
>+
>+        # Ensure the agent has its cert
>+        on agent, puppet_agent("--test --server #{master}")
>+
>+        # Double check to ensure we don't have stale data
>+        on master, "rm -rf #{exploited}"
>+        on agent, "rm -rf #{testdir}/exploit.rb"
>+      end
>+
>+      step "Exploit the master" do
>+        # Copy over our exploit code and execute it
>+        create_remote_file( agent, "#{testdir}/exploit.rb", exploit_code( agent, master, 'report', 8140, exploited ))
>+        on agent, "chmod +x #{testdir}/exploit.rb"
>+        on agent, "#{testdir}/exploit.rb"
>+
>+        # Did it exploit the SUT?
>+        fail_test( "Found exploit file #{exploited}" ) if
>+          on( master, "[ ! -f #{exploited} ]",
>+             :acceptable_exit_codes => [0,1] ).exit_code == 1
>+      end
>+    end
>+  end
>+end
>diff --git a/acceptance/tests/security/cve-2013-2275_report_acl.rb b/acceptance/tests/security/cve-2013-2275_report_acl.rb
>new file mode 100644
>index 0000000..dae05c5
>--- /dev/null
>+++ b/acceptance/tests/security/cve-2013-2275_report_acl.rb
>@@ -0,0 +1,30 @@
>+test_name "(#19531) report save access control"
>+step "Verify puppet only allows saving reports from the node matching the certificate"
>+
>+fake_report = <<-EOYAML
>+--- !ruby/object:Puppet::Transaction::Report
>+  host: mccune
>+  metrics: {}
>+  logs: []
>+  kind: inspect
>+  puppet_version: "2.7.20"
>+  status: failed
>+  report_format: 3
>+EOYAML
>+
>+with_master_running_on(master) do
>+  submit_fake_report_cmd = [
>+    "curl -k -X PUT",
>+    "--cacert \"$(puppet master --configprint cacert)\"",
>+    "--cert \"$(puppet master --configprint hostcert)\"",
>+    "--key \"$(puppet master --configprint hostprivkey)\"",
>+    "-H 'Content-Type: text/yaml'",
>+    "-d '#{fake_report}'",
>+    "\"https://#{master}:8140/production/report/mccune\"",
>+  ].join(" ")
>+
>+  on master, submit_fake_report_cmd, :acceptable_exit_codes => [0] do
>+    msg = "(#19531) (CVE-2013-2275) Puppet master accepted a report for a node that does not match the certname"
>+    assert_match(/Forbidden request/, stdout, msg)
>+  end
>+end
>diff --git a/conf/auth.conf b/conf/auth.conf
>index ba389c0..92aae26 100644
>--- a/conf/auth.conf
>+++ b/conf/auth.conf
>@@ -63,10 +63,10 @@ path /certificate_revocation_list/ca
> method find
> allow *
> 
>-# allow all nodes to store their reports
>-path /report
>+# allow all nodes to store their own reports
>+path ~ ^/report/([^/]+)$
> method save
>-allow *
>+allow $1
> 
> # inconditionnally allow access to all files services
> # which means in practice that fileserver.conf will
>diff --git a/lib/puppet/indirector/catalog/compiler.rb b/lib/puppet/indirector/catalog/compiler.rb
>index 19f8386..757ec6e 100644
>--- a/lib/puppet/indirector/catalog/compiler.rb
>+++ b/lib/puppet/indirector/catalog/compiler.rb
>@@ -13,7 +13,9 @@ class Puppet::Resource::Catalog::Compiler < Puppet::Indirector::Code
> 
>   def extract_facts_from_request(request)
>     return unless text_facts = request.options[:facts]
>-    raise ArgumentError, "Facts but no fact format provided for #{request.name}" unless format = request.options[:facts_format]
>+    unless format = request.options[:facts_format]
>+      raise ArgumentError, "Facts but no fact format provided for #{request.key}"
>+    end
> 
>     # If the facts were encoded as yaml, then the param reconstitution system
>     # in Network::HTTP::Handler will automagically deserialize the value.
>@@ -22,6 +24,11 @@ class Puppet::Resource::Catalog::Compiler < Puppet::Indirector::Code
>     else
>       facts = Puppet::Node::Facts.convert_from(format, text_facts)
>     end
>+
>+    unless facts.name == request.key
>+      raise Puppet::Error, "Catalog for #{request.key.inspect} was requested with fact definition for the wrong node (#{facts.name.inspect})."
>+    end
>+
>     facts.add_timestamp
>     Puppet::Node::Facts.indirection.save(facts)
>   end
>@@ -104,7 +111,11 @@ class Puppet::Resource::Catalog::Compiler < Puppet::Indirector::Code
>   # to find the node.
>   def node_from_request(request)
>     if node = request.options[:use_node]
>-      return node
>+      if request.remote?
>+        raise Puppet::Error, "Invalid option use_node for a remote request"
>+      else
>+        return node
>+      end
>     end
> 
>     # We rely on our authorization system to determine whether the connected
>diff --git a/lib/puppet/indirector/certificate_status/file.rb b/lib/puppet/indirector/certificate_status/file.rb
>index 9061d94..db316f6 100644
>--- a/lib/puppet/indirector/certificate_status/file.rb
>+++ b/lib/puppet/indirector/certificate_status/file.rb
>@@ -79,4 +79,9 @@ class Puppet::Indirector::CertificateStatus::File < Puppet::Indirector::Code
>       nil
>     end
>   end
>+
>+  def validate_key(request)
>+    # We only use desired_state from the instance and use request.key
>+    # otherwise, so the name does not need to match
>+  end
> end
>diff --git a/lib/puppet/indirector/errors.rb b/lib/puppet/indirector/errors.rb
>new file mode 100644
>index 0000000..6b599b8
>--- /dev/null
>+++ b/lib/puppet/indirector/errors.rb
>@@ -0,0 +1,5 @@
>+require 'puppet/error'
>+
>+module Puppet::Indirector
>+  class ValidationError < Puppet::Error; end
>+end
>diff --git a/lib/puppet/indirector/file_bucket_file/file.rb b/lib/puppet/indirector/file_bucket_file/file.rb
>index d32788a..99006a3 100644
>--- a/lib/puppet/indirector/file_bucket_file/file.rb
>+++ b/lib/puppet/indirector/file_bucket_file/file.rb
>@@ -48,6 +48,10 @@ module Puppet::FileBucketFile
>       instance.to_s
>     end
> 
>+    def validate_key(request)
>+      # There are no ACLs on filebucket files so validating key is not important
>+    end
>+
>     private
> 
>     def path_match(dir_path, files_original_path)
>diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb
>index 7296be2..75a7867 100644
>--- a/lib/puppet/indirector/indirection.rb
>+++ b/lib/puppet/indirector/indirection.rb
>@@ -309,6 +309,7 @@ class Puppet::Indirector::Indirection
> 
>     dest_terminus = terminus(terminus_name)
>     check_authorization(request, dest_terminus)
>+    dest_terminus.validate(request)
> 
>     dest_terminus
>   end
>diff --git a/lib/puppet/indirector/request.rb b/lib/puppet/indirector/request.rb
>index b6b69ea..683c9a5 100644
>--- a/lib/puppet/indirector/request.rb
>+++ b/lib/puppet/indirector/request.rb
>@@ -150,6 +150,10 @@ class Puppet::Indirector::Request
>     return(uri ? uri : "/#{indirection_name}/#{key}")
>   end
> 
>+  def remote?
>+    self.node or self.ip
>+  end
>+
>   private
> 
>   def set_attributes(options)
>diff --git a/lib/puppet/indirector/resource/active_record.rb b/lib/puppet/indirector/resource/active_record.rb
>index e886e0e..b2d6a91 100644
>--- a/lib/puppet/indirector/resource/active_record.rb
>+++ b/lib/puppet/indirector/resource/active_record.rb
>@@ -1,6 +1,9 @@
> require 'puppet/indirector/active_record'
>+require 'puppet/indirector/resource/validator'
> 
> class Puppet::Resource::ActiveRecord < Puppet::Indirector::ActiveRecord
>+  include Puppet::Resource::Validator
>+
>   def search(request)
>     type   = request_to_type_name(request)
>     host   = request.options[:host]
>diff --git a/lib/puppet/indirector/resource/ral.rb b/lib/puppet/indirector/resource/ral.rb
>index b7f827a..99e0b73 100644
>--- a/lib/puppet/indirector/resource/ral.rb
>+++ b/lib/puppet/indirector/resource/ral.rb
>@@ -1,4 +1,8 @@
>+require 'puppet/indirector/resource/validator'
>+
> class Puppet::Resource::Ral < Puppet::Indirector::Code
>+  include Puppet::Resource::Validator
>+
>   def find( request )
>     # find by name
>     res   = type(request).instances.find { |o| o.name == resource_name(request) }
>diff --git a/lib/puppet/indirector/resource/store_configs.rb b/lib/puppet/indirector/resource/store_configs.rb
>index 69b57c0..28be7a2 100644
>--- a/lib/puppet/indirector/resource/store_configs.rb
>+++ b/lib/puppet/indirector/resource/store_configs.rb
>@@ -1,3 +1,6 @@
> require 'puppet/indirector/store_configs'
>+require 'puppet/indirector/resource/validator'
>+
> class Puppet::Resource::StoreConfigs < Puppet::Indirector::StoreConfigs
>+  include Puppet::Resource::Validator
> end
>diff --git a/lib/puppet/indirector/resource/validator.rb b/lib/puppet/indirector/resource/validator.rb
>new file mode 100644
>index 0000000..dff6ce5
>--- /dev/null
>+++ b/lib/puppet/indirector/resource/validator.rb
>@@ -0,0 +1,8 @@
>+module Puppet::Resource::Validator
>+  def validate_key(request)
>+    type, title = request.key.split('/', 2)
>+    unless type.downcase == request.instance.type.downcase and title == request.instance.title
>+      raise Puppet::Indirector::ValidationError, "Resource instance does not match request key"
>+    end
>+  end
>+end
>diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb
>index d76a289..da8ae82 100644
>--- a/lib/puppet/indirector/rest.rb
>+++ b/lib/puppet/indirector/rest.rb
>@@ -102,6 +102,10 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus
>       msg = valid_certnames.length > 1 ? "one of #{valid_certnames.join(', ')}" : valid_certnames.first
> 
>       raise Puppet::Error, "Server hostname '#{http_connection.address}' did not match server certificate; expected #{msg}"
>+    elsif error.message.empty?
>+      # This may be because the server is speaking SSLv2 and we
>+      # monkey patch OpenSSL::SSL:SSLContext to reject SSLv2.
>+      raise error.exception("#{error.class} with no message")
>     else
>       raise
>     end
>@@ -152,6 +156,10 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus
>     deserialize http_put(request, indirection2uri(request), request.instance.render, headers.merge({ "Content-Type" => request.instance.mime }))
>   end
> 
>+  def validate_key(request)
>+    # Validation happens on the remote end
>+  end
>+
>   private
> 
>   def environment
>diff --git a/lib/puppet/indirector/run/local.rb b/lib/puppet/indirector/run/local.rb
>index 8cf65d1..473c3d6 100644
>--- a/lib/puppet/indirector/run/local.rb
>+++ b/lib/puppet/indirector/run/local.rb
>@@ -5,4 +5,8 @@ class Puppet::Run::Local < Puppet::Indirector::Code
>   def save( request )
>     request.instance.run
>   end
>+
>+  def validate_key(request)
>+    # No key is necessary for kick
>+  end
> end
>diff --git a/lib/puppet/indirector/terminus.rb b/lib/puppet/indirector/terminus.rb
>index d488869..e86f8fd 100644
>--- a/lib/puppet/indirector/terminus.rb
>+++ b/lib/puppet/indirector/terminus.rb
>@@ -1,4 +1,5 @@
> require 'puppet/indirector'
>+require 'puppet/indirector/errors'
> require 'puppet/indirector/indirection'
> require 'puppet/util/instance_loader'
> 
>@@ -142,4 +143,23 @@ class Puppet::Indirector::Terminus
>   def terminus_type
>     self.class.terminus_type
>   end
>+
>+  def validate(request)
>+    if request.instance
>+      validate_key(request)
>+      validate_model(request)
>+    end
>+  end
>+
>+  def validate_key(request)
>+    unless request.key == request.instance.name
>+      raise Puppet::Indirector::ValidationError, "Instance name #{request.instance.name.inspect} does not match requested key #{request.key.inspect}"
>+    end
>+  end
>+
>+  def validate_model(request)
>+    unless request.instance.kind_of?(model)
>+      raise Puppet::Indirector::ValidationError, "Invalid instance type #{request.instance.class.inspect}, expected #{model.inspect}"
>+    end
>+  end
> end
>diff --git a/lib/puppet/network/formats.rb b/lib/puppet/network/formats.rb
>index 082c83e..e12aa6d 100644
>--- a/lib/puppet/network/formats.rb
>+++ b/lib/puppet/network/formats.rb
>@@ -3,12 +3,12 @@ require 'puppet/network/format_handler'
> Puppet::Network::FormatHandler.create_serialized_formats(:yaml) do
>   # Yaml doesn't need the class name; it's serialized.
>   def intern(klass, text)
>-    YAML.load(text)
>+    YAML.safely_load(text)
>   end
> 
>   # Yaml doesn't need the class name; it's serialized.
>   def intern_multiple(klass, text)
>-    YAML.load(text)
>+    YAML.safely_load(text)
>   end
> 
>   def render(instance)
>@@ -72,7 +72,7 @@ Puppet::Network::FormatHandler.create_serialized_formats(:b64_zlib_yaml) do
> 
>   def decode(yaml)
>     requiring_zlib do
>-      YAML.load(Zlib::Inflate.inflate(Base64.decode64(yaml)))
>+      YAML.safely_load(Zlib::Inflate.inflate(Base64.decode64(yaml)))
>     end
>   end
> end
>diff --git a/lib/puppet/network/handler/master.rb b/lib/puppet/network/handler/master.rb
>index fd2bb95..15c229d 100644
>--- a/lib/puppet/network/handler/master.rb
>+++ b/lib/puppet/network/handler/master.rb
>@@ -69,7 +69,7 @@ class Puppet::Network::Handler
>         Puppet.debug "Our client is remote"
> 
>         begin
>-          facts = YAML.load(CGI.unescape(facts))
>+          facts = YAML.safely_load(CGI.unescape(facts))
>         rescue => detail
>           raise XMLRPC::FaultException.new(
>             1, "Could not rebuild facts"
>diff --git a/lib/puppet/network/handler/report.rb b/lib/puppet/network/handler/report.rb
>index 5e3ee26..540a43e 100755
>--- a/lib/puppet/network/handler/report.rb
>+++ b/lib/puppet/network/handler/report.rb
>@@ -45,7 +45,7 @@ class Puppet::Network::Handler
> 
>       # First convert the report to real objects
>       begin
>-        report = YAML.load(yaml)
>+        report = YAML.safely_load(yaml)
>       rescue => detail
>         Puppet.warning "Could not load report: #{detail}"
>         return
>diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb
>index 2c78a02..d6a9d06 100644
>--- a/lib/puppet/network/http/handler.rb
>+++ b/lib/puppet/network/http/handler.rb
>@@ -70,6 +70,8 @@ module Puppet::Network::HTTP::Handler
>     raise
>   rescue Exception => e
>     return do_exception(response, e)
>+  ensure
>+    cleanup(request)
>   end
> 
>   # Set the response up, with the body and status.
>@@ -217,6 +219,10 @@ module Puppet::Network::HTTP::Handler
>     raise NotImplementedError
>   end
> 
>+  def cleanup(request)
>+    # By default, there is nothing to cleanup.
>+  end
>+
>   def decode_params(params)
>     params.inject({}) do |result, ary|
>       param, value = ary
>@@ -230,7 +236,7 @@ module Puppet::Network::HTTP::Handler
>       next result if param == :ip
>       value = CGI.unescape(value)
>       if value =~ /^---/
>-        value = YAML.load(value)
>+        value = YAML.safely_load(value)
>       else
>         value = true if value == "true"
>         value = false if value == "false"
>diff --git a/lib/puppet/network/http/rack/rest.rb b/lib/puppet/network/http/rack/rest.rb
>index 602927a..61ef529 100644
>--- a/lib/puppet/network/http/rack/rest.rb
>+++ b/lib/puppet/network/http/rack/rest.rb
>@@ -73,12 +73,17 @@ class Puppet::Network::HTTP::RackREST < Puppet::Network::HTTP::RackHttpHandler
>   end
> 
>   # return the request body
>-  # request.body has some limitiations, so we need to concat it back
>-  # into a regular string, which is something puppet can use.
>   def body(request)
>     request.body.read
>   end
> 
>+  # Passenger freaks out if we finish handling the request without reading any
>+  # part of the body, so make sure we have.
>+  def cleanup(request)
>+    request.body.read(1)
>+    nil
>+  end
>+
>   def extract_client_info(request)
>     result = {}
>     result[:ip] = request.ip
>diff --git a/lib/puppet/network/http/webrick.rb b/lib/puppet/network/http/webrick.rb
>index 52aec1b..d645e6d 100644
>--- a/lib/puppet/network/http/webrick.rb
>+++ b/lib/puppet/network/http/webrick.rb
>@@ -104,6 +104,7 @@ class Puppet::Network::HTTP::WEBrick
>     results[:SSLCertificate] = host.certificate.content
>     results[:SSLStartImmediately] = true
>     results[:SSLEnable] = true
>+    results[:SSLOptions] = OpenSSL::SSL::OP_NO_SSLv2
> 
>     raise Puppet::Error, "Could not find CA certificate" unless Puppet::SSL::Certificate.indirection.find(Puppet::SSL::CA_NAME)
> 
>diff --git a/lib/puppet/network/http_pool.rb b/lib/puppet/network/http_pool.rb
>index 8baf48c..96e20f7 100644
>--- a/lib/puppet/network/http_pool.rb
>+++ b/lib/puppet/network/http_pool.rb
>@@ -11,14 +11,23 @@ module Puppet::Network::HttpPool
> 
>   # Use cert information from a Puppet client to set up the http object.
>   def self.cert_setup(http)
>-    # Just no-op if we don't have certs.
>-    return false unless FileTest.exist?(Puppet[:hostcert]) and FileTest.exist?(Puppet[:localcacert])
>-
>-    http.cert_store = ssl_host.ssl_store
>-    http.ca_file = Puppet[:localcacert]
>-    http.cert = ssl_host.certificate.content
>-    http.verify_mode = OpenSSL::SSL::VERIFY_PEER
>-    http.key = ssl_host.key.content
>+    if FileTest.exist?(Puppet[:hostcert]) and FileTest.exist?(Puppet[:localcacert])
>+      http.cert_store  = ssl_host.ssl_store
>+      http.ca_file     = Puppet[:localcacert]
>+      http.cert        = ssl_host.certificate.content
>+      http.verify_mode = OpenSSL::SSL::VERIFY_PEER
>+      http.key         = ssl_host.key.content
>+    else
>+      # We don't have the local certificates, so we don't do any verification
>+      # or setup at this early stage.  REVISIT: Shouldn't we supply the local
>+      # certificate details if we have them?  The original code didn't.
>+      # --daniel 2012-06-03
>+
>+      # Ruby 1.8 defaulted to this, but 1.9 defaults to peer verify, and we
>+      # almost always talk to a dedicated, not-standard CA that isn't trusted
>+      # out of the box.  This forces the expected state.
>+      http.verify_mode = OpenSSL::SSL::VERIFY_NONE
>+    end
>   end
> 
>   # Retrieve a cached http instance if caching is enabled, else return
>diff --git a/lib/puppet/network/rest_authconfig.rb b/lib/puppet/network/rest_authconfig.rb
>index 8016710..643dd23 100644
>--- a/lib/puppet/network/rest_authconfig.rb
>+++ b/lib/puppet/network/rest_authconfig.rb
>@@ -13,7 +13,7 @@ module Puppet
>       # to fileserver.conf
>       { :acl => "/file" },
>       { :acl => "/certificate_revocation_list/ca", :method => :find, :authenticated => true },
>-      { :acl => "/report", :method => :save, :authenticated => true },
>+      { :acl => "~ ^\/report\/([^\/]+)$", :method => :save, :allow => '$1', :authenticated => true },
>       # These allow `auth any`, because if you can do them anonymously you
>       # should probably also be able to do them when trusted.
>       { :acl => "/certificate/ca", :method => :find, :authenticated => :any },
>diff --git a/lib/puppet/parser/templatewrapper.rb b/lib/puppet/parser/templatewrapper.rb
>index 27d75bf..3df03c4 100644
>--- a/lib/puppet/parser/templatewrapper.rb
>+++ b/lib/puppet/parser/templatewrapper.rb
>@@ -5,8 +5,6 @@ require 'erb'
> 
> class Puppet::Parser::TemplateWrapper
>   attr_writer :scope
>-  attr_reader :file
>-  attr_accessor :string
>   include Puppet::Util
>   Puppet::Util.logmethods(self)
> 
>@@ -14,18 +12,22 @@ class Puppet::Parser::TemplateWrapper
>     @__scope__ = scope
>   end
> 
>+  def file
>+    @__file__
>+  end
>+
>   def scope
>     @__scope__
>   end
> 
>   def script_line
>     # find which line in the template (if any) we were called from
>-    (caller.find { |l| l =~ /#{file}:/ }||"")[/:(\d+):/,1]
>+    (caller.find { |l| l =~ /#{@__file__}:/ }||"")[/:(\d+):/,1]
>   end
> 
>   # Should return true if a variable is defined, false if it is not
>   def has_variable?(name)
>-    scope.lookupvar(name.to_s, :file => file, :line => script_line) != :undefined
>+    scope.lookupvar(name.to_s, :file => @__file__, :line => script_line) != :undefined
>   end
> 
>   # Allow templates to access the defined classes
>@@ -56,53 +58,51 @@ class Puppet::Parser::TemplateWrapper
>   # the missing_method definition here until we declare the syntax finally
>   # dead.
>   def method_missing(name, *args)
>-    value = scope.lookupvar(name.to_s,:file => file,:line => script_line)
>+    value = scope.lookupvar(name.to_s,:file => @__file__,:line => script_line)
>     if value != :undefined
>       return value
>     else
>       # Just throw an error immediately, instead of searching for
>       # other missingmethod things or whatever.
>-      raise Puppet::ParseError.new("Could not find value for '#{name}'",@file,script_line)
>+      raise Puppet::ParseError.new("Could not find value for '#{name}'", @__file__, script_line)
>     end
>   end
> 
>   def file=(filename)
>-    unless @file = Puppet::Parser::Files.find_template(filename, scope.compiler.environment.to_s)
>+    unless @__file__ = Puppet::Parser::Files.find_template(filename, scope.compiler.environment.to_s)
>       raise Puppet::ParseError, "Could not find template '#{filename}'"
>     end
> 
>     # We'll only ever not have a parser in testing, but, eh.
>-    scope.known_resource_types.watch_file(file)
>-
>-    @string = File.read(file)
>+    scope.known_resource_types.watch_file(@__file__)
>   end
> 
>   def result(string = nil)
>     if string
>-      self.string = string
>       template_source = "inline template"
>     else
>-      template_source = file
>+      string = File.read(@__file__)
>+      template_source = @__file__
>     end
> 
>     # Expose all the variables in our scope as instance variables of the
>     # current object, making it possible to access them without conflict
>     # to the regular methods.
>     benchmark(:debug, "Bound template variables for #{template_source}") do
>-      scope.to_hash.each { |name, value|
>+      scope.to_hash.each do |name, value|
>         if name.kind_of?(String)
>           realname = name.gsub(/[^\w]/, "_")
>         else
>           realname = name
>         end
>         instance_variable_set("@#{realname}", value)
>-      }
>+      end
>     end
> 
>     result = nil
>     benchmark(:debug, "Interpolated template #{template_source}") do
>-      template = ERB.new(self.string, 0, "-")
>-      template.filename = file
>+      template = ERB.new(string, 0, "-")
>+      template.filename = @__file__
>       result = template.result(binding)
>     end
> 
>@@ -110,6 +110,6 @@ class Puppet::Parser::TemplateWrapper
>   end
> 
>   def to_s
>-    "template[#{(file ? file : "inline")}]"
>+    "template[#{(@__file__ ? @__file__ : "inline")}]"
>   end
> end
>diff --git a/lib/puppet/util/monkey_patches.rb b/lib/puppet/util/monkey_patches.rb
>index 5186a4e..69c868d 100644
>--- a/lib/puppet/util/monkey_patches.rb
>+++ b/lib/puppet/util/monkey_patches.rb
>@@ -34,6 +34,21 @@ end
>   end
> }
> 
>+if defined?(YAML::ENGINE) and YAML::ENGINE.yamler == 'psych'
>+  def Psych.safely_load(str)
>+    result = Psych.parse(str)
>+    if invalid_node = result.find { |node| node.tag =~ /!map:(.*)/ || node.tag =~ /!ruby\/hash:(.*)/ }
>+      raise ArgumentError, "Illegal YAML mapping found with tag #{invalid_node.tag}; please use !ruby/object:#{$1} instead"
>+    else
>+      result.to_ruby
>+    end
>+  end
>+else
>+  def YAML.safely_load(str)
>+    self.load(str)
>+  end
>+end
>+
> def YAML.dump(*args)
>   ZAML.dump(*args)
> end
>@@ -138,3 +153,91 @@ module Kernel
>     self
>   end unless method_defined?(:tap)
> end
>+
>+# The mv method in Ruby 1.8.5 can't mv directories across devices
>+# File.rename causes "Invalid cross-device link", which is rescued, but in Ruby
>+# 1.8.5 it tries to recover with a copy and unlink, but the unlink causes the
>+# error "Is a directory".  In newer Rubies remove_entry is used
>+# The implementation below is what's used in Ruby 1.8.7 and Ruby 1.9
>+if RUBY_VERSION == '1.8.5'
>+  require 'fileutils'
>+
>+  module FileUtils
>+    def mv(src, dest, options = {})
>+      fu_check_options options, OPT_TABLE['mv']
>+      fu_output_message "mv#{options[:force] ? ' -f' : ''} #{[src,dest].flatten.join ' '}" if options[:verbose]
>+      return if options[:noop]
>+      fu_each_src_dest(src, dest) do |s, d|
>+        destent = Entry_.new(d, nil, true)
>+        begin
>+          if destent.exist?
>+            if destent.directory?
>+              raise Errno::EEXIST, dest
>+            else
>+              destent.remove_file if rename_cannot_overwrite_file?
>+            end
>+          end
>+          begin
>+            File.rename s, d
>+          rescue Errno::EXDEV
>+            copy_entry s, d, true
>+            if options[:secure]
>+              remove_entry_secure s, options[:force]
>+            else
>+              remove_entry s, options[:force]
>+            end
>+          end
>+        rescue SystemCallError
>+          raise unless options[:force]
>+        end
>+      end
>+    end
>+    module_function :mv
>+
>+    alias move mv
>+    module_function :move
>+  end
>+end
>+
>+# (#19151) Reject all SSLv2 ciphers and handshakes
>+require 'openssl'
>+class OpenSSL::SSL::SSLContext
>+  if match = /^1\.8\.(\d+)/.match(RUBY_VERSION)
>+    older_than_187 = match[1].to_i < 7
>+  else
>+    older_than_187 = false
>+  end
>+
>+  alias __original_initialize initialize
>+  private :__original_initialize
>+
>+  if older_than_187
>+    def initialize(*args)
>+      __original_initialize(*args)
>+      if bitmask = self.options
>+        self.options = bitmask | OpenSSL::SSL::OP_NO_SSLv2
>+      else
>+        self.options = OpenSSL::SSL::OP_NO_SSLv2
>+      end
>+      # These are the default ciphers in recent MRI versions.  See
>+      # https://github.com/ruby/ruby/blob/v1_9_3_392/ext/openssl/lib/openssl/ssl-internal.rb#L26
>+      self.ciphers = "ALL:!ADH:!EXPORT:!SSLv2:RC4+RSA:+HIGH:+MEDIUM:+LOW"
>+    end
>+  else
>+    if DEFAULT_PARAMS[:options]
>+      DEFAULT_PARAMS[:options] |= OpenSSL::SSL::OP_NO_SSLv2
>+    else
>+      DEFAULT_PARAMS[:options] = OpenSSL::SSL::OP_NO_SSLv2
>+    end
>+    DEFAULT_PARAMS[:ciphers] << ':!SSLv2'
>+
>+    def initialize(*args)
>+      __original_initialize(*args)
>+      params = {
>+        :options => DEFAULT_PARAMS[:options],
>+        :ciphers => DEFAULT_PARAMS[:ciphers],
>+      }
>+      set_params(params)
>+    end
>+  end
>+end
>diff --git a/spec/integration/indirector/catalog/compiler_spec.rb b/spec/integration/indirector/catalog/compiler_spec.rb
>index f51a3f2..79f3d90 100755
>--- a/spec/integration/indirector/catalog/compiler_spec.rb
>+++ b/spec/integration/indirector/catalog/compiler_spec.rb
>@@ -11,6 +11,7 @@ describe Puppet::Resource::Catalog::Compiler do
>     @catalog = Puppet::Resource::Catalog.new
>     @catalog.add_resource(@one = Puppet::Resource.new(:file, "/one"))
>     @catalog.add_resource(@two = Puppet::Resource.new(:file, "/two"))
>+    Puppet::Resource::Catalog.indirection.terminus.stubs(:validate)
>   end
> 
>   after { Puppet.settings.clear }
>diff --git a/spec/integration/indirector/catalog/queue_spec.rb b/spec/integration/indirector/catalog/queue_spec.rb
>index 940c8ba..bfc74f8 100755
>--- a/spec/integration/indirector/catalog/queue_spec.rb
>+++ b/spec/integration/indirector/catalog/queue_spec.rb
>@@ -6,7 +6,7 @@ require 'puppet/resource/catalog'
> describe "Puppet::Resource::Catalog::Queue", :if => Puppet.features.pson? do
>   before do
>     Puppet::Resource::Catalog.indirection.terminus(:queue)
>-    @catalog = Puppet::Resource::Catalog.new
>+    @catalog = Puppet::Resource::Catalog.new("foo")
> 
>     @one = Puppet::Resource.new(:file, "/one")
>     @two = Puppet::Resource.new(:file, "/two")
>diff --git a/spec/integration/resource/catalog_spec.rb b/spec/integration/resource/catalog_spec.rb
>index df310b1..f545546 100755
>--- a/spec/integration/resource/catalog_spec.rb
>+++ b/spec/integration/resource/catalog_spec.rb
>@@ -48,6 +48,7 @@ describe Puppet::Resource::Catalog do
>       Puppet::Resource::Catalog.indirection.stubs(:terminus).returns terminus
> 
>       node = mock 'node'
>+      terminus.stubs(:validate)
>       terminus.expects(:find).with { |request| request.options[:use_node] == node }
>       Puppet::Resource::Catalog.indirection.find("me", :use_node => node)
>     end
>diff --git a/spec/unit/indirector/catalog/compiler_spec.rb b/spec/unit/indirector/catalog/compiler_spec.rb
>index 3c559e3..950d29e 100755
>--- a/spec/unit/indirector/catalog/compiler_spec.rb
>+++ b/spec/unit/indirector/catalog/compiler_spec.rb
>@@ -53,13 +53,22 @@ describe Puppet::Resource::Catalog::Compiler do
>       @request = stub 'request', :key => @name, :node => @name, :options => {}
>     end
> 
>-    it "should directly use provided nodes" do
>+    it "should directly use provided nodes for a local request" do
>       Puppet::Node.indirection.expects(:find).never
>       @compiler.expects(:compile).with(@node)
>       @request.stubs(:options).returns(:use_node => @node)
>+      @request.stubs(:remote?).returns(false)
>       @compiler.find(@request)
>     end
> 
>+    it "rejects a provided node if the request is remote" do
>+      @request.stubs(:options).returns(:use_node => @node)
>+      @request.stubs(:remote?).returns(true)
>+      expect {
>+        @compiler.find(@request)
>+      }.to raise_error Puppet::Error, /invalid option use_node/i
>+    end
>+
>     it "should use the authenticated node name if no request key is provided" do
>       @request.stubs(:key).returns(nil)
>       Puppet::Node.indirection.expects(:find).with(@name).returns(@node)
>@@ -99,6 +108,24 @@ describe Puppet::Resource::Catalog::Compiler do
>       @compiler.find(@request)
>     end
> 
>+    it "requires `facts_format` option if facts are passed in" do
>+      facts = Puppet::Node::Facts.new("mynode", :afact => "avalue")
>+      request = Puppet::Indirector::Request.new(:catalog, :find, "mynode", :facts => facts)
>+      expect {
>+        @compiler.find(request)
>+      }.to raise_error ArgumentError, /no fact format provided for mynode/
>+    end
>+
>+    it "rejects facts in the request from a different node" do
>+      facts = Puppet::Node::Facts.new("differentnode", :afact => "avalue")
>+      request = Puppet::Indirector::Request.new(
>+        :catalog, :find, "mynode", :facts => facts, :facts_format => "unused"
>+      )
>+      expect {
>+        @compiler.find(request)
>+      }.to raise_error Puppet::Error, /fact definition for the wrong node/i
>+    end
>+
>     it "should return the results of compiling as the catalog" do
>       Puppet::Node.indirection.stubs(:find).returns(@node)
>       config = mock 'config'
>@@ -133,7 +160,7 @@ describe Puppet::Resource::Catalog::Compiler do
>     before do
>       Facter.stubs(:value).returns "something"
>       @compiler = Puppet::Resource::Catalog::Compiler.new
>-      @request = stub 'request', :options => {}
>+      @request = Puppet::Indirector::Request.new(:catalog, :find, "hostname", nil)
> 
>       @facts = Puppet::Node::Facts.new('hostname', "fact" => "value", "architecture" => "i386")
>       Puppet::Node::Facts.indirection.stubs(:save).returns(nil)
>diff --git a/spec/unit/indirector/indirection_spec.rb b/spec/unit/indirector/indirection_spec.rb
>index c33fdf1..a0fb4d5 100755
>--- a/spec/unit/indirector/indirection_spec.rb
>+++ b/spec/unit/indirector/indirection_spec.rb
>@@ -40,7 +40,7 @@ shared_examples_for "Indirection Delegator" do
>       end
>     end
> 
>-    request = stub 'request', :key => "me", :options => {}
>+    request = Puppet::Indirector::Request.new(:indirection, :find, "me", nil)
> 
>     @indirection.stubs(:request).returns request
> 
>@@ -101,6 +101,16 @@ shared_examples_for "Delegation Authorizer" do
>   end
> end
> 
>+shared_examples_for "Request validator" do
>+  it "asks the terminus to validate the request" do
>+    @terminus.expects(:validate).raises(Puppet::Indirector::ValidationError, "Invalid")
>+    @terminus.expects(@method).never
>+    expect {
>+      @indirection.send(@method, "key")
>+    }.to raise_error Puppet::Indirector::ValidationError
>+  end
>+end
>+
> describe Puppet::Indirector::Indirection do
>   describe "when initializing" do
>     # (LAK) I've no idea how to test this, really.
>@@ -141,6 +151,7 @@ describe Puppet::Indirector::Indirection do
>     before :each do
>       @terminus_class = mock 'terminus_class'
>       @terminus = mock 'terminus'
>+      @terminus.stubs(:validate)
>       @terminus_class.stubs(:new).returns(@terminus)
>       @cache = stub 'cache', :name => "mycache"
>       @cache_class = mock 'cache_class'
>@@ -211,6 +222,7 @@ describe Puppet::Indirector::Indirection do
> 
>       it_should_behave_like "Indirection Delegator"
>       it_should_behave_like "Delegation Authorizer"
>+      it_should_behave_like "Request validator"
> 
>       it "should return the results of the delegation" do
>         @terminus.expects(:find).returns(@instance)
>@@ -251,6 +263,7 @@ describe Puppet::Indirector::Indirection do
>         before do
>           @indirection.cache_class = :cache_terminus
>           @cache_class.stubs(:new).returns(@cache)
>+          @cache.stubs(:validate)
> 
>           @instance.stubs(:expired?).returns false
>         end
>@@ -384,6 +397,7 @@ describe Puppet::Indirector::Indirection do
> 
>       it_should_behave_like "Indirection Delegator"
>       it_should_behave_like "Delegation Authorizer"
>+      it_should_behave_like "Request validator"
> 
>       it "should return true if the head method returned true" do
>         @terminus.expects(:head).returns(true)
>@@ -501,6 +515,7 @@ describe Puppet::Indirector::Indirection do
> 
>       it_should_behave_like "Indirection Delegator"
>       it_should_behave_like "Delegation Authorizer"
>+      it_should_behave_like "Request validator"
> 
>       it "should return the result of removing the instance" do
>         @terminus.stubs(:destroy).returns "yayness"
>@@ -539,6 +554,7 @@ describe Puppet::Indirector::Indirection do
> 
>       it_should_behave_like "Indirection Delegator"
>       it_should_behave_like "Delegation Authorizer"
>+      it_should_behave_like "Request validator"
> 
>       it "should set the expiration date on any instances without one set" do
>         @terminus.stubs(:search).returns([@instance])
>@@ -707,6 +723,7 @@ describe Puppet::Indirector::Indirection do
>     before do
>       @indirection = Puppet::Indirector::Indirection.new(mock('model'), :test)
>       @terminus = mock 'terminus'
>+      @terminus.stubs(:validate)
>       @terminus_class = stub 'terminus class', :new => @terminus
>     end
> 
>diff --git a/spec/unit/indirector/request_spec.rb b/spec/unit/indirector/request_spec.rb
>index d330248..19bdf04 100755
>--- a/spec/unit/indirector/request_spec.rb
>+++ b/spec/unit/indirector/request_spec.rb
>@@ -311,4 +311,26 @@ describe Puppet::Indirector::Request do
>       lambda { @request.query_string }.should raise_error(ArgumentError)
>     end
>   end
>+
>+  describe "#remote?" do
>+    def request(options = {})
>+      Puppet::Indirector::Request.new('node', 'find', 'localhost', options)
>+    end
>+
>+    it "should not be unless node or ip is set" do
>+      request.should_not be_remote
>+    end
>+
>+    it "should be remote if node is set" do
>+      request(:node => 'example.com').should be_remote
>+    end
>+
>+    it "should be remote if ip is set" do
>+      request(:ip => '127.0.0.1').should be_remote
>+    end
>+
>+    it "should be remote if node and ip are set" do
>+      request(:node => 'example.com', :ip => '127.0.0.1').should be_remote
>+    end
>+  end
> end
>diff --git a/spec/unit/indirector/terminus_spec.rb b/spec/unit/indirector/terminus_spec.rb
>index ccd6fd2..18062bb 100755
>--- a/spec/unit/indirector/terminus_spec.rb
>+++ b/spec/unit/indirector/terminus_spec.rb
>@@ -1,250 +1,264 @@
>-#!/usr/bin/env rspec
>+#! /usr/bin/env ruby
> require 'spec_helper'
> require 'puppet/defaults'
> require 'puppet/indirector'
> require 'puppet/indirector/memory'
> 
>-describe Puppet::Indirector::Terminus, :'fails_on_ruby_1.9.2' => true do
>-  before :each do
>-    Puppet::Indirector::Terminus.stubs(:register_terminus_class)
>-    @indirection = stub 'indirection', :name => :my_stuff, :register_terminus_type => nil
>-    Puppet::Indirector::Indirection.stubs(:instance).with(:my_stuff).returns(@indirection)
>-    @abstract_terminus = Class.new(Puppet::Indirector::Terminus) do
>-      def self.to_s
>-        "Testing::Abstract"
>+describe Puppet::Indirector::Terminus do
>+  before :all do
>+    class Puppet::AbstractConcept
>+      extend Puppet::Indirector
>+      indirects :abstract_concept
>+      attr_accessor :name
>+      def initialize(name = "name")
>+        @name = name
>       end
>     end
>-    @terminus_class = Class.new(@abstract_terminus) do
>-      def self.to_s
>-        "MyStuff::TermType"
>-      end
>+
>+    class Puppet::AbstractConcept::Freedom < Puppet::Indirector::Code
>     end
>-    @terminus = @terminus_class.new
>   end
> 
>-  describe Puppet::Indirector::Terminus do
>+  after :all do
>+    # Remove the class, unlinking it from the rest of the system.
>+    Puppet.send(:remove_const, :AbstractConcept)
>+  end
> 
>-    it "should provide a method for setting terminus class documentation" do
>-      @terminus_class.should respond_to(:desc)
>-    end
>+  let :terminus_class do Puppet::AbstractConcept::Freedom end
>+  let :terminus       do terminus_class.new end
>+  let :indirection    do Puppet::AbstractConcept.indirection end
>+  let :model          do Puppet::AbstractConcept end
> 
>-    it "should support a class-level name attribute" do
>-      @terminus_class.should respond_to(:name)
>-    end
>+  it "should provide a method for setting terminus class documentation" do
>+    terminus_class.should respond_to(:desc)
>+  end
> 
>-    it "should support a class-level indirection attribute" do
>-      @terminus_class.should respond_to(:indirection)
>-    end
>+  it "should support a class-level name attribute" do
>+    terminus_class.should respond_to(:name)
>+  end
> 
>-    it "should support a class-level terminus-type attribute" do
>-      @terminus_class.should respond_to(:terminus_type)
>-    end
>+  it "should support a class-level indirection attribute" do
>+    terminus_class.should respond_to(:indirection)
>+  end
> 
>-    it "should support a class-level model attribute" do
>-      @terminus_class.should respond_to(:model)
>-    end
>+  it "should support a class-level terminus-type attribute" do
>+    terminus_class.should respond_to(:terminus_type)
>+  end
> 
>-    it "should accept indirection instances as its indirection" do
>-      indirection = stub 'indirection', :is_a? => true, :register_terminus_type => nil
>-      proc { @terminus_class.indirection = indirection }.should_not raise_error
>-      @terminus_class.indirection.should equal(indirection)
>-    end
>+  it "should support a class-level model attribute" do
>+    terminus_class.should respond_to(:model)
>+  end
> 
>-    it "should look up indirection instances when only a name has been provided" do
>-      indirection = mock 'indirection'
>-      Puppet::Indirector::Indirection.expects(:instance).with(:myind).returns(indirection)
>-      @terminus_class.indirection = :myind
>-      @terminus_class.indirection.should equal(indirection)
>-    end
>+  it "should accept indirection instances as its indirection" do
>+    # The test is that this shouldn't raise, and should preserve the object
>+    # instance exactly, hence "equal", not just "==".
>+    terminus_class.indirection = indirection
>+    terminus_class.indirection.should equal indirection
>+  end
> 
>-    it "should fail when provided a name that does not resolve to an indirection" do
>-      Puppet::Indirector::Indirection.expects(:instance).with(:myind).returns(nil)
>-      proc { @terminus_class.indirection = :myind }.should raise_error(ArgumentError)
>+  it "should look up indirection instances when only a name has been provided" do
>+    terminus_class.indirection = :abstract_concept
>+    terminus_class.indirection.should equal indirection
>+  end
> 
>-      # It shouldn't overwrite our existing one (or, more normally, it shouldn't set
>-      # anything).
>-      @terminus_class.indirection.should equal(@indirection)
>-    end
>+  it "should fail when provided a name that does not resolve to an indirection" do
>+    expect {
>+      terminus_class.indirection = :exploding_whales
>+    }.to raise_error(ArgumentError, /Could not find indirection instance/)
>+
>+    # We should still have the default indirection.
>+    terminus_class.indirection.should equal indirection
>   end
> 
>-  describe Puppet::Indirector::Terminus, " when creating terminus classes" do
>-    it "should associate the subclass with an indirection based on the subclass constant" do
>-      @terminus.indirection.should equal(@indirection)
>+  describe "when a terminus instance" do
>+    it "should return the class's name as its name" do
>+      terminus.name.should == :freedom
>     end
> 
>-    it "should set the subclass's type to the abstract terminus name" do
>-      @terminus.terminus_type.should == :abstract
>+    it "should return the class's indirection as its indirection" do
>+      terminus.indirection.should equal indirection
>     end
> 
>-    it "should set the subclass's name to the indirection name" do
>-      @terminus.name.should == :term_type
>+    it "should set the instances's type to the abstract terminus type's name" do
>+      terminus.terminus_type.should == :code
>     end
> 
>-    it "should set the subclass's model to the indirection model" do
>-      @indirection.expects(:model).returns :yay
>-      @terminus.model.should == :yay
>+    it "should set the instances's model to the indirection's model" do
>+      terminus.model.should equal indirection.model
>     end
>   end
> 
>-  describe Puppet::Indirector::Terminus, " when a terminus instance" do
>+  describe "when managing terminus classes" do
>+    it "should provide a method for registering terminus classes" do
>+      Puppet::Indirector::Terminus.should respond_to(:register_terminus_class)
>+    end
> 
>-    it "should return the class's name as its name" do
>-      @terminus.name.should == :term_type
>+    it "should provide a method for returning terminus classes by name and type" do
>+      terminus = stub 'terminus_type', :name => :abstract, :indirection_name => :whatever
>+      Puppet::Indirector::Terminus.register_terminus_class(terminus)
>+      Puppet::Indirector::Terminus.terminus_class(:whatever, :abstract).should equal(terminus)
>     end
> 
>-    it "should return the class's indirection as its indirection" do
>-      @terminus.indirection.should equal(@indirection)
>+    it "should set up autoloading for any terminus class types requested" do
>+      Puppet::Indirector::Terminus.expects(:instance_load).with(:test2, "puppet/indirector/test2")
>+      Puppet::Indirector::Terminus.terminus_class(:test2, :whatever)
>     end
> 
>-    it "should set the instances's type to the abstract terminus type's name" do
>-      @terminus.terminus_type.should == :abstract
>+    it "should load terminus classes that are not found" do
>+      # Set up instance loading; it would normally happen automatically
>+      Puppet::Indirector::Terminus.instance_load :test1, "puppet/indirector/test1"
>+
>+      Puppet::Indirector::Terminus.instance_loader(:test1).expects(:load).with(:yay)
>+      Puppet::Indirector::Terminus.terminus_class(:test1, :yay)
>     end
> 
>-    it "should set the instances's model to the indirection's model" do
>-      @indirection.expects(:model).returns :yay
>-      @terminus.model.should == :yay
>+    it "should fail when no indirection can be found" do
>+      Puppet::Indirector::Indirection.expects(:instance).with(:abstract_concept).returns(nil)
>+      expect {
>+        class Puppet::AbstractConcept::Physics < Puppet::Indirector::Code
>+        end
>+      }.to raise_error(ArgumentError, /Could not find indirection instance/)
>     end
>-  end
>-end
> 
>-# LAK: This could reasonably be in the Indirection instances, too.  It doesn't make
>-# a whole heckuva lot of difference, except that with the instance loading in
>-# the Terminus base class, we have to have a check to see if we're already
>-# instance-loading a given terminus class type.
>-describe Puppet::Indirector::Terminus, " when managing terminus classes" do
>-  it "should provide a method for registering terminus classes" do
>-    Puppet::Indirector::Terminus.should respond_to(:register_terminus_class)
>-  end
>+    it "should register the terminus class with the terminus base class" do
>+      Puppet::Indirector::Terminus.expects(:register_terminus_class).with do |type|
>+        type.indirection_name == :abstract_concept and type.name == :intellect
>+      end
> 
>-  it "should provide a method for returning terminus classes by name and type" do
>-    terminus = stub 'terminus_type', :name => :abstract, :indirection_name => :whatever
>-    Puppet::Indirector::Terminus.register_terminus_class(terminus)
>-    Puppet::Indirector::Terminus.terminus_class(:whatever, :abstract).should equal(terminus)
>+      begin
>+        class Puppet::AbstractConcept::Intellect < Puppet::Indirector::Code
>+        end
>+      ensure
>+        Puppet::AbstractConcept.send(:remove_const, :Intellect) rescue nil
>+      end
>+    end
>   end
> 
>-  it "should set up autoloading for any terminus class types requested" do
>-    Puppet::Indirector::Terminus.expects(:instance_load).with(:test2, "puppet/indirector/test2")
>-    Puppet::Indirector::Terminus.terminus_class(:test2, :whatever)
>-  end
>+  describe "when parsing class constants for indirection and terminus names" do
>+    before :each do
>+      Puppet::Indirector::Terminus.stubs(:register_terminus_class)
>+    end
> 
>-  it "should load terminus classes that are not found" do
>-    # Set up instance loading; it would normally happen automatically
>-    Puppet::Indirector::Terminus.instance_load :test1, "puppet/indirector/test1"
>+    let :subclass do
>+      subclass = mock 'subclass'
>+      subclass.stubs(:to_s).returns("TestInd::OneTwo")
>+      subclass.stubs(:mark_as_abstract_terminus)
>+      subclass
>+    end
> 
>-    Puppet::Indirector::Terminus.instance_loader(:test1).expects(:load).with(:yay)
>-    Puppet::Indirector::Terminus.terminus_class(:test1, :yay)
>-  end
>+    it "should fail when anonymous classes are used" do
>+      expect {
>+        Puppet::Indirector::Terminus.inherited(Class.new)
>+      }.to raise_error(Puppet::DevError, /Terminus subclasses must have associated constants/)
>+    end
> 
>-  it "should fail when no indirection can be found", :'fails_on_ruby_1.9.2' => true do
>-    Puppet::Indirector::Indirection.expects(:instance).with(:my_indirection).returns(nil)
>+    it "should use the last term in the constant for the terminus class name" do
>+      subclass.expects(:name=).with(:one_two)
>+      subclass.stubs(:indirection=)
>+      Puppet::Indirector::Terminus.inherited(subclass)
>+    end
> 
>-    @abstract_terminus = Class.new(Puppet::Indirector::Terminus) do
>-      def self.to_s
>-        "Abstract"
>-      end
>+    it "should convert the terminus name to a downcased symbol" do
>+      subclass.expects(:name=).with(:one_two)
>+      subclass.stubs(:indirection=)
>+      Puppet::Indirector::Terminus.inherited(subclass)
>     end
>-    proc {
>-      @terminus = Class.new(@abstract_terminus) do
>-        def self.to_s
>-          "MyIndirection::TestType"
>-        end
>-      end
>-    }.should raise_error(ArgumentError)
>-  end
> 
>-  it "should register the terminus class with the terminus base class", :'fails_on_ruby_1.9.2' => true do
>-    Puppet::Indirector::Terminus.expects(:register_terminus_class).with do |type|
>-      type.indirection_name == :my_indirection and type.name == :test_terminus
>+    it "should use the second to last term in the constant for the indirection name" do
>+      subclass.expects(:indirection=).with(:test_ind)
>+      subclass.stubs(:name=)
>+      subclass.stubs(:terminus_type=)
>+      Puppet::Indirector::Memory.inherited(subclass)
>     end
>-    @indirection = stub 'indirection', :name => :my_indirection, :register_terminus_type => nil
>-    Puppet::Indirector::Indirection.expects(:instance).with(:my_indirection).returns(@indirection)
> 
>-    @abstract_terminus = Class.new(Puppet::Indirector::Terminus) do
>-      def self.to_s
>-        "Abstract"
>-      end
>+    it "should convert the indirection name to a downcased symbol" do
>+      subclass.expects(:indirection=).with(:test_ind)
>+      subclass.stubs(:name=)
>+      subclass.stubs(:terminus_type=)
>+      Puppet::Indirector::Memory.inherited(subclass)
>     end
> 
>-    @terminus = Class.new(@abstract_terminus) do
>-      def self.to_s
>-        "MyIndirection::TestTerminus"
>-      end
>+    it "should convert camel case to lower case with underscores as word separators" do
>+      subclass.expects(:name=).with(:one_two)
>+      subclass.stubs(:indirection=)
>+
>+      Puppet::Indirector::Terminus.inherited(subclass)
>     end
>   end
>-end
> 
>-describe Puppet::Indirector::Terminus, " when parsing class constants for indirection and terminus names" do
>-  before do
>-    @subclass = mock 'subclass'
>-    @subclass.stubs(:to_s).returns("TestInd::OneTwo")
>-    @subclass.stubs(:mark_as_abstract_terminus)
>-    Puppet::Indirector::Terminus.stubs(:register_terminus_class)
>-  end
>+  describe "when creating terminus class types" do
>+    before :all do
>+      Puppet::Indirector::Terminus.stubs(:register_terminus_class)
> 
>-  it "should fail when anonymous classes are used" do
>-    proc { Puppet::Indirector::Terminus.inherited(Class.new) }.should raise_error(Puppet::DevError)
>-  end
>+      class Puppet::Indirector::Terminus::TestTerminusType < Puppet::Indirector::Terminus
>+      end
>+    end
> 
>-  it "should use the last term in the constant for the terminus class name" do
>-    @subclass.expects(:name=).with(:one_two)
>-    @subclass.stubs(:indirection=)
>-    Puppet::Indirector::Terminus.inherited(@subclass)
>-  end
>+    after :all do
>+      Puppet::Indirector::Terminus.send(:remove_const, :TestTerminusType)
>+    end
> 
>-  it "should convert the terminus name to a downcased symbol" do
>-    @subclass.expects(:name=).with(:one_two)
>-    @subclass.stubs(:indirection=)
>-    Puppet::Indirector::Terminus.inherited(@subclass)
>-  end
>+    let :subclass do
>+      Puppet::Indirector::Terminus::TestTerminusType
>+    end
>+
>+    it "should set the name of the abstract subclass to be its class constant" do
>+      subclass.name.should == :test_terminus_type
>+    end
>+
>+    it "should mark abstract terminus types as such" do
>+      subclass.should be_abstract_terminus
>+    end
> 
>-  it "should use the second to last term in the constant for the indirection name" do
>-    @subclass.expects(:indirection=).with(:test_ind)
>-    @subclass.stubs(:name=)
>-    @subclass.stubs(:terminus_type=)
>-    Puppet::Indirector::Memory.inherited(@subclass)
>+    it "should not allow instances of abstract subclasses to be created" do
>+      expect { subclass.new }.to raise_error(Puppet::DevError)
>+    end
>   end
> 
>-  it "should convert the indirection name to a downcased symbol" do
>-    @subclass.expects(:indirection=).with(:test_ind)
>-    @subclass.stubs(:name=)
>-    @subclass.stubs(:terminus_type=)
>-    Puppet::Indirector::Memory.inherited(@subclass)
>+  describe "when listing terminus classes" do
>+    it "should list the terminus files available to load" do
>+      Puppet::Util::Autoload.any_instance.stubs(:files_to_load).returns ["/foo/bar/baz", "/max/runs/marathon"]
>+      Puppet::Indirector::Terminus.terminus_classes('my_stuff').should == [:baz, :marathon]
>+    end
>   end
> 
>-  it "should convert camel case to lower case with underscores as word separators" do
>-    @subclass.expects(:name=).with(:one_two)
>-    @subclass.stubs(:indirection=)
>+  describe "when validating a request" do
>+    let :request do
>+      Puppet::Indirector::Request.new(indirection.name, :find, "the_key", instance)
>+    end
> 
>-    Puppet::Indirector::Terminus.inherited(@subclass)
>-  end
>-end
>+    describe "`instance.name` does not match the key in the request" do
>+      let(:instance) { model.new("wrong_key") }
> 
>-describe Puppet::Indirector::Terminus, " when creating terminus class types", :'fails_on_ruby_1.9.2' => true do
>-  before do
>-    Puppet::Indirector::Terminus.stubs(:register_terminus_class)
>-    @subclass = Class.new(Puppet::Indirector::Terminus) do
>-      def self.to_s
>-        "Puppet::Indirector::Terminus::MyTermType"
>+      it "raises an error " do
>+        expect {
>+          terminus.validate(request)
>+        }.to raise_error(
>+          Puppet::Indirector::ValidationError,
>+          /Instance name .* does not match requested key/
>+        )
>       end
>     end
>-  end
> 
>-  it "should set the name of the abstract subclass to be its class constant" do
>-    @subclass.name.should equal(:my_term_type)
>-  end
>+    describe "`instance` is not an instance of the model class" do
>+      let(:instance) { mock "instance", :name => "the_key" }
> 
>-  it "should mark abstract terminus types as such" do
>-    @subclass.should be_abstract_terminus
>-  end
>+      it "raises an error" do
>+        expect {
>+          terminus.validate(request)
>+        }.to raise_error(
>+          Puppet::Indirector::ValidationError,
>+          /Invalid instance type/
>+        )
>+      end
>+    end
> 
>-  it "should not allow instances of abstract subclasses to be created" do
>-    proc { @subclass.new }.should raise_error(Puppet::DevError)
>-  end
>-end
>+    describe "the instance key and class match the request key and model class" do
>+      let(:instance) { model.new("the_key") }
> 
>-describe Puppet::Indirector::Terminus, " when listing terminus classes" do
>-  it "should list the terminus files available to load" do
>-    Puppet::Util::Autoload.any_instance.stubs(:files_to_load).returns ["/foo/bar/baz", "/max/runs/marathon"]
>-    Puppet::Indirector::Terminus.terminus_classes('my_stuff').should == [:baz, :marathon]
>+      it "passes" do
>+        terminus.validate(request)
>+      end
>+    end
>   end
> end
>diff --git a/spec/unit/network/formats_spec.rb b/spec/unit/network/formats_spec.rb
>index 62c2dbb..be076b0 100755
>--- a/spec/unit/network/formats_spec.rb
>+++ b/spec/unit/network/formats_spec.rb
>@@ -55,15 +55,15 @@ describe "Puppet Network Format" do
>       @yaml.render_multiple(instances).should == "foo"
>     end
> 
>-    it "should intern by calling 'YAML.load'" do
>+    it "should safely load YAML when interning" do
>       text = "foo"
>-      YAML.expects(:load).with("foo").returns "bar"
>+      YAML.expects(:safely_load).with("foo").returns "bar"
>       @yaml.intern(String, text).should == "bar"
>     end
> 
>-    it "should intern multiples by calling 'YAML.load'" do
>+    it "should safely load YAML when interning multiples" do
>       text = "foo"
>-      YAML.expects(:load).with("foo").returns "bar"
>+      YAML.expects(:safely_load).with("foo").returns "bar"
>       @yaml.intern_multiple(String, text).should == "bar"
>     end
>   end
>@@ -120,10 +120,10 @@ describe "Puppet Network Format" do
>       @yaml.intern_multiple(String, text).should == "bar"
>     end
> 
>-    it "should decode by base64 decoding, uncompressing and Yaml loading" do
>+    it "should decode by base64 decoding, uncompressing and safely Yaml loading" do
>       Base64.expects(:decode64).with("zorg").returns "foo"
>       Zlib::Inflate.expects(:inflate).with("foo").returns "baz"
>-      YAML.expects(:load).with("baz").returns "bar"
>+      YAML.expects(:safely_load).with("baz").returns "bar"
>       @yaml.decode("zorg").should == "bar"
>     end
> 
>diff --git a/spec/unit/network/http/handler_spec.rb b/spec/unit/network/http/handler_spec.rb
>index c709d82..7d9b5ae 100755
>--- a/spec/unit/network/http/handler_spec.rb
>+++ b/spec/unit/network/http/handler_spec.rb
>@@ -125,6 +125,31 @@ describe Puppet::Network::HTTP::Handler do
>       @handler.request_format(@request).should == "s"
>     end
> 
>+    it "should deserialize YAML parameters" do
>+      params = {'my_param' => [1,2,3].to_yaml}
>+
>+      decoded_params = @handler.send(:decode_params, params)
>+
>+      decoded_params.should == {:my_param => [1,2,3]}
>+    end
>+
>+    it "should accept YAML parameters with !ruby/hash tags on Ruby 1.8", :if => RUBY_VERSION =~ /^1\.8/ do
>+      params = {'my_param' => "--- !ruby/hash:Array {}"}
>+
>+      decoded_params = @handler.send(:decode_params, params)
>+
>+      decoded_params[:my_param].should be_an(Array)
>+    end
>+
>+    # These are only dangerous with Psych, which is Ruby 1.9-only. Since
>+    # there's no real way to change the yamler in Puppet, assume that 1.9 means
>+    # Psych, especially in tests.
>+    it "should fail if YAML parameters have !ruby/hash tags on Ruby 1.9", :unless => RUBY_VERSION =~ /^1\.8/ do
>+      params = {'my_param' => "--- !ruby/hash:Array {}"}
>+
>+      expect { @handler.send(:decode_params, params) }.to raise_error(ArgumentError, /Illegal YAML mapping found/)
>+    end
>+
>     describe "when finding a model instance" do
>       before do
>         @indirection.stubs(:find).returns @result
>diff --git a/spec/unit/network/http/rack/rest_spec.rb b/spec/unit/network/http/rack/rest_spec.rb
>index 8a5666f..d2e889a 100755
>--- a/spec/unit/network/http/rack/rest_spec.rb
>+++ b/spec/unit/network/http/rack/rest_spec.rb
>@@ -91,6 +91,23 @@ describe "Puppet::Network::HTTP::RackREST", :if => Puppet.features.rack? do
>           @handler.set_response(@response, @file, 200)
>         end
>       end
>+
>+      it "should ensure the body has been read on success" do
>+        req = mk_req('/production/report/foo', :method => 'PUT')
>+        req.body.expects(:read).at_least_once
>+
>+        Puppet::Transaction::Report.stubs(:save)
>+
>+        @handler.process(req, @response)
>+      end
>+
>+      it "should ensure the body has been partially read on failure" do
>+        req = mk_req('/production/report/foo')
>+        req.body.expects(:read).with(1)
>+        req.stubs(:check_authorization).raises(Exception)
>+
>+        @handler.process(req, @response)
>+      end
>     end
> 
>     describe "and determining the request parameters" do
>diff --git a/spec/unit/network/http/webrick_spec.rb b/spec/unit/network/http/webrick_spec.rb
>index f84e78e..4a50bbc 100755
>--- a/spec/unit/network/http/webrick_spec.rb
>+++ b/spec/unit/network/http/webrick_spec.rb
>@@ -316,6 +316,10 @@ describe Puppet::Network::HTTP::WEBrick, :unless => Puppet.features.microsoft_wi
>       @server.setup_ssl[:SSLEnable].should be_true
>     end
> 
>+    it "should reject SSLv2" do
>+      @server.setup_ssl[:SSLOptions].should == OpenSSL::SSL::OP_NO_SSLv2
>+    end
>+
>     it "should configure the verification method as 'OpenSSL::SSL::VERIFY_PEER'" do
>       @server.setup_ssl[:SSLVerifyClient].should == OpenSSL::SSL::VERIFY_PEER
>     end
>diff --git a/spec/unit/network/http_pool_spec.rb b/spec/unit/network/http_pool_spec.rb
>index c86f7a6..0e5d29d 100755
>--- a/spec/unit/network/http_pool_spec.rb
>+++ b/spec/unit/network/http_pool_spec.rb
>@@ -14,122 +14,119 @@ describe Puppet::Network::HttpPool do
>   end
> 
>   describe "when managing http instances" do
>-    def stub_settings(settings)
>-      settings.each do |param, value|
>-        Puppet.settings.stubs(:value).with(param).returns(value)
>-      end
>-    end
>-
>-    before do
>+    before :each do
>       # All of the cert stuff is tested elsewhere
>       Puppet::Network::HttpPool.stubs(:cert_setup)
>     end
> 
>     it "should return an http instance created with the passed host and port" do
>-      http = stub 'http', :use_ssl= => nil, :read_timeout= => nil, :open_timeout= => nil, :started? => false
>-      Net::HTTP.expects(:new).with("me", 54321, nil, nil).returns(http)
>-      Puppet::Network::HttpPool.http_instance("me", 54321).should equal(http)
>+      http = Puppet::Network::HttpPool.http_instance("me", 54321)
>+      http.should be_an_instance_of Net::HTTP
>+      http.address.should == 'me'
>+      http.port.should    == 54321
>     end
> 
>     it "should enable ssl on the http instance" do
>-      Puppet::Network::HttpPool.http_instance("me", 54321).instance_variable_get("@use_ssl").should be_true
>-    end
>-
>-    it "should set the read timeout" do
>-      Puppet::Network::HttpPool.http_instance("me", 54321).read_timeout.should == 120
>+      Puppet::Network::HttpPool.http_instance("me", 54321).should be_use_ssl
>     end
> 
>-    it "should set the open timeout" do
>-      Puppet::Network::HttpPool.http_instance("me", 54321).open_timeout.should == 120
>+    context "proxy and timeout settings should propagate" do
>+      subject { Puppet::Network::HttpPool.http_instance("me", 54321) }
>     end
> 
>-    it "should create the http instance with the proxy host and port set if the http_proxy is not set to 'none'" do
>-      stub_settings :http_proxy_host => "myhost", :http_proxy_port => 432, :configtimeout => 120
>-      Puppet::Network::HttpPool.http_instance("me", 54321).open_timeout.should == 120
>+    it "should not set a proxy if the value is 'none'" do
>+      Puppet[:http_proxy_host] = 'none'
>+      Puppet::Network::HttpPool.http_instance("me", 54321).proxy_address.should be_nil
>     end
> 
>     it "should not cache http instances" do
>-      stub_settings :http_proxy_host => "myhost", :http_proxy_port => 432, :configtimeout => 120
>+      Puppet[:http_proxy_host] = "myhost"
>+      Puppet[:http_proxy_port] = 432
>+      Puppet[:configtimeout]   = 120
>       old = Puppet::Network::HttpPool.http_instance("me", 54321)
>       Puppet::Network::HttpPool.http_instance("me", 54321).should_not equal(old)
>     end
>   end
> 
>-  describe "when adding certificate information to http instances" do
>-    before do
>-      @http = mock 'http'
>-      [:cert_store=, :verify_mode=, :ca_file=, :cert=, :key=].each { |m| @http.stubs(m) }
>-      @store = stub 'store'
>-
>-      @cert = stub 'cert', :content => "real_cert"
>-      @key = stub 'key', :content => "real_key"
>-      @host = stub 'host', :certificate => @cert, :key => @key, :ssl_store => @store
>-
>-      Puppet[:confdir] = "/sometthing/else"
>-      Puppet.settings.stubs(:value).returns "/some/file"
>-      Puppet.settings.stubs(:value).with(:hostcert).returns "/host/cert"
>-      Puppet.settings.stubs(:value).with(:localcacert).returns "/local/ca/cert"
>-
>-      FileTest.stubs(:exist?).with("/host/cert").returns true
>-      FileTest.stubs(:exist?).with("/local/ca/cert").returns true
>-
>-      Puppet::Network::HttpPool.stubs(:ssl_host).returns @host
>+  describe "when doing SSL setup for http instances" do
>+    let :http do
>+      http = Net::HTTP.new('localhost', 443)
>+      http.use_ssl = true
>+      http
>     end
> 
>-    after do
>-      Puppet.settings.clear
>-    end
>-
>-    it "should do nothing if no host certificate is on disk" do
>-      FileTest.expects(:exist?).with("/host/cert").returns false
>-      @http.expects(:cert=).never
>-      Puppet::Network::HttpPool.cert_setup(@http)
>-    end
>+    let :store do stub('store') end
> 
>-    it "should do nothing if no local certificate is on disk" do
>-      FileTest.expects(:exist?).with("/local/ca/cert").returns false
>-      @http.expects(:cert=).never
>-      Puppet::Network::HttpPool.cert_setup(@http)
>+    before :each do
>+      Puppet[:hostcert]    = '/host/cert'
>+      Puppet[:localcacert] = '/local/ca/cert'
>+      cert  = stub 'cert', :content => 'real_cert'
>+      key   = stub 'key',  :content => 'real_key'
>+      host  = stub 'host', :certificate => cert, :key => key, :ssl_store => store
>+      Puppet::Network::HttpPool.stubs(:ssl_host).returns(host)
>     end
> 
>-    it "should add a certificate store from the ssl host" do
>-      @http.expects(:cert_store=).with(@store)
>+    shared_examples "HTTPS setup without all certificates" do
>+      subject { Puppet::Network::HttpPool.cert_setup(http); http }
> 
>-      Puppet::Network::HttpPool.cert_setup(@http)
>+      it                { should be_use_ssl }
>+      its(:cert)        { should be_nil }
>+      its(:ca_file)     { should be_nil }
>+      its(:key)         { should be_nil }
>+      its(:verify_mode) { should == OpenSSL::SSL::VERIFY_NONE }
>     end
> 
>-    it "should add the client certificate" do
>-      @http.expects(:cert=).with("real_cert")
>+    context "with neither a host cert or a local CA cert" do
>+      before :each do
>+        FileTest.stubs(:exist?).with(Puppet[:hostcert]).returns(false)
>+        FileTest.stubs(:exist?).with(Puppet[:localcacert]).returns(false)
>+      end
> 
>-      Puppet::Network::HttpPool.cert_setup(@http)
>+      include_examples "HTTPS setup without all certificates"
>     end
> 
>-    it "should add the client key" do
>-      @http.expects(:key=).with("real_key")
>+    context "with there is no host certificate" do
>+      before :each do
>+        FileTest.stubs(:exist?).with(Puppet[:hostcert]).returns(false)
>+        FileTest.stubs(:exist?).with(Puppet[:localcacert]).returns(true)
>+      end
> 
>-      Puppet::Network::HttpPool.cert_setup(@http)
>+      include_examples "HTTPS setup without all certificates"
>     end
> 
>-    it "should set the verify mode to OpenSSL::SSL::VERIFY_PEER" do
>-      @http.expects(:verify_mode=).with(OpenSSL::SSL::VERIFY_PEER)
>+    context "with there is no local CA certificate" do
>+      before :each do
>+        FileTest.stubs(:exist?).with(Puppet[:hostcert]).returns(true)
>+        FileTest.stubs(:exist?).with(Puppet[:localcacert]).returns(false)
>+      end
> 
>-      Puppet::Network::HttpPool.cert_setup(@http)
>+      include_examples "HTTPS setup without all certificates"
>     end
> 
>-    it "should set the ca file" do
>-      Puppet.settings.stubs(:value).returns "/some/file"
>-      FileTest.stubs(:exist?).with(Puppet[:hostcert]).returns true
>+    context "with both the host and CA cert" do
>+      subject { Puppet::Network::HttpPool.cert_setup(http); http }
> 
>-      Puppet.settings.stubs(:value).with(:localcacert).returns "/ca/cert/file"
>-      FileTest.stubs(:exist?).with("/ca/cert/file").returns true
>-      @http.expects(:ca_file=).with("/ca/cert/file")
>+      before :each do
>+        FileTest.expects(:exist?).with(Puppet[:hostcert]).returns(true)
>+        FileTest.expects(:exist?).with(Puppet[:localcacert]).returns(true)
>+      end
> 
>-      Puppet::Network::HttpPool.cert_setup(@http)
>+      it                { should be_use_ssl }
>+      its(:cert_store)  { should equal store }
>+      its(:cert)        { should == "real_cert" }
>+      its(:key)         { should == "real_key" }
>+      its(:verify_mode) { should == OpenSSL::SSL::VERIFY_PEER }
>+      its(:ca_file)     { should == Puppet[:localcacert] }
>     end
> 
>     it "should set up certificate information when creating http instances" do
>-      Puppet::Network::HttpPool.expects(:cert_setup).with { |i| i.is_a?(Net::HTTP) }
>-      Puppet::Network::HttpPool.http_instance("one", "two")
>+      Puppet::Network::HttpPool.expects(:cert_setup).with do |http|
>+        http.should be_an_instance_of Net::HTTP
>+        http.address.should == "one"
>+        http.port.should == 2
>+      end
>+
>+      Puppet::Network::HttpPool.http_instance("one", 2)
>     end
>   end
> end
>diff --git a/spec/unit/network/rest_authconfig_spec.rb b/spec/unit/network/rest_authconfig_spec.rb
>index bebbb87..8d4a2d4 100755
>--- a/spec/unit/network/rest_authconfig_spec.rb
>+++ b/spec/unit/network/rest_authconfig_spec.rb
>@@ -85,6 +85,9 @@ describe Puppet::Network::RestAuthConfig do
>   end
> 
>   it "should create default ACL entries if no file have been read" do
>+    # The singleton instance is stored as an instance variable we don't have
>+    # access to, so.. instance_variable_set. Alas.
>+    Puppet::Network::RestAuthConfig.instance_variable_set(:@main, nil)
>     Puppet::Network::RestAuthConfig.any_instance.stubs(:exists?).returns(false)
> 
>     Puppet::Network::RestAuthConfig.any_instance.expects(:insert_default_acl)
>@@ -122,6 +125,18 @@ describe Puppet::Network::RestAuthConfig do
>       @authconfig.insert_default_acl
>     end
> 
>-  end
>+    it '(CVE-2013-2275) allows report submission only for the node matching the certname by default' do
>+      acl = {
>+        :acl => "~ ^\/report\/([^\/]+)$",
>+        :method => :save,
>+        :allow => '$1',
>+        :authenticated => true
>+      }
>+      @authconfig.rights.stubs(:[]).returns(true)
>+      @authconfig.rights.stubs(:[]).with(acl[:acl]).returns(nil)
> 
>+      @authconfig.expects(:mk_acl).with(acl)
>+      @authconfig.insert_default_acl
>+    end
>+  end
> end
>diff --git a/spec/unit/parser/functions/inline_template_spec.rb b/spec/unit/parser/functions/inline_template_spec.rb
>index 47dcae1..b761e32 100755
>--- a/spec/unit/parser/functions/inline_template_spec.rb
>+++ b/spec/unit/parser/functions/inline_template_spec.rb
>@@ -58,4 +58,17 @@ describe "the inline_template function", :'fails_on_ruby_1.9.2' => true do
>     lambda { @scope.function_inline_template("1") }.should raise_error(Puppet::ParseError)
>   end
> 
>+  it "is not interfered with by a variable called 'string' (#14093)" do
>+    @scope.setvar("string", "this is a variable")
>+    inline_template("this is a template").should == "this is a template"
>+  end
>+
>+  it "has access to a variable called 'string' (#14093)" do
>+    @scope.setvar('string', "this is a variable")
>+    inline_template("string was: <%= @string %>").should == "string was: this is a variable"
>+  end
>+
>+  def inline_template(*templates)
>+    @scope.function_inline_template(templates)
>+  end
> end
>diff --git a/spec/unit/parser/functions/template_spec.rb b/spec/unit/parser/functions/template_spec.rb
>index 6bce695..b3de21a 100755
>--- a/spec/unit/parser/functions/template_spec.rb
>+++ b/spec/unit/parser/functions/template_spec.rb
>@@ -53,6 +53,22 @@ describe "the template function", :'fails_on_ruby_1.9.2' => true do
>     @scope.function_template(["1","2"]).should == "result1result2"
>   end
> 
>+  it "is not interfered with by having a variable named 'string' (#14093)" do
>+    @scope.setvar('string', "this output should not be seen")
>+    @scope.stubs(:compiler => stub(:environment => 'production'))
>+    @scope.stubs(:known_resource_types => stub(:watch_file))
>+
>+    eval_template("some text that is static").should == "some text that is static"
>+  end
>+
>+  it "has access to a variable named 'string' (#14093)" do
>+    @scope.setvar('string', "the string value")
>+    @scope.stubs(:compiler => stub(:environment => 'production'))
>+    @scope.stubs(:known_resource_types => stub(:watch_file))
>+
>+    eval_template("string was: <%= @string %>").should == "string was: the string value"
>+  end
>+
>   it "should raise an error if the template raises an error" do
>     tw = stub_everything 'template_wrapper'
>     Puppet::Parser::TemplateWrapper.stubs(:new).returns(tw)
>@@ -61,4 +77,9 @@ describe "the template function", :'fails_on_ruby_1.9.2' => true do
>     lambda { @scope.function_template("1") }.should raise_error(Puppet::ParseError)
>   end
> 
>+  def eval_template(content)
>+    File.stubs(:read).with("template").returns(content)
>+    Puppet::Parser::Files.stubs(:find_template).returns("template")
>+    @scope.function_template(['template'])
>+  end
> end
>diff --git a/spec/unit/parser/templatewrapper_spec.rb b/spec/unit/parser/templatewrapper_spec.rb
>index 600293b..c2b9086 100755
>--- a/spec/unit/parser/templatewrapper_spec.rb
>+++ b/spec/unit/parser/templatewrapper_spec.rb
>@@ -30,16 +30,14 @@ describe Puppet::Parser::TemplateWrapper do
> 
>   it "should check template file existance and read its content" do
>     Puppet::Parser::Files.expects(:find_template).with("fake_template", @scope.environment.to_s).returns("/tmp/fake_template")
>-    File.expects(:read).with("/tmp/fake_template").returns("template content")
> 
>     @tw.file = @file
>   end
> 
>   it "should mark the file for watching" do
>-    Puppet::Parser::Files.expects(:find_template).returns("/tmp/fake_template")
>-    File.stubs(:read)
>+    full_file_name = given_a_template_file("fake_template", "content")
> 
>-    @known_resource_types.expects(:watch_file).with("/tmp/fake_template")
>+    @known_resource_types.expects(:watch_file).with(full_file_name)
>     @tw.file = @file
>   end
> 
>@@ -66,6 +64,13 @@ describe Puppet::Parser::TemplateWrapper do
>     @tw.result.should eql("woot!")
>   end
> 
>+  it "provides access to the name of the template via #file" do
>+    full_file_name = given_a_template_file("fake_template", "<%= file %>")
>+
>+    @tw.file = "fake_template"
>+    @tw.result.should == full_file_name
>+  end
>+
>   it "should return the processed template contents with a call to result and a string" do
>     mock_template
>     @tw.result("template contents").should eql("woot!")
>@@ -139,4 +144,14 @@ describe Puppet::Parser::TemplateWrapper do
>       @tw.instance_variable_get("@one_").should == "foo"
>     end
>   end
>+
>+  def given_a_template_file(name, contents)
>+    full_name = "/full/path/to/#{name}"
>+    Puppet::Parser::Files.stubs(:find_template).
>+      with(name, anything()).
>+      returns(full_name)
>+    File.stubs(:read).with(full_name).returns(contents)
>+
>+    full_name
>+  end
> end
>diff --git a/spec/unit/ssl/certificate_request_spec.rb b/spec/unit/ssl/certificate_request_spec.rb
>index 9a27397..e697d82 100755
>--- a/spec/unit/ssl/certificate_request_spec.rb
>+++ b/spec/unit/ssl/certificate_request_spec.rb
>@@ -254,6 +254,7 @@ describe Puppet::SSL::CertificateRequest do
> 
>         csr = Puppet::SSL::CertificateRequest.new("me")
>         terminus = mock 'terminus'
>+        terminus.stubs(:validate)
>         Puppet::SSL::CertificateRequest.indirection.expects(:prepare).returns(terminus)
>         terminus.expects(:save).with { |request| request.instance == csr && request.key == "me" }
> 
>@@ -267,6 +268,7 @@ describe Puppet::SSL::CertificateRequest do
> 
>         csr = Puppet::SSL::CertificateRequest.new("me")
>         terminus = mock 'terminus'
>+        terminus.stubs(:validate)
>         Puppet::SSL::CertificateRequest.indirection.expects(:prepare).returns(terminus)
>         terminus.expects(:save).with { |request| request.instance == csr && request.key == "me" }
> 
>diff --git a/spec/unit/ssl/host_spec.rb b/spec/unit/ssl/host_spec.rb
>index 3f94407..4ed20b7 100755
>--- a/spec/unit/ssl/host_spec.rb
>+++ b/spec/unit/ssl/host_spec.rb
>@@ -489,6 +489,7 @@ describe Puppet::SSL::Host do
>       @request.stubs(:generate)
>       @request.stubs(:name).returns("myname")
>       terminus = stub 'terminus'
>+      terminus.stubs(:validate)
>       Puppet::SSL::CertificateRequest.indirection.expects(:prepare).returns(terminus)
>       terminus.expects(:save).with { |req| req.instance == @request && req.key == "myname" }.raises "eh"
> 
>diff --git a/spec/unit/util/monkey_patches_spec.rb b/spec/unit/util/monkey_patches_spec.rb
>index 4b609ad..fe0b5fe 100755
>--- a/spec/unit/util/monkey_patches_spec.rb
>+++ b/spec/unit/util/monkey_patches_spec.rb
>@@ -53,3 +53,15 @@ describe "Array#combination" do
>     [1,2,3,4].combination(3).to_a.should == [[1, 2, 3], [1, 2, 4], [1, 3, 4], [2, 3, 4]]
>   end
> end
>+
>+describe OpenSSL::SSL::SSLContext do
>+  it 'disables SSLv2 via the SSLContext#options bitmask' do
>+    (subject.options & OpenSSL::SSL::OP_NO_SSLv2).should == OpenSSL::SSL::OP_NO_SSLv2
>+  end
>+  it 'has no ciphers with version SSLv2 enabled' do
>+    ciphers = subject.ciphers.select do |name, version, bits, alg_bits|
>+      /SSLv2/.match(version)
>+    end
>+    ciphers.should be_empty
>+  end
>+end
>-- 
>1.8.1.1
>

Actions: View | Diff

Attachments on bug 919783: 710424 | 710425 | 710426 | 710427 | 710428