Commit fdd4ceff authored by Rob Browning's avatar Rob Browning

(PDB-3322) Redact sensitive parameters in terminus

A resource parameter marked Sensitive()

  notify {'hi':  message => Sensitive('there')}

will show up in the terminus
like this:

        input = {...
                 'environment'=>'production',
                 'resources'=>
                 [...
                  {'type'=>'Notify',
                   'title'=>'hi',
                   'tags'=>Puppet::Util::TagSet.new(['notify', 'hi', 'class']),
                   'file'=> 'site.pp',
                   'line'=>1,
                   'exported'=>false,
                   'parameters'=>{:message=>'there'},
                   'sensitive_parameters'=>[:message]}],
                 'edges'=>
                 [{'source'=>'Stage[main]', 'target'=>'Class[Settings]'},
                  {'source'=>'Stage[main]', 'target'=>'Class[main]'},
                  {'source'=>'Class[main]', 'target'=>'Notify[hi]'}],
                 'classes'=>['settings']}

Remove any sensitive values from 'parameters', and remove
'sensitive_parameters' before sending data to PuppetDB.

Aside from the fact that we don't want senstitive parameters to be
stored in (or even make it to) PuppetDB, the 'sensitive_parameters'
element can cause command processing to fail, e.g. when an existing
parameter becomes sensitive.  Add an integration test for that.
parent 3e1fd1bc
......@@ -32,6 +32,21 @@ class Puppet::Resource::Catalog::Puppetdb < Puppet::Indirector::REST
hash
end
def redact_sensitive_params(hash)
resources = hash['resources']
resources.each do |resource|
sensitive_params = resource['sensitive_parameters']
unless sensitive_params.nil?
parameters = resource['parameters']
sensitive_params.each do |sensitive_param|
parameters.delete sensitive_param
end
resource.delete 'sensitive_parameters'
end
end
nil
end
def munge_catalog(catalog, producer_timestamp, extra_request_data = {})
profile("Munge catalog", [:puppetdb, :catalog, :munge]) do
data = profile("Convert catalog to JSON data hash", [:puppetdb, :catalog, :convert_to_hash]) do
......@@ -54,6 +69,7 @@ class Puppet::Resource::Catalog::Puppetdb < Puppet::Indirector::REST
add_environment(data, extra_request_data[:environment])
add_producer_timestamp(data, producer_timestamp)
add_producer(data, Puppet[:node_name_value])
redact_sensitive_params(data)
data
end
......
......@@ -702,5 +702,55 @@ describe Puppet::Resource::Catalog::Puppetdb do
"catalog_uuid", "producer"]
end
end
describe "#redact_sensitive_params" do
let(:secret) {'xyzzy XKPGMKIB253ZVJHKOKZZXPQSSE'}
let(:has_secret?) {
lambda { |r| r['parameters'] && r['parameters'][:message] == secret}
}
let(:input) {{'tags' => Puppet::Util::TagSet.new(['settings']),
'name' => 'my_agent',
'version' => 1490991352,
'code_id' => nil,
'catalog_uuid' => 'aa4759d3-f1f1-47a0-925c-a4acd0c1b4ed',
'catalog_format' => 1,
'environment' => 'production',
'resources' =>
[{'type' => 'Stage',
'title' => :main,
'tags' => Puppet::Util::TagSet.new(['stage']),
'exported' => false,
'parameters' => {:name => 'main'}},
{'type' => 'Class',
'title' => 'Settings',
'tags' => Puppet::Util::TagSet.new(['class', 'settings']),
'exported' => false},
{'type' => 'Class',
'title' => :main,
'tags' => Puppet::Util::TagSet.new(['class']),
'exported' => false,
'parameters' => {:name => 'main'}},
{'type' => 'Notify',
'title' => 'hi',
'tags' => Puppet::Util::TagSet.new(['notify', 'hi', 'class']),
'file' => 'site.pp',
'line' => 1,
'exported' => false,
'parameters' => {:message => secret},
'sensitive_parameters' => [:message]}],
'edges' =>
[{'source' => 'Stage[main]', 'target' => 'Class[Settings]'},
{'source' => 'Stage[main]', 'target' => 'Class[main]'},
{'source' => 'Class[main]', 'target' => 'Notify[hi]'}],
'classes' => ['settings']}}
it 'should remove any sensitive resource parameters' do
resources = input['resources']
resources.any? {|r| r['sensitive_parameters']}.should be true
resources.any? {|r| has_secret?.call(r) }.should be true
subject.redact_sensitive_params(input)
resources.any? {|r| r['sensitive_parameters']}.should_not be true
resources.any? {|r| has_secret?.call(r) }.should_not be true
end
end
end
end
(ns puppetlabs.puppetdb.integration.sensitive-params
(:require
[clojure.java.shell :refer [sh]]
[clojure.string :as str]
[clojure.test :refer :all]
[puppetlabs.puppetdb.integration.fixtures :as int])
(:import
[java.net URI]))
(defn pg-dump
"Returns the content of pg as a string via pg_dump."
[pg]
(let [db-config (:db-config pg)
db-uri (URI. (:subname db-config))
result (sh "pg_dump"
"--username" (:user db-config)
"--host" (.getHost db-uri)
"--port" (str (.getPort db-uri))
"--dbname" (subs (.getPath db-uri) 1)
"--data-only"
"--no-owner"
"--encoding" "utf-8"
"--format" "plain"
:out-enc "utf-8")]
(when-not (zero? (:exit result))
(is (= 0 (:exit result)))
(binding [*out* *err*]
(print (:err result))))
(:out result)))
(deftest ^:integration sensitive-parameter-redaction
(with-open [pg (int/setup-postgres)
pdb (int/run-puppetdb pg {})
ps (int/run-puppet-server-as "my_puppetserver" [pdb] {})]
(let [not-secret "friend R6P2BMFD3XD3PA53SNM5U7RNEE"
secret "xyzzy XKPGMKIB253ZVJHKOKZZXPQSSE"]
;; Create an initial parameter, and make sure it's visible
(int/run-puppet-as "my_agent" ps pdb
(format "notify {'hi': message => '%s'}"
not-secret))
(let [notifications (filter #(and (= ["Notify" "hi"]
[(:type %) (:title %)]))
(int/pql-query pdb "resources {}"))
[notify] notifications]
(is (= 1 (count notifications)))
(is (= {:message not-secret} (:parameters notify))))
(let [dump (pg-dump pg)]
(is (str/includes? dump not-secret))
(is (not (str/includes? dump secret))))
;; Now change the parameter to be a secret, and make sure it's redacted
(int/run-puppet-as "my_agent" ps pdb
(format "notify {'hi': message => Sensitive('%s')}"
secret))
(let [[notify & other-bits] (filter #(and (= ["Notify" "hi"]
[(:type %) (:title %)]))
(int/pql-query pdb "resources {}"))]
(is (empty? other-bits))
(is (= {} (:parameters notify))))
(let [dump (pg-dump pg)
;; Once PUP-7417 is resolved, these tests will fail, and
;; then everything below here should be replaced with this:
;; (is (not (str/includes? dump secret)))
;; but for now, check for the issue and then ignore it.
dump-lines (str/split-lines dump)
expected-leak (fn [line]
(every? (partial str/includes? line)
["\"file\": null"
"\"line\": null"
"\"tags\": [\"notice\"]"
"\"level\": \"notice\""
"\"source\": \"Puppet\""
(format "\"message\": \"%s\"" secret)]))]
(is (some expected-leak dump-lines))
(is (not-any? #(str/includes? % secret)
(remove expected-leak dump-lines)))))))
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment