Merge pull request #7401 from joshcooper/certmismatch_8213

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

Author: jhelwig

lib/puppet/network/http/connection.rb

@@ -328,11 +328,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(', ') }
@@ -342,6 +338,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

spec/unit/network/http/connection_spec.rb

@@ -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',
@@ -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')]))
 
@@ -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,
@@ -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')