Moving the password file handling into the SSL::Key class.

This was necessary because when the Indirector is used, there
isn't necessarily enough context available to know when a
password file should be used (e.g., when reading a Key from disk,
you don't know if that key was encrypted).

Now, the Key class automatically uses the right password file, and
only tries to use those files that actually exist.

This isn't very flexible, in that it only allows one CA file and
one non-CA file, but no one really uses anything but
the CA file anyway.

7 files changed, 78 insertions, 47 deletions

diff --git a/lib/puppet/ssl/base.rb b/lib/puppet/ssl/base.rb
index ab040152d..80bfcae84 100644
--- a/lib/puppet/ssl/base.rb
+++ b/lib/puppet/ssl/base.rb
@@ -13,6 +13,11 @@ class Puppet::SSL::Base
 
     attr_accessor :name, :content
 
+    # Is this file for the CA?
+    def ca?
+        name == Puppet::SSL::Host.ca_name
+    end
+
     def generate
         raise Puppet::DevError, "%s did not override 'generate'" % self.class
     end
diff --git a/lib/puppet/ssl/certificate_authority.rb b/lib/puppet/ssl/certificate_authority.rb
index 6d5ca1bb2..2ed45e08b 100644
--- a/lib/puppet/ssl/certificate_authority.rb
+++ b/lib/puppet/ssl/certificate_authority.rb
@@ -18,6 +18,8 @@ class Puppet::SSL::CertificateAuthority
     def generate_ca_certificate
         generate_password unless password?
 
+        host.generate_key unless host.key
+
         # Create a new cert request.  We do this
         # specially, because we don't want to actually
         # save the request anywhere.
@@ -34,7 +36,6 @@ class Puppet::SSL::CertificateAuthority
         @name = Puppet[:certname]
 
         @host = Puppet::SSL::Host.new(Puppet::SSL::Host.ca_name)
-        @host.password_file = Puppet[:capass]
     end
 
     # Sign a given certificate request.
@@ -55,7 +56,7 @@ class Puppet::SSL::CertificateAuthority
 
         cert = Puppet::SSL::Certificate.new(hostname)
         cert.content = Puppet::SSL::CertificateFactory.new(cert_type, csr.content, issuer, next_serial).result
-        cert.content.sign(key, OpenSSL::Digest::SHA1.new)
+        cert.content.sign(host.key, OpenSSL::Digest::SHA1.new)
 
         Puppet.notice "Signed certificate request for %s" % hostname
 
diff --git a/lib/puppet/ssl/host.rb b/lib/puppet/ssl/host.rb
index a6c721b1c..42f881568 100644
--- a/lib/puppet/ssl/host.rb
+++ b/lib/puppet/ssl/host.rb
@@ -15,7 +15,7 @@ class Puppet::SSL::Host
     extend Puppet::Util::ConstantInflector
 
     attr_reader :name
-    attr_accessor :ca, :password_file
+    attr_accessor :ca
 
     CA_NAME = "ca"
 
@@ -114,10 +114,6 @@ 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
         true
diff --git a/lib/puppet/ssl/key.rb b/lib/puppet/ssl/key.rb
index 65294ac00..a1d436090 100644
--- a/lib/puppet/ssl/key.rb
+++ b/lib/puppet/ssl/key.rb
@@ -8,7 +8,7 @@ class Puppet::SSL::Key < Puppet::SSL::Base
     extend Puppet::Indirector
     indirects :key, :terminus_class => :file
 
-    attr_reader :password_file
+    attr_accessor :password_file
 
     # Knows how to create keys with our system defaults.
     def generate
@@ -16,23 +16,27 @@ class Puppet::SSL::Key < Puppet::SSL::Base
         @content = OpenSSL::PKey::RSA.new(Puppet[:keylength].to_i)
     end
 
-    def password
-        return nil unless password_file
+    def initialize(name)
+        super
 
-        ::File.read(password_file)
+        if ca?
+            @password_file = Puppet[:capass]
+        else
+            @password_file = Puppet[:passfile]
+        end
     end
 
-    # Set our password file.
-    def password_file=(file)
-        raise ArgumentError, "Password file %s does not exist" % file unless FileTest.exist?(file)
+    def password
+        return nil unless password_file and FileTest.exist?(password_file)
 
-        @password_file = file
+        ::File.read(password_file)
     end
 
     # Optionally support specifying a password file.
     def read(path)
         return super unless password_file
 
+        #@content = wrapped_class.new(::File.read(path), password)
         @content = wrapped_class.new(::File.read(path), password)
     end
 
diff --git a/spec/unit/ssl/certificate_authority.rb b/spec/unit/ssl/certificate_authority.rb
index c6ffcb809..285877301 100755
--- a/spec/unit/ssl/certificate_authority.rb
+++ b/spec/unit/ssl/certificate_authority.rb
@@ -22,23 +22,12 @@ describe Puppet::SSL::CertificateAuthority do
         it "should create an SSL::Host instance whose name is the 'ca_name'" do
             Puppet::SSL::Host.expects(:ca_name).returns "caname"
 
-            host = stub 'host', :password_file= => nil
+            host = stub 'host'
             Puppet::SSL::Host.expects(:new).with("caname").returns host
 
             Puppet::SSL::CertificateAuthority.new
         end
 
-        it "should set the Host instance's password file to the :capass setting" do
-            Puppet.settings.stubs(:value).with(:capass).returns "/ca/pass"
-
-            host = mock 'host'
-            Puppet::SSL::Host.expects(:new).returns host
-
-            host.expects(:password_file=).with "/ca/pass"
-
-            Puppet::SSL::CertificateAuthority.new
-        end
-
         it "should use the :main, :ca, and :ssl settings sections" do
             Puppet.settings.expects(:use).with(:main, :ssl, :ca)
             Puppet::SSL::CertificateAuthority.new
@@ -74,6 +63,16 @@ describe Puppet::SSL::CertificateAuthority do
             @ca.generate_ca_certificate
         end
 
+        it "should generate a key if one does not exist" do
+            @ca.stubs :generate_password
+            @ca.stubs :sign
+
+            @ca.host.expects(:key).returns nil
+            @ca.host.expects(:generate_key)
+
+            @ca.generate_ca_certificate
+        end
+
         it "should create and sign a self-signed cert" do
             request = mock 'request'
             Puppet::SSL::CertificateRequest.expects(:new).with(@ca.host.name).returns request
@@ -254,7 +253,7 @@ describe Puppet::SSL::CertificateAuthority do
                 OpenSSL::Digest::SHA1.expects(:new).returns digest
 
                 key = mock 'key'
-                @ca.stubs(:key).returns key
+                @ca.host.stubs(:key).returns key
 
                 @cert.content.expects(:sign).with(key, digest)
                 @ca.sign(@name)
diff --git a/spec/unit/ssl/host.rb b/spec/unit/ssl/host.rb
index e1d6b5c9e..482286b93 100755
--- a/spec/unit/ssl/host.rb
+++ b/spec/unit/ssl/host.rb
@@ -33,10 +33,6 @@ describe Puppet::SSL::Host do
         @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
-
     it "should have a method for determining the CA location" do
         Puppet::SSL::Host.should respond_to(:ca_location)
     end
@@ -160,17 +156,6 @@ 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)
diff --git a/spec/unit/ssl/key.rb b/spec/unit/ssl/key.rb
index 8d89c0039..4cd8e856d 100755
--- a/spec/unit/ssl/key.rb
+++ b/spec/unit/ssl/key.rb
@@ -21,6 +21,32 @@ describe Puppet::SSL::Key do
         @class.indirection.terminus_class.should == :file
     end
 
+    it "should have a method for determining whether it's a CA key" do
+        @class.new("test").should respond_to(:ca?)
+    end
+
+    it "should consider itself a ca key if its name matches the CA_NAME" do
+        @class.new(Puppet::SSL::Host.ca_name).should be_ca
+    end
+
+    describe "when initializing" do
+        it "should set its password file to the :capass if it's a CA key" do
+            Puppet.settings.stubs(:value).returns "whatever"
+            Puppet.settings.stubs(:value).with(:capass).returns "/ca/pass"
+
+            key = Puppet::SSL::Key.new(Puppet::SSL::Host.ca_name)
+            key.password_file.should == "/ca/pass"
+        end
+
+        it "should set its password file to the default password file if it is not the CA key" do
+            Puppet.settings.stubs(:value).returns "whatever"
+            Puppet.settings.stubs(:value).with(:passfile).returns "/normal/pass"
+
+            key = Puppet::SSL::Key.new("notca")
+            key.password_file.should == "/normal/pass"
+        end
+    end
+
     describe "when managing instances" do
         before do
             @key = @class.new("myname")
@@ -38,11 +64,24 @@ describe Puppet::SSL::Key do
             path = "/my/path"
             File.expects(:read).with(path).returns("my key")
             key = mock 'key'
-            OpenSSL::PKey::RSA.expects(:new).with("my key").returns(key)
+            OpenSSL::PKey::RSA.expects(:new).returns(key)
             @key.read(path).should equal(key)
             @key.content.should equal(key)
         end
 
+        it "should not try to use the provided password file if the file does not exist" do
+            FileTest.stubs(:exist?).returns false
+            @key.password_file = "/path/to/password"
+
+            path = "/my/path"
+
+            File.stubs(:read).with(path).returns("my key")
+            OpenSSL::PKey::RSA.expects(:new).with("my key", nil).returns(mock('key'))
+            File.expects(:read).with("/path/to/password").never
+
+            @key.read(path)
+        end
+
         it "should read the key with the password retrieved from the password file if one is provided" do
             FileTest.stubs(:exist?).returns true
             @key.password_file = "/path/to/password"
@@ -113,10 +152,13 @@ describe Puppet::SSL::Key do
         end
 
         describe "with a password file set" do
-            it "should fail if the password file does not exist" do
+            it "should return a nil password if the password file does not exist" do
                 FileTest.expects(:exist?).with("/path/to/pass").returns false
+                File.expects(:read).with("/path/to/pass").never
 
-                lambda { @instance.password_file = "/path/to/pass" }.should raise_error(ArgumentError)
+                @instance.password_file = "/path/to/pass"
+
+                @instance.password.should be_nil
             end
 
             it "should return the contents of the password file as its password" do
@@ -131,7 +173,6 @@ describe Puppet::SSL::Key do
             it "should export the private key to text using the password" do
                 Puppet.settings.stubs(:value).with(:keylength).returns("50")
 
-                FileTest.expects(:exist?).with("/path/to/pass").returns true
                 @instance.password_file = "/path/to/pass"
                 @instance.stubs(:password).returns "my password"