Changing all of the SSL terminus classes to treat CA files specially.

This is a kind of weird design situation.  For instance, we've got a
collection of certificates in the :certdir, but then there's a special
CA certificate off by itself.  Rather than build a whole separate
infrastructure for managing those separate files (cert and key, at least),
I decided to add special support for specifying where to find the CA-specific
bits, and then code for handling them when necessary.

This requires that we have a standard way of knowing whether we should be
managing the CA bits or normal host files.  The Puppet::SSL::Host class now has
a 'ca_name' method that returns the string we're using for the CA name; this
name is currently 'ca'.  We have to use a name, because the name is the only
thing that all methods have access to (e.g., when trying to 'find' the right
cert, we only have the name available).

What this means is that if you want access to the CA key or cert, then create
a Puppet::SSL::Host instance with the name 'ca'.

You'll still get the CA cert created with the host's :certname; it will just
be stored in a different location.

7 files changed, 258 insertions, 120 deletions

diff --git a/spec/unit/indirector/certificate/file.rb b/spec/unit/indirector/certificate/file.rb
index 18fe9a1c3..ffaf12047 100755
--- a/spec/unit/indirector/certificate/file.rb
+++ b/spec/unit/indirector/certificate/file.rb
@@ -16,4 +16,13 @@ describe Puppet::SSL::Certificate::File do
         Puppet.settings.expects(:value).with(:certdir).returns "/cert/dir"
         Puppet::SSL::Certificate::File.collection_directory.should == "/cert/dir"
     end
+
+    it "should store the ca certificate at the :cacert location" do
+        Puppet.settings.stubs(:use)
+        Puppet.settings.stubs(:value).returns "whatever"
+        Puppet.settings.stubs(:value).with(:cacert).returns "/ca/cert"
+        file = Puppet::SSL::Certificate::File.new
+        file.stubs(:ca?).returns true
+        file.path("whatever").should == "/ca/cert"
+    end
 end
diff --git a/spec/unit/indirector/certificate_revocation_list/ca_file.rb b/spec/unit/indirector/certificate_revocation_list/ca_file.rb
new file mode 100755
index 000000000..fa5379dfa
--- /dev/null
+++ b/spec/unit/indirector/certificate_revocation_list/ca_file.rb
@@ -0,0 +1,21 @@
+#!/usr/bin/env ruby
+#
+#  Created by Luke Kanies on 2008-3-7.
+#  Copyright (c) 2007. All rights reserved.
+
+require File.dirname(__FILE__) + '/../../../spec_helper'
+
+require 'puppet/indirector/certificate_revocation_list/ca_file'
+
+describe Puppet::SSL::CertificateRevocationList::CaFile do
+    it "should have documentation" do
+        Puppet::SSL::CertificateRevocationList::CaFile.doc.should be_instance_of(String)
+    end
+
+    it "should use the :cacrl setting as the crl location" do
+        Puppet.settings.stubs(:value).returns "whatever"
+        Puppet.settings.stubs(:use)
+        Puppet.settings.stubs(:value).with(:cacrl).returns "/request/dir"
+        Puppet::SSL::CertificateRevocationList::CaFile.new.path("whatever").should == "/request/dir"
+    end
+end
diff --git a/spec/unit/indirector/certificate_revocation_list/file.rb b/spec/unit/indirector/certificate_revocation_list/file.rb
new file mode 100755
index 000000000..5db4f8c06
--- /dev/null
+++ b/spec/unit/indirector/certificate_revocation_list/file.rb
@@ -0,0 +1,20 @@
+#!/usr/bin/env ruby
+#
+#  Created by Luke Kanies on 2008-3-7.
+#  Copyright (c) 2007. All rights reserved.
+
+require File.dirname(__FILE__) + '/../../../spec_helper'
+
+require 'puppet/indirector/certificate_revocation_list/file'
+
+describe Puppet::SSL::CertificateRevocationList::File do
+    it "should have documentation" do
+        Puppet::SSL::CertificateRevocationList::File.doc.should be_instance_of(String)
+    end
+
+    it "should always store the file to :hostcrl location" do
+        Puppet.settings.expects(:value).with(:hostcrl).returns "/host/crl"
+        Puppet.settings.stubs(:use)
+        Puppet::SSL::CertificateRevocationList::File.file_location.should == "/host/crl"
+    end
+end
diff --git a/spec/unit/indirector/key/file.rb b/spec/unit/indirector/key/file.rb
index a7297d522..8a1cb04bd 100755
--- a/spec/unit/indirector/key/file.rb
+++ b/spec/unit/indirector/key/file.rb
@@ -17,18 +17,49 @@ describe Puppet::SSL::Key::File do
         Puppet::SSL::Key::File.collection_directory.should == "/key/dir"
     end
 
-    describe "when managing private keys" do
-        before do
-            @private = "/private/key/dir"
-            @public = "/public/key/dir"
-            Puppet.settings.stubs(:value).with(:privatekeydir).returns @private
-            Puppet.settings.stubs(:value).with(:publickeydir).returns @public
+    it "should store the ca key at the :cakey location" do
+        Puppet.settings.stubs(:use)
+        Puppet.settings.stubs(:value).returns "whatever"
+        Puppet.settings.stubs(:value).with(:cakey).returns "/ca/key"
+        file = Puppet::SSL::Key::File.new
+        file.stubs(:ca?).returns true
+        file.path("whatever").should == "/ca/key"
+    end
+
+    describe "when choosing the path for the public key" do
+        it "should use the :capub setting location if the key is for the certificate authority" do
+            Puppet.settings.stubs(:value).returns "/fake/dir"
+            Puppet.settings.stubs(:value).with(:capub).returns "/ca/pubkey"
             Puppet.settings.stubs(:use)
 
             @searcher = Puppet::SSL::Key::File.new
+            @searcher.stubs(:ca?).returns true
+            @searcher.public_key_path("whatever").should == "/ca/pubkey"
+        end
+
+        it "should use the host name plus '.pem' in :publickeydir for normal hosts" do
+            Puppet.settings.stubs(:value).with(:privatekeydir).returns "/private/key/dir"
+            Puppet.settings.stubs(:value).with(:publickeydir).returns "/public/key/dir"
+            Puppet.settings.stubs(:use)
+
+            @searcher = Puppet::SSL::Key::File.new
+            @searcher.stubs(:ca?).returns false
+            @searcher.public_key_path("whatever").should == "/public/key/dir/whatever.pem"
+        end
+    end
+
+    describe "when managing private keys" do
+        before do
+            @searcher = Puppet::SSL::Key::File.new
+
+            @private_key_path = File.join("/fake/key/path")
+            @public_key_path = File.join("/other/fake/key/path")
 
-            @privatekey = File.join(@private, "myname" + ".pem")
-            @publickey = File.join(@public, "myname" + ".pem")
+            @searcher.stubs(:public_key_path).returns @public_key_path
+            @searcher.stubs(:path).returns @private_key_path
+
+            FileTest.stubs(:directory?).returns true
+            FileTest.stubs(:writable?).returns true
 
             @public_key = stub 'public_key'
             @real_key = stub 'sslkey', :public_key => @public_key
@@ -39,14 +70,11 @@ describe Puppet::SSL::Key::File do
         end
 
         it "should save the public key when saving the private key" do
-            FileTest.stubs(:directory?).returns true
-            FileTest.stubs(:writable?).returns true
-
-            File.stubs(:open).with(@privatekey, "w")
+            File.stubs(:open).with(@private_key_path, "w")
 
             fh = mock 'filehandle'
 
-            File.expects(:open).with(@publickey, "w").yields fh
+            File.expects(:open).with(@public_key_path, "w").yields fh
             @public_key.expects(:to_pem).returns "my pem"
 
             fh.expects(:print).with "my pem"
@@ -55,20 +83,20 @@ describe Puppet::SSL::Key::File do
         end
 
         it "should destroy the public key when destroying the private key" do
-            File.stubs(:unlink).with(@privatekey)
-            FileTest.stubs(:exist?).with(@privatekey).returns true
-            FileTest.expects(:exist?).with(@publickey).returns true
-            File.expects(:unlink).with(@publickey)
+            File.stubs(:unlink).with(@private_key_path)
+            FileTest.stubs(:exist?).with(@private_key_path).returns true
+            FileTest.expects(:exist?).with(@public_key_path).returns true
+            File.expects(:unlink).with(@public_key_path)
 
             @searcher.destroy(@request)
         end
 
         it "should not fail if the public key does not exist when deleting the private key" do
-            File.stubs(:unlink).with(@privatekey)
+            File.stubs(:unlink).with(@private_key_path)
 
-            FileTest.stubs(:exist?).with(@privatekey).returns true
-            FileTest.expects(:exist?).with(@publickey).returns false
-            File.expects(:unlink).with(@publickey).never
+            FileTest.stubs(:exist?).with(@private_key_path).returns true
+            FileTest.expects(:exist?).with(@public_key_path).returns false
+            File.expects(:unlink).with(@public_key_path).never
 
             @searcher.destroy(@request)
         end
diff --git a/spec/unit/indirector/ssl_file.rb b/spec/unit/indirector/ssl_file.rb
index 2671f964f..aed2a8769 100755
--- a/spec/unit/indirector/ssl_file.rb
+++ b/spec/unit/indirector/ssl_file.rb
@@ -20,7 +20,9 @@ describe Puppet::Indirector::SslFile do
         @setting = :mydir
         @file_class.store_in @setting
         @path = "/my/directory"
-        Puppet.settings.stubs(:[]).with(@setting).returns(@path)
+        Puppet.settings.stubs(:value).returns "stubbed_setting"
+        Puppet.settings.stubs(:value).with(@setting).returns(@path)
+        Puppet.settings.stubs(:value).with(:trace).returns(false)
     end
 
     it "should use ssl upon initialization" do
@@ -28,9 +30,20 @@ describe Puppet::Indirector::SslFile do
         @file_class.new
     end
 
-    it "should fail if no store directory has been set" do
+    it "should return a nil collection directory if no directory setting has been provided" do
         @file_class.store_in nil
-        lambda { @file_class.collection_directory }.should raise_error(Puppet::DevError)
+        @file_class.collection_directory.should be_nil
+    end
+
+    it "should return a nil file location if no location has been provided" do
+        @file_class.store_at nil
+        @file_class.file_location.should be_nil
+    end
+
+    it "should fail if no store directory or file location has been set" do
+        @file_class.store_in nil
+        @file_class.store_at nil
+        lambda { @file_class.new }.should raise_error(Puppet::DevError)
     end
 
     describe "when managing ssl files" do
@@ -43,9 +56,34 @@ describe Puppet::Indirector::SslFile do
 
             @request = stub 'request', :key => @cert.name, :instance => @cert
         end
+
+        it "should consider the file a ca file if the name is equal to what the SSL::Host class says is the CA name" do
+            Puppet::SSL::Host.expects(:ca_name).returns "amaca"
+            @searcher.should be_ca("amaca")
+        end
         
         describe "when choosing the location for certificates" do
-            it "should set them in the setting directory, with the certificate name plus '.pem'" do
+            it "should set them at the ca setting's path if a ca setting is available and the name resolves to the CA name" do
+                @file_class.store_in nil
+                @file_class.store_at :mysetting
+                @file_class.store_ca_at :casetting
+
+                Puppet.settings.stubs(:value).with(:casetting).returns "/ca/file"
+
+                @searcher.expects(:ca?).with(@cert.name).returns true
+                @searcher.path(@cert.name).should == "/ca/file"
+            end
+
+            it "should set them at the file location if a file setting is available" do
+                @file_class.store_in nil
+                @file_class.store_at :mysetting
+
+                Puppet.settings.stubs(:value).with(:mysetting).returns "/some/file"
+
+                @searcher.path(@cert.name).should == "/some/file"
+            end
+
+            it "should set them in the setting directory, with the certificate name plus '.pem', if a directory setting is available" do
                 @searcher.path(@cert.name).should == @certpath
             end
         end
@@ -79,6 +117,11 @@ describe Puppet::Indirector::SslFile do
         end
 
         describe "when saving certificates to disk" do
+            before do
+                FileTest.stubs(:directory?).returns true
+                FileTest.stubs(:writable?).returns true
+            end
+
             it "should fail if the directory is absent" do
                 FileTest.expects(:directory?).with(File.dirname(@certpath)).returns false
                 lambda { @searcher.save(@request) }.should raise_error(Puppet::Error)
@@ -91,18 +134,49 @@ describe Puppet::Indirector::SslFile do
             end
 
             it "should save to the path the output of converting the certificate to a string" do
-                FileTest.stubs(:directory?).returns true
-                FileTest.stubs(:writable?).returns true
-
                 fh = mock 'filehandle'
-                File.expects(:open).with(@certpath, "w").yields(fh)
+                fh.expects(:print).with("mycert")
 
+                @searcher.stubs(:write).yields fh
                 @cert.expects(:to_s).returns "mycert"
 
-                fh.expects(:print).with("mycert")
-
                 @searcher.save(@request)
             end
+
+            describe "and a directory setting is set" do
+                it "should open the file in write mode" do
+                    @searcher.class.store_in @setting
+                    fh = mock 'filehandle'
+                    fh.stubs :print
+                    File.expects(:open).with(@certpath, "w").yields(fh)
+
+                    @searcher.save(@request)
+                end
+            end
+
+            describe "and a file location is set" do
+                it "should use the filehandle provided by the Settings" do
+                    @searcher.class.store_at @setting
+
+                    fh = mock 'filehandle'
+                    fh.stubs :print
+                    Puppet.settings.expects(:write).with(@setting).yields fh
+                    @searcher.save(@request)
+                end
+            end
+
+            describe "and the name is the CA name and a ca setting is set" do
+                it "should use the filehandle provided by the Settings" do
+                    @searcher.class.store_at @setting
+                    @searcher.class.store_ca_at :castuff
+
+                    fh = mock 'filehandle'
+                    fh.stubs :print
+                    Puppet.settings.expects(:write).with(:castuff).yields fh
+                    @searcher.stubs(:ca?).returns true
+                    @searcher.save(@request)
+                end
+            end
         end
 
         describe "when destroying certificates" do
@@ -111,7 +185,7 @@ describe Puppet::Indirector::SslFile do
                     FileTest.expects(:exist?).with(@certpath).returns false
                 end
 
-                it "should return nil" do
+                it "should return false" do
                     @searcher.destroy(@request).should be_false
                 end
             end
diff --git a/spec/unit/ssl/certificate_revocation_list.rb b/spec/unit/ssl/certificate_revocation_list.rb
index 0281b0947..01c197b25 100755
--- a/spec/unit/ssl/certificate_revocation_list.rb
+++ b/spec/unit/ssl/certificate_revocation_list.rb
@@ -8,8 +8,6 @@ describe Puppet::SSL::CertificateRevocationList do
     before do
         @cert = stub 'cert', :subject => "mysubject"
 
-        @key = stub 'key'
-
         @class = Puppet::SSL::CertificateRevocationList
     end
 
@@ -17,84 +15,27 @@ describe Puppet::SSL::CertificateRevocationList do
         before do
             @class.any_instance.stubs(:read_or_generate)
 
-            @crl = @class.new("myname", @cert, @key)
+            @crl = @class.new
         end
 
-        it "should have a name attribute" do
-            @crl.name.should == "myname"
+        it "should always use 'crl' for its name" do
+            @crl.name.should == "crl"
         end
 
         it "should have a content attribute" do
             @crl.should respond_to(:content)
         end
-
-        it "should be able to read the crl from disk" do
-            path = "/my/path"
-            File.expects(:read).with(path).returns("my crl")
-            crl = mock 'crl'
-            OpenSSL::X509::CRL.expects(:new).with("my crl").returns(crl)
-            @crl.read(path).should equal(crl)
-            @crl.content.should equal(crl)
-        end
-
-        it "should return an empty string when converted to a string with no crl" do
-            @crl.to_s.should == ""
-        end
-
-        it "should convert the crl to pem format when converted to a string" do
-            crl = mock 'crl', :to_pem => "pem"
-            @crl.content = crl
-            @crl.to_s.should == "pem"
-        end
-
-        it "should have a :to_text method that it delegates to the actual crl" do
-            real_crl = mock 'crl'
-            real_crl.expects(:to_text).returns "crltext"
-            @crl.content = real_crl
-            @crl.to_text.should == "crltext"
-        end
     end
 
     describe "when initializing" do
-        it "should require the CA cert and key" do
-            lambda { @class.new("myname") }.should raise_error(ArgumentError)
-        end
-
         it "should fail if :cacrl is set to false" do
             Puppet.settings.expects(:value).with(:cacrl).returns false
-            lambda { @class.new("myname", @cert, @key) }.should raise_error(Puppet::Error)
+            lambda { @class.new }.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("myname", @cert, @key) }.should raise_error(Puppet::Error)
-        end
-
-        it "should read the CRL from disk" do
-            Puppet.settings.stubs(:value).with(:cacrl).returns "/path/to/crl"
-            @class.any_instance.expects(:read).with("/path/to/crl").returns("my key")
-
-            @class.new("myname", @cert, @key)
-        end
-
-        describe "and no CRL exists on disk" do
-            before do
-                @class.any_instance.stubs(:read).returns(false)
-                @class.any_instance.stubs(:generate)
-                @class.any_instance.stubs(:save)
-            end
-
-            it "should generate a new CRL" do
-                @class.any_instance.expects(:generate).with(@cert, @key)
-
-                @class.new("myname", @cert, @key)
-            end
-
-            it "should save the CRL" do
-                @class.any_instance.expects(:save).with(@key)
-
-                @class.new("myname", @cert, @key)
-            end
+            lambda { @class.new }.should raise_error(Puppet::Error)
         end
     end
 
@@ -107,61 +48,91 @@ describe Puppet::SSL::CertificateRevocationList do
 
             @class.any_instance.stubs(:read_or_generate)
 
-            @crl = @class.new("myname", @cert, @key)
+            @crl = @class.new
         end
 
         it "should set its issuer to the subject of the passed certificate" do
             @real_crl.expects(:issuer=).with(@cert.subject)
 
-            @crl.generate(@cert, @key)
+            @crl.generate(@cert)
         end
 
         it "should set its version to 1" do
             @real_crl.expects(:version=).with(1)
 
-            @crl.generate(@cert, @key)
+            @crl.generate(@cert)
         end
 
         it "should create an instance of OpenSSL::X509::CRL" do
             OpenSSL::X509::CRL.expects(:new).returns(@real_crl)
 
-            @crl.generate(@cert, @key)
+            @crl.generate(@cert)
         end
 
         it "should set the content to the generated crl" do
-            @crl.generate(@cert, @key)
+            @crl.generate(@cert)
             @crl.content.should equal(@real_crl)
         end
 
         it "should return the generated crl" do
-            @crl.generate(@cert, @key).should equal(@real_crl)
-        end
-
-        it "should return the crl in pem format" do
-            @crl.generate(@cert, @key)
-            @crl.content.expects(:to_pem).returns "my normal crl"
-            @crl.to_s.should == "my normal crl"
+            @crl.generate(@cert).should equal(@real_crl)
         end
     end
 
-    describe "when saving the CRL" do
+    # This test suite isn't exactly complete, because the
+    # SSL stuff is very complicated.  It just hits the high points.
+    describe "when revoking a certificate" do
         before do
-            @class.any_instance.stubs(:read_or_generate)
             @class.wrapped_class.any_instance.stubs(:issuer=)
 
-            @crl = @class.new("myname", @cert, @key)
-            @crl.generate(@cert, @key)
+            @crl = @class.new
+            @crl.generate(@cert)
+            @crl.content.stubs(:sign)
+
+            @crl.stubs :save
+
+            @key = mock 'key'
+        end
+
+        it "should require a serial number and the CA's private key" do
+            lambda { @crl.revoke }.should raise_error(ArgumentError)
         end
 
-        it "should use the Settings#write method to write the file" do
-            pending("Not fully ported") do
-                fh = mock 'filehandle'
-                Puppet.settings.expects(:write).with(:cacrl).yields fh
+        it "should default to OpenSSL::OCSP::REVOKED_STATUS_KEYCOMPROMISE as the revocation reason" do
+            # This makes it a bit more of an integration test than we'd normally like, but that's life
+            # with openssl.
+            reason = OpenSSL::ASN1::Enumerated(OpenSSL::OCSP::REVOKED_STATUS_KEYCOMPROMISE)
+            OpenSSL::ASN1.expects(:Enumerated).with(OpenSSL::OCSP::REVOKED_STATUS_KEYCOMPROMISE).returns reason
+
+            @crl.revoke(1, @key)
+        end
 
-                fh.expects :print
+        it "should mark the CRL as updated" do
+            time = Time.now
+            Time.expects(:now).returns time
+
+            @crl.content.expects(:last_update=).with(time)
+
+            @crl.revoke(1, @key)
+        end
+
+        it "should mark the CRL valid for five years" do
+            time = Time.now
+            Time.expects(:now).returns time
+
+            @crl.content.expects(:next_update=).with(time + (5 * 365*24*60*60))
+
+            @crl.revoke(1, @key)
+        end
+
+        it "should sign the CRL with the CA's private key and a digest instance" do
+            @crl.content.expects(:sign).with { |key, digest| key == @key and digest.is_a?(OpenSSL::Digest::SHA1) }
+            @crl.revoke(1, @key)
+        end
 
-                @crl.save(@key)
-            end
+        it "should save the CRL" do
+            @crl.expects :save
+            @crl.revoke(1, @key)
         end
     end
 end
diff --git a/spec/unit/ssl/host.rb b/spec/unit/ssl/host.rb
index e82971683..97d6c27d8 100755
--- a/spec/unit/ssl/host.rb
+++ b/spec/unit/ssl/host.rb
@@ -30,7 +30,11 @@ describe Puppet::SSL::Host do
 
     it "should be able to be a ca host" do
         @host.ca = true
-        @host.ca.should be_true
+        @host.ca?.should be_true
+    end
+
+    it "should support having a password file set" do
+        lambda { @host.password_file = "/my/file" }.should_not raise_error
     end
 
     describe "when managing its private key" do
@@ -59,6 +63,17 @@ describe Puppet::SSL::Host do
             @host.key.should equal(@realkey)
         end
 
+        it "should pass its password file on to the key if one is set" do
+            @host.password_file = "/my/password"
+            Puppet::SSL::Key.expects(:new).with("myname").returns(@key)
+
+            @key.stub_everything
+
+            @key.expects(:password_file=).with("/my/password")
+
+            @host.generate_key
+        end
+
         it "should return any previously found key without requerying" do
             Puppet::SSL::Key.expects(:find).with("myname").returns(@key).once
             @host.key.should equal(@realkey)