Skip to content

(PUP-8213) Display correct message when certname is mismatched #7401

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions lib/puppet/network/http/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -318,11 +318,7 @@ def with_connection(site, &block)
# can be nil
peer_cert = @verify.peer_certs.last

if error.message.include? "certificate verify failed"
msg = error.message
msg << ": [" + @verify.verify_errors.join('; ') + "]"
raise Puppet::Error, msg, error.backtrace
elsif peer_cert && !OpenSSL::SSL.verify_certificate_identity(peer_cert.content, site.host)
if peer_cert && !OpenSSL::SSL.verify_certificate_identity(peer_cert.content, site.host)
valid_certnames = [peer_cert.name, *peer_cert.subject_alt_names].uniq
if valid_certnames.size > 1
expected_certnames = _("expected one of %{certnames}") % { certnames: valid_certnames.join(', ') }
Expand All @@ -332,6 +328,10 @@ def with_connection(site, &block)

msg = _("Server hostname '%{host}' did not match server certificate; %{expected_certnames}") % { host: site.host, expected_certnames: expected_certnames }
raise Puppet::Error, msg, error.backtrace
elsif !@verify.verify_errors.empty?
msg = error.message
msg << ": [" + @verify.verify_errors.join('; ') + "]"
raise Puppet::Error, msg, error.backtrace
else
raise
end
Expand Down
26 changes: 22 additions & 4 deletions spec/unit/network/http/connection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def verify_errors
WebMock.enable!
end

it "should provide a useful error message when one is available and certificate validation fails", :unless => Puppet.features.microsoft_windows? do
it "should provide a useful error message when one is available and certificate validation fails in ruby 2.4 and up" do
connection = Puppet::Network::HTTP::Connection.new(
host, port,
:verify => ConstantErrorValidator.new(:fails_with => 'certificate verify failed',
Expand All @@ -124,13 +124,13 @@ def verify_errors
end.to raise_error(Puppet::Error, "certificate verify failed: [shady looking signature]")
end

it "should provide a helpful error message when hostname was not match with server certificate", :unless => Puppet.features.microsoft_windows? do
it "should provide a helpful error message when hostname does not match server certificate before ruby 2.4" do
Puppet[:confdir] = tmpdir('conf')

connection = Puppet::Network::HTTP::Connection.new(
host, port,
:verify => ConstantErrorValidator.new(
:fails_with => 'hostname was not match with server certificate',
:fails_with => "hostname 'myserver' does not match the server certificate",
:peer_certs => [Puppet::SSL::CertificateAuthority.new.generate(
'not_my_server', :dns_alt_names => 'foo,bar,baz')]))

Expand All @@ -142,6 +142,24 @@ def verify_errors
end
end

it "should provide a helpful error message when hostname does not match server certificate in ruby 2.4 or greater" do
Puppet[:confdir] = tmpdir('conf')

connection = Puppet::Network::HTTP::Connection.new(
host, port,
:verify => ConstantErrorValidator.new(
:fails_with => "certificate verify failed",
:peer_certs => [Puppet::SSL::CertificateAuthority.new.generate(
'not_my_server', :dns_alt_names => 'foo,bar,baz')]))

expect do
connection.get('request')
end.to raise_error(Puppet::Error) do |error|
error.message =~ /\AServer hostname 'my_server' did not match server certificate; expected one of (.+)/
expect($1.split(', ')).to match_array(%w[DNS:foo DNS:bar DNS:baz DNS:not_my_server not_my_server])
end
end

it "should pass along the error message otherwise" do
connection = Puppet::Network::HTTP::Connection.new(
host, port,
Expand All @@ -152,7 +170,7 @@ def verify_errors
end.to raise_error(/some other message/)
end

it "should check all peer certificates for upcoming expiration", :unless => Puppet.features.microsoft_windows? do
it "should check all peer certificates for upcoming expiration" do
Puppet[:confdir] = tmpdir('conf')
cert = Puppet::SSL::CertificateAuthority.new.generate(
'server', :dns_alt_names => 'foo,bar,baz')
Expand Down