12 files changed, 335 insertions, 75 deletions

diff --git a/lib/puppet/indirector/certificate_request/file.rb b/lib/puppet/indirector/certificate_request/file.rb
index 5eb6745fd..274311e2c 100644
--- a/lib/puppet/indirector/certificate_request/file.rb
+++ b/lib/puppet/indirector/certificate_request/file.rb
@@ -1,8 +1,8 @@
 require 'puppet/indirector/ssl_file'
 require 'puppet/ssl/certificate_request'
 
-class Puppet::SSL::CertificateRequest::CaFile < Puppet::Indirector::SslFile
-    desc "Manage the CA collection of certificate requests on disk."
+class Puppet::SSL::CertificateRequest::File < Puppet::Indirector::SslFile
+    desc "Manage the collection of certificate requests on disk."
 
     store_in :requestdir
 end
diff --git a/lib/puppet/indirector/ssl_file.rb b/lib/puppet/indirector/ssl_file.rb
index 6125d46e4..7a1501dbf 100644
--- a/lib/puppet/indirector/ssl_file.rb
+++ b/lib/puppet/indirector/ssl_file.rb
@@ -25,7 +25,7 @@ class Puppet::Indirector::SslFile < Puppet::Indirector::Terminus
     end
 
     # Remove our file.
-    def destroy(file)
+    def destroy(file, options = {})
         path = path(file.name)
         raise Puppet::Error.new("File %s does not exist; cannot destroy" % [file]) unless FileTest.exist?(path)
 
@@ -37,7 +37,7 @@ class Puppet::Indirector::SslFile < Puppet::Indirector::Terminus
     end
 
     # Find the file on disk, returning an instance of the model.
-    def find(name)
+    def find(name, options = {})
         path = path(name)
 
         return nil unless FileTest.exist?(path)
@@ -48,7 +48,7 @@ class Puppet::Indirector::SslFile < Puppet::Indirector::Terminus
     end
 
     # Save our file to disk.
-    def save(file)
+    def save(file, options = {})
         path = path(file.name)
         dir = File.dirname(path)
 
diff --git a/lib/puppet/ssl/base.rb b/lib/puppet/ssl/base.rb
index 87cbea4b5..781ccb805 100644
--- a/lib/puppet/ssl/base.rb
+++ b/lib/puppet/ssl/base.rb
@@ -1,4 +1,5 @@
 require 'puppet/ssl'
+require 'puppet/ssl/indirection_hooks'
 
 # The base class for wrapping SSL instances.
 class Puppet::SSL::Base
diff --git a/lib/puppet/ssl/certificate.rb b/lib/puppet/ssl/certificate.rb
index d1687e6f0..697b2e785 100644
--- a/lib/puppet/ssl/certificate.rb
+++ b/lib/puppet/ssl/certificate.rb
@@ -6,9 +6,40 @@ class Puppet::SSL::Certificate < Puppet::SSL::Base
     wraps OpenSSL::X509::Certificate
 
     extend Puppet::Indirector
-    indirects :certificate, :terminus_class => :file
+    indirects :certificate, :extend => Puppet::SSL::IndirectionHooks
 
-    def generate
-        raise Puppet::DevError, "Cannot generate certificates directly; they must be generated during signing"
+    # Indicate where we should get our signed certs from.
+    def self.ca_is(dest)
+        raise(ArgumentError, "Invalid location '%s' for ca; valid values are :local and :remote" % dest) unless [:local, :remote].include?(dest)
+        @ca_location = dest
+    end
+
+    # Default to :local for the ca location.
+    def self.ca_location
+        if defined?(@ca_location) and @ca_location
+            @ca_location
+        else
+            :local
+        end
+    end
+
+    # Request a certificate from our CA.
+    def generate(request)
+        if self.class.ca_location == :local
+            terminus = :ca_file
+        else
+            terminus = :rest
+        end
+
+        # Save our certificate request.
+        request.save :in => terminus
+
+        # And see if we can retrieve the certificate.
+        if cert = self.class.find(name, :in => terminus)
+            @content = cert.content
+            return true
+        else
+            return false
+        end
     end
 end
diff --git a/lib/puppet/ssl/certificate_request.rb b/lib/puppet/ssl/certificate_request.rb
index fec9e1733..e8cbbbade 100644
--- a/lib/puppet/ssl/certificate_request.rb
+++ b/lib/puppet/ssl/certificate_request.rb
@@ -5,7 +5,7 @@ class Puppet::SSL::CertificateRequest < Puppet::SSL::Base
     wraps OpenSSL::X509::Request
 
     extend Puppet::Indirector
-    indirects :certificate_request #, :terminus_class => :file
+    indirects :certificate_request, :extend => Puppet::SSL::IndirectionHooks
 
     # How to create a certificate request with our system defaults.
     def generate(key)
@@ -17,6 +17,8 @@ class Puppet::SSL::CertificateRequest < Puppet::SSL::Base
         csr.public_key = key.public_key
         csr.sign(key, OpenSSL::Digest::MD5.new)
 
+        raise Puppet::Error, "CSR sign verification failed; you need to clean the certificate request for %s on the server" % name unless csr.verify(key.public_key)
+
         @content = csr
     end
 end
diff --git a/lib/puppet/ssl/host.rb b/lib/puppet/ssl/host.rb
index 8df9ef385..bae33b23e 100644
--- a/lib/puppet/ssl/host.rb
+++ b/lib/puppet/ssl/host.rb
@@ -13,23 +13,28 @@ class Puppet::SSL::Host
 
     attr_reader :name
 
+    attr_accessor :ca
+
+    # Is this a ca host, meaning that all of its files go in the CA collections?
+    def ca?
+        ca
+    end
+
     # Read our cert if necessary, fail if we can't find it (since it should
     # be created by someone else and returned through 'find').
     def certificate
         unless @certificate ||= Certificate.find(name)
-            Certificate.new(name).generate # throws an exception
+            return nil
         end
-        @certificate
+        @certificate.content
     end
 
     # Read or create, then return, our certificate request.
     def certificate_request
         unless @certificate_request ||= CertificateRequest.find(name)
-            @certificate_request = CertificateRequest.new(name)
-            @certificate_request.generate(key)
-            @certificate_request.save
+            return nil
         end
-        @certificate_request
+        @certificate_request.content
     end
 
     # Remove all traces of this ssl host
@@ -39,20 +44,49 @@ class Puppet::SSL::Host
         end
     end
 
+    # Request a signed certificate from a ca, if we can find one.
+    def generate_certificate
+        generate_certificate_request unless certificate_request
+
+        @certificate = Certificate.new(name)
+        if @certificate.generate(certificate_request)
+            @certificate.save
+            return true
+        else
+            return false
+        end
+    end
+
+    # Generate and save a new certificate request.
+    def generate_certificate_request
+        generate_key unless key
+        @certificate_request = CertificateRequest.new(name)
+        @certificate_request.generate(key)
+        @certificate_request.save
+        return true
+    end
+
+    # Generate and save a new key.
+    def generate_key
+        @key = Key.new(name)
+        @key.generate
+        @key.save
+        return true
+    end
+
     # Read or create, then return, our key. The public key is part
-    # of the private key.
+    # of the private key.  We 
     def key
         unless @key ||= Key.find(name)
-            @key = Key.new(name)
-            @key.generate
-            @key.save
+            return nil
         end
-        @key
+        @key.content
     end
 
     def initialize(name)
         @name = name
         @key = @certificate = @certificate_request = nil
+        @ca = false
     end
 
     # Extract the public key from the private key.
diff --git a/lib/puppet/ssl/indirection_hooks.rb b/lib/puppet/ssl/indirection_hooks.rb
new file mode 100644
index 000000000..c2a3442c0
--- /dev/null
+++ b/lib/puppet/ssl/indirection_hooks.rb
@@ -0,0 +1,17 @@
+#
+#  Created by Luke Kanies on 2008-3-10.
+#  Copyright (c) 2008. All rights reserved.
+
+require 'uri'
+require 'puppet/ssl'
+
+# This module is used to pick the appropriate terminus
+# in certificate indirections.  This is necessary because
+# we need the ability to choose between interacting with the CA
+# or the local certs.
+module Puppet::SSL::IndirectionHooks
+    # Pick an appropriate terminus based on what's specified, defaulting to :file.
+    def select_terminus(full_uri, options = {})
+        return options[:to] || options[:in] || :file
+    end
+end
diff --git a/spec/unit/indirector/certificate_request/file.rb b/spec/unit/indirector/certificate_request/file.rb
index c4595b932..e1f442e2a 100755
--- a/spec/unit/indirector/certificate_request/file.rb
+++ b/spec/unit/indirector/certificate_request/file.rb
@@ -7,13 +7,13 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 
 require 'puppet/indirector/certificate_request/file'
 
-describe Puppet::SSL::CertificateRequest::CaFile do
+describe Puppet::SSL::CertificateRequest::File do
     it "should have documentation" do
-        Puppet::SSL::CertificateRequest::CaFile.doc.should be_instance_of(String)
+        Puppet::SSL::CertificateRequest::File.doc.should be_instance_of(String)
     end
 
     it "should use the :requestdir as the collection directory" do
         Puppet.settings.expects(:value).with(:requestdir).returns "/request/dir"
-        Puppet::SSL::CertificateRequest::CaFile.collection_directory.should == "/request/dir"
+        Puppet::SSL::CertificateRequest::File.collection_directory.should == "/request/dir"
     end
 end
diff --git a/spec/unit/ssl/certificate.rb b/spec/unit/ssl/certificate.rb
index 1df9c42e1..1f847e16e 100755
--- a/spec/unit/ssl/certificate.rb
+++ b/spec/unit/ssl/certificate.rb
@@ -9,6 +9,10 @@ describe Puppet::SSL::Certificate do
         @class = Puppet::SSL::Certificate
     end
 
+    after do
+        @class.instance_variable_set("@ca_location", nil)
+    end
+
     it "should be extended with the Indirector module" do
         @class.metaclass.should be_include(Puppet::Indirector)
     end
@@ -18,7 +22,22 @@ describe Puppet::SSL::Certificate do
     end
 
     it "should default to the :file terminus class" do
-        @class.indirection.terminus_class.should == :file
+        @class.indirection.terminus(:file).expects(:find).with "myname"
+        @class.find("myname")
+    end
+
+    it "should allow specification of a different terminus class" do
+        @class.indirection.terminus(:ca_file).expects(:find).with { |*args| args[0] == "myname" }
+        @class.find("myname", :in => :ca_file)
+    end
+
+    it "should default to a local certificate authority" do
+        @class.ca_location.should == :local
+    end
+
+    it "should allow overriding the ca location" do
+        @class.ca_is :remote
+        @class.ca_location.should == :remote
     end
 
     describe "when managing instances" do
@@ -55,9 +74,64 @@ describe Puppet::SSL::Certificate do
     end
 
     describe "when generating the certificate" do
-        it "should fail because certificates must be created by a certificate authority" do
-            @instance = @class.new("test")
-            lambda { @instance.generate }.should raise_error(Puppet::DevError)
+        before do
+            @cert = @class.new("test")
+            @request = mock 'request'
+        end
+
+        describe "from a local ca" do
+            before do
+                @class.stubs(:ca_location).returns :local
+            end
+
+            it "should save the certificate request to and try to find the cert in the :ca_file terminus" do
+                @request.expects(:save).with(:in => :ca_file)
+                @cert.class.expects(:find).with("test", :in => :ca_file)
+
+                @cert.generate(@request)
+            end
+        end
+
+        describe "from a remote ca" do
+            before do
+                @class.stubs(:ca_location).returns :remote
+            end
+
+            it "should save the certificate request to and try to find the cert in the :rest terminus" do
+                @request.expects(:save).with(:in => :rest)
+                @cert.class.expects(:find).with("test", :in => :rest)
+
+                @cert.generate(@request)
+            end
+        end
+
+        describe "successfully" do
+            it "should set its content to the content of the retrieved certificate" do
+                @request.stubs(:save)
+                newcert = mock 'newcert', :content => "realcert"
+                @cert.class.expects(:find).returns(newcert)
+
+                @cert.generate(@request)
+
+                @cert.content.should == "realcert"
+            end
+
+            it "should return true" do
+                @request.stubs(:save)
+                newcert = mock 'newcert', :content => "realcert"
+                @cert.class.expects(:find).returns(newcert)
+
+                @cert.generate(@request).should be_true
+            end
+        end
+
+        describe "unsuccessfully" do
+            it "should return false" do
+                @request.stubs(:save)
+                @cert.class.expects(:find).returns(nil)
+
+                @cert.generate(@request).should be_false
+            end
         end
     end
 end
diff --git a/spec/unit/ssl/certificate_request.rb b/spec/unit/ssl/certificate_request.rb
index 48755a614..9b2823bf9 100755
--- a/spec/unit/ssl/certificate_request.rb
+++ b/spec/unit/ssl/certificate_request.rb
@@ -22,6 +22,16 @@ describe Puppet::SSL::CertificateRequest do
         @class.new("myname").name.should == "myname"
     end
 
+    it "should default to the :file terminus class" do
+        @class.indirection.terminus(:file).expects(:find).with "myname"
+        @class.find("myname")
+    end
+
+    it "should allow specification of a different terminus class" do
+        @class.indirection.terminus(:ca_file).expects(:find).with { |*args| args[0] == "myname" }
+        @class.find("myname", :in => :ca_file)
+    end
+
     describe "when managing instances" do
         before do
             @request = @class.new("myname")
@@ -86,9 +96,39 @@ describe Puppet::SSL::CertificateRequest do
             # Again, this is weirdly failing, even though it's painfully simple.
             @request.expects(:sign)
 
+            @request.stubs(:verify).returns(true)
+
+            @instance.generate(@key).should == @request
+        end
+
+        it "should verify the generated request using the public key" do
+            @request = mock 'request'
+            OpenSSL::X509::Request.expects(:new).returns(@request)
+
+            subject = mock 'subject'
+            OpenSSL::X509::Name.stubs(:new)
+
+            @request.stubs(:version=)
+            @request.stubs(:public_key=)
+            @request.stubs(:subject=)
+            @request.stubs(:sign)
+
+            # Grr, mocha is broken in this class for some reason; I can't get
+            # the 'with' arguments to register correctly.
+            @request.expects(:verify).returns true
+
             @instance.generate(@key).should == @request
         end
 
+        it "should fail if verification fails" do
+            @request = OpenSSL::X509::Request.new
+            OpenSSL::X509::Request.expects(:new).returns(@request)
+
+            @request.expects(:verify).returns false
+
+            lambda { @instance.generate(@key) }.should raise_error(Puppet::Error)
+        end
+
         it "should return the generated request" do
             @instance.generate(@key).should be_instance_of(OpenSSL::X509::Request)
         end
diff --git a/spec/unit/ssl/host.rb b/spec/unit/ssl/host.rb
index 90729454b..f3ead362a 100755
--- a/spec/unit/ssl/host.rb
+++ b/spec/unit/ssl/host.rb
@@ -15,96 +15,156 @@ describe Puppet::SSL::Host do
     end
 
     it "should retrieve its public key from its private key" do
-        key = mock 'key'
+        realkey = mock 'realkey'
+        key = stub 'key', :content => realkey
         Puppet::SSL::Key.stubs(:find).returns(key)
         pubkey = mock 'public_key'
-        key.expects(:public_key).returns pubkey
+        realkey.expects(:public_key).returns pubkey
 
         @host.public_key.should equal(pubkey)
     end
 
+    it "should default to being a non-ca host" do
+        @host.ca?.should be_false
+    end
+
+    it "should be able to be a ca host" do
+        @host.ca = true
+        @host.ca.should be_true
+    end
+
     describe "when managing its private key" do
-        it "should find the key in the Key class and return it" do
-            key = mock 'key'
-            Puppet::SSL::Key.expects(:find).with("myname").returns(key)
-            @host.key.should equal(key)
+        before do
+            @realkey = "mykey"
+            @key = stub 'key', :content => @realkey
+        end
+
+        it "should return nil if the key is not set and cannot be found" do
+            Puppet::SSL::Key.expects(:find).with("myname").returns(nil)
+            @host.key.should be_nil
         end
 
-        it "should generate and save a new key if none is found" do
-            key = mock 'key'
-            Puppet::SSL::Key.stubs(:find).with("myname").returns(nil)
+        it "should find the key in the Key class and return the SSL key, not the wrapper" do
+            Puppet::SSL::Key.expects(:find).with("myname").returns(@key)
+            @host.key.should equal(@realkey)
+        end
 
-            Puppet::SSL::Key.expects(:new).with("myname").returns(key)
+        it "should be able to generate and save a new key" do
+            Puppet::SSL::Key.expects(:new).with("myname").returns(@key)
 
-            key.expects(:generate)
-            key.expects(:save)
+            @key.expects(:generate)
+            @key.expects(:save)
 
-            @host.key.should equal(key)
+            @host.generate_key.should be_true
+            @host.key.should equal(@realkey)
         end
 
         it "should return any previously found key without requerying" do
-            key = mock 'key'
-            Puppet::SSL::Key.expects(:find).with("myname").returns(key).once
-            @host.key.should equal(key)
-            @host.key.should equal(key)
+            Puppet::SSL::Key.expects(:find).with("myname").returns(@key).once
+            @host.key.should equal(@realkey)
+            @host.key.should equal(@realkey)
         end
     end
 
     describe "when managing its certificate request" do
-        it "should find the request in the Key class and return it" do
-            request = mock 'request'
-            Puppet::SSL::CertificateRequest.expects(:find).with("myname").returns request
+        before do
+            @realrequest = "real request"
+            @request = stub 'request', :content => @realrequest
+        end
+
+        it "should return nil if the key is not set and cannot be found" do
+            Puppet::SSL::CertificateRequest.expects(:find).with("myname").returns(nil)
+            @host.certificate_request.should be_nil
+        end
+
+        it "should find the request in the Key class and return it and return the SSL request, not the wrapper" do
+            Puppet::SSL::CertificateRequest.expects(:find).with("myname").returns @request
+
+            @host.certificate_request.should equal(@realrequest)
+        end
 
-            @host.certificate_request.should equal(request)
+        it "should generate a new key when generating the cert request if no key exists" do
+            Puppet::SSL::CertificateRequest.expects(:new).with("myname").returns @request
+
+            key = stub 'key', :public_key => mock("public_key")
+            @host.expects(:generate_key).returns(key)
+
+            @request.stubs(:generate)
+            @request.stubs(:save)
+
+            @host.generate_certificate_request
         end
 
-        it "should generate a new request using the private key if none is found" do
-            request = mock 'request'
-            Puppet::SSL::CertificateRequest.expects(:find).with("myname").returns nil
-            Puppet::SSL::CertificateRequest.expects(:new).with("myname").returns request
+        it "should be able to generate and save a new request using the private key" do
+            Puppet::SSL::CertificateRequest.expects(:new).with("myname").returns @request
 
             key = stub 'key', :public_key => mock("public_key")
             @host.stubs(:key).returns(key)
-            request.expects(:generate).with(key)
-            request.expects(:save)
+            @request.expects(:generate).with(key)
+            @request.expects(:save)
 
-            @host.certificate_request.should equal(request)
+            @host.generate_certificate_request.should be_true
+            @host.certificate_request.should equal(@realrequest)
         end
 
         it "should return any previously found request without requerying" do
-            request = mock 'request'
-            Puppet::SSL::CertificateRequest.expects(:find).with("myname").returns(request).once
+            Puppet::SSL::CertificateRequest.expects(:find).with("myname").returns(@request).once
 
-            @host.certificate_request.should equal(request)
-            @host.certificate_request.should equal(request)
+            @host.certificate_request.should equal(@realrequest)
+            @host.certificate_request.should equal(@realrequest)
         end
     end
 
     describe "when managing its certificate" do
-        it "should find the certificate in the Certificate class" do
-            cert = mock 'cert'
-            Puppet::SSL::Certificate.expects(:find).with("myname").returns cert
+        before do
+            @realcert = mock 'certificate'
+            @cert = stub 'cert', :content => @realcert
+        end
+        it "should find the certificate in the Certificate class and return the SSL certificate, not the wrapper" do
+            Puppet::SSL::Certificate.expects(:find).with("myname").returns @cert
+
+            @host.certificate.should equal(@realcert)
+        end
+
+        it "should generate a new certificate request when generating the cert if no request exists" do
+            Puppet::SSL::Certificate.expects(:new).with("myname").returns @cert
+
+            request = stub 'request'
+            @host.expects(:generate_certificate_request)
+
+            @cert.stubs(:generate)
+            @cert.stubs(:save)
+
+            @host.generate_certificate
+        end
+
+        it "should be able to generate and save a new certificate using the certificate request" do
+            Puppet::SSL::Certificate.expects(:new).with("myname").returns @cert
+
+            request = stub 'request'
+            @host.stubs(:certificate_request).returns(request)
+            @cert.expects(:generate).with(request).returns(true)
+            @cert.expects(:save)
 
-            @host.certificate.should equal(cert)
+            @host.generate_certificate.should be_true
+            @host.certificate.should equal(@realcert)
         end
 
-        it "should generate a new certificate if none is found" do
-            cert = mock 'cert'
-            Puppet::SSL::Certificate.expects(:find).with("myname").returns nil
-            Puppet::SSL::Certificate.expects(:new).with("myname").returns cert
+        it "should return false if no certificate could be generated" do
+            Puppet::SSL::Certificate.expects(:new).with("myname").returns @cert
 
-            # This will normally fail.
-            cert.expects(:generate)
+            request = stub 'request'
+            @host.stubs(:certificate_request).returns(request)
+            @cert.expects(:generate).with(request).returns(false)
 
-            @host.certificate
+            @host.generate_certificate.should be_false
         end
 
         it "should return any previously found certificate" do
-            cert = mock 'cert'
-            Puppet::SSL::Certificate.expects(:find).with("myname").returns(cert).once
+            Puppet::SSL::Certificate.expects(:find).with("myname").returns(@cert).once
 
-            @host.certificate.should equal(cert)
-            @host.certificate.should equal(cert)
+            @host.certificate.should equal(@realcert)
+            @host.certificate.should equal(@realcert)
         end
     end
 
diff --git a/spec/unit/ssl/key.rb b/spec/unit/ssl/key.rb
index d6cdc8266..4dec78a0d 100755
--- a/spec/unit/ssl/key.rb
+++ b/spec/unit/ssl/key.rb
@@ -18,7 +18,8 @@ describe Puppet::SSL::Key do
     end
 
     it "should default to the :file terminus class" do
-        @class.indirection.terminus_class.should == :file
+        @class.indirection.terminus(:file).expects(:find).with "myname"
+        @class.find("myname")
     end
 
     describe "when managing instances" do