Skip to content

Revert "Merge pull request #7401 from joshcooper/certmismatch_8213" #7416

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.

Sign up for GitHub

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 1 commit into from
Mar 4, 2019
Merged

Revert "Merge pull request #7401 from joshcooper/certmismatch_8213" #7416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,11 @@ def with_connection(site, &block)
# can be nil
peer_cert = @verify.peer_certs.last

if peer_cert && !OpenSSL::SSL.verify_certificate_identity(peer_cert.content, site.host)
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)
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 @@ -338,10 +342,6 @@ 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: 4 additions & 22 deletions spec/unit/network/http/connection_spec.rb
View file
Open in desktop
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 in ruby 2.4 and up" do
it "should provide a useful error message when one is available and certificate validation fails", :unless => Puppet.features.microsoft_windows? 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 does not match server certificate before ruby 2.4" do
it "should provide a helpful error message when hostname was not match with server certificate", :unless => Puppet.features.microsoft_windows? do
Puppet[:confdir] = tmpdir('conf')

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

Expand All @@ -142,24 +142,6 @@ 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 @@ -170,7 +152,7 @@ def verify_errors
end.to raise_error(/some other message/)
end

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