From 778127d0c3cf01701d227ec88a8e5e6550638c35 Mon Sep 17 00:00:00 2001 From: Matt Robinson Date: Mon, 25 Jul 2011 16:18:54 -0700 Subject: maint: Fix cert app to print help and exit if no subcommand In 2.6.x this was the behavior, but the changes to the way options parsing worked in 2.7.x to support faces changed the behavior of how help was called. This resulted in the follow unhelpful error message when you just called `puppet cert`: /Users/matthewrobinson/work/puppet/lib/puppet/ssl/certificate_authority/interface.rb:85:in `method=' Invalid method to apply Reviewed-by: Pieter van de Bruggen --- lib/puppet/application/cert.rb | 3 ++- spec/unit/application/cert_spec.rb | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/puppet/application/cert.rb b/lib/puppet/application/cert.rb index 162672b6a..330fba8bd 100644 --- a/lib/puppet/application/cert.rb +++ b/lib/puppet/application/cert.rb @@ -218,7 +218,8 @@ Copyright (c) 2011 Puppet Labs, LLC Licensed under the Apache 2.0 License if sub = self.command_line.args.shift then self.subcommand = sub else - help + puts help + exit end end result diff --git a/spec/unit/application/cert_spec.rb b/spec/unit/application/cert_spec.rb index 7510f0783..300234c2b 100755 --- a/spec/unit/application/cert_spec.rb +++ b/spec/unit/application/cert_spec.rb @@ -208,5 +208,15 @@ describe Puppet::Application::Cert, :'fails_on_ruby_1.9.2' => true do args.should == ["fun.example.com"] end end + + it "should print help and exit if there is no subcommand" do + args = [] + @cert_app.command_line.stubs(:args).returns(args) + @cert_app.stubs(:help).returns("I called for help!") + @cert_app.expects(:puts).with("I called for help!") + + expect { @cert_app.parse_options }.to exit_with 0 + @cert_app.subcommand.should be_nil + end end end -- cgit From 1d4acb5afda61b1f2c05223afff19c68248a3996 Mon Sep 17 00:00:00 2001 From: Matt Robinson Date: Tue, 19 Jul 2011 11:27:03 -0700 Subject: maint: Suggest where to start troubleshooting SSL error message Much like the infamous "hostname was not match" error message, there's another SSL error that people run into that isn't clear how to troubleshoot. err: Could not send report: SSL_connect returned=1 errno=0 state=SSLv3 read server certificate B: certificate verify failed. As far as I can tell this only ever happens when the clock is off on the master or client. People seem to think it will happen other times, but I haven't been able to reproduce it other ways - missing private key, revoked cert, offline CA all have their own errors. I googled around and the only thing I've seen for this error in relation to puppet is the time sync problem. So the error message text just has some additional info to suggest you check your clocks. Reviewed-by: Nick Lewis --- lib/puppet/indirector/rest.rb | 4 +- spec/unit/indirector/rest_spec.rb | 83 ++++++++++++++++++++++----------------- 2 files changed, 50 insertions(+), 37 deletions(-) diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 8018fe8e3..19daff51d 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -93,7 +93,9 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus http_connection.send(method, *args) rescue OpenSSL::SSL::SSLError => error - if error.message.include? "hostname was not match" + if error.message.include? "certificate verify failed" + raise Puppet::Error, "#{error.message}. This is often because the time is out of sync on the server or client" + elsif error.message.include? "hostname was not match" raise unless cert = peer_certs.find { |c| c.name !~ /^puppet ca/i } valid_certnames = [cert.name, *cert.alternate_names].uniq diff --git a/spec/unit/indirector/rest_spec.rb b/spec/unit/indirector/rest_spec.rb index ee0111a77..042b7ca16 100755 --- a/spec/unit/indirector/rest_spec.rb +++ b/spec/unit/indirector/rest_spec.rb @@ -90,42 +90,53 @@ describe Puppet::Indirector::REST do @rest_class.port.should == 543 end - describe "when making http requests" do - it "should provide a helpful error message when hostname was not match with server certificate" do - Puppet[:certdnsnames] = 'foo:bar:baz' - csr = OpenSSL::X509::Request.new - csr.subject = OpenSSL::X509::Name.new([['CN', 'not_my_server']]) - csr.public_key = OpenSSL::PKey::RSA.generate(Puppet[:keylength]).public_key - cert = Puppet::SSL::CertificateFactory.new('server', csr, csr, 14).result - - connection = Net::HTTP.new('my_server', 8140) - @searcher.stubs(:network).returns(connection) - ssl_context = OpenSSL::SSL::SSLContext.new - ssl_context.stubs(:current_cert).returns(cert) - connection.stubs(:get).with do - connection.verify_callback.call(true, ssl_context) - end.raises(OpenSSL::SSL::SSLError.new('hostname was not match with server certificate')) - - msg = /Server hostname 'my_server' did not match server certificate; expected one of (.+)/ - expect { @searcher.http_request(:get, stub('request')) }.to( - raise_error(Puppet::Error, msg) do |error| - error.message =~ msg - $1.split(', ').should =~ ['foo', 'bar', 'baz', 'not_my_server'] - end - ) - end - - it "should pass along the error message otherwise" do - connection = Net::HTTP.new('my_server', 8140) - @searcher.stubs(:network).returns(connection) - - connection.stubs(:get).raises(OpenSSL::SSL::SSLError.new('certificate verify failed')) - - expect do - @searcher.http_request(:get, stub('request')) - end.to raise_error(/certificate verify failed/) - end - end + describe "when making http requests" do + it "should provide a suggestive error message when certificate verify failed" do + connection = Net::HTTP.new('my_server', 8140) + @searcher.stubs(:network).returns(connection) + + connection.stubs(:get).raises(OpenSSL::SSL::SSLError.new('certificate verify failed')) + + expect do + @searcher.http_request(:get, stub('request')) + end.to raise_error(/This is often because the time is out of sync on the server or client/) + end + + it "should provide a helpful error message when hostname was not match with server certificate" do + Puppet[:certdnsnames] = 'foo:bar:baz' + csr = OpenSSL::X509::Request.new + csr.subject = OpenSSL::X509::Name.new([['CN', 'not_my_server']]) + csr.public_key = OpenSSL::PKey::RSA.generate(Puppet[:keylength]).public_key + cert = Puppet::SSL::CertificateFactory.new('server', csr, csr, 14).result + + connection = Net::HTTP.new('my_server', 8140) + @searcher.stubs(:network).returns(connection) + ssl_context = OpenSSL::SSL::SSLContext.new + ssl_context.stubs(:current_cert).returns(cert) + connection.stubs(:get).with do + connection.verify_callback.call(true, ssl_context) + end.raises(OpenSSL::SSL::SSLError.new('hostname was not match with server certificate')) + + msg = /Server hostname 'my_server' did not match server certificate; expected one of (.+)/ + expect { @searcher.http_request(:get, stub('request')) }.to( + raise_error(Puppet::Error, msg) do |error| + error.message =~ msg + $1.split(', ').should =~ ['foo', 'bar', 'baz', 'not_my_server'] + end + ) + end + + it "should pass along the error message otherwise" do + connection = Net::HTTP.new('my_server', 8140) + @searcher.stubs(:network).returns(connection) + + connection.stubs(:get).raises(OpenSSL::SSL::SSLError.new('some other message')) + + expect do + @searcher.http_request(:get, stub('request')) + end.to raise_error(/some other message/) + end + end describe "when deserializing responses" do it "should return nil if the response code is 404" do -- cgit