Fixing #1729 - puppetmasterd can now read certs at startup

lak · lak · commit 566bf7820e56 · 2008-12-18T18:06:45.000-06:00
The main aspect of this solution is to create a site-wide
Puppet::SSL::Host instance to cache ssl key and certificate,
so that by the time we've switched UIDs, we've got the key and
cert in memory.  Then webrick just uses that, rather than creating
a new Host instance.

Signed-off-by: Luke Kanies &lt;luke@madstop.com&gt;
diff --git a/bin/puppetmasterd b/bin/puppetmasterd
@@ -155,6 +155,8 @@ if Puppet.settings.print_configs?
     exit(Puppet.settings.print_configs ? 0 : 1)
 end
 
+Puppet.settings.use :main, :puppetmasterd, :ssl
+
 # A temporary solution, to at least make the master work for now.
 Puppet::Node::Facts.terminus_class = :yaml
 
@@ -164,7 +166,7 @@ Puppet::Node.cache_class = :yaml
 # Configure all of the SSL stuff.
 if Puppet::SSL::CertificateAuthority.ca?
     Puppet::SSL::Host.ca_location = :local
-    Puppet.settings.use :main, :ssl, :ca
+    Puppet.settings.use :ca
     Puppet::SSL::CertificateAuthority.instance
 else
     Puppet::SSL::Host.ca_location = :none
@@ -197,6 +199,16 @@ end
 
 server = Puppet::Network::Server.new(:handlers => rest_handlers, :xmlrpc_handlers => xmlrpc_handlers)
 
+# Make sure we've got a localhost ssl cert
+Puppet::SSL::Host.localhost
+
+# And now configure our server to *only* hit the CA for data, because that's
+# all it will have write access to.
+if Puppet::SSL::CertificateAuthority.ca?
+    Puppet::SSL::Host.ca_location = :only
+    Puppet::SSL::Host.ca_location = :none
+end
+
 if Process.uid == 0
     begin
         Puppet::Util.chuser
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
@@ -148,7 +148,12 @@ module Puppet
             "The HTTP proxy port to use for outgoing connections"],
         :http_enable_post_connection_check => [true,
             "Boolean; wheter or not puppetd should validate the server
-            SSL certificate against the request hostname."]
+            SSL certificate against the request hostname."],
+        :filetimeout => [ 15,
+            "The minimum time to wait (in seconds) between checking for updates in
+            configuration files.  This timeout determines how quickly Puppet checks whether
+            a file (such as manifests or templates) has changed on disk."
+        ]
     )
 
     hostname = Facter["hostname"].value
@@ -380,7 +385,31 @@ module Puppet
         :yamldir => {:default => "$vardir/yaml", :owner => "$user", :group => "$user", :mode => "750",
             :desc => "The directory in which YAML data is stored, usually in a subdirectory."},
         :clientyamldir => {:default => "$vardir/client_yaml", :mode => "750",
-            :desc => "The directory in which client-side YAML data is stored."}
+            :desc => "The directory in which client-side YAML data is stored."},
+        :reports => ["store",
+            "The list of reports to generate.  All reports are looked for
+            in puppet/reports/<name>.rb, and multiple report names should be
+            comma-separated (whitespace is okay)."
+        ],
+        :reportdir => {:default => "$vardir/reports",
+                :mode => 0750,
+                :owner => "$user",
+                :group => "$group",
+                :desc => "The directory in which to store reports
+                    received from the client.  Each client gets a separate
+                    subdirectory."},
+        :fileserverconfig => ["$confdir/fileserver.conf",
+            "Where the fileserver configuration is stored."],
+        :rrddir => {:default => "$vardir/rrd",
+            :owner => "$user",
+            :group => "$group",
+            :desc => "The directory where RRD database files are stored.
+                Directories for each reporting host will be created under
+                this directory."
+        },
+        :rrdgraph => [false, "Whether RRD information should be graphed."],
+        :rrdinterval => ["$runinterval", "How often RRD should expect data.
+            This should match how often the hosts report back to the server."]
     )
 
     self.setdefaults(:puppetd,
@@ -428,35 +457,7 @@ module Puppet
         :ca_port => ["$masterport", "The port to use for the certificate authority."],
         :catalog_format => ["yaml", "What format to use to dump the catalog.  Only supports
             'marshal' and 'yaml'.  Only matters on the client, since it asks the server
-            for a specific format."]
-    )
-        
-    self.setdefaults(:filebucket,
-        :clientbucketdir => {
-            :default => "$vardir/clientbucket",
-            :mode => 0750,
-            :desc => "Where FileBucket files are stored locally."
-        }
-    )
-    self.setdefaults(:fileserver,
-        :fileserverconfig => ["$confdir/fileserver.conf",
-            "Where the fileserver configuration is stored."]
-    )
-    self.setdefaults(:reporting,
-        :reports => ["store",
-            "The list of reports to generate.  All reports are looked for
-            in puppet/reports/<name>.rb, and multiple report names should be
-            comma-separated (whitespace is okay)."
-        ],
-        :reportdir => {:default => "$vardir/reports",
-                :mode => 0750,
-                :owner => "$user",
-                :group => "$group",
-                :desc => "The directory in which to store reports
-                    received from the client.  Each client gets a separate
-                    subdirectory."}
-    )
-    self.setdefaults(:puppetd,
+            for a specific format."],
         :puppetdlockfile => [ "$statedir/puppetdlock",
             "A lock file to temporarily stop puppetd from doing anything."],
         :usecacheonfailure => [true,
@@ -482,10 +483,12 @@ module Puppet
             run interval."],
         :splay => [false,
             "Whether to sleep for a pseudo-random (but consistent) amount of time before
-            a run."]
-    )
-
-    self.setdefaults(:puppetd,
+            a run."],
+        :clientbucketdir => {
+            :default => "$vardir/clientbucket",
+            :mode => 0750,
+            :desc => "Where FileBucket files are stored locally."
+        },
         :configtimeout => [120,
             "How long the client should wait for the configuration to be retrieved
             before considering it a failure.  This can help reduce flapping if too
@@ -496,7 +499,14 @@ module Puppet
         ],
         :report => [false,
             "Whether to send reports after every transaction."
-        ]
+        ],
+        :graph => [false, "Whether to create dot graph files for the different
+            configuration graphs.  These dot files can be interpreted by tools
+            like OmniGraffle or dot (which is part of ImageMagick)."],
+        :graphdir => ["$statedir/graphs", "Where to store dot-outputted graphs."],
+        :storeconfigs => [false,
+            "Whether to store each client's configuration.  This
+             requires ActiveRecord from Ruby on Rails."]
     )
 
     # Plugin information.
@@ -582,13 +592,6 @@ module Puppet
             and other environments normally use ``debug``."]
     )
 
-    setdefaults(:graphing,
-        :graph => [false, "Whether to create dot graph files for the different
-            configuration graphs.  These dot files can be interpreted by tools
-            like OmniGraffle or dot (which is part of ImageMagick)."],
-        :graphdir => ["$statedir/graphs", "Where to store dot-outputted graphs."]
-    )
-
     setdefaults(:transaction,
         :tags => ["", "Tags to use to find resources.  If this is set, then
             only resources tagged with the specified tags will be applied.
@@ -665,12 +668,6 @@ module Puppet
             branch under your main directory."]
     )
 
-    setdefaults(:puppetmasterd,
-        :storeconfigs => [false,
-            "Whether to store each client's configuration.  This
-             requires ActiveRecord from Ruby on Rails."]
-    )
-
     # This doesn't actually work right now.
     setdefaults(:parser,
         :lexical => [false, "Whether to use lexical scoping (vs. dynamic)."],
@@ -679,26 +676,4 @@ module Puppet
              directories."
         ]
     )
-
-    setdefaults(:main,
-        :filetimeout => [ 15,
-            "The minimum time to wait (in seconds) between checking for updates in
-            configuration files.  This timeout determines how quickly Puppet checks whether
-            a file (such as manifests or templates) has changed on disk."
-        ]
-    )
-
-    setdefaults(:metrics,
-        :rrddir => {:default => "$vardir/rrd",
-            :owner => "$user",
-            :group => "$group",
-            :desc => "The directory where RRD database files are stored.
-                Directories for each reporting host will be created under
-                this directory."
-        },
-        :rrdgraph => [false, "Whether RRD information should be graphed."],
-        :rrdinterval => ["$runinterval", "How often RRD should expect data.
-            This should match how often the hosts report back to the server."]
-    )
 end
-
diff --git a/lib/puppet/network/http/webrick.rb b/lib/puppet/network/http/webrick.rb
@@ -89,9 +89,8 @@ def setup_logger
     def setup_ssl
         results = {}
 
-        host = Puppet::SSL::Host.new
-
-        host.generate unless host.certificate
+        # Get the cached copy.  We know it's been generated, too.
+        host = Puppet::SSL::Host.localhost
 
         raise Puppet::Error, "Could not retrieve certificate for %s and not running on a valid certificate authority" % host.name unless host.certificate
 
diff --git a/lib/puppet/network/http_pool.rb b/lib/puppet/network/http_pool.rb
@@ -8,13 +8,17 @@ module Puppet::Network; end
 module Puppet::Network::HttpPool
     class << self
         include Puppet::Util::Cacher
-        cached_attr(:ssl_host) { Puppet::SSL::Host.new }
 
         private
         
         cached_attr(:http_cache) { Hash.new }
     end
 
+    # Use the global localhost instance.
+    def self.ssl_host
+        Puppet::SSL::Host.localhost
+    end
+
     # 2008/03/23
     # LAK:WARNING: Enabling this has a high propability of
     # causing corrupt files and who knows what else.  See #1010.
diff --git a/lib/puppet/ssl/host.rb b/lib/puppet/ssl/host.rb
@@ -3,7 +3,7 @@
 require 'puppet/ssl/certificate'
 require 'puppet/ssl/certificate_request'
 require 'puppet/ssl/certificate_revocation_list'
-require 'puppet/util/constant_inflector'
+require 'puppet/util/cacher'
 
 # The class that manages all aspects of our SSL certificates --
 # private keys, public keys, requests, etc.
@@ -14,15 +14,23 @@ class Puppet::SSL::Host
     CertificateRequest = Puppet::SSL::CertificateRequest
     CertificateRevocationList = Puppet::SSL::CertificateRevocationList
 
-    extend Puppet::Util::ConstantInflector
-
     attr_reader :name
     attr_accessor :ca
 
     attr_writer :key, :certificate, :certificate_request
 
-    CA_NAME = "ca"
+    class << self
+        include Puppet::Util::Cacher
+
+        cached_attr(:localhost) do
+            result = new()
+            result.generate unless result.certificate
+            result.key # Make sure it's read in
+            result
+        end
+    end
 
+    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
@@ -40,7 +48,7 @@ def self.configure_indirection(terminus, cache = nil)
         CertificateRevocationList.terminus_class = terminus
 
         if cache
-            # This is weird; we don't actually cache our keys or CRL, we
+            # This is weird; we don't actually cache our keys, we
             # use what would otherwise be the cache as our normal
             # terminus.
             Key.terminus_class = cache
@@ -55,23 +63,25 @@ def self.configure_indirection(terminus, cache = nil)
         end
     end
 
+    CA_MODES = {
+        # Our ca is local, so we use it as the ultimate source of information
+        # And we cache files locally.
+        :local => [:ca, :file],
+        # We're a remote CA client.
+        :remote => [:rest, :file],
+        # We are the CA, so we don't have read/write access to the normal certificates.
+        :only => [:ca],
+        # We have no CA, so we just look in the local file store.
+        :none => [:file]
+    }
+
     # Specify how we expect to interact with our certificate authority.
     def self.ca_location=(mode)
-        raise ArgumentError, "CA Mode can only be :local, :remote, or :none" unless [:local, :remote, :none].include?(mode)
-
-        @ca_mode = mode
-
-        case @ca_mode
-        when :local:
-            # Our ca is local, so we use it as the ultimate source of information
-            # And we cache files locally.
-            configure_indirection :ca, :file
-        when :remote:
-            configure_indirection :rest, :file
-        when :none:
-            # We have no CA, so we just look in the local file store.
-            configure_indirection :file
-        end
+        raise ArgumentError, "CA Mode can only be %s" % CA_MODES.collect { |m| m.to_s }.join(", ") unless CA_MODES.include?(mode)
+
+        @ca_location = mode
+
+        configure_indirection(*CA_MODES[@ca_location])
     end
 
     # Remove all traces of a given host
diff --git a/spec/unit/network/http/webrick.rb b/spec/unit/network/http/webrick.rb
@@ -298,24 +298,16 @@
 
             Puppet::SSL::Certificate.stubs(:find).with('ca').returns @cert
 
-            Puppet::SSL::Host.stubs(:new).returns @host
+            Puppet::SSL::Host.stubs(:localhost).returns @host
         end
 
-        it "should use the key from an SSL::Host instance created with the default name" do
-            Puppet::SSL::Host.expects(:new).returns @host
+        it "should use the key from the localhost SSL::Host instance" do
+            Puppet::SSL::Host.expects(:localhost).returns @host
             @host.expects(:key).returns @key
 
             @server.setup_ssl[:SSLPrivateKey].should == "mykey"
         end
 
-        it "should generate its files if no certificate can be found" do
-            @host.expects(:certificate).times(2).returns(nil).then.returns(@cert)
-
-            @host.expects(:generate)
-
-            @server.setup_ssl
-        end
-
         it "should configure the certificate" do
             @server.setup_ssl[:SSLCertificate].should == "mycert"
         end
diff --git a/spec/unit/network/http_pool.rb b/spec/unit/network/http_pool.rb
@@ -17,16 +17,9 @@
         Puppet::Network::HttpPool::HTTP_KEEP_ALIVE.should be_false
     end
 
-    it "should use an SSL::Host instance to get its certificate information" do
+    it "should use the global SSL::Host instance to get its certificate information" do
         host = mock 'host'
-        Puppet::SSL::Host.expects(:new).with().returns host
-        Puppet::Network::HttpPool.ssl_host.should equal(host)
-    end
-
-    it "should reuse the same host instance" do
-        host = mock 'host'
-        Puppet::SSL::Host.expects(:new).with().once.returns host
-        Puppet::Network::HttpPool.ssl_host.should equal(host)
+        Puppet::SSL::Host.expects(:localhost).with().returns host
         Puppet::Network::HttpPool.ssl_host.should equal(host)
     end
 
@@ -122,16 +115,6 @@ def stub_settings(settings)
                 one.expects(:finish).never
                 Puppet::Network::HttpPool.clear_http_instances
             end
-
-            it "should reset its ssl host when clearing the cache" do
-                stub_settings :http_proxy_host => "myhost", :http_proxy_port => 432, :configtimeout => 120, :http_enable_post_connection_check => true, :certname => "a"
-                one = Puppet::Network::HttpPool.http_instance("me", 54321)
-                one.expects(:started?).returns(false)
-                one.expects(:finish).never
-                id = Puppet::Network::HttpPool.ssl_host.object_id
-                Puppet::Network::HttpPool.clear_http_instances
-                Puppet::Network::HttpPool.ssl_host.object_id.should_not == id
-            end
         end
 
         describe "and http keep-alive is disabled" do
diff --git a/spec/unit/ssl/host.rb b/spec/unit/ssl/host.rb
