merge revision(s) 64234,64252: [Backport #15219]

	net/http, net/ftp: fix session resumption with TLS 1.3

	When TLS 1.3 is in use, the session ticket may not have been sent yet
	even though a handshake has finished. Also, the ticket could change if
	multiple session ticket messages are sent by the server. Use
	SSLContext#session_new_cb instead of calling SSLSocket#session
	immediately after a handshake. This way also works with earlier protocol
	versions.

	net/http, net/ftp: skip SSL/TLS session resumption tests

	Due to a bug in OpenSSL 1.1.0h[1] (it's only in this specific version;
	it was introduced just before the release and is already fixed in their
	stable branch), the callback set by SSLContext#session_new_cb= does not
	get called for clients, making net/http and net/ftp not attempt session
	resumption.

	Let's disable the affected test cases for now. Another option would be
	to fallback to using SSLSocket#session as we did before r64234. But
	since only a single version is affected and hopefully a new stable
	version containing the fix will be released in near future, I chose not
	to add such workaround code to lib/.

	[1] https://github.com/openssl/openssl/pull/5967

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_5@67237 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

5 files changed, 40 insertions, 29 deletions

diff --git a/lib/net/ftp.rb b/lib/net/ftp.rb
index c3ee47ef4d..9902f9dc65 100644
--- a/lib/net/ftp.rb
+++ b/lib/net/ftp.rb
@@ -230,6 +230,10 @@ module Net
         if defined?(VerifyCallbackProc)
           @ssl_context.verify_callback = VerifyCallbackProc
         end
+        @ssl_context.session_cache_mode =
+          OpenSSL::SSL::SSLContext::SESSION_CACHE_CLIENT |
+          OpenSSL::SSL::SSLContext::SESSION_CACHE_NO_INTERNAL_STORE
+        @ssl_context.session_new_cb = proc {|sock, sess| @ssl_session = sess }
         @ssl_session = nil
         if options[:private_data_connection].nil?
           @private_data_connection = true
@@ -349,7 +353,6 @@ module Net
       if @ssl_context.verify_mode != VERIFY_NONE
         ssl_sock.post_connection_check(@host)
       end
-      @ssl_session = ssl_sock.session
       return ssl_sock
     end
     private :start_tls_session
diff --git a/lib/net/http.rb b/lib/net/http.rb
index e3b5619e8a..961ef398c3 100644
--- a/lib/net/http.rb
+++ b/lib/net/http.rb
@@ -969,6 +969,10 @@ module Net   #:nodoc:
         end
         @ssl_context = OpenSSL::SSL::SSLContext.new
         @ssl_context.set_params(ssl_parameters)
+        @ssl_context.session_cache_mode =
+          OpenSSL::SSL::SSLContext::SESSION_CACHE_CLIENT |
+          OpenSSL::SSL::SSLContext::SESSION_CACHE_NO_INTERNAL_STORE
+        @ssl_context.session_new_cb = proc {|sock, sess| @ssl_session = sess }
         D "starting SSL for #{conn_address}:#{conn_port}..."
         s = OpenSSL::SSL::SSLSocket.new(s, @ssl_context)
         s.sync_close = true
@@ -976,13 +980,12 @@ module Net   #:nodoc:
         s.hostname = @address if s.respond_to? :hostname=
         if @ssl_session and
            Process.clock_gettime(Process::CLOCK_REALTIME) < @ssl_session.time.to_f + @ssl_session.timeout
-          s.session = @ssl_session if @ssl_session
+          s.session = @ssl_session
         end
         ssl_socket_connect(s, @open_timeout)
         if @ssl_context.verify_mode != OpenSSL::SSL::VERIFY_NONE
           s.post_connection_check(@address)
         end
-        @ssl_session = s.session
         D "SSL established"
       end
       @socket = BufferedIO.new(s, read_timeout: @read_timeout,
diff --git a/test/net/ftp/test_ftp.rb b/test/net/ftp/test_ftp.rb
index 03204ebc52..8177bfe36e 100644
--- a/test/net/ftp/test_ftp.rb
+++ b/test/net/ftp/test_ftp.rb
@@ -1755,6 +1755,7 @@ EOF
       server = TCPServer.new(SERVER_ADDR, 0)
       port = server.addr[1]
       commands = []
+      session_reused_for_data_connection = nil
       binary_data = (0..0xff).map {|i| i.chr}.join * 4 * 3
       @thread = Thread.start do
         sock = server.accept
@@ -1793,6 +1794,7 @@ EOF
             conn = OpenSSL::SSL::SSLSocket.new(conn, ctx)
             conn.sync_close = true
             conn.accept
+            session_reused_for_data_connection = conn.session_reused?
             binary_data.scan(/.{1,1024}/nm) do |s|
               conn.print(s)
             end
@@ -1823,6 +1825,11 @@ EOF
         assert_match(/\A(PORT|EPRT) /, commands.shift)
         assert_equal("RETR foo\r\n", commands.shift)
         assert_equal(nil, commands.shift)
+        # 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.
+        if OpenSSL::OPENSSL_LIBRARY_VERSION !~ /OpenSSL 1.1.0h/
+          assert_equal(true, session_reused_for_data_connection)
+        end
       ensure
         ftp.close
       end
@@ -1832,6 +1839,7 @@ EOF
       server = TCPServer.new(SERVER_ADDR, 0)
       port = server.addr[1]
       commands = []
+      session_reused_for_data_connection = nil
       binary_data = (0..0xff).map {|i| i.chr}.join * 4 * 3
       @thread = Thread.start do
         sock = server.accept
@@ -1869,6 +1877,7 @@ EOF
             conn = OpenSSL::SSL::SSLSocket.new(conn, ctx)
             conn.sync_close = true
             conn.accept
+            session_reused_for_data_connection = conn.session_reused?
             binary_data.scan(/.{1,1024}/nm) do |s|
               conn.print(s)
             end
@@ -1900,6 +1909,10 @@ EOF
         assert_match(/\A(PASV|EPSV)\r\n/, commands.shift)
         assert_equal("RETR foo\r\n", commands.shift)
         assert_equal(nil, commands.shift)
+        # FIXME: The new_session_cb is known broken for clients in OpenSSL 1.1.0h.
+        if OpenSSL::OPENSSL_LIBRARY_VERSION !~ /OpenSSL 1.1.0h/
+          assert_equal(true, session_reused_for_data_connection)
+        end
       ensure
         ftp.close
       end
diff --git a/test/net/http/test_https.rb b/test/net/http/test_https.rb
index 8004d5c5f2..c1d486470a 100644
--- a/test/net/http/test_https.rb
+++ b/test/net/http/test_https.rb
@@ -63,6 +63,10 @@ class TestNetHTTPS < Test::Unit::TestCase
   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.
+    skip if OpenSSL::OPENSSL_LIBRARY_VERSION =~ /OpenSSL 1.1.0h/
+
     http = Net::HTTP.new("localhost", config("port"))
     http.use_ssl = true
     http.cert_store = TEST_STORE
@@ -73,18 +77,9 @@ class TestNetHTTPS < Test::Unit::TestCase
 
     http.start
     http.get("/")
-    http.finish # three times due to possible bug in OpenSSL 0.9.8
-
-    sid = http.instance_variable_get(:@ssl_session).id
-
-    http.start
-    http.get("/")
 
     socket = http.instance_variable_get(:@socket).io
-
-    assert socket.session_reused?
-
-    assert_equal sid, http.instance_variable_get(:@ssl_session).id
+    assert_equal true, socket.session_reused?
 
     http.finish
   rescue SystemCallError
@@ -92,6 +87,9 @@ class TestNetHTTPS < Test::Unit::TestCase
   end
 
   def test_session_reuse_but_expire
+    # FIXME: The new_session_cb is known broken for clients in OpenSSL 1.1.0h.
+    skip if OpenSSL::OPENSSL_LIBRARY_VERSION =~ /OpenSSL 1.1.0h/
+
     http = Net::HTTP.new("localhost", config("port"))
     http.use_ssl = true
     http.cert_store = TEST_STORE
@@ -101,16 +99,12 @@ class TestNetHTTPS < Test::Unit::TestCase
     http.get("/")
     http.finish
 
-    sid = http.instance_variable_get(:@ssl_session).id
-
     http.start
     http.get("/")
 
     socket = http.instance_variable_get(:@socket).io
     assert_equal false, socket.session_reused?
 
-    assert_not_equal sid, http.instance_variable_get(:@ssl_session).id
-
     http.finish
   rescue SystemCallError
     skip $!
@@ -160,15 +154,16 @@ class TestNetHTTPS < Test::Unit::TestCase
   end
 
   def test_identity_verify_failure
+    # the certificate's subject has CN=localhost
     http = Net::HTTP.new("127.0.0.1", config("port"))
     http.use_ssl = true
-    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| }
     }
-    assert_match(/hostname \"127.0.0.1\" does not match/, ex.message)
+    re_msg = /certificate verify failed|hostname \"127.0.0.1\" does not match/
+    assert_match(re_msg, ex.message)
   end
 
   def test_timeout_during_SSL_handshake
@@ -193,16 +188,13 @@ class TestNetHTTPS < Test::Unit::TestCase
   end
 
   def test_min_version
-    http = Net::HTTP.new("127.0.0.1", config("port"))
+    http = Net::HTTP.new("localhost", config("port"))
     http.use_ssl = true
     http.min_version = :TLS1
-    http.verify_callback = Proc.new do |preverify_ok, store_ctx|
-      true
-    end
-    ex = assert_raise(OpenSSL::SSL::SSLError){
-      http.request_get("/") {|res| }
+    http.cert_store = TEST_STORE
+    http.request_get("/") {|res|
+      assert_equal($test_net_http_data, res.body)
     }
-    assert_match(/hostname \"127.0.0.1\" does not match/, ex.message)
   end
 
   def test_max_version
diff --git a/version.h b/version.h
index f1792fe296..a2e53cc0c4 100644
--- a/version.h
+++ b/version.h
@@ -1,6 +1,6 @@
 #define RUBY_VERSION "2.5.4"
 #define RUBY_RELEASE_DATE "2019-03-13"
-#define RUBY_PATCHLEVEL 149
+#define RUBY_PATCHLEVEL 150
 
 #define RUBY_RELEASE_YEAR 2019
 #define RUBY_RELEASE_MONTH 3