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 new file mode 100644 index 00000000000..8513fb03e35 --- /dev/null +++ b/acceptance/tests/ssl/autosign_command.rb @@ -0,0 +1,47 @@ +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 + + testdir = master.tmpdir('autosign_command') + + teardown do + step "Remove autosign configuration" + on(master, host_command("rm -rf '#{testdir}'") ) + reset_agent_ssl + end + + 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| + next if agent == master + + on(agent, puppet("agent --test --server #{master} --waitforcert 0")) + assert_key_generated(agent) + assert_match(/Caching certificate for #{agent}/, stdout, "Expected certificate to be autosigned") + end + end + + reset_agent_ssl(false) + + 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_match(/no certificate found/, stdout, "Expected certificate to not be autosigned") + end + end +end 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: diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb index 462466af402..ca00ac567de 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.rb @@ -714,8 +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