ruby-changes:55030
From: nagachika <ko1@a...>
Date: Wed, 13 Mar 2019 08:23:23 +0900 (JST)
Subject: [ruby-changes:55030] nagachika:r67237 (ruby_2_5): merge revision(s) 64234, 64252: [Backport #15219]
nagachika 2019-03-13 08:23:17 +0900 (Wed, 13 Mar 2019) New Revision: 67237 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=67237 Log: 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 Modified directories: branches/ruby_2_5/ Modified files: branches/ruby_2_5/lib/net/ftp.rb branches/ruby_2_5/lib/net/http.rb branches/ruby_2_5/test/net/ftp/test_ftp.rb branches/ruby_2_5/test/net/http/test_https.rb branches/ruby_2_5/version.h Index: ruby_2_5/test/net/http/test_https.rb =================================================================== --- ruby_2_5/test/net/http/test_https.rb (revision 67236) +++ ruby_2_5/test/net/http/test_https.rb (revision 67237) @@ -63,6 +63,10 @@ class TestNetHTTPS < Test::Unit::TestCas https://github.com/ruby/ruby/blob/trunk/ruby_2_5/test/net/http/test_https.rb#L63 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::TestCas https://github.com/ruby/ruby/blob/trunk/ruby_2_5/test/net/http/test_https.rb#L77 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::TestCas https://github.com/ruby/ruby/blob/trunk/ruby_2_5/test/net/http/test_https.rb#L87 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::TestCas https://github.com/ruby/ruby/blob/trunk/ruby_2_5/test/net/http/test_https.rb#L99 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::TestCas https://github.com/ruby/ruby/blob/trunk/ruby_2_5/test/net/http/test_https.rb#L154 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::TestCas https://github.com/ruby/ruby/blob/trunk/ruby_2_5/test/net/http/test_https.rb#L188 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 Index: ruby_2_5/test/net/ftp/test_ftp.rb =================================================================== --- ruby_2_5/test/net/ftp/test_ftp.rb (revision 67236) +++ ruby_2_5/test/net/ftp/test_ftp.rb (revision 67237) @@ -1755,6 +1755,7 @@ EOF https://github.com/ruby/ruby/blob/trunk/ruby_2_5/test/net/ftp/test_ftp.rb#L1755 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 https://github.com/ruby/ruby/blob/trunk/ruby_2_5/test/net/ftp/test_ftp.rb#L1794 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 https://github.com/ruby/ruby/blob/trunk/ruby_2_5/test/net/ftp/test_ftp.rb#L1825 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 https://github.com/ruby/ruby/blob/trunk/ruby_2_5/test/net/ftp/test_ftp.rb#L1839 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 https://github.com/ruby/ruby/blob/trunk/ruby_2_5/test/net/ftp/test_ftp.rb#L1877 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 https://github.com/ruby/ruby/blob/trunk/ruby_2_5/test/net/ftp/test_ftp.rb#L1909 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 Index: ruby_2_5/lib/net/http.rb =================================================================== --- ruby_2_5/lib/net/http.rb (revision 67236) +++ ruby_2_5/lib/net/http.rb (revision 67237) @@ -969,6 +969,10 @@ module Net #:nodoc: https://github.com/ruby/ruby/blob/trunk/ruby_2_5/lib/net/http.rb#L969 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: https://github.com/ruby/ruby/blob/trunk/ruby_2_5/lib/net/http.rb#L980 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, Index: ruby_2_5/lib/net/ftp.rb =================================================================== --- ruby_2_5/lib/net/ftp.rb (revision 67236) +++ ruby_2_5/lib/net/ftp.rb (revision 67237) @@ -230,6 +230,10 @@ module Net https://github.com/ruby/ruby/blob/trunk/ruby_2_5/lib/net/ftp.rb#L230 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 https://github.com/ruby/ruby/blob/trunk/ruby_2_5/lib/net/ftp.rb#L353 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 Index: ruby_2_5/version.h =================================================================== --- ruby_2_5/version.h (revision 67236) +++ ruby_2_5/version.h (revision 67237) @@ -1,6 +1,6 @@ https://github.com/ruby/ruby/blob/trunk/ruby_2_5/version.h#L1 #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 Index: ruby_2_5 =================================================================== --- ruby_2_5 (revision 67236) +++ ruby_2_5 (revision 67237) Property changes on: ruby_2_5 ___________________________________________________________________ Modified: svn:mergeinfo ## -0,0 +0,1 ## Merged /trunk:r64234,64252 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/