Fixing #578 -- Invalid certs are no longer written to disk.

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

3 files changed, 32 insertions, 3 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 051a5ebc0..7658a46e8 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,5 @@
+    Invalid certificates are no longer written to disk (#578).
+
     Added a package provider (appdmg) able to install .app packages
     on .dmg files on OS X (#641).
 
diff --git a/lib/puppet/network/client/ca.rb b/lib/puppet/network/client/ca.rb
index 50d761ccf..412c9c59f 100644
--- a/lib/puppet/network/client/ca.rb
+++ b/lib/puppet/network/client/ca.rb
@@ -34,8 +34,6 @@ class Puppet::Network::Client::CA < Puppet::Network::Client
         if cert.nil? or cert == ""
             return nil
         end
-        Puppet.config.write(:hostcert) do |f| f.print cert end
-        Puppet.config.write(:localcacert) do |f| f.print cacert end
 
         begin
             @cert = OpenSSL::X509::Certificate.new(cert)
@@ -47,8 +45,13 @@ class Puppet::Network::Client::CA < Puppet::Network::Client
         end
 
         unless @cert.check_private_key(key)
-            raise InvalidCertificate, "Certificate does not match private key"
+            raise InvalidCertificate, "Certificate does not match private key.  Try 'puppetca --clean %s' on the server." % Facter.value(:fqdn)
         end
+
+        # Only write the cert out if it passes validating.
+        Puppet.config.write(:hostcert) do |f| f.print cert end
+        Puppet.config.write(:localcacert) do |f| f.print cacert end
+
         return @cert
     end
 end
diff --git a/test/network/client/ca.rb b/test/network/client/ca.rb
index bdae140ef..0fdbda537 100755
--- a/test/network/client/ca.rb
+++ b/test/network/client/ca.rb
@@ -33,6 +33,30 @@ class TestClientCA < Test::Unit::TestCase
             client = Puppet::Network::Client.ca.new
         end
     end
+
+    # #578
+    def test_invalid_certs_are_not_written
+        # Run the get once, which should be valid
+
+        assert_nothing_raised("Could not get a certificate") do
+            @client.request_cert
+        end
+
+        # Now remove the cert and keys, so we get a broken cert
+        File.unlink(Puppet[:hostcert])
+        File.unlink(Puppet[:localcacert])
+        File.unlink(Puppet[:hostprivkey])
+
+        @client = Puppet::Network::Client.ca.new :CA => @ca
+        # 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
+        end
+
+        # And then make sure the cert isn't written to disk
+        assert(! FileTest.exists?(Puppet[:hostcert]),
+            "Invalid cert got written to disk")
+    end
 end
 
 # $Id$