Fix for #2890 (the cached certificates that would not die)

This patch implements the two-part suggestion from the ticket;

1) a client that receives a certificate that doesn't match its current
private key does not accept, store or use the certificate--instead it
removes any locally cached copies and acts as if the certificate had
never been found.

2) a puppetmaster that receives a csr from a client for whom it already
has a signed certificate now honors the request and considers it to
supercede any previously signed certificates.

In order to make the cache expiration work as expected, I changed a few
assumptions in the caching system:

* The expiration of a cached certificate is the earlier of the envelope
  expiration and the certificate's expiration, as opposed to just overriding
  the cache value
* Telling the cache to expire an item now removes it from the cache if
  possible, rather than just setting an expiration date in the past and
  hoping that somebody notices.

Signed-off-by: Markus Roberts <Markus@reality.com>

1 files changed, 19 insertions, 57 deletions

diff --git a/spec/unit/ssl/host.rb b/spec/unit/ssl/host.rb
index 4e4d3f570..9174f2e49 100755
--- a/spec/unit/ssl/host.rb
+++ b/spec/unit/ssl/host.rb
@@ -90,55 +90,6 @@ describe Puppet::SSL::Host do
         Puppet::SSL::Host.localhost.should equal(two)
     end
 
-    it "should be able to verify its certificate matches its key" do
-        Puppet::SSL::Host.new("foo").should respond_to(:certificate_matches_key?)
-    end
-
-    it "should consider the certificate invalid if it cannot find a key" do
-        host = Puppet::SSL::Host.new("foo")
-        host.expects(:key).returns nil
-
-        host.should_not be_certificate_matches_key
-    end
-
-    it "should consider the certificate invalid if it cannot find a certificate" do
-        host = Puppet::SSL::Host.new("foo")
-        host.expects(:key).returns mock("key")
-        host.expects(:certificate).returns nil
-
-        host.should_not be_certificate_matches_key
-    end
-
-    it "should consider the certificate invalid if the SSL certificate's key verification fails" do
-        host = Puppet::SSL::Host.new("foo")
-
-        key = mock 'key', :content => "private_key"
-        sslcert = mock 'sslcert'
-        certificate = mock 'cert', :content => sslcert
-
-        host.stubs(:key).returns key
-        host.stubs(:certificate).returns certificate
-
-        sslcert.expects(:check_private_key).with("private_key").returns false
-
-        host.should_not be_certificate_matches_key
-    end
-
-    it "should consider the certificate valid if the SSL certificate's key verification succeeds" do
-        host = Puppet::SSL::Host.new("foo")
-
-        key = mock 'key', :content => "private_key"
-        sslcert = mock 'sslcert'
-        certificate = mock 'cert', :content => sslcert
-
-        host.stubs(:key).returns key
-        host.stubs(:certificate).returns certificate
-
-        sslcert.expects(:check_private_key).with("private_key").returns true
-
-        host.should be_certificate_matches_key
-    end
-
     describe "when specifying the CA location" do
         before do
             [Puppet::SSL::Key, Puppet::SSL::Certificate, Puppet::SSL::CertificateRequest, Puppet::SSL::CertificateRevocationList].each do |klass|
@@ -408,10 +359,11 @@ describe Puppet::SSL::Host do
     describe "when managing its certificate" do
         before do
             @realcert = mock 'certificate'
-            @cert = stub 'cert', :content => @realcert
+            @realcert.stubs(:check_private_key).with('private key').returns true
+
+            @cert = stub 'cert', :content => @realcert, :expired? => false
 
-            @host.stubs(:key).returns mock("key")
-            @host.stubs(:certificate_matches_key?).returns true
+            @host.stubs(:key).returns stub("key",:content => 'private key' )
         end
 
         it "should find the CA certificate if it does not have a certificate" do
@@ -459,12 +411,22 @@ describe Puppet::SSL::Host do
             @host.certificate.should equal(@cert)
         end
 
-        it "should fail if the found certificate does not match the private key" do
-            @host.expects(:certificate_matches_key?).returns false
+        it "should immediately expire the cached copy if the found certificate does not match the private key" do
+            @realcert.expects(:check_private_key).with('private key').returns false
 
             Puppet::SSL::Certificate.stubs(:find).returns @cert
+            Puppet::SSL::Certificate.expects(:expire).with("myname")
 
-            lambda { @host.certificate }.should raise_error(Puppet::Error)
+            @host.certificate
+        end
+
+        it "should not return a certificate if it does not match the private key" do
+            @realcert.expects(:check_private_key).with('private key').returns false
+
+            Puppet::SSL::Certificate.stubs(:find).returns @cert
+            Puppet::SSL::Certificate.stubs(:expire).with("myname")
+
+            @host.certificate.should == nil
         end
 
         it "should return any previously found certificate" do
@@ -654,14 +616,14 @@ describe Puppet::SSL::Host do
 
         it "should catch and log errors during CSR saving" do
             @host.expects(:certificate).times(2).returns(nil).then.returns "foo"
-            @host.expects(:generate).raises(RuntimeError)
+            @host.expects(:generate).raises(RuntimeError).then.returns nil
             @host.stubs(:sleep)
             @host.wait_for_cert(1)
         end
 
         it "should sleep and retry after failures saving the CSR if waitforcert is enabled" do
             @host.expects(:certificate).times(2).returns(nil).then.returns "foo"
-            @host.expects(:generate).raises(RuntimeError)
+            @host.expects(:generate).raises(RuntimeError).then.returns nil
             @host.expects(:sleep).with(1)
             @host.wait_for_cert(1)
         end