Refactor HTTPS tests

This contains various improvements in tests for openssl integration:

  - Remove DHE parameters from test servers. OpenSSL is almost always
    compiled with ECC support nowadays and will prefer ECDHE over DHE.
  - Remove an outdated omission for a bug in OpenSSL 1.1.0h released in
    2018. None of our CI systems use this specific OpenSSL version.
  - Use top-level return to skip tests if openssl is unavailable.
  - Refactor tests for Net::HTTP#verify_callback.

Author: rhenium

test/net/fixtures/dhparams.pem

@@ -7,6 +7,8 @@
   # should skip this test
 end
 
+return unless defined?(OpenSSL::SSL)
+
 class TestNetHTTPS < Test::Unit::TestCase
   include TestNetHTTPUtils
 
@@ -19,7 +21,6 @@ def self.read_fixture(key)
   CA_CERT = OpenSSL::X509::Certificate.new(read_fixture("cacert.pem"))
   SERVER_KEY = OpenSSL::PKey.read(read_fixture("server.key"))
   SERVER_CERT = OpenSSL::X509::Certificate.new(read_fixture("server.crt"))
-  DHPARAMS = OpenSSL::PKey::DH.new(read_fixture("dhparams.pem"))
   TEST_STORE = OpenSSL::X509::Store.new.tap {|s| s.add_cert(CA_CERT) }
 
   CONFIG = {
@@ -29,44 +30,27 @@ def self.read_fixture(key)
     'ssl_enable' => true,
     'ssl_certificate' => SERVER_CERT,
     'ssl_private_key' => SERVER_KEY,
-    'ssl_tmp_dh_callback' => proc { DHPARAMS },
   }
 
   def test_get
     http = Net::HTTP.new(HOST, config("port"))
     http.use_ssl = true
     http.cert_store = TEST_STORE
-    certs = []
-    http.verify_callback = Proc.new do |preverify_ok, store_ctx|
-      certs << store_ctx.current_cert
-      preverify_ok
-    end
     http.request_get("/") {|res|
       assert_equal($test_net_http_data, res.body)
+      assert_equal(SERVER_CERT.to_der, http.peer_cert.to_der)
     }
-    # TODO: OpenSSL 1.1.1h seems to yield only SERVER_CERT; need to check the incompatibility
-    certs.zip([CA_CERT, SERVER_CERT][-certs.size..-1]) do |actual, expected|
-      assert_equal(expected.to_der, actual.to_der)
-    end
   end
 
   def test_get_SNI
     http = Net::HTTP.new(HOST, config("port"))
     http.ipaddr = config('host')
     http.use_ssl = true
     http.cert_store = TEST_STORE
-    certs = []
-    http.verify_callback = Proc.new do |preverify_ok, store_ctx|
-      certs << store_ctx.current_cert
-      preverify_ok
-    end
     http.request_get("/") {|res|
       assert_equal($test_net_http_data, res.body)
+      assert_equal(SERVER_CERT.to_der, http.peer_cert.to_der)
     }
-    # TODO: OpenSSL 1.1.1h seems to yield only SERVER_CERT; need to check the incompatibility
-    certs.zip([CA_CERT, SERVER_CERT][-certs.size..-1]) do |actual, expected|
-      assert_equal(expected.to_der, actual.to_der)
-    end
   end
 
   def test_get_SNI_proxy
@@ -78,11 +62,6 @@ def test_get_SNI_proxy
         http.ipaddr = "192.0.2.1"
         http.use_ssl = true
         http.cert_store = TEST_STORE
-        certs = []
-        http.verify_callback = Proc.new do |preverify_ok, store_ctx|
-          certs << store_ctx.current_cert
-          preverify_ok
-        end
         begin
           http.start
         rescue EOFError
@@ -114,11 +93,6 @@ def test_get_SNI_failure
       http.ipaddr = config('host')
       http.use_ssl = true
       http.cert_store = TEST_STORE
-      certs = []
-      http.verify_callback = Proc.new do |preverify_ok, store_ctx|
-        certs << store_ctx.current_cert
-        preverify_ok
-      end
       @log_tester = lambda {|_| }
       assert_raise(OpenSSL::SSL::SSLError){ http.start }
     end
@@ -135,10 +109,6 @@ def test_post
   end
 
   def test_session_reuse
-    # FIXME: The new_session_cb is known broken for clients in OpenSSL 1.1.0h.
-    # See https://github.com/openssl/openssl/pull/5967 for details.
-    omit if OpenSSL::OPENSSL_LIBRARY_VERSION.include?('OpenSSL 1.1.0h')
-
     http = Net::HTTP.new(HOST, config("port"))
     http.use_ssl = true
     http.cert_store = TEST_STORE
@@ -165,9 +135,6 @@ def test_session_reuse
   end
 
   def test_session_reuse_but_expire
-    # FIXME: The new_session_cb is known broken for clients in OpenSSL 1.1.0h.
-    omit if OpenSSL::OPENSSL_LIBRARY_VERSION.include?('OpenSSL 1.1.0h')
-
     http = Net::HTTP.new(HOST, config("port"))
     http.use_ssl = true
     http.cert_store = TEST_STORE
@@ -240,6 +207,21 @@ def test_certificate_verify_failure
     assert_match(/certificate verify failed/, ex.message)
   end
 
+  def test_verify_callback
+    http = Net::HTTP.new(HOST, config("port"))
+    http.use_ssl = true
+    http.cert_store = TEST_STORE
+    certs = []
+    http.verify_callback = Proc.new {|preverify_ok, store_ctx|
+      certs << store_ctx.current_cert
+      preverify_ok
+    }
+    http.request_get("/") {|res|
+      assert_equal($test_net_http_data, res.body)
+    }
+    assert_equal(SERVER_CERT.to_der, certs.last.to_der)
+  end
+
   def test_timeout_during_SSL_handshake
     bug4246 = "expected the SSL connection to have timed out but have not. [ruby-core:34203]"
 
@@ -275,9 +257,7 @@ def test_max_version
     http = Net::HTTP.new(HOST, config("port"))
     http.use_ssl = true
     http.max_version = :SSL2
-    http.verify_callback = Proc.new do |preverify_ok, store_ctx|
-      true
-    end
+    http.cert_store = TEST_STORE
     @log_tester = lambda {|_| }
     ex = assert_raise(OpenSSL::SSL::SSLError){
       http.request_get("/") {|res| }
@@ -286,7 +266,7 @@ def test_max_version
     assert_match(re_msg, ex.message)
   end
 
-end if defined?(OpenSSL::SSL)
+end
 
 class TestNetHTTPSIdentityVerifyFailure < Test::Unit::TestCase
   include TestNetHTTPUtils
@@ -300,7 +280,6 @@ def self.read_fixture(key)
   CA_CERT = OpenSSL::X509::Certificate.new(read_fixture("cacert.pem"))
   SERVER_KEY = OpenSSL::PKey.read(read_fixture("server.key"))
   SERVER_CERT = OpenSSL::X509::Certificate.new(read_fixture("server.crt"))
-  DHPARAMS = OpenSSL::PKey::DH.new(read_fixture("dhparams.pem"))
   TEST_STORE = OpenSSL::X509::Store.new.tap {|s| s.add_cert(CA_CERT) }
 
   CONFIG = {
@@ -310,7 +289,6 @@ def self.read_fixture(key)
     'ssl_enable' => true,
     'ssl_certificate' => SERVER_CERT,
     'ssl_private_key' => SERVER_KEY,
-    'ssl_tmp_dh_callback' => proc { DHPARAMS },
   }
 
   def test_identity_verify_failure
@@ -326,4 +304,4 @@ def test_identity_verify_failure
     re_msg = /certificate verify failed|hostname \"#{HOST_IP}\" does not match/
     assert_match(re_msg, ex.message)
   end
-end if defined?(OpenSSL::SSL)
+end

test/net/http/test_https.rb

@@ -5,14 +5,10 @@
 end
 require 'test/unit'
 
+return unless defined?(OpenSSL::SSL)
+
 class HTTPSProxyTest < Test::Unit::TestCase
   def test_https_proxy_authentication
-    begin
-      OpenSSL
-    rescue LoadError
-      omit 'autoload problem. see [ruby-dev:45021][Bug #5786]'
-    end
-
     TCPServer.open("127.0.0.1", 0) {|serv|
       _, port, _, _ = serv.addr
       client_thread = Thread.new {
@@ -50,12 +46,6 @@ def read_fixture(key)
   end
 
   def test_https_proxy_ssl_connection
-    begin
-      OpenSSL
-    rescue LoadError
-      omit 'autoload problem. see [ruby-dev:45021][Bug #5786]'
-    end
-
     TCPServer.open("127.0.0.1", 0) {|tcpserver|
       ctx = OpenSSL::SSL::SSLContext.new
       ctx.key = OpenSSL::PKey.read(read_fixture("server.key"))
@@ -91,4 +81,4 @@ def test_https_proxy_ssl_connection
       assert_join_threads([client_thread, server_thread])
     }
   end
-end if defined?(OpenSSL)
+end

test/net/http/test_https_proxy.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: false
 require 'socket'
-require 'openssl'
 
 module TestNetHTTPUtils
 
@@ -14,10 +13,10 @@ def initialize(config, &block)
       @procs = {}
 
       if @config['ssl_enable']
+        require 'openssl'
         context = OpenSSL::SSL::SSLContext.new
         context.cert = @config['ssl_certificate']
         context.key = @config['ssl_private_key']
-        context.tmp_dh_callback = @config['ssl_tmp_dh_callback']
         @ssl_server = OpenSSL::SSL::SSLServer.new(@server, context)
       end