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.

15 files changed, 397 insertions, 173 deletions

diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 300f9bad4..7b206901c 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -183,7 +183,7 @@ module Puppet
         },
         :hostcsr => { :default => "$ssldir/csr_$certname.pem",
             :mode => 0644,
-            :desc => "Where individual hosts store and look for their certificates."
+            :desc => "Where individual hosts store and look for their certificate requests."
         },
         :hostcert => { :default => "$certdir/$certname.pem",
             :mode => 0644,
@@ -200,6 +200,11 @@ module Puppet
         :localcacert => { :default => "$certdir/ca.pem",
             :mode => 0644,
             :desc => "Where each client stores the CA certificate."
+        },
+        :hostcrl => { :default => "$ssldir/crl.pem",
+            :mode => 0644,
+            :desc => "Where the host's certificate revocation list can be found.
+                This is distinct from the certificate authority's CRL."
         }
     )
 
diff --git a/lib/puppet/indirector/certificate/file.rb b/lib/puppet/indirector/certificate/file.rb
index 9e2e8ed99..5f4ade051 100644
--- a/lib/puppet/indirector/certificate/file.rb
+++ b/lib/puppet/indirector/certificate/file.rb
@@ -5,4 +5,5 @@ class Puppet::SSL::Certificate::File < Puppet::Indirector::SslFile
     desc "Manage SSL certificates on disk."
 
     store_in :certdir
+    store_ca_at :cacert
 end
diff --git a/lib/puppet/indirector/certificate_revocation_list/ca_file.rb b/lib/puppet/indirector/certificate_revocation_list/ca_file.rb
new file mode 100644
index 000000000..d29a5ce3b
--- /dev/null
+++ b/lib/puppet/indirector/certificate_revocation_list/ca_file.rb
@@ -0,0 +1,8 @@
+require 'puppet/indirector/ssl_file'
+require 'puppet/ssl/certificate_revocation_list'
+
+class Puppet::SSL::CertificateRevocationList::CaFile < Puppet::Indirector::SslFile
+    desc "Manage the CA collection of certificate requests on disk."
+
+    store_at :cacrl
+end
diff --git a/lib/puppet/indirector/certificate_revocation_list/file.rb b/lib/puppet/indirector/certificate_revocation_list/file.rb
new file mode 100644
index 000000000..037aa6b8c
--- /dev/null
+++ b/lib/puppet/indirector/certificate_revocation_list/file.rb
@@ -0,0 +1,8 @@
+require 'puppet/indirector/ssl_file'
+require 'puppet/ssl/certificate_revocation_list'
+
+class Puppet::SSL::CertificateRevocationList::File < Puppet::Indirector::SslFile
+    desc "Manage the global certificate revocation list."
+
+    store_at :hostcrl
+end
diff --git a/lib/puppet/indirector/key/file.rb b/lib/puppet/indirector/key/file.rb
index 41d30a2d4..4536f8aa7 100644
--- a/lib/puppet/indirector/key/file.rb
+++ b/lib/puppet/indirector/key/file.rb
@@ -5,9 +5,15 @@ class Puppet::SSL::Key::File < Puppet::Indirector::SslFile
     desc "Manage SSL private and public keys on disk."
 
     store_in :privatekeydir
+    store_ca_at :cakey
 
+    # Where should we store the public key?
     def public_key_path(name)
-        File.join(Puppet[:publickeydir], name.to_s + ".pem")
+        if ca?(name)
+            Puppet[:capub]
+        else
+            File.join(Puppet[:publickeydir], name.to_s + ".pem")
+        end
     end
 
     # Remove the public key, in addition to the private key
diff --git a/lib/puppet/indirector/ssl_file.rb b/lib/puppet/indirector/ssl_file.rb
index eddf82ac5..582d282ff 100644
--- a/lib/puppet/indirector/ssl_file.rb
+++ b/lib/puppet/indirector/ssl_file.rb
@@ -1,27 +1,67 @@
 require 'puppet/indirector/file'
+require 'puppet/ssl/host'
 
 class Puppet::Indirector::SslFile < Puppet::Indirector::Terminus
+    # Specify the directory in which multiple files are stored.
     def self.store_in(setting)
         @directory_setting = setting
     end
 
+    # Specify a single file location for storing just one file.
+    # This is used for things like the CRL.
+    def self.store_at(setting)
+        @file_setting = setting
+    end
+
+    # Specify where a specific ca file should be stored.
+    def self.store_ca_at(setting)
+        @ca_setting = setting
+    end
+
     class << self
-        attr_reader :directory_setting
+        attr_reader :directory_setting, :file_setting, :ca_setting
     end
 
     # The full path to where we should store our files.
     def self.collection_directory
-        raise(Puppet::DevError, "No setting defined for %s" % self) unless @directory_setting
-        Puppet.settings[@directory_setting]
+        return nil unless directory_setting
+        Puppet.settings[directory_setting]
+    end
+
+    # The full path to an individual file we would be managing.
+    def self.file_location
+        return nil unless file_setting
+        Puppet.settings[file_setting]
+    end
+
+    # The full path to a ca file we would be managing.
+    def self.ca_location
+        return nil unless ca_setting
+        Puppet.settings[ca_setting]
+    end
+
+    # We assume that all files named 'ca' are pointing to individual ca files,
+    # rather than normal host files.  It's a bit hackish, but all the other
+    # solutions seemed even more hackish.
+    def ca?(name)
+        name == Puppet::SSL::Host.ca_name
     end
 
     def initialize
         Puppet.settings.use(:ssl)
+
+        (collection_directory || file_location) or raise Puppet::DevError, "No file or directory setting provided; terminus %s cannot function" % self.class.name
     end
 
     # Use a setting to determine our path.
     def path(name)
-        File.join(collection_directory, name.to_s + ".pem")
+        if ca?(name) and ca_location 
+            ca_location
+        elsif collection_directory
+            File.join(collection_directory, name.to_s + ".pem")
+        else
+            file_location
+        end
     end
 
     # Remove our file.
@@ -53,13 +93,9 @@ class Puppet::Indirector::SslFile < Puppet::Indirector::Terminus
         dir = File.dirname(path)
 
         raise Puppet::Error.new("Cannot save %s; parent directory %s does not exist" % [request.key, dir]) unless FileTest.directory?(dir)
-        raise Puppet::Error.new("Cannot save %s; parent directory %s does not exist" % [request.key, dir]) unless FileTest.writable?(dir)
+        raise Puppet::Error.new("Cannot save %s; parent directory %s is not writable" % [request.key, dir]) unless FileTest.writable?(dir)
 
-        begin
-            File.open(path, "w") { |f| f.print request.instance.to_s }
-        rescue => detail
-            raise Puppet::Error, "Could not write %s: %s" % [request.key, detail]
-        end
+        write(request.key, path) { |f| f.print request.instance.to_s }
     end
 
     # Search for more than one file.  At this point, it just returns
@@ -76,8 +112,32 @@ class Puppet::Indirector::SslFile < Puppet::Indirector::Terminus
 
     private
 
-    # A demeterish pointer to the collection directory.
+    # Demeterish pointers to class info.
     def collection_directory
         self.class.collection_directory
     end
+
+    def file_location
+        self.class.file_location
+    end
+
+    def ca_location
+        self.class.ca_location
+    end
+
+    # Yield a filehandle set up appropriately, either with our settings doing
+    # the work or opening a filehandle manually.
+    def write(name, path)
+        if ca?(name) and ca_location
+            Puppet.settings.write(self.class.ca_setting) { |f| yield f }
+        elsif file_location
+            Puppet.settings.write(self.class.file_setting) { |f| yield f }
+        else
+            begin
+                File.open(path, "w") { |f| yield f }
+            rescue => detail
+                raise Puppet::Error, "Could not write %s: %s" % [path, detail]
+            end
+        end
+    end
 end
diff --git a/lib/puppet/ssl/certificate_revocation_list.rb b/lib/puppet/ssl/certificate_revocation_list.rb
index e892e276a..939b48443 100644
--- a/lib/puppet/ssl/certificate_revocation_list.rb
+++ b/lib/puppet/ssl/certificate_revocation_list.rb
@@ -1,12 +1,16 @@
 require 'puppet/ssl/base'
+require 'puppet/indirector'
 
 # Manage the CRL.
 class Puppet::SSL::CertificateRevocationList < Puppet::SSL::Base
     wraps OpenSSL::X509::CRL
 
+    extend Puppet::Indirector
+    indirects :certificate_revocation_list
+
     # Knows how to create a CRL with our system defaults.
-    def generate(cert, key)
-        Puppet.info "Creating a new SSL key for %s" % name
+    def generate(cert)
+        Puppet.info "Creating a new certificate revocation list"
         @content = wrapped_class.new
         @content.issuer = cert.subject
         @content.version = 1
@@ -14,29 +18,19 @@ class Puppet::SSL::CertificateRevocationList < Puppet::SSL::Base
         @content
     end
 
-    def initialize(name, cert, key)
+    def initialize
         raise Puppet::Error, "Cannot manage the CRL when :cacrl is set to false" if [false, "false"].include?(Puppet[:cacrl])
 
-        @name = name
-
-        read_or_generate(cert, key)
-    end
-
-    # A stupid indirection method to make this easier to test.  Yay.
-    def read_or_generate(cert, key)
-        unless read(Puppet[:cacrl])
-            generate(cert, key)
-            save(key)
-        end
+        @name = "crl"
     end
 
     # Revoke the certificate with serial number SERIAL issued by this
-    # CA. The REASON must be one of the OpenSSL::OCSP::REVOKED_* reasons
-    def revoke(serial, reason = OpenSSL::OCSP::REVOKED_STATUS_KEYCOMPROMISE)
-        if @config[:cacrl] == 'false'
-            raise Puppet::Error, "Revocation requires a CRL, but ca_crl is set to 'false'"
-        end
+    # CA, then write the CRL back to disk. The REASON must be one of the
+    # OpenSSL::OCSP::REVOKED_* reasons
+    def revoke(serial, cakey, reason = OpenSSL::OCSP::REVOKED_STATUS_KEYCOMPROMISE)
         time = Time.now
+
+        # Add our revocation to the CRL.
         revoked = OpenSSL::X509::Revoked.new
         revoked.serial = serial
         revoked.time = time
@@ -44,13 +38,7 @@ class Puppet::SSL::CertificateRevocationList < Puppet::SSL::Base
         ext = OpenSSL::X509::Extension.new("CRLReason", enum)
         revoked.add_extension(ext)
         @content.add_revoked(revoked)
-        store_crl
-    end
 
-    # Save the CRL to disk.  Note that none of the other Base subclasses
-    # have this method, because they all use the indirector to find and save
-    # the CRL.
-    def save(key)
         # Increment the crlNumber
         e = @content.extensions.find { |e| e.oid == 'crlNumber' }
         ext = @content.extensions.reject { |e| e.oid == 'crlNumber' }
@@ -59,14 +47,12 @@ class Puppet::SSL::CertificateRevocationList < Puppet::SSL::Base
         @content.extensions = ext
 
         # Set last/next update
-        now = Time.now
-        @content.last_update = now
+        @content.last_update = time
         # Keep CRL valid for 5 years
-        @content.next_update = now + 5 * 365*24*60*60
+        @content.next_update = time + 5 * 365*24*60*60
+
+        @content.sign(cakey, OpenSSL::Digest::SHA1.new)
 
-        sign_with_key(@content)
-        Puppet.settings.write(:cacrl) do |f|
-            f.puts @content.to_pem
-        end
+        save
     end
 end
diff --git a/lib/puppet/ssl/host.rb b/lib/puppet/ssl/host.rb
index 6f49175aa..dbd885316 100644
--- a/lib/puppet/ssl/host.rb
+++ b/lib/puppet/ssl/host.rb
@@ -15,7 +15,15 @@ class Puppet::SSL::Host
     extend Puppet::Util::ConstantInflector
 
     attr_reader :name
-    attr_accessor :ca
+    attr_accessor :ca, :password_file
+
+    CA_NAME = "ca"
+
+    # This is the constant that people will use to mark that a given host is
+    # a certificate authority.
+    def self.ca_name
+        CA_NAME
+    end
 
     # Search for more than one host, optionally only specifying
     # an interest in hosts with a given file type.
@@ -36,6 +44,11 @@ class Puppet::SSL::Host
         end
     end
 
+    # Is this a ca host, meaning that all of its files go in the CA location?
+    def ca?
+        ca
+    end
+
     def key
         return nil unless (defined?(@key) and @key) or @key = Key.find(name)
         @key.content
@@ -45,8 +58,12 @@ class Puppet::SSL::Host
     # with no inputs.
     def generate_key
         @key = Key.new(name)
+
+        # If a password file is set, then the key will be stored
+        # encrypted by the password.
+        @key.password_file = password_file if password_file
         @key.generate
-        @key.save :in => :file
+        @key.save
         true
     end
 
@@ -60,7 +77,7 @@ class Puppet::SSL::Host
         generate_key unless key
         @certificate_request = CertificateRequest.new(name)
         @certificate_request.generate(key)
-        @certificate_request.save :in => :file
+        @certificate_request.save
         return true
     end
 
@@ -71,11 +88,6 @@ class Puppet::SSL::Host
         @certificate.content
     end
 
-    # Is this a ca host, meaning that all of its files go in the CA collections?
-    def ca?
-        ca
-    end
-
     # Remove all traces of this ssl host
     def destroy
         [key, certificate, certificate_request].each do |instance|
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)