Finishing the interface between the CA and the CRL.

Certificate revocation now works, the CA knows how
to generate the CRL, and the SSL::Host class knows
how to configure the CRL class for indirection.

4 files changed, 262 insertions, 27 deletions

diff --git a/spec/unit/ssl/certificate_authority.rb b/spec/unit/ssl/certificate_authority.rb
index c102d05fe..a4d8568fe 100755
--- a/spec/unit/ssl/certificate_authority.rb
+++ b/spec/unit/ssl/certificate_authority.rb
@@ -40,6 +40,62 @@ describe Puppet::SSL::CertificateAuthority do
         end
     end
 
+    describe "when retrieving the certificate revocation list" do
+        before do
+            Puppet.settings.stubs(:use)
+            Puppet.settings.stubs(:value).returns "whatever"
+
+            Puppet::SSL::CertificateAuthority.any_instance.stubs(:generate_ca_certificate)
+            @ca = Puppet::SSL::CertificateAuthority.new
+        end
+
+        describe "and the CRL is disabled" do
+            it "should return nil when the :cacrl is false" do
+                Puppet.settings.stubs(:value).with(:cacrl).returns false
+
+                Puppet::SSL::CertificateRevocationList.expects(:new).never
+
+                @ca.crl.should be_nil
+            end
+
+            it "should return nil when the :cacrl is 'false'" do
+                Puppet.settings.stubs(:value).with(:cacrl).returns 'false'
+
+                Puppet::SSL::CertificateRevocationList.expects(:new).never
+
+                @ca.crl.should be_nil
+            end
+        end
+
+        describe "and the CRL is enabled" do
+            before do
+                Puppet.settings.stubs(:value).with(:cacrl).returns "/my/crl"
+
+                cert = stub("certificate", :content => "real_cert")
+                @host = stub 'host', :certificate => cert, :name => "hostname"
+
+                @ca.stubs(:host).returns @host
+            end
+
+            it "should return any found CRL instance" do
+                crl = mock 'crl'
+                Puppet::SSL::CertificateRevocationList.expects(:find).returns crl
+                @ca.crl.should equal(crl)
+            end
+
+            it "should create and generate a new CRL instance of no CRL can be found" do
+                crl = mock 'crl'
+                Puppet::SSL::CertificateRevocationList.expects(:find).returns nil
+
+                Puppet::SSL::CertificateRevocationList.expects(:new).returns crl
+
+                crl.expects(:generate).with(@ca.host.certificate.content)
+
+                @ca.crl.should equal(crl)
+            end
+        end
+    end
+
     describe "when generating a self-signed CA certificate" do
         before do
             Puppet.settings.stubs(:use)
@@ -121,11 +177,13 @@ describe Puppet::SSL::CertificateAuthority do
             @factory = stub 'factory', :result => "my real cert"
             Puppet::SSL::CertificateFactory.stubs(:new).returns @factory
 
-            @request = stub 'request', :content => "myrequest"
+            @request = stub 'request', :content => "myrequest", :name => @name
 
             # And the inventory
             @inventory = stub 'inventory', :add => nil
             @ca.stubs(:inventory).returns @inventory
+
+            Puppet::SSL::CertificateRequest.stubs(:destroy)
         end
 
         describe "and calculating the next certificate serial number" do
@@ -275,6 +333,12 @@ describe Puppet::SSL::CertificateAuthority do
                 @cert.expects(:save)
                 @ca.sign(@name)
             end
+
+            it "should remove the host's certificate request" do
+                Puppet::SSL::CertificateRequest.expects(:destroy).with(@name)
+
+                @ca.sign(@name)
+            end
         end
 
         it "should create a certificate instance with the content set to the newly signed x509 certificate" do
@@ -315,16 +379,169 @@ describe Puppet::SSL::CertificateAuthority do
             Puppet::SSL::CertificateAuthority.any_instance.stubs(:key).returns @key
             @cacert = mock 'certificate'
             @cacert.stubs(:content).returns "cacertificate"
-            Puppet::SSL::CertificateAuthority.any_instance.stubs(:certificate).returns @cacert
             @ca = Puppet::SSL::CertificateAuthority.new
         end
+        
+        it "should be able to list waiting certificate requests" do
+            req1 = stub 'req1', :name => "one"
+            req2 = stub 'req2', :name => "two"
+            Puppet::SSL::CertificateRequest.expects(:search).with("*").returns [req1, req2]
+
+            @ca.waiting?.should == %w{one two}
+        end
+        
+        it "should delegate removing hosts to the Host class" do
+            Puppet::SSL::Host.expects(:destroy).with("myhost")
+
+            @ca.destroy("myhost")
+        end
+
+        it "should be able to verify certificates" do
+            @ca.should respond_to(:verify)
+        end
+
+        describe "and verifying certificates" do
+            before do
+                @store = stub 'store', :verify => true, :add_file => nil, :purpose= => nil, :add_crl => true
+
+                OpenSSL::X509::Store.stubs(:new).returns @store
+
+                Puppet.settings.stubs(:value).returns "crtstuff"
+
+                @cert = stub 'cert', :content => "mycert"
+                Puppet::SSL::Certificate.stubs(:find).returns @cert
+
+                @crl = mock('crl')
+
+                @ca.stubs(:crl).returns @crl
+            end
+
+            it "should fail if the host's certificate cannot be found" do
+                Puppet::SSL::Certificate.expects(:find).with("me").returns(nil)
+
+                lambda { @ca.verify("me") }.should raise_error(ArgumentError)
+            end
+
+            it "should create an SSL Store to verify" do
+                OpenSSL::X509::Store.expects(:new).returns @store
+
+                @ca.verify("me")
+            end
+
+            it "should add the CA Certificate to the store" do
+                Puppet.settings.stubs(:value).with(:cacert).returns "/ca/cert"
+                @store.expects(:add_file).with "/ca/cert"
+
+                @ca.verify("me")
+            end
+
+            it "should add the CRL to the store if the crl is enabled" do
+                @store.expects(:add_crl).with @crl
+
+                @ca.verify("me")
+            end
+
+            it "should set the store purpose to OpenSSL::X509::PURPOSE_SSL_CLIENT" do
+                Puppet.settings.stubs(:value).with(:cacert).returns "/ca/cert"
+                @store.expects(:add_file).with "/ca/cert"
+
+                @ca.verify("me")
+            end
+
+            it "should use the store to verify the certificate" do
+                @cert.expects(:content).returns "mycert"
+
+                @store.expects(:verify).with("mycert").returns true
+
+                @ca.verify("me")
+            end
+
+            it "should fail if the verification returns false" do
+                @cert.expects(:content).returns "mycert"
+
+                @store.expects(:verify).with("mycert").returns false
+
+                lambda { @ca.verify("me") }.should raise_error
+            end
+        end
+
+        describe "and revoking certificates" do
+            before do
+                @crl = mock 'crl'
+                @ca.stubs(:crl).returns @crl
+            end
+
+            it "should fail if the certificate revocation list is disabled" do
+                @ca.stubs(:crl).returns false
+
+                lambda { @ca.revoke('whatever') }.should raise_error(ArgumentError)
+
+            end
+
+            it "should delegate the revocation to its CRL" do
+                @ca.crl.expects(:revoke)
+
+                @ca.revoke('host')
+            end
+
+            it "should get the serial number from the local certificate if it exists" do
+                real_cert = stub 'real_cert', :serial => 15
+                cert = stub 'cert', :content => real_cert
+                Puppet::SSL::Certificate.expects(:find).with("host").returns cert
+
+                @ca.crl.expects(:revoke).with { |serial, key| serial == 15 }
+
+                @ca.revoke('host')
+            end
+
+            it "should get the serial number from inventory if no local certificate exists" do
+                real_cert = stub 'real_cert', :serial => 15
+                cert = stub 'cert', :content => real_cert
+                Puppet::SSL::Certificate.expects(:find).with("host").returns nil
+
+                @ca.inventory.expects(:serial).with("host").returns 16
+
+                @ca.crl.expects(:revoke).with { |serial, key| serial == 16 }
+                @ca.revoke('host')
+            end
+        end
 
-        describe "when revoking certificates" do
-            it "should fail if the certificate revocation list is disabled"
+        it "should be able to generate a complete new SSL host" do
+            @ca.should respond_to(:generate)
+        end
 
-            it "should default to OpenSSL::OCSP::REVOKED_STATUS_KEYCOMPROMISE as the reason"
+        describe "and generating certificates" do
+            before do
+                @host = stub 'host', :generate_certificate_request => nil
+                Puppet::SSL::Host.stubs(:new).returns @host
+                Puppet::SSL::Certificate.stubs(:find).returns nil
 
-            it "should require a serial number"
+                @ca.stubs(:sign)
+            end
+
+            it "should fail if a certificate already exists for the host" do
+                Puppet::SSL::Certificate.expects(:find).with("him").returns "something"
+
+                lambda { @ca.generate("him") }.should raise_error(ArgumentError)
+            end
+
+            it "should create a new Host instance with the correct name" do
+                Puppet::SSL::Host.expects(:new).with("him").returns @host
+
+                @ca.generate("him")
+            end
+
+            it "should use the Host to generate the certificate request" do
+                @host.expects :generate_certificate_request
+
+                @ca.generate("him")
+            end
+
+            it "should sign the generated request" do
+                @ca.expects(:sign).with("him")
+
+                @ca.generate("him")
+            end
         end
     end
 end
diff --git a/spec/unit/ssl/certificate_revocation_list.rb b/spec/unit/ssl/certificate_revocation_list.rb
index 3513607d9..042a12e15 100755
--- a/spec/unit/ssl/certificate_revocation_list.rb
+++ b/spec/unit/ssl/certificate_revocation_list.rb
@@ -19,7 +19,7 @@ describe Puppet::SSL::CertificateRevocationList do
         before do
             @class.any_instance.stubs(:read_or_generate)
 
-            @crl = @class.new
+            @crl = @class.new("whatever")
         end
 
         it "should always use 'crl' for its name" do
@@ -34,12 +34,12 @@ describe Puppet::SSL::CertificateRevocationList do
     describe "when initializing" do
         it "should fail if :cacrl is set to false" do
             Puppet.settings.expects(:value).with(:cacrl).returns false
-            lambda { @class.new }.should raise_error(Puppet::Error)
+            lambda { @class.new("crl") }.should raise_error(Puppet::Error)
         end
 
         it "should fail if :cacrl is set to the string 'false'" do
             Puppet.settings.expects(:value).with(:cacrl).returns "false"
-            lambda { @class.new }.should raise_error(Puppet::Error)
+            lambda { @class.new("crl") }.should raise_error(Puppet::Error)
         end
     end
 
@@ -52,7 +52,7 @@ describe Puppet::SSL::CertificateRevocationList do
 
             @class.any_instance.stubs(:read_or_generate)
 
-            @crl = @class.new
+            @crl = @class.new("crl")
         end
 
         it "should set its issuer to the subject of the passed certificate" do
@@ -89,7 +89,7 @@ describe Puppet::SSL::CertificateRevocationList do
         before do
             @class.wrapped_class.any_instance.stubs(:issuer=)
 
-            @crl = @class.new
+            @crl = @class.new("crl")
             @crl.generate(@cert)
             @crl.content.stubs(:sign)
 
diff --git a/spec/unit/ssl/host.rb b/spec/unit/ssl/host.rb
index 743d1dcef..d3756a016 100755
--- a/spec/unit/ssl/host.rb
+++ b/spec/unit/ssl/host.rb
@@ -43,7 +43,7 @@ describe Puppet::SSL::Host do
 
     describe "when specifying the CA location" do
         before do
-            [Puppet::SSL::Key, Puppet::SSL::Certificate, Puppet::SSL::CertificateRequest].each do |klass|
+            [Puppet::SSL::Key, Puppet::SSL::Certificate, Puppet::SSL::CertificateRequest, Puppet::SSL::CertificateRevocationList].each do |klass|
                 klass.stubs(:terminus_class=)
                 klass.stubs(:cache_class=)
             end
@@ -66,9 +66,10 @@ describe Puppet::SSL::Host do
         end
 
         describe "as 'local'" do
-            it "should set the cache class for Certificate and CertificateRequest as :file" do
+            it "should set the cache class for Certificate, CertificateRevocationList, and CertificateRequest as :file" do
                 Puppet::SSL::Certificate.expects(:cache_class=).with :file
                 Puppet::SSL::CertificateRequest.expects(:cache_class=).with :file
+                Puppet::SSL::CertificateRevocationList.expects(:cache_class=).with :file
 
                 Puppet::SSL::Host.ca_location = :local
             end
@@ -79,18 +80,20 @@ describe Puppet::SSL::Host do
                 Puppet::SSL::Host.ca_location = :local
             end
 
-            it "should set the terminus class for Certificate and CertificateRequest as :ca_file" do
+            it "should set the terminus class for Certificate, CertificateRevocationList, and CertificateRequest as :ca_file" do
                 Puppet::SSL::Certificate.expects(:terminus_class=).with :ca_file
                 Puppet::SSL::CertificateRequest.expects(:terminus_class=).with :ca_file
+                Puppet::SSL::CertificateRevocationList.expects(:terminus_class=).with :ca_file
 
                 Puppet::SSL::Host.ca_location = :local
             end
         end
 
         describe "as 'remote'" do
-            it "should set the cache class for Certificate and CertificateRequest as :file" do
+            it "should set the cache class for Certificate, CertificateRevocationList, and CertificateRequest as :file" do
                 Puppet::SSL::Certificate.expects(:cache_class=).with :file
                 Puppet::SSL::CertificateRequest.expects(:cache_class=).with :file
+                Puppet::SSL::CertificateRevocationList.expects(:cache_class=).with :file
 
                 Puppet::SSL::Host.ca_location = :remote
             end
@@ -101,29 +104,21 @@ describe Puppet::SSL::Host do
                 Puppet::SSL::Host.ca_location = :remote
             end
 
-            it "should set the terminus class for Certificate and CertificateRequest as :rest" do
+            it "should set the terminus class for Certificate, CertificateRevocationList, and CertificateRequest as :rest" do
                 Puppet::SSL::Certificate.expects(:terminus_class=).with :rest
                 Puppet::SSL::CertificateRequest.expects(:terminus_class=).with :rest
+                Puppet::SSL::CertificateRevocationList.expects(:terminus_class=).with :rest
 
                 Puppet::SSL::Host.ca_location = :remote
             end
         end
 
-        describe "as 'only'" do
-            it "should set the terminus class for Key, Certificate, and CertificateRequest as :ca_file" do
-                Puppet::SSL::Key.expects(:terminus_class=).with :ca_file
-                Puppet::SSL::Certificate.expects(:terminus_class=).with :ca_file
-                Puppet::SSL::CertificateRequest.expects(:terminus_class=).with :ca_file
-
-                Puppet::SSL::Host.ca_location = :only
-            end
-        end
-
         describe "as 'none'" do
-            it "should set the terminus class for Key, Certificate, and CertificateRequest as :file" do
+            it "should set the terminus class for Key, Certificate, CertificateRevocationList, and CertificateRequest as :file" do
                 Puppet::SSL::Key.expects(:terminus_class=).with :file
                 Puppet::SSL::Certificate.expects(:terminus_class=).with :file
                 Puppet::SSL::CertificateRequest.expects(:terminus_class=).with :file
+                Puppet::SSL::CertificateRevocationList.expects(:terminus_class=).with :file
 
                 Puppet::SSL::Host.ca_location = :none
             end
diff --git a/spec/unit/ssl/inventory.rb b/spec/unit/ssl/inventory.rb
index 10eb42d95..bf1dbfb48 100755
--- a/spec/unit/ssl/inventory.rb
+++ b/spec/unit/ssl/inventory.rb
@@ -153,5 +153,28 @@ describe Puppet::SSL::Inventory do
                 @inventory.format(@cert).should =~ /^0x.+mycert$/
             end
         end
+
+        it "should be able to find a given host's serial number" do
+            @inventory.should respond_to(:serial)
+        end
+
+        describe "and finding a serial number" do
+            it "should return nil if the inventory file is missing" do
+                FileTest.expects(:exist?).with("/inven/tory").returns false
+                @inventory.serial(:whatever).should be_nil
+            end
+
+            it "should return the serial number from the line matching the provided name" do
+                File.expects(:readlines).with("/inven/tory").returns ["0x00f blah blah /CN=me\n", "0x001 blah blah /CN=you\n"]
+
+                @inventory.serial("me").should == 15
+            end
+
+            it "should return the number as an integer" do
+                File.expects(:readlines).with("/inven/tory").returns ["0x00f blah blah /CN=me\n", "0x001 blah blah /CN=you\n"]
+
+                @inventory.serial("me").should == 15
+            end
+        end
     end
 end