From aad83a8be02bd1ceaca2115dfed1a68df33d7015 Mon Sep 17 00:00:00 2001 From: Adrien Thebo Date: Fri, 1 Nov 2013 09:15:51 -0700 Subject: [PATCH 1/4] (maint) Ensure that the autosign setting is validated. Original patch by Patrick Hemmer --- lib/puppet/defaults.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb index 462466af402..4ea012739fd 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.rb @@ -715,7 +715,13 @@ def self.default_diffargs :desc => "Whether to enable autosign. Valid values are true (which autosigns any key request, and is a very bad idea), false (which never autosigns any key request), and the path to a file, which - uses that configuration file to determine which keys to sign."}, + uses that configuration file to determine which keys to sign.", + :hook => proc do |value| + unless [false, 'false', true, 'true'].include?(value) or Puppet::Util.absolute_path?(value) + raise ArgumentError, "The autosign parameter must be 'true'/'false' or an absolute path" + end + end + }, :allow_duplicate_certs => { :default => false, :type => :boolean, From 561b67dcee15ccc378d770b7a952fd630bc28d79 Mon Sep 17 00:00:00 2001 From: Adrien Thebo Date: Fri, 1 Nov 2013 09:35:13 -0700 Subject: [PATCH 2/4] (#7244) Add autosign_command for programmatic CSR autosigning This commit adds support for delegating control of autosigning to an outside command. This allows users to define their own criteria for signing CSRs. This commit adds a new setting, 'autosign_command', for the command to use for testing CSRs, and extends the logic of the CA to test for autosigning based on both the 'autosign' setting and the output of the autosign_command. Original patch by Patrick Hemmer --- acceptance/tests/ssl/autosign_command.rb | 85 ++++++++++++ lib/puppet/defaults.rb | 26 +++- lib/puppet/ssl/certificate_authority.rb | 109 +++++++++------ .../certificate_authority/autosign_command.rb | 48 +++++++ .../autosign_command_spec.rb | 43 ++++++ spec/unit/ssl/certificate_authority_spec.rb | 127 +++++++++++++----- 6 files changed, 355 insertions(+), 83 deletions(-) create mode 100644 acceptance/tests/ssl/autosign_command.rb create mode 100644 lib/puppet/ssl/certificate_authority/autosign_command.rb create mode 100644 spec/unit/ssl/certificate_authority/autosign_command_spec.rb diff --git a/acceptance/tests/ssl/autosign_command.rb b/acceptance/tests/ssl/autosign_command.rb new file mode 100644 index 00000000000..8f592ecdac0 --- /dev/null +++ b/acceptance/tests/ssl/autosign_command.rb @@ -0,0 +1,85 @@ +test_name "autosign_command behavior (#7244)" do + + def assert_key_generated(name) + assert_match(/Creating a new SSL key for #{name}/, stdout, "Expected agent to create a new SSL key for autosigning") + end + + def reset_agent_ssl + return if master.is_pe? + clear_agent_ssl + + hostname = master.execute('facter hostname') + fqdn = master.execute('facter fqdn') + + step "Master: Ensure the master bootstraps CA" + with_puppet_running_on(master, + :master => { + :dns_alt_names => "puppet,#{hostname},#{fqdn}", + :autosign => true, + } + ) do + + agents.each do |agent| + next if agent == master + step "Clear old agent certificate from master" do + agent_cn = on(agent, puppet('agent --configprint certname')).stdout.chomp + puts "agent cn: #{agent_cn}" + clean_cert(master, agent_cn) if agent_cn + end + end + end + end + + + def clean_cert(host, cn) + on(host, puppet('cert', 'clean', cn), :acceptable_exit_codes => [0, 24]) + end + + def clear_agent_ssl + return if master.is_pe? + step "All: Clear agent only ssl settings (do not clear master)" + hosts.each do |host| + next if host == master + ssldir = on(host, puppet('agent --configprint ssldir')).stdout.chomp + on( host, host_command("rm -rf '#{ssldir}'") ) + end + end + + testdir = master.tmpdir('autosign_command') + + teardown do + step "Remove autosign configuration" + on(master, host_command("rm -rf '#{testdir}'") ) + reset_agent_ssl + end + + step "Step 1: ensure autosign_command can approve CSRs" + + reset_agent_ssl + + master_opts = {'master' => {'autosign' => 'false', 'autosign_command' => '/bin/true'}} + with_puppet_running_on(master, master_opts) do + agents.each do |agent| + next if agent == master + + on(agent, puppet("agent --test --server #{master} --waitforcert 0")) + assert_key_generated(agent) + assert_no_match(/failed to retrieve certificate/, stdout, "Expected certificate to be autosigned") + end + end + + reset_agent_ssl + + step "Step 2: ensure autosign_command can reject CSRs" + + master_opts = {'master' => {'autosign' => 'false', 'autosign_command' => '/bin/false'}} + with_puppet_running_on(master, master_opts) do + agents.each do |agent| + next if agent == master + + on(agent, puppet("agent --test --server #{master} --waitforcert 0"), :acceptable_exit_codes => [1]) + assert_key_generated(agent) + assert_no_match(/failed to retrieve certificate/, stdout, "Expected certificate to not be autosigned") + end + end +end diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb index 4ea012739fd..ca00ac567de 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.rb @@ -714,14 +714,36 @@ def self.default_diffargs :mode => 0644, :desc => "Whether to enable autosign. Valid values are true (which autosigns any key request, and is a very bad idea), false (which - never autosigns any key request), and the path to a file, which - uses that configuration file to determine which keys to sign.", + disables autosigning certificates based on an autosign file), and the + path to a file, which uses that configuration file to determine which + keys to sign.", :hook => proc do |value| unless [false, 'false', true, 'true'].include?(value) or Puppet::Util.absolute_path?(value) raise ArgumentError, "The autosign parameter must be 'true'/'false' or an absolute path" end end }, + :autosign_command => { + :default => nil, + :type => :string, + :desc => "Command to run which determines whether the certificate + request should be automatically signed. The script is called + on each request and is passed the certificate name as an + argument and the contents of the CSR in PEM format on stdin. It should + exit with a status of 0 if the cert should be signed, 1 if the cert + should not be signed, and any other value if an error occured while + running the command. + + The 'autosign' parameter has precedence over this one. If 'autosign' + determines a cert should be signed, 'autosign_command' will never be + called. However if 'autosign' does not validate a request, it will + pass through to 'autosign_command'.", + :hook => proc do |value| + if value and !Puppet::Util.absolute_path?(value) + raise ArgumentError, "The autosign_command parameter must be an absolute path" + end + end, + }, :allow_duplicate_certs => { :default => false, :type => :boolean, diff --git a/lib/puppet/ssl/certificate_authority.rb b/lib/puppet/ssl/certificate_authority.rb index 7b76476ea9e..e45f061e611 100644 --- a/lib/puppet/ssl/certificate_authority.rb +++ b/lib/puppet/ssl/certificate_authority.rb @@ -25,6 +25,7 @@ class Puppet::SSL::CertificateAuthority require 'puppet/ssl/inventory' require 'puppet/ssl/certificate_revocation_list' require 'puppet/ssl/certificate_authority/interface' + require 'puppet/ssl/certificate_authority/autosign_command' require 'puppet/network/authstore' class CertificateVerificationError < RuntimeError @@ -61,53 +62,18 @@ def self.instance # If autosign is configured, then autosign all CSRs that match our configuration. def autosign(name) - case auto = autosign? - - when false, nil - # auto-signing is off - return - - when true - # auto-signing is on and all are allowed - - else - # auto-signing if allowed by store - return unless autosign_store(auto).allowed?(name, "127.1.1.1") - end - - Puppet.info "Autosigning #{name}" - sign(name) - end - - # Do we autosign? This returns true, false, or a filename. - def autosign? - case auto = Puppet[:autosign] - - when 'false', false - return false - - when 'true', true - return true - - else - # it must be an absolute path to an existing file - unless Puppet::Util.absolute_path?(auto) - raise ArgumentError, "The autosign configuration '#{auto}' must be a fully qualified file" - end - Puppet::FileSystem::File.exist?(auto) && auto + if autosign?(name) + Puppet.info "Autosigning #{name}" + sign(name) end end - # Creates an AuthStore for autosigning - def autosign_store(file) - auth = Puppet::Network::AuthStore.new - File.readlines(file).each do |line| - next if line =~ /^\s*#/ - next if line =~ /^\s*$/ - auth.allow(line.chomp) - end - - auth + # Determine if a CSR can be autosigned by the autosign store or autosign command + # + # @param name [String] The name of the CSR to check + # @return [true, false] + def autosign?(name) + autosign_store?(name) or autosign_command?(name) end # Retrieves (or creates, if necessary) the certificate revocation list. @@ -472,4 +438,59 @@ def fingerprint(name, md = :SHA256) def waiting? Puppet::SSL::CertificateRequest.indirection.search("*").collect { |r| r.name } end + + private + + # Check the autosign setting to see if this cert can be signed. + # + # @api private + # @param name [String] The name of the CSR to check + # @return [true, false] + def autosign_store?(name) + auto = Puppet[:autosign] + + case auto + + when 'false', false, nil + return false + + when 'true', true + return true + + else + if Puppet::FileSystem::File.exist?(auto) + store = autosign_store(auto) + store.allowed?(name, '127.1.1.1') + end + end + end + + # Construct an autosign store for the contents of the autosign file + # + # @api private + # @param file [String] The path to the autosign file + # @return [Puppet::Network::AuthStore] + def autosign_store(file) + auth = Puppet::Network::AuthStore.new + File.readlines(file).each do |line| + next if line =~ /^\s*#/ + next if line =~ /^\s*$/ + auth.allow(line.chomp) + end + + auth + end + + # Try to autosign a certificate based an external script + # + # @api private + # @param name [String] The name of the CSR to check + # @return [true, false] + def autosign_command?(name) + cmd = Puppet[:autosign_command] + return false if cmd.nil? + + autosign_cmd = Puppet::SSL::CertificateAuthority::AutosignCommand.new(cmd) + autosign_cmd.allowed?(name) + end end diff --git a/lib/puppet/ssl/certificate_authority/autosign_command.rb b/lib/puppet/ssl/certificate_authority/autosign_command.rb new file mode 100644 index 00000000000..1310c2c5e58 --- /dev/null +++ b/lib/puppet/ssl/certificate_authority/autosign_command.rb @@ -0,0 +1,48 @@ +require 'puppet/ssl/certificate_authority' + +# This class wraps a given command and invokes it with a CSR name and body to +# determine if the given CSR should be autosigned +# +# @api private +class Puppet::SSL::CertificateAuthority::AutosignCommand + + class CheckFailure < Puppet::Error; end + + def initialize(path) + @path = path + end + + # Run the autosign command with the given CSR name as an argument and the + # CSR body on stdin. + # + # @param name [String] The CSR name to check for autosigning + # @return [true, false] If the CSR should be autosigned + def allowed?(name) + csr = Puppet::SSL::CertificateRequest.indirection.find(name) + if csr.nil? + raise CheckFailure, "Could not run autosign_command for #{name}: no CSR for #{name}" + end + + cmd = "#{@path} #{name}" + csr_file = Tempfile.new('puppet-csr') + csr_file.write(csr.to_s) + + execute_options = {:stdinfile => csr_file.path, :combine => true, :failonfail => false} + output = Puppet::Util::Execution.execute(cmd, execute_options) + + output.chomp! if output.is_a? String + exitstatus = $CHILD_STATUS.exitstatus + + Puppet.info "autosign_command '#{cmd}' completed with exit status #{exitstatus}" + Puppet.debug "Output of autosign_command '#{cmd}': #{output}" + + case exitstatus + when 0 + true + when 1 + false + else + raise CheckFailure, "autosign_command '#{cmd}' failed while checking #{name}: exit status #{exitstatus}" + end + end +end diff --git a/spec/unit/ssl/certificate_authority/autosign_command_spec.rb b/spec/unit/ssl/certificate_authority/autosign_command_spec.rb new file mode 100644 index 00000000000..7a1d51ec555 --- /dev/null +++ b/spec/unit/ssl/certificate_authority/autosign_command_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' + +require 'puppet/ssl/certificate_authority/autosign_command' + +describe Puppet::SSL::CertificateAuthority::AutosignCommand do + + subject { described_class.new('/autosign/command') } + + before :each do + Puppet::Util::Execution.stubs(:execute).with('/autosign/command host', anything).returns('') + Puppet::SSL::CertificateRequest.indirection.stubs(:find).returns(stub 'csr', :to_s => 'CSR PEM goes here') + end + + + it "runs the command with the CSR certname and body" do + tmpfile = stub('tempfile', :path => '/path/to/csr/tempfile', :write => nil) + Tempfile.stubs(:new).returns tmpfile + + Puppet::Util::Execution.expects(:execute).with do |cmd, args| + cmd.should eq '/autosign/command host' + args.should include(:stdinfile => '/path/to/csr/tempfile') + end.returns '' + subject.allowed?('host') + end + + it "returns true if the command succeeded" do + $CHILD_STATUS.stubs(:exitstatus).returns 0 + subject.allowed?('host').should == true + end + + it "returns false if the command failed" do + $CHILD_STATUS.stubs(:exitstatus).returns 1 + subject.allowed?('host').should == false + end + + it "raises an error if the command errored" do + $CHILD_STATUS.stubs(:exitstatus).returns 255 + expect do + subject.allowed?('host').should == false + end.to raise_error Puppet::SSL::CertificateAuthority::AutosignCommand::CheckFailure, + /autosign_command.*failed.*exit status 255/ + end +end diff --git a/spec/unit/ssl/certificate_authority_spec.rb b/spec/unit/ssl/certificate_authority_spec.rb index 3a2d8a94ee9..38e15bc9eff 100755 --- a/spec/unit/ssl/certificate_authority_spec.rb +++ b/spec/unit/ssl/certificate_authority_spec.rb @@ -570,61 +570,114 @@ def stub_ca_host end describe "when autosigning certificates" do - let(:autosign) { File.expand_path("/auto/sign") } - it "should do nothing if autosign is disabled" do - Puppet[:autosign] = 'false' + describe "using the autosign setting" do + let(:autosign) { File.expand_path("/auto/sign") } - Puppet::SSL::CertificateRequest.indirection.expects(:search).never - @ca.autosign("host") - end + it "should do nothing if autosign is disabled" do + Puppet[:autosign] = false + + @ca.expects(:sign).never + @ca.autosign("host") + end + + it "should do nothing if no autosign.conf exists" do + Puppet[:autosign] = autosign + Puppet::FileSystem::File.expects(:exist?).with(autosign).returns false + + @ca.expects(:sign).never + @ca.autosign("host") + end + + describe "and autosign is enabled and the autosign.conf file exists" do + before do + Puppet[:autosign] = autosign + Puppet::FileSystem::File.expects(:exist?).with(autosign).returns(true).at_least_once + File.stubs(:readlines).with(autosign).returns ["one\n", "two\n"] + + @store = stub 'store', :allow => nil, :allowed? => false + Puppet::Network::AuthStore.stubs(:new).returns @store + end + + describe "when creating the AuthStore instance to verify autosigning" do + it "should create an AuthStore with each line in the configuration file allowed to be autosigned" do + @store.expects(:allow).with("one") + @store.expects(:allow).with("two") + + @ca.autosign("host") + end + + it "should reparse the autosign configuration on each call" do + Puppet::Network::AuthStore.expects(:new).times(2).returns @store + + @ca.autosign("host") + @ca.autosign("host") + end - it "should do nothing if no autosign.conf exists" do - Puppet[:autosign] = autosign - Puppet::FileSystem::File.expects(:exist?).with(autosign).returns false + it "should ignore comments" do + File.stubs(:readlines).with(autosign).returns ["one\n", "#two\n"] - Puppet::SSL::CertificateRequest.indirection.expects(:search).never - @ca.autosign("host") + @store.expects(:allow).with("one") + @ca.autosign("host") + end + + it "should ignore blank lines" do + File.stubs(:readlines).with(autosign).returns ["one\n", "\n"] + + @store.expects(:allow).with("one") + @ca.autosign("host") + end + end + end end - describe "and autosign is enabled and the autosign.conf file exists" do - before do - Puppet[:autosign] = autosign - Puppet::FileSystem::File.stubs(:exist?).with(autosign).returns true - File.stubs(:readlines).with(autosign).returns ["one\n", "two\n"] + describe "using the autosign_command setting" do + after(:all) do + Puppet[:autosign_command] = nil + end - Puppet::SSL::CertificateRequest.indirection.stubs(:search).returns [] + it "checks the autosign setting first" do + Puppet[:autosign] = true + + @ca.expects(:autosign_command?).never + @ca.expects(:sign).with('host') - @store = stub 'store', :allow => nil, :allowed? => false - Puppet::Network::AuthStore.stubs(:new).returns @store + @ca.autosign("host") end - describe "when creating the AuthStore instance to verify autosigning" do - it "should create an AuthStore with each line in the configuration file allowed to be autosigned" do - @store.expects(:allow).with("one") - @store.expects(:allow).with("two") + it "doesn't autosign the CSR if the autosign_command is unset" do + Puppet[:autosign] = false + Puppet[:autosign_command] = nil - @ca.autosign("host") - end + @ca.expects(:sign).never + @ca.autosign("host") + end + + describe "invoking the autosign_command" do + let(:autosign_cmd) { mock 'autosign_command' } + + before do + Puppet[:autosign] = false - it "should reparse the autosign configuration on each call" do - Puppet::Network::AuthStore.expects(:new).times(2).returns @store + Puppet::Util.stubs(:absolute_path?).with('autosign_cmd').returns true + Puppet[:autosign_command] = 'autosign_cmd' - @ca.autosign("host") - @ca.autosign("host") + Puppet::SSL::CertificateAuthority::AutosignCommand.stubs(:new).returns autosign_cmd end - it "should ignore comments" do - File.stubs(:readlines).with(autosign).returns ["one\n", "#two\n"] + it "autosigns the CSR if the autosign_command returned true" do + autosign_cmd.expects(:allowed?).with('host').returns true + Puppet::SSL::CertificateAuthority::AutosignCommand.expects(:new).returns autosign_cmd - @store.expects(:allow).with("one") - @ca.autosign("host") + @ca.expects(:sign).with('host') + @ca.autosign('host') end - it "should ignore blank lines" do - File.stubs(:readlines).with(autosign).returns ["one\n", "\n"] + it "doesn't autosign the CSR if the autosign_command returned false" do + autosign_cmd.expects(:allowed?).with('host').returns false + Puppet::SSL::CertificateAuthority::AutosignCommand.expects(:new).returns autosign_cmd - @store.expects(:allow).with("one") - @ca.autosign("host") + @ca.expects(:sign).never + @ca.autosign('host') end end end From 17a6d5aaec6e964d86251954e26335c4f6748bbd Mon Sep 17 00:00:00 2001 From: Josh Partlow Date: Fri, 8 Nov 2013 11:40:12 -0800 Subject: [PATCH 3/4] (maint) Extract common ssl setup for acceptance tests Extracts cert initialization from git/package certificate provisioning, and common ssl reset steps from the autosign_command and puppet_cert_generate_and_autosign tests into the puppet/acceptance/common_utils helper lib. --- .../git/pre-suite/04_ValidateSignCert.rb | 26 +----- .../packages/pre-suite/04_ValidateSignCert.rb | 29 +------ .../lib/puppet/acceptance/common_utils.rb | 85 +++++++++++++++++++ acceptance/tests/ssl/autosign_command.rb | 48 ++--------- .../ssl/puppet_cert_generate_and_autosign.rb | 52 +----------- 5 files changed, 99 insertions(+), 141 deletions(-) diff --git a/acceptance/config/el6/setup/git/pre-suite/04_ValidateSignCert.rb b/acceptance/config/el6/setup/git/pre-suite/04_ValidateSignCert.rb index 0d2e457bfb0..549432427bd 100755 --- a/acceptance/config/el6/setup/git/pre-suite/04_ValidateSignCert.rb +++ b/acceptance/config/el6/setup/git/pre-suite/04_ValidateSignCert.rb @@ -1,26 +1,6 @@ test_name "Validate Sign Cert" -hostname = on(master, 'facter hostname').stdout.strip -fqdn = on(master, 'facter fqdn').stdout.strip +require 'puppet/acceptance/common_utils' +extend Puppet::Acceptance::CAUtils -step "Clear SSL on all hosts" -hosts.each { |host| on host, "rm -rf #{host["puppetpath"]}/ssl" } - -step "Master: Start Puppet Master" do - with_puppet_running_on(master, :main => { :dns_alt_names => "puppet,#{hostname},#{fqdn}", :verbose => true, :daemonize => true }) do - - hosts.each do |host| - next if host['roles'].include? 'master' - - step "Agents: Run agent --test first time to gen CSR" - on host, puppet("agent --test --server #{master}"), :acceptable_exit_codes => [1] - end - - # Sign all waiting certs - step "Master: sign all certs" - on master, puppet("cert --sign --all"), :acceptable_exit_codes => [0,24] - - step "Agents: Run agent --test second time to obtain signed cert" - on agents, puppet("agent --test --server #{master}"), :acceptable_exit_codes => [0,2] - end -end +initialize_ssl diff --git a/acceptance/config/el6/setup/packages/pre-suite/04_ValidateSignCert.rb b/acceptance/config/el6/setup/packages/pre-suite/04_ValidateSignCert.rb index f7aef381541..549432427bd 100755 --- a/acceptance/config/el6/setup/packages/pre-suite/04_ValidateSignCert.rb +++ b/acceptance/config/el6/setup/packages/pre-suite/04_ValidateSignCert.rb @@ -1,29 +1,6 @@ test_name "Validate Sign Cert" -hostname = on(master, 'facter hostname').stdout.strip -fqdn = on(master, 'facter fqdn').stdout.strip +require 'puppet/acceptance/common_utils' +extend Puppet::Acceptance::CAUtils -step "Clear SSL on all hosts" -hosts.each do |host| - ssldir = on(host, puppet('agent --configprint ssldir')).stdout.chomp - on(host, "rm -rf '#{ssldir}'") -end - -step "Master: Start Puppet Master" do - with_puppet_running_on(master, :main => { :dns_alt_names => "puppet,#{hostname},#{fqdn}", :verbose => true, :daemonize => true }) do - - hosts.each do |host| - next if host['roles'].include? 'master' - - step "Agents: Run agent --test first time to gen CSR" - on host, puppet("agent --test --server #{master}"), :acceptable_exit_codes => [1] - end - - # Sign all waiting certs - step "Master: sign all certs" - on master, puppet("cert --sign --all"), :acceptable_exit_codes => [0,24] - - step "Agents: Run agent --test second time to obtain signed cert" - on agents, puppet("agent --test --server #{master}"), :acceptable_exit_codes => [0,2] - end -end +initialize_ssl diff --git a/acceptance/lib/puppet/acceptance/common_utils.rb b/acceptance/lib/puppet/acceptance/common_utils.rb index 191820c0b02..2e8efc76e82 100644 --- a/acceptance/lib/puppet/acceptance/common_utils.rb +++ b/acceptance/lib/puppet/acceptance/common_utils.rb @@ -17,5 +17,90 @@ def setup(agent, o={}) package {'cron': name=> $cron, ensure=>present, }]) end end + + module CAUtils + + def initialize_ssl + hostname = on(master, 'facter hostname').stdout.strip + fqdn = on(master, 'facter fqdn').stdout.strip + + step "Clear SSL on all hosts" + hosts.each do |host| + ssldir = on(host, puppet('agent --configprint ssldir')).stdout.chomp + on(host, "rm -rf '#{ssldir}'") + end + + step "Master: Start Puppet Master" do + with_puppet_running_on(master, :main => { :dns_alt_names => "puppet,#{hostname},#{fqdn}", :verbose => true, :daemonize => true }) do + + hosts.each do |host| + next if host['roles'].include? 'master' + + step "Agents: Run agent --test first time to gen CSR" + on host, puppet("agent --test --server #{master}"), :acceptable_exit_codes => [1] + end + + # Sign all waiting certs + step "Master: sign all certs" + on master, puppet("cert --sign --all"), :acceptable_exit_codes => [0,24] + + step "Agents: Run agent --test second time to obtain signed cert" + on agents, puppet("agent --test --server #{master}"), :acceptable_exit_codes => [0,2] + end + end + end + + def clean_cert(host, cn, check = true) + on(host, puppet('cert', 'clean', cn), :acceptable_exit_codes => check ? [0] : [0, 24]) + if check + assert_match(/remov.*Certificate.*#{cn}/i, stdout, "Should see a log message that certificate request was removed.") + on(host, puppet('cert', 'list', '--all')) + assert_no_match(/#{cn}/, stdout, "Should not see certificate in list anymore.") + end + end + + def clear_agent_ssl + return if master.is_pe? + step "All: Clear agent only ssl settings (do not clear master)" + hosts.each do |host| + next if host == master + ssldir = on(host, puppet('agent --configprint ssldir')).stdout.chomp + on( host, host_command("rm -rf '#{ssldir}'") ) + end + end + + def reset_agent_ssl(resign = true) + return if master.is_pe? + clear_agent_ssl + + hostname = master.execute('facter hostname') + fqdn = master.execute('facter fqdn') + + step "Master: Ensure the master bootstraps CA" + with_puppet_running_on(master, + :master => { + :dns_alt_names => "puppet,#{hostname},#{fqdn}", + :autosign => true, + } + ) do + + agents.each do |agent| + next if agent == master + + step "Clear old agent certificate from master" do + agent_cn = on(agent, puppet('agent --configprint certname')).stdout.chomp + clean_cert(master, agent_cn, false) if agent_cn + end + if resign + step "Agents: Run agent --test once to obtained auto-signed cert" do + on agent, puppet('agent', "--test --server #{master}"), :acceptable_exit_codes => [0,2] + end + end + end + + end + end + + end end end diff --git a/acceptance/tests/ssl/autosign_command.rb b/acceptance/tests/ssl/autosign_command.rb index 8f592ecdac0..34989c32447 100644 --- a/acceptance/tests/ssl/autosign_command.rb +++ b/acceptance/tests/ssl/autosign_command.rb @@ -1,50 +1,12 @@ +require 'puppet/acceptance/common_utils' +extend Puppet::Acceptance::CAUtils + test_name "autosign_command behavior (#7244)" do def assert_key_generated(name) assert_match(/Creating a new SSL key for #{name}/, stdout, "Expected agent to create a new SSL key for autosigning") end - def reset_agent_ssl - return if master.is_pe? - clear_agent_ssl - - hostname = master.execute('facter hostname') - fqdn = master.execute('facter fqdn') - - step "Master: Ensure the master bootstraps CA" - with_puppet_running_on(master, - :master => { - :dns_alt_names => "puppet,#{hostname},#{fqdn}", - :autosign => true, - } - ) do - - agents.each do |agent| - next if agent == master - step "Clear old agent certificate from master" do - agent_cn = on(agent, puppet('agent --configprint certname')).stdout.chomp - puts "agent cn: #{agent_cn}" - clean_cert(master, agent_cn) if agent_cn - end - end - end - end - - - def clean_cert(host, cn) - on(host, puppet('cert', 'clean', cn), :acceptable_exit_codes => [0, 24]) - end - - def clear_agent_ssl - return if master.is_pe? - step "All: Clear agent only ssl settings (do not clear master)" - hosts.each do |host| - next if host == master - ssldir = on(host, puppet('agent --configprint ssldir')).stdout.chomp - on( host, host_command("rm -rf '#{ssldir}'") ) - end - end - testdir = master.tmpdir('autosign_command') teardown do @@ -55,7 +17,7 @@ def clear_agent_ssl step "Step 1: ensure autosign_command can approve CSRs" - reset_agent_ssl + reset_agent_ssl(false) master_opts = {'master' => {'autosign' => 'false', 'autosign_command' => '/bin/true'}} with_puppet_running_on(master, master_opts) do @@ -68,7 +30,7 @@ def clear_agent_ssl end end - reset_agent_ssl + reset_agent_ssl(false) step "Step 2: ensure autosign_command can reject CSRs" diff --git a/acceptance/tests/ssl/puppet_cert_generate_and_autosign.rb b/acceptance/tests/ssl/puppet_cert_generate_and_autosign.rb index b2e511e5f79..83b912cd94e 100644 --- a/acceptance/tests/ssl/puppet_cert_generate_and_autosign.rb +++ b/acceptance/tests/ssl/puppet_cert_generate_and_autosign.rb @@ -1,3 +1,6 @@ +require 'puppet/acceptance/common_utils' +extend Puppet::Acceptance::CAUtils + test_name "Puppet cert generate behavior (#6112)" do # This acceptance test documents the behavior of `puppet cert generate` calls @@ -51,15 +54,6 @@ clean_cert(master, test_cn, false) end - def clean_cert(host, cn, check = true) - on(host, puppet('cert', 'clean', cn), :acceptable_exit_codes => check ? [0] : [0, 24]) - if check - assert_match(/remov.*Certificate.*#{cn}/i, stdout, "Should see a log message that certificate request was removed.") - on(host, puppet('cert', 'list', '--all')) - assert_no_match(/#{cn}/, stdout, "Should not see certificate in list anymore.") - end - end - def generate_and_clean_cert(host, cn, autosign) on(host, puppet('cert', 'generate', cn, '--autosign', autosign)) assert_no_match(/Could not find certificate request for.*cert\.test/i, stderr, "Should not see an error message for a missing certificate request.") @@ -85,46 +79,6 @@ def host_is_master?(host) host['roles'].include?('master') end - def clear_agent_ssl - return if master.is_pe? - step "All: Clear agent only ssl settings (do not clear master)" - hosts.each do |host| - next if host == master - ssldir = on(host, puppet('agent --configprint ssldir')).stdout.chomp - on( host, host_command("rm -rf '#{ssldir}'") ) - end - end - - def reset_agent_ssl - return if master.is_pe? - clear_agent_ssl - - hostname = master.execute('facter hostname') - fqdn = master.execute('facter fqdn') - - step "Master: Ensure the master bootstraps CA" - with_puppet_running_on(master, - :master => { - :dns_alt_names => "puppet,#{hostname},#{fqdn}", - :autosign => true, - } - ) do - - agents.each do |agent| - next if agent == master - - step "Clear old agent certificate from master" do - agent_cn = on(agent, puppet('agent --configprint certname')).stdout.chomp - clean_cert(master, agent_cn, false) if agent_cn - end - step "Agents: Run agent --test once to obtained auto-signed cert" do - on agent, puppet('agent', "--test --server #{master}"), :acceptable_exit_codes => [0,2] - end - end - - end - end - ################ # Cases 1 and 2: From 38088e30c063bb1c43fd40a6f0ae0d9c8357be1b Mon Sep 17 00:00:00 2001 From: Josh Partlow Date: Fri, 8 Nov 2013 13:51:56 -0800 Subject: [PATCH 4/4] (maint) more explicit stdout tests for autosign_command acceptance We were assert_no_matching for the same string in both cases; instead assert_match output for more surety. --- acceptance/tests/ssl/autosign_command.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/acceptance/tests/ssl/autosign_command.rb b/acceptance/tests/ssl/autosign_command.rb index 34989c32447..8513fb03e35 100644 --- a/acceptance/tests/ssl/autosign_command.rb +++ b/acceptance/tests/ssl/autosign_command.rb @@ -15,10 +15,10 @@ def assert_key_generated(name) reset_agent_ssl end - step "Step 1: ensure autosign_command can approve CSRs" - reset_agent_ssl(false) + step "Step 1: ensure autosign_command can approve CSRs" + master_opts = {'master' => {'autosign' => 'false', 'autosign_command' => '/bin/true'}} with_puppet_running_on(master, master_opts) do agents.each do |agent| @@ -26,7 +26,7 @@ def assert_key_generated(name) on(agent, puppet("agent --test --server #{master} --waitforcert 0")) assert_key_generated(agent) - assert_no_match(/failed to retrieve certificate/, stdout, "Expected certificate to be autosigned") + assert_match(/Caching certificate for #{agent}/, stdout, "Expected certificate to be autosigned") end end @@ -41,7 +41,7 @@ def assert_key_generated(name) on(agent, puppet("agent --test --server #{master} --waitforcert 0"), :acceptable_exit_codes => [1]) assert_key_generated(agent) - assert_no_match(/failed to retrieve certificate/, stdout, "Expected certificate to not be autosigned") + assert_match(/no certificate found/, stdout, "Expected certificate to not be autosigned") end end end