Let's move all SSL related stuff from the agent and the http network client to the new ssl client class plugin.
Signed-off-by: Brice Figureau <[email protected]> --- lib/puppet/application/agent.rb | 12 +-- lib/puppet/auth/client/ssl.rb | 36 +++++++++ lib/puppet/network/http_pool.rb | 19 +---- spec/unit/application/agent_spec.rb | 53 +------------ spec/unit/auth/client/ssl_spec.rb | 146 +++++++++++++++++++++++++++++++++++ spec/unit/network/http_pool_spec.rb | 95 ++--------------------- 6 files changed, 199 insertions(+), 162 deletions(-) create mode 100644 lib/puppet/auth/client/ssl.rb create mode 100644 spec/unit/auth/client/ssl_spec.rb diff --git a/lib/puppet/application/agent.rb b/lib/puppet/application/agent.rb index 2b75505..2fccd94 100644 --- a/lib/puppet/application/agent.rb +++ b/lib/puppet/application/agent.rb @@ -1,4 +1,5 @@ require 'puppet/application' +require 'puppet/auth' class Puppet::Application::Agent < Puppet::Application @@ -189,9 +190,7 @@ class Puppet::Application::Agent < Puppet::Application end def setup_host - @host = Puppet::SSL::Host.new - waitforcert = options[:waitforcert] || (Puppet[:onetime] ? 0 : 120) - cert = @host.wait_for_cert(waitforcert) unless options[:fingerprint] + Puppet::Auth.client.setup(options) end def setup @@ -217,16 +216,13 @@ class Puppet::Application::Agent < Puppet::Application Puppet::Util::Log.newdestination(logdest) end - Puppet.settings.use :main, :agent, :ssl + Puppet.settings.use :main, :agent # Always ignoreimport for agent. It really shouldn't even try to import, # but this is just a temporary band-aid. Puppet[:ignoreimport] = true - # We need to specify a ca location for all of the SSL-related i - # indirected classes to work; in fingerprint mode we just need - # access to the local files and we don't need a ca. - Puppet::SSL::Host.ca_location = options[:fingerprint] ? :none : :remote + Puppet::Auth.client.init(options) Puppet::Transaction::Report.terminus_class = :rest diff --git a/lib/puppet/auth/client/ssl.rb b/lib/puppet/auth/client/ssl.rb new file mode 100644 index 0000000..0b4f0fc --- /dev/null +++ b/lib/puppet/auth/client/ssl.rb @@ -0,0 +1,36 @@ +require 'puppet/ssl/host' + +Puppet::Auth.new_client(:ssl) do + + def self.init(options) + Puppet.settings.use :ssl + + # We need to specify a ca location for all of the SSL-related i + # indirected classes to work; in fingerprint mode we just need + # access to the local files and we don't need a ca. + Puppet::SSL::Host.ca_location = options[:fingerprint] ? :none : :remote + end + + def self.setup(options) + waitforcert = options[:waitforcert] || (Puppet[:onetime] ? 0 : 120) + cert = Puppet::SSL::Host.new.wait_for_cert(waitforcert) unless options[:fingerprint] + end + + def self.setup_http_client(http) + http.use_ssl = true + + # Just no-op if we don't have certs. + return false unless FileTest.exist?(Puppet[:hostcert]) and FileTest.exist?(Puppet[:localcacert]) + + http.cert_store = ssl_host.ssl_store + http.ca_file = Puppet[:localcacert] + http.cert = ssl_host.certificate.content + http.verify_mode = OpenSSL::SSL::VERIFY_PEER + http.key = ssl_host.key.content + end + + # Use the global localhost instance. + def self.ssl_host + Puppet::SSL::Host.localhost + end +end \ No newline at end of file diff --git a/lib/puppet/network/http_pool.rb b/lib/puppet/network/http_pool.rb index 7d227b4..432e34b 100644 --- a/lib/puppet/network/http_pool.rb +++ b/lib/puppet/network/http_pool.rb @@ -14,11 +14,6 @@ module Puppet::Network::HttpPool 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. @@ -49,15 +44,8 @@ module Puppet::Network::HttpPool end # Use cert information from a Puppet client to set up the http object. - def self.cert_setup(http) - # Just no-op if we don't have certs. - return false unless FileTest.exist?(Puppet[:hostcert]) and FileTest.exist?(Puppet[:localcacert]) - - http.cert_store = ssl_host.ssl_store - http.ca_file = Puppet[:localcacert] - http.cert = ssl_host.certificate.content - http.verify_mode = OpenSSL::SSL::VERIFY_PEER - http.key = ssl_host.key.content + def self.auth_setup(http) + Puppet::Auth.client.setup_http_client(http) end # Retrieve a cached http instance if caching is enabled, else return @@ -90,12 +78,11 @@ module Puppet::Network::HttpPool # give us a reader for ca_file... Grr... class << http; attr_accessor :ca_file; end - http.use_ssl = true # Use configured timeout (#1176) http.read_timeout = Puppet[:configtimeout] http.open_timeout = Puppet[:configtimeout] - cert_setup(http) + auth_setup(http) http_cache[key] = http if keep_alive? diff --git a/spec/unit/application/agent_spec.rb b/spec/unit/application/agent_spec.rb index 8fc98b8..9124a2a 100755 --- a/spec/unit/application/agent_spec.rb +++ b/spec/unit/application/agent_spec.rb @@ -95,6 +95,7 @@ describe Puppet::Application::Agent do describe "when handling options" do before do @puppetd.command_line.stubs(:args).returns([]) + @puppetd.stubs(:setup_host) end [:centrallogging, :disable, :enable, :debug, :fqdn, :test, :verbose, :digest].each do |option| @@ -120,24 +121,6 @@ describe Puppet::Application::Agent do @puppetd.options[:client].should be_false end - it "should set waitforcert to 0 with --onetime and if --waitforcert wasn't given" do - Puppet[:onetime] = true - Puppet::SSL::Host.any_instance.expects(:wait_for_cert).with(0) - @puppetd.setup_host - end - - it "should use supplied waitforcert when --onetime is specified" do - Puppet[:onetime] = true - @puppetd.handle_waitforcert(60) - Puppet::SSL::Host.any_instance.expects(:wait_for_cert).with(60) - @puppetd.setup_host - end - - it "should use a default value for waitforcert when --onetime and --waitforcert are not specified" do - Puppet::SSL::Host.any_instance.expects(:wait_for_cert).with(120) - @puppetd.setup_host - end - it "should set the log destination with --logdest" do @puppetd.options.stubs(:[]=).with { |opt,val| opt == :setdest } Puppet::Log.expects(:newdestination).with("console") @@ -183,9 +166,9 @@ describe Puppet::Application::Agent do Puppet::Resource::Catalog.stubs(:terminus_class=) Puppet::Resource::Catalog.stubs(:cache_class=) Puppet::Node::Facts.stubs(:terminus_class=) - @host = stub_everything 'host' - Puppet::SSL::Host.stubs(:new).returns(@host) Puppet.stubs(:settraps) + @authclient = stub_everything 'auth client' + Puppet::Auth.stubs(:client).returns(@authclient) end describe "with --test" do @@ -287,20 +270,7 @@ describe Puppet::Application::Agent do end it "should use :main, :puppetd, and :ssl" do - Puppet.settings.expects(:use).with(:main, :agent, :ssl) - - @puppetd.setup - end - - it "should install a remote ca location" do - Puppet::SSL::Host.expects(:ca_location=).with(:remote) - - @puppetd.setup - end - - it "should install a none ca location in fingerprint mode" do - @puppetd.options.stubs(:[]).with(:fingerprint).returns(true) - Puppet::SSL::Host.expects(:ca_location=).with(:none) + Puppet.settings.expects(:use).with(:main, :agent) @puppetd.setup end @@ -382,21 +352,6 @@ describe Puppet::Application::Agent do @puppetd.setup end - it "should wait for a certificate" do - @puppetd.options.stubs(:[]).with(:waitforcert).returns(123) - @host.expects(:wait_for_cert).with(123) - - @puppetd.setup - end - - it "should not wait for a certificate in fingerprint mode" do - @puppetd.options.stubs(:[]).with(:fingerprint).returns(true) - @puppetd.options.stubs(:[]).with(:waitforcert).returns(123) - @host.expects(:wait_for_cert).never - - @puppetd.setup - end - it "should setup listen if told to and not onetime" do Puppet[:listen] = true @puppetd.options.stubs(:[]).with(:onetime).returns(false) diff --git a/spec/unit/auth/client/ssl_spec.rb b/spec/unit/auth/client/ssl_spec.rb new file mode 100644 index 0000000..f7f3ece --- /dev/null +++ b/spec/unit/auth/client/ssl_spec.rb @@ -0,0 +1,146 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../../../spec_helper' + +require 'puppet/auth' +require 'puppet/ssl/host' + +describe Puppet::Auth, "ssl client" do + before(:each) do + Puppet[:auth] = "ssl" + + @host = stub_everything 'host' + Puppet::SSL::Host.stubs(:new).returns(@host) + end + + describe "when initializing" do + it "should use ssl" do + Puppet.settings.expects(:use).with(:ssl) + Puppet::Auth.client.init({}) + end + + it "should install a remote ca location" do + Puppet::SSL::Host.expects(:ca_location=).with(:remote) + + Puppet::Auth.client.init({}) + end + + it "should install a none ca location in fingerprint mode" do + Puppet::SSL::Host.expects(:ca_location=).with(:none) + + Puppet::Auth.client.init({:fingerprint => true}) + end + end + + describe "when setting up" do + it "should set waitforcert to 0 with --onetime and if --waitforcert wasn't given" do + Puppet[:onetime] = true + @host.expects(:wait_for_cert).with(0) + Puppet::Auth.client.setup({}) + end + + it "should use supplied waitforcert when --onetime is specified" do + Puppet[:onetime] = true + @host.expects(:wait_for_cert).with(60) + Puppet::Auth.client.setup({:waitforcert => 60}) + end + + it "should use a default value for waitforcert when --onetime and --waitforcert are not specified" do + @host.expects(:wait_for_cert).with(120) + Puppet::Auth.client.setup({}) + end + + it "should wait for a certificate" do + @host.expects(:wait_for_cert).with(123) + Puppet::Auth.client.setup({:waitforcert => 123}) + end + + it "should not wait for a certificate in fingerprint mode" do + @host.expects(:wait_for_cert).never + Puppet::Auth.client.setup({:waitforcert => 123, :fingerprint => true}) + end + end + + describe "when setting ssl for the http client" do + before do + @client = stub_everything 'client' + FileTest.stubs(:exists?).returns(false) + end + + it "should enable ssl on the http instance" do + @client.expects(:use_ssl=).with(true) + Puppet::Auth.client.setup_http_client(@client) + end + + describe "when adding certificate information to http instances" do + before do + @http = mock 'http' + [:cert_store=, :verify_mode=, :ca_file=, :cert=, :key=, :use_ssl=].each { |m| @http.stubs(m) } + @store = stub 'store' + + @cert = stub 'cert', :content => "real_cert" + @key = stub 'key', :content => "real_key" + @host = stub 'host', :certificate => @cert, :key => @key, :ssl_store => @store + + Puppet[:confdir] = "/sometthing/else" + Puppet[:hostcert] = "/host/cert" + Puppet[:localcacert] = "/local/ca/cert" + + FileTest.stubs(:exist?).with("/host/cert").returns true + FileTest.stubs(:exist?).with("/local/ca/cert").returns true + + Puppet::Auth::client.stubs(:ssl_host).returns @host + end + + after do + Puppet.settings.clear + end + + it "should do nothing if no host certificate is on disk" do + FileTest.expects(:exist?).with("/host/cert").returns false + @http.expects(:cert=).never + Puppet::Auth.client.setup_http_client(@http) + end + + it "should do nothing if no local certificate is on disk" do + FileTest.expects(:exist?).with("/local/ca/cert").returns false + @http.expects(:cert=).never + Puppet::Auth.client.setup_http_client(@http) + end + + it "should add a certificate store from the ssl host" do + @http.expects(:cert_store=).with(@store) + + Puppet::Auth.client.setup_http_client(@http) + end + + it "should add the client certificate" do + @http.expects(:cert=).with("real_cert") + + Puppet::Auth.client.setup_http_client(@http) + end + + it "should add the client key" do + @http.expects(:key=).with("real_key") + + Puppet::Auth.client.setup_http_client(@http) + end + + it "should set the verify mode to OpenSSL::SSL::VERIFY_PEER" do + @http.expects(:verify_mode=).with(OpenSSL::SSL::VERIFY_PEER) + + Puppet::Auth.client.setup_http_client(@http) + end + + it "should set the ca file" do + FileTest.stubs(:exist?).with(Puppet[:hostcert]).returns true + + Puppet[:localcacert] = "/ca/cert/file" + FileTest.stubs(:exist?).with("/ca/cert/file").returns true + @http.expects(:ca_file=).with("/ca/cert/file") + + Puppet::Auth.client.setup_http_client(@http) + end + end + end +end diff --git a/spec/unit/network/http_pool_spec.rb b/spec/unit/network/http_pool_spec.rb index 8fa7de8..02d4914 100755 --- a/spec/unit/network/http_pool_spec.rb +++ b/spec/unit/network/http_pool_spec.rb @@ -10,19 +10,12 @@ describe Puppet::Network::HttpPool do after do Puppet::Util::Cacher.expire Puppet::Network::HttpPool.clear_http_instances - Puppet::Network::HttpPool.instance_variable_set("@ssl_host", nil) end it "should have keep-alive disabled" do Puppet::Network::HttpPool::HTTP_KEEP_ALIVE.should be_false end - it "should use the global SSL::Host instance to get its certificate information" do - host = mock 'host' - Puppet::SSL::Host.expects(:localhost).with.returns host - Puppet::Network::HttpPool.ssl_host.should equal(host) - end - describe "when managing http instances" do def stub_settings(settings) settings.each do |param, value| @@ -32,7 +25,8 @@ describe Puppet::Network::HttpPool do before do # All of the cert stuff is tested elsewhere - Puppet::Network::HttpPool.stubs(:cert_setup) + @auth = stub_everything 'auth' + Puppet::Auth.stubs(:client).returns(@auth) end it "should return an http instance created with the passed host and port" do @@ -41,10 +35,6 @@ describe Puppet::Network::HttpPool do Puppet::Network::HttpPool.http_instance("me", 54321).should equal(http) end - it "should enable ssl on the http instance" do - Puppet::Network::HttpPool.http_instance("me", 54321).instance_variable_get("@use_ssl").should be_true - end - it "should set the read timeout" do Puppet::Network::HttpPool.http_instance("me", 54321).read_timeout.should == 120 end @@ -121,86 +111,13 @@ describe Puppet::Network::HttpPool do end end - after do - Puppet::Network::HttpPool.clear_http_instances - end - end - - describe "when adding certificate information to http instances" do - before do - @http = mock 'http' - [:cert_store=, :verify_mode=, :ca_file=, :cert=, :key=].each { |m| @http.stubs(m) } - @store = stub 'store' - - @cert = stub 'cert', :content => "real_cert" - @key = stub 'key', :content => "real_key" - @host = stub 'host', :certificate => @cert, :key => @key, :ssl_store => @store - - Puppet[:confdir] = "/sometthing/else" - Puppet.settings.stubs(:value).returns "/some/file" - Puppet.settings.stubs(:value).with(:hostcert).returns "/host/cert" - Puppet.settings.stubs(:value).with(:localcacert).returns "/local/ca/cert" - - FileTest.stubs(:exist?).with("/host/cert").returns true - FileTest.stubs(:exist?).with("/local/ca/cert").returns true - - Puppet::Network::HttpPool.stubs(:ssl_host).returns @host + it "should set up certificate information when creating http instances" do + @auth.with { |i| i.is_a?(Net::HTTP) } + Puppet::Network::HttpPool.http_instance("one", "two") end after do - Puppet.settings.clear - end - - it "should do nothing if no host certificate is on disk" do - FileTest.expects(:exist?).with("/host/cert").returns false - @http.expects(:cert=).never - Puppet::Network::HttpPool.cert_setup(@http) - end - - it "should do nothing if no local certificate is on disk" do - FileTest.expects(:exist?).with("/local/ca/cert").returns false - @http.expects(:cert=).never - Puppet::Network::HttpPool.cert_setup(@http) - end - - it "should add a certificate store from the ssl host" do - @http.expects(:cert_store=).with(@store) - - Puppet::Network::HttpPool.cert_setup(@http) - end - - it "should add the client certificate" do - @http.expects(:cert=).with("real_cert") - - Puppet::Network::HttpPool.cert_setup(@http) - end - - it "should add the client key" do - @http.expects(:key=).with("real_key") - - Puppet::Network::HttpPool.cert_setup(@http) - end - - it "should set the verify mode to OpenSSL::SSL::VERIFY_PEER" do - @http.expects(:verify_mode=).with(OpenSSL::SSL::VERIFY_PEER) - - Puppet::Network::HttpPool.cert_setup(@http) - end - - it "should set the ca file" do - Puppet.settings.stubs(:value).returns "/some/file" - FileTest.stubs(:exist?).with(Puppet[:hostcert]).returns true - - Puppet.settings.stubs(:value).with(:localcacert).returns "/ca/cert/file" - FileTest.stubs(:exist?).with("/ca/cert/file").returns true - @http.expects(:ca_file=).with("/ca/cert/file") - - Puppet::Network::HttpPool.cert_setup(@http) - end - - it "should set up certificate information when creating http instances" do - Puppet::Network::HttpPool.expects(:cert_setup).with { |i| i.is_a?(Net::HTTP) } - Puppet::Network::HttpPool.http_instance("one", "two") + Puppet::Network::HttpPool.clear_http_instances end end end -- 1.7.2.1 -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
