(#3360) Add an allow_duplicate_certs option

If this option is true, a certificate request with the same CN as an existing
certificate will override the existing certificate when signed. With the option
false, the new certificate request will be rejected. This option will default
to false.

Paired-With: Max Martin

3 files changed, 55 insertions, 0 deletions

diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 989ef3f35..76c40824c 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -353,6 +353,8 @@ module Puppet
         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."},
+    :allow_duplicate_certs => [false, "Whether to allow a new certificate
+      request to overwrite an existing certificate."],
     :ca_days => ["", "How long a certificate should be valid.
       This parameter is deprecated, use ca_ttl instead"],
     :ca_ttl => ["5y", "The default TTL for new certificates; valid values
diff --git a/lib/puppet/indirector/certificate_request/ca.rb b/lib/puppet/indirector/certificate_request/ca.rb
index f4c924fe1..5d76ee52a 100644
--- a/lib/puppet/indirector/certificate_request/ca.rb
+++ b/lib/puppet/indirector/certificate_request/ca.rb
@@ -7,6 +7,14 @@ class Puppet::SSL::CertificateRequest::Ca < Puppet::Indirector::SslFile
   store_in :csrdir
 
   def save(request)
+    if host = Puppet::SSL::Host.indirection.find(request.key)
+      if Puppet[:allow_duplicate_certs]
+        Puppet.notice "#{request.key} already has a #{host.state} certificate; new certificate will overwrite it"
+      else
+        raise "#{request.key} already has a #{host.state} certificate; ignoring certificate request"
+      end
+    end
+
     result = super
     Puppet.notice "#{request.key} has a waiting certificate request"
     result
diff --git a/spec/unit/indirector/certificate_request/ca_spec.rb b/spec/unit/indirector/certificate_request/ca_spec.rb
index 8c25e40f7..38d8a1bb1 100755
--- a/spec/unit/indirector/certificate_request/ca_spec.rb
+++ b/spec/unit/indirector/certificate_request/ca_spec.rb
@@ -5,9 +5,28 @@
 
 require File.expand_path(File.dirname(__FILE__) + '/../../../spec_helper')
 
+require 'puppet/ssl/host'
+require 'puppet/sslcertificates'
+require 'puppet/sslcertificates/ca'
 require 'puppet/indirector/certificate_request/ca'
 
 describe Puppet::SSL::CertificateRequest::Ca do
+  include PuppetSpec::Files
+
+  before :each do
+    Puppet[:ssldir] = tmpdir('ssl')
+
+    Puppet::SSL::Host.ca_location = :local
+    Puppet[:localcacert] = Puppet[:cacert]
+    Puppet::SSLCertificates::CA.new.mkrootcert
+
+    @ca = Puppet::SSL::CertificateAuthority.new
+  end
+
+  after :all do
+    Puppet::SSL::Host.ca_location = :none
+  end
+
   it "should have documentation" do
     Puppet::SSL::CertificateRequest::Ca.doc.should be_instance_of(String)
   end
@@ -16,4 +35,30 @@ describe Puppet::SSL::CertificateRequest::Ca do
     Puppet.settings.expects(:value).with(:csrdir).returns "/request/dir"
     Puppet::SSL::CertificateRequest::Ca.collection_directory.should == "/request/dir"
   end
+
+  it "should overwrite the previous certificate request if allow_duplicate_certs is true" do
+    Puppet[:allow_duplicate_certs] = true
+    host = Puppet::SSL::Host.new("foo")
+    host.generate_certificate_request
+    @ca.sign(host.name)
+
+    Puppet::SSL::Host.indirection.find("foo").generate_certificate_request
+
+    Puppet::SSL::Certificate.indirection.find("foo").name.should == "foo"
+    Puppet::SSL::CertificateRequest.indirection.find("foo").name.should == "foo"
+    Puppet::SSL::Host.indirection.find("foo").state.should == "requested"
+  end
+
+  it "should reject a new certificate request if allow_duplicate_certs is false" do
+    Puppet[:allow_duplicate_certs] = false
+    host = Puppet::SSL::Host.new("bar")
+    host.generate_certificate_request
+    @ca.sign(host.name)
+
+    expect { Puppet::SSL::Host.indirection.find("bar").generate_certificate_request }.should raise_error(/ignoring certificate request/)
+
+    Puppet::SSL::Certificate.indirection.find("bar").name.should == "bar"
+    Puppet::SSL::CertificateRequest.indirection.find("bar").should be_nil
+    Puppet::SSL::Host.indirection.find("bar").state.should == "signed"
+  end
 end