Modifying the CA server so that it will not send back a cert whose public key does not match the csr. We have been getting a lot of instances of this, so this should cut down that problem.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2612 980ebf18-57e1-0310-9a29-db15c13687c0

4 files changed, 43 insertions, 1 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 7658a46e8..8b9c8c7e7 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,7 @@
+    Fixed the CA server so that it refuses to send back a certificate
+    whose public key doesn't match the CSR.  Instead, it tells the
+    user to run 'puppetca --clean'.
+
     Invalid certificates are no longer written to disk (#578).
 
     Added a package provider (appdmg) able to install .app packages
diff --git a/lib/puppet/network/handler/ca.rb b/lib/puppet/network/handler/ca.rb
index 875cfc926..422b21ae1 100644
--- a/lib/puppet/network/handler/ca.rb
+++ b/lib/puppet/network/handler/ca.rb
@@ -104,7 +104,9 @@ class Puppet::Network::Handler
             cert, cacert = ca.getclientcert(hostname)
             if cert and cacert
                 Puppet.info "Retrieving existing certificate for %s" % hostname
-                #Puppet.info "Cert: %s; Cacert: %s" % [cert.class, cacert.class]
+                unless csr.public_key.to_s == cert.public_key.to_s
+                    raise Puppet::Error, "Certificate request does not match existing certificate; run 'puppetca --clean %s'." % hostname
+                end
                 return [cert.to_pem, cacert.to_pem]
             elsif @ca
                 if self.autosign?(hostname) or client.nil?
diff --git a/test/network/client/ca.rb b/test/network/client/ca.rb
index 0fdbda537..26fb72f40 100755
--- a/test/network/client/ca.rb
+++ b/test/network/client/ca.rb
@@ -2,6 +2,7 @@
 
 $:.unshift("../../lib") if __FILE__ =~ /\.rb$/
 
+require 'mocha'
 require 'puppettest'
 require 'puppet/network/client/ca'
 require 'puppet/sslcertificates/support'
@@ -48,6 +49,7 @@ class TestClientCA < Test::Unit::TestCase
         File.unlink(Puppet[:hostprivkey])
 
         @client = Puppet::Network::Client.ca.new :CA => @ca
+        @ca.expects(:getcert).returns("yay") # not a valid cert
         # Now make sure it fails, since we'll get the old cert but have new keys
         assert_raise(Puppet::Network::Client::CA::InvalidCertificate, "Did not fail on invalid cert") do
             @client.request_cert
diff --git a/test/network/handler/ca.rb b/test/network/handler/ca.rb
index fe2fdbd2e..3c89f597b 100755
--- a/test/network/handler/ca.rb
+++ b/test/network/handler/ca.rb
@@ -229,6 +229,40 @@ class TestCA < Test::Unit::TestCase
         # And try a different host
         assert(! caserv.autosign?("other.yay.com"), "Host was autosigned")
     end
+
+    # Make sure that a CSR created with keys that don't match the existing
+    # cert throws an exception on the server.
+    def test_mismatched_public_keys_throws_exception
+        ca = Puppet::Network::Handler.ca.new()
+
+        # First initialize the server
+        client = Puppet::Network::Client.ca.new :CA => ca
+        client.request_cert
+        File.unlink(Puppet[:hostcsr])
+
+        # Now use a different cert name
+        Puppet[:certname] = "my.host.com"
+        client = Puppet::Network::Client.ca.new :CA => ca
+        firstcsr = client.csr
+        File.unlink(Puppet[:hostcsr]) if FileTest.exists?(Puppet[:hostcsr])
+
+        assert_nothing_raised("Could not get cert") do
+            ca.getcert(firstcsr.to_s)
+        end
+
+        # Now get rid of the public key, forcing a new csr
+        File.unlink(Puppet[:hostprivkey])
+
+        client = Puppet::Network::Client.ca.new :CA => ca
+
+        second_csr = client.csr
+
+        assert(firstcsr.to_s != second_csr.to_s, "CSR did not change")
+
+        assert_raise(Puppet::Error, "CA allowed mismatched keys") do
+            ca.getcert(second_csr.to_s)
+        end
+    end
 end
 
 # $Id$