diff --git a/lib/puppet/ssl/certificate_request_attributes.rb b/lib/puppet/ssl/certificate_request_attributes.rb index 092a8dc9f..e65b01443 100644 --- a/lib/puppet/ssl/certificate_request_attributes.rb +++ b/lib/puppet/ssl/certificate_request_attributes.rb @@ -1,34 +1,37 @@ require 'puppet/ssl' require 'puppet/util/yaml' # This class transforms simple key/value pairs into the equivalent ASN1 # structures. Values may be strings or arrays of strings. # # @api private class Puppet::SSL::CertificateRequestAttributes attr_reader :path, :custom_attributes, :extension_requests def initialize(path) @path = path @custom_attributes = {} @extension_requests = {} end # Attempt to load a yaml file at the given @path. # @return true if we are able to load the file, false otherwise # @raise [Puppet::Error] if there are unexpected attribute keys def load Puppet.info("csr_attributes file loading from #{path}") if Puppet::FileSystem::File.exist?(path) - hash = Puppet::Util::Yaml.load_file(path) + hash = Puppet::Util::Yaml.load_file(path, {}) + if ! hash.is_a?(Hash) + raise Puppet::Error, "invalid CSR attributes, expected instance of Hash, received instance of #{hash.class}" + end @custom_attributes = hash.delete('custom_attributes') || {} @extension_requests = hash.delete('extension_requests') || {} if not hash.keys.empty? raise Puppet::Error, "unexpected attributes #{hash.keys.inspect} in #{@path.inspect}" end return true end return false end end diff --git a/lib/puppet/util/yaml.rb b/lib/puppet/util/yaml.rb index d0c37ae77..246080843 100644 --- a/lib/puppet/util/yaml.rb +++ b/lib/puppet/util/yaml.rb @@ -1,23 +1,24 @@ require 'yaml' module Puppet::Util::Yaml if defined?(::Psych::SyntaxError) YamlLoadExceptions = [::StandardError, ::Psych::SyntaxError] else YamlLoadExceptions = [::StandardError] end class YamlLoadError < Puppet::Error; end - def self.load_file(filename) - YAML.load_file(filename) + def self.load_file(filename, default_value = false) + yaml = YAML.load_file(filename) + yaml || default_value rescue *YamlLoadExceptions => detail raise YamlLoadError.new(detail.message, detail) end def self.dump(structure, filename) Puppet::Util.replace_file(filename, 0660) do |fh| YAML.dump(structure, fh) end end end diff --git a/spec/integration/ssl/autosign_spec.rb b/spec/integration/ssl/autosign_spec.rb index 3a3c6c115..003796ef0 100644 --- a/spec/integration/ssl/autosign_spec.rb +++ b/spec/integration/ssl/autosign_spec.rb @@ -1,90 +1,130 @@ require 'spec_helper' describe "autosigning" do include PuppetSpec::Files let(:puppet_dir) { tmpdir("ca_autosigning") } let(:csr_attributes_content) do { 'custom_attributes' => { '1.3.6.1.4.1.34380.2.0' => 'hostname.domain.com', '1.3.6.1.4.1.34380.2.1' => 'my passphrase', '1.3.6.1.4.1.34380.2.2' => # system IPs in hex [ 0xC0A80001, # 192.168.0.1 0xC0A80101 ], # 192.168.1.1 }, 'extension_requests' => { 'pp_uuid' => 'abcdef', '1.3.6.1.4.1.34380.1.1.2' => '1234', # pp_instance_id '1.3.6.1.4.1.34380.1.2.1' => 'some-value', # private extension }, } end let(:host) { Puppet::SSL::Host.new } before do Puppet.settings[:confdir] = puppet_dir Puppet.settings[:vardir] = puppet_dir # This is necessary so the terminus instances don't lie around. Puppet::SSL::Key.indirection.termini.clear end - context "with extension requests from csr_attributes file" do - let(:ca) { Puppet::SSL::CertificateAuthority.new } + def write_csr_attributes(yaml) + File.open(Puppet.settings[:csr_attributes], 'w') do |file| + file.puts YAML.dump(yaml) + end + end + + context "when the csr_attributes file is valid, but empty" do + it "generates a CSR when the file is empty" do + Puppet::FileSystem::File.new(Puppet.settings[:csr_attributes]).touch + + host.generate_certificate_request + end - def write_csr_attributes + it "generates a CSR when the file contains whitespace" do File.open(Puppet.settings[:csr_attributes], 'w') do |file| - file.puts YAML.dump(csr_attributes_content) + file.puts "\n\n" end + + host.generate_certificate_request + end + end + + context "when the csr_attributes file doesn't contain a YAML encoded hash" do + it "raises when the file contains a string" do + write_csr_attributes('a string') + + expect { + host.generate_certificate_request + }.to raise_error(Puppet::Error, /invalid CSR attributes, expected instance of Hash, received instance of String/) + end + + it "raises when the file contains an empty array" do + write_csr_attributes([]) + + expect { + host.generate_certificate_request + }.to raise_error(Puppet::Error, /invalid CSR attributes, expected instance of Hash, received instance of Array/) + end + end + + context "with extension requests from csr_attributes file" do + let(:ca) { Puppet::SSL::CertificateAuthority.new } + + it "generates a CSR when the csr_attributes file is an empty hash" do + write_csr_attributes(csr_attributes_content) + + host.generate_certificate_request end context "and subjectAltName" do it "raises an error if you include subjectAltName in csr_attributes" do csr_attributes_content['extension_requests']['subjectAltName'] = 'foo' - write_csr_attributes + write_csr_attributes(csr_attributes_content) expect { host.generate_certificate_request }.to raise_error(Puppet::Error, /subjectAltName.*conflicts with internally used extension request/) end it "properly merges subjectAltName when in settings" do Puppet.settings[:dns_alt_names] = 'althostname.nowhere' - write_csr_attributes + write_csr_attributes(csr_attributes_content) host.generate_certificate_request csr = Puppet::SSL::CertificateRequest.indirection.find(host.name) expect(csr.subject_alt_names).to include('DNS:althostname.nowhere') end end context "without subjectAltName" do before do - write_csr_attributes + write_csr_attributes(csr_attributes_content) host.generate_certificate_request end it "pulls extension attributes from the csr_attributes file into the certificate" do csr = Puppet::SSL::CertificateRequest.indirection.find(host.name) expect(csr.request_extensions).to have(3).items expect(csr.request_extensions).to include('oid' => 'pp_uuid', 'value' => 'abcdef') expect(csr.request_extensions).to include('oid' => 'pp_instance_id', 'value' => '1234') expect(csr.request_extensions).to include('oid' => '1.3.6.1.4.1.34380.1.2.1', 'value' => 'some-value') end it "copies extension requests to certificate" do cert = ca.sign(host.name) expect(cert.custom_extensions).to include('oid' => 'pp_uuid', 'value' => 'abcdef') expect(cert.custom_extensions).to include('oid' => 'pp_instance_id', 'value' => '1234') expect(cert.custom_extensions).to include('oid' => '1.3.6.1.4.1.34380.1.2.1', 'value' => 'some-value') end it "does not copy custom attributes to certificate" do cert = ca.sign(host.name) cert.custom_extensions.each do |ext| expect(Puppet::SSL::Oids.subtree_of?('1.3.6.1.4.1.34380.2', ext['oid'])).to be_false end end end end end diff --git a/spec/unit/ssl/certificate_request_attributes_spec.rb b/spec/unit/ssl/certificate_request_attributes_spec.rb index 70a4df520..6165330aa 100644 --- a/spec/unit/ssl/certificate_request_attributes_spec.rb +++ b/spec/unit/ssl/certificate_request_attributes_spec.rb @@ -1,61 +1,61 @@ require 'spec_helper' require 'puppet/ssl/certificate_request_attributes' describe Puppet::SSL::CertificateRequestAttributes do let(:expected) do { "custom_attributes" => { "1.3.6.1.4.1.34380.2.2"=>[3232235521, 3232235777], # system IPs in hex "1.3.6.1.4.1.34380.2.0"=>"hostname.domain.com", } } end let(:csr_attributes_hash) { expected.dup } let(:csr_attributes_path) { '/some/where/csr_attributes.yaml' } let(:csr_attributes) { Puppet::SSL::CertificateRequestAttributes.new(csr_attributes_path) } it "initializes with a path" do expect(csr_attributes.path).to eq(csr_attributes_path) end describe "loading" do it "returns nil when loading from a non-existent file" do expect(csr_attributes.load).to be_false end context "with an available attributes file" do before do Puppet::FileSystem::File.expects(:exist?).with(csr_attributes_path).returns(true) - Puppet::Util::Yaml.expects(:load_file).with(csr_attributes_path).returns(csr_attributes_hash) + Puppet::Util::Yaml.expects(:load_file).with(csr_attributes_path, {}).returns(csr_attributes_hash) end it "loads csr attributes from a file when the file is present" do expect(csr_attributes.load).to be_true end it "exposes custom_attributes" do csr_attributes.load expect(csr_attributes.custom_attributes).to eq(expected['custom_attributes']) end it "returns an empty hash if custom_attributes points to nil" do csr_attributes_hash["custom_attributes"] = nil csr_attributes.load expect(csr_attributes.custom_attributes).to eq({}) end it "returns an empty hash if custom_attributes key is not present" do csr_attributes_hash.delete("custom_attributes") csr_attributes.load expect(csr_attributes.custom_attributes).to eq({}) end it "raise a Puppet::Error if an unexpected root key is defined" do csr_attributes_hash['unintentional'] = 'data' expect { csr_attributes.load }.to raise_error(Puppet::Error, /unexpected attributes.*unintentional/) end end end end diff --git a/spec/unit/util/yaml_spec.rb b/spec/unit/util/yaml_spec.rb index 960388ade..978afb0a2 100644 --- a/spec/unit/util/yaml_spec.rb +++ b/spec/unit/util/yaml_spec.rb @@ -1,41 +1,55 @@ require 'spec_helper' require 'puppet/util/yaml' describe Puppet::Util::Yaml do include PuppetSpec::Files let(:filename) { tmpfile("yaml") } it "reads a YAML file from disk" do write_file(filename, YAML.dump({ "my" => "data" })) expect(Puppet::Util::Yaml.load_file(filename)).to eq({ "my" => "data" }) end it "writes data formatted as YAML to disk" do Puppet::Util::Yaml.dump({ "my" => "data" }, filename) expect(Puppet::Util::Yaml.load_file(filename)).to eq({ "my" => "data" }) end it "raises an error when the file is invalid YAML" do write_file(filename, "{ invalid") expect { Puppet::Util::Yaml.load_file(filename) }.to raise_error(Puppet::Util::Yaml::YamlLoadError) end it "raises an error when the file does not exist" do expect { Puppet::Util::Yaml.load_file("no") }.to raise_error(Puppet::Util::Yaml::YamlLoadError, /No such file or directory/) end it "raises an error when the filename is illegal" do expect { Puppet::Util::Yaml.load_file("not\0allowed") }.to raise_error(Puppet::Util::Yaml::YamlLoadError, /null byte/) end + context "when the file is empty" do + it "returns false" do + Puppet::FileSystem::File.new(filename).touch + + expect(Puppet::Util::Yaml.load_file(filename)).to be_false + end + + it "allows return value to be overridden" do + Puppet::FileSystem::File.new(filename).touch + + expect(Puppet::Util::Yaml.load_file(filename, {})).to eq({}) + end + end + def write_file(name, contents) File.open(name, "w") do |fh| fh.write(contents) end end end