Skip to content

Commit

Permalink
Fixing puppetlabs#1729 - puppetmasterd can now read certs at startup
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@lak
lak committed Dec 19, 2008
1 parent 0cf9dec commit 566bf78
Show file tree
Hide file tree
Showing 8 changed files with 165 additions and 126 deletions.
14 changes: 13 additions & 1 deletion bin/puppetmasterd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
117 changes: 46 additions & 71 deletions lib/puppet/defaults.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)."],
Expand All @@ -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

5 changes: 2 additions & 3 deletions lib/puppet/network/http/webrick.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 5 additions & 1 deletion lib/puppet/network/http_pool.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
50 changes: 30 additions & 20 deletions lib/puppet/ssl/host.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
14 changes: 3 additions & 11 deletions spec/unit/network/http/webrick.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 2 additions & 19 deletions spec/unit/network/http_pool.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 566bf78

Please sign in to comment.