From 0dc2dbafe65b59bfbb3ab66e26f595260bdde356 Mon Sep 17 00:00:00 2001 From: Markus Roberts Date: Wed, 16 Dec 2009 16:26:05 -0800 Subject: 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 --- spec/unit/ssl/host.rb | 76 +++++++++++++-------------------------------------- 1 file changed, 19 insertions(+), 57 deletions(-) (limited to 'spec/unit/ssl') 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 -- cgit