It looks like all of the new ssl classes for managing

keys, certificates, and requests now work, including
talking to the certificate authority.  Now we just
need the authority itself, along with the necessary
REST terminii.

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