diff --git a/lib/puppet/network/http_pool.rb b/lib/puppet/network/http_pool.rb index 8baf48c77..96e20f7c2 100644 --- a/lib/puppet/network/http_pool.rb +++ b/lib/puppet/network/http_pool.rb @@ -1,48 +1,57 @@ require 'puppet/ssl/host' require 'net/https' module Puppet::Network; end module Puppet::Network::HttpPool # Use the global localhost instance. def self.ssl_host Puppet::SSL::Host.localhost 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 + if 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 + else + # We don't have the local certificates, so we don't do any verification + # or setup at this early stage. REVISIT: Shouldn't we supply the local + # certificate details if we have them? The original code didn't. + # --daniel 2012-06-03 + + # Ruby 1.8 defaulted to this, but 1.9 defaults to peer verify, and we + # almost always talk to a dedicated, not-standard CA that isn't trusted + # out of the box. This forces the expected state. + http.verify_mode = OpenSSL::SSL::VERIFY_NONE + end end # Retrieve a cached http instance if caching is enabled, else return # a new one. def self.http_instance(host, port, reset = false) args = [host, port] if Puppet[:http_proxy_host] == "none" args << nil << nil else args << Puppet[:http_proxy_host] << Puppet[:http_proxy_port] end http = Net::HTTP.new(*args) # Pop open the http client a little; older versions of Net::HTTP(s) didn't # 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) http end end diff --git a/spec/unit/network/http_pool_spec.rb b/spec/unit/network/http_pool_spec.rb index c86f7a660..81258aade 100755 --- a/spec/unit/network/http_pool_spec.rb +++ b/spec/unit/network/http_pool_spec.rb @@ -1,135 +1,141 @@ #!/usr/bin/env rspec require 'spec_helper' require 'puppet/network/http_pool' describe Puppet::Network::HttpPool do after do Puppet::Network::HttpPool.instance_variable_set("@ssl_host", nil) 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| - Puppet.settings.stubs(:value).with(param).returns(value) - end - end - - before do + before :each do # All of the cert stuff is tested elsewhere Puppet::Network::HttpPool.stubs(:cert_setup) end it "should return an http instance created with the passed host and port" do - http = stub 'http', :use_ssl= => nil, :read_timeout= => nil, :open_timeout= => nil, :started? => false - Net::HTTP.expects(:new).with("me", 54321, nil, nil).returns(http) - Puppet::Network::HttpPool.http_instance("me", 54321).should equal(http) + http = Puppet::Network::HttpPool.http_instance("me", 54321) + http.should be_an_instance_of Net::HTTP + http.address.should == 'me' + http.port.should == 54321 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 + Puppet::Network::HttpPool.http_instance("me", 54321).should be_use_ssl end - it "should set the read timeout" do - Puppet::Network::HttpPool.http_instance("me", 54321).read_timeout.should == 120 - end + context "proxy and timeout settings should propagate" do + subject { Puppet::Network::HttpPool.http_instance("me", 54321) } + before :each do + Puppet[:http_proxy_host] = "myhost" + Puppet[:http_proxy_port] = 432 + Puppet[:configtimeout] = 120 + end - it "should set the open timeout" do - Puppet::Network::HttpPool.http_instance("me", 54321).open_timeout.should == 120 + its(:open_timeout) { should == Puppet[:configtimeout] } + its(:read_timeout) { should == Puppet[:configtimeout] } + its(:proxy_address) { should == Puppet[:http_proxy_host] } + its(:proxy_port) { should == Puppet[:http_proxy_port] } end - it "should create the http instance with the proxy host and port set if the http_proxy is not set to 'none'" do - stub_settings :http_proxy_host => "myhost", :http_proxy_port => 432, :configtimeout => 120 - Puppet::Network::HttpPool.http_instance("me", 54321).open_timeout.should == 120 + it "should not set a proxy if the value is 'none'" do + Puppet[:http_proxy_host] = 'none' + Puppet::Network::HttpPool.http_instance("me", 54321).proxy_address.should be_nil end it "should not cache http instances" do - stub_settings :http_proxy_host => "myhost", :http_proxy_port => 432, :configtimeout => 120 - old = Puppet::Network::HttpPool.http_instance("me", 54321) - Puppet::Network::HttpPool.http_instance("me", 54321).should_not equal(old) + Puppet::Network::HttpPool.http_instance("me", 54321). + should_not equal Puppet::Network::HttpPool.http_instance("me", 54321) 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 + describe "when doing SSL setup for http instances" do + let :http do + http = Net::HTTP.new('localhost', 443) + http.use_ssl = true + http end - after do - Puppet.settings.clear - end + let :store do stub('store') 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 + before :each do + Puppet[:hostcert] = '/host/cert' + Puppet[:localcacert] = '/local/ca/cert' - 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) + cert = stub 'cert', :content => 'real_cert' + key = stub 'key', :content => 'real_key' + host = stub 'host', :certificate => cert, :key => key, :ssl_store => store + Puppet::Network::HttpPool.stubs(:ssl_host).returns(host) end - it "should add a certificate store from the ssl host" do - @http.expects(:cert_store=).with(@store) + shared_examples "HTTPS setup without all certificates" do + subject { Puppet::Network::HttpPool.cert_setup(http); http } - Puppet::Network::HttpPool.cert_setup(@http) + it { should be_use_ssl } + its(:cert) { should be_nil } + its(:cert_store) { should be_nil } + its(:ca_file) { should be_nil } + its(:key) { should be_nil } + its(:verify_mode) { should == OpenSSL::SSL::VERIFY_NONE } end - it "should add the client certificate" do - @http.expects(:cert=).with("real_cert") + context "with neither a host cert or a local CA cert" do + before :each do + FileTest.stubs(:exist?).with(Puppet[:hostcert]).returns(false) + FileTest.stubs(:exist?).with(Puppet[:localcacert]).returns(false) + end - Puppet::Network::HttpPool.cert_setup(@http) + include_examples "HTTPS setup without all certificates" end - it "should add the client key" do - @http.expects(:key=).with("real_key") + context "with there is no host certificate" do + before :each do + FileTest.stubs(:exist?).with(Puppet[:hostcert]).returns(false) + FileTest.stubs(:exist?).with(Puppet[:localcacert]).returns(true) + end - Puppet::Network::HttpPool.cert_setup(@http) + include_examples "HTTPS setup without all certificates" end - it "should set the verify mode to OpenSSL::SSL::VERIFY_PEER" do - @http.expects(:verify_mode=).with(OpenSSL::SSL::VERIFY_PEER) + context "with there is no local CA certificate" do + before :each do + FileTest.stubs(:exist?).with(Puppet[:hostcert]).returns(true) + FileTest.stubs(:exist?).with(Puppet[:localcacert]).returns(false) + end - Puppet::Network::HttpPool.cert_setup(@http) + include_examples "HTTPS setup without all certificates" end - it "should set the ca file" do - Puppet.settings.stubs(:value).returns "/some/file" - FileTest.stubs(:exist?).with(Puppet[:hostcert]).returns true + context "with both the host and CA cert" do + subject { Puppet::Network::HttpPool.cert_setup(http); http } - 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") + before :each do + FileTest.expects(:exist?).with(Puppet[:hostcert]).returns(true) + FileTest.expects(:exist?).with(Puppet[:localcacert]).returns(true) + end - Puppet::Network::HttpPool.cert_setup(@http) + it { should be_use_ssl } + its(:cert_store) { should equal store } + its(:cert) { should == "real_cert" } + its(:key) { should == "real_key" } + its(:verify_mode) { should == OpenSSL::SSL::VERIFY_PEER } + its(:ca_file) { should == Puppet[:localcacert] } 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.expects(:cert_setup).with do |http| + http.should be_an_instance_of Net::HTTP + http.address.should == "one" + http.port.should == 2 + end + + Puppet::Network::HttpPool.http_instance("one", 2) end end end