ruby-changes:49123
From: nagachika <ko1@a...>
Date: Thu, 14 Dec 2017 22:31:47 +0900 (JST)
Subject: [ruby-changes:49123] nagachika:r61239 (ruby_2_4): merge revision(s) 60123, 60172, 60189, 60208, 60210, 60211: [Backport #14005]
nagachika 2017-12-14 22:31:41 +0900 (Thu, 14 Dec 2017) New Revision: 61239 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=61239 Log: merge revision(s) 60123,60172,60189,60208,60210,60211: [Backport #14005] webrick: avoid unnecessary IO#sync= call Sockets and pipes are always created with FMODE_SYNC flag already set (otherwise many things would be broken). * lib/webrick/server.rb (accept_client): remove unnecessary IO#sync= call webrick: do not hang acceptor on slow TLS connections OpenSSL::SSL::SSLSocket#accept may block indefinitely on clients which negotiate the TCP connection, but fail (or are slow) to negotiate the subsequent TLS handshake. This prevents the multi-threaded WEBrick server from accepting other connections. Since the TLS handshake (via OpenSSL::SSL::SSLSocket#accept) consists of normal read/write traffic over TCP, handle it in the per-client thread, instead. Furthermore, using non-blocking accept() is useful for non-TLS sockets anyways because spurious wakeups are possible from select(2). * lib/webrick/server.rb (accept_client): use TCPServer#accept_nonblock and remove OpenSSL::SSL::SSLSocket#accept call * lib/webrick/server.rb (start_thread): call OpenSSL::SSL::SSLSocket#accept * test/webrick/test_ssl_server.rb (test_slow_connect): new test [ruby-core:83221] [Bug #14005] webrick: fix up r60172 By making the socket non-blocking in r60172, TLS/SSL negotiation via the SSL_accept function must handle non-blocking sockets properly and retry on SSL_ERROR_WANT_READ/SSL_ERROR_WANT_WRITE. OpenSSL::SSL::SSLSocket#accept cannot do that properly with a non-blocking socket, so it must use non-blocking logic of OpenSSL::SSL::SSLSocket#accept_nonblock. Thanks to MSP-Greg (Greg L) for finding this. * lib/webrick/server.rb (start_thread): use SSL_accept properly with non-blocking socket. [Bug #14013] [Bug #14005] webrick: fix up r60172 and revert r60189 Thanks to MSP-Greg (Greg L) for helping with this. * lib/webrick/server.rb (start_thread): ignore ECONNRESET, ECONNABORTED, EPROTO, and EINVAL on TLS negotiation errors the same way they were ignored before r60172 in the accept_client method of the main acceptor thread. [Bug #14013] [Bug #14005] webrick: fix up r60172 and r60208 Thanks to MSP-Greg (Greg L) for helping with this. * lib/webrick/server.rb (start_thread): fix non-local return introduced in r60208 webrick: fix up r60172 and r60210 Thanks to MSP-Greg (Greg L) for helping with this. * lib/webrick/server.rb (start_thread): properly fix non-local return introduced in r60208 and r60210 Modified directories: branches/ruby_2_4/ Modified files: branches/ruby_2_4/lib/webrick/server.rb branches/ruby_2_4/test/webrick/test_ssl_server.rb branches/ruby_2_4/version.h Index: ruby_2_4/lib/webrick/server.rb =================================================================== --- ruby_2_4/lib/webrick/server.rb (revision 61238) +++ ruby_2_4/lib/webrick/server.rb (revision 61239) @@ -252,18 +252,26 @@ module WEBrick https://github.com/ruby/ruby/blob/trunk/ruby_2_4/lib/webrick/server.rb#L252 # the client socket. def accept_client(svr) - sock = nil - begin - sock = svr.accept - sock.sync = true - Utils::set_non_blocking(sock) - rescue Errno::ECONNRESET, Errno::ECONNABORTED, - Errno::EPROTO, Errno::EINVAL - rescue StandardError => ex - msg = "#{ex.class}: #{ex.message}\n\t#{ex.backtrace[0]}" - @logger.error msg + case sock = svr.to_io.accept_nonblock(exception: false) + when :wait_readable + nil + else + if svr.respond_to?(:start_immediately) + sock = OpenSSL::SSL::SSLSocket.new(sock, ssl_context) + sock.sync_close = true + # we cannot do OpenSSL::SSL::SSLSocket#accept here because + # a slow client can prevent us from accepting connections + # from other clients + end + sock end - return sock + rescue Errno::ECONNRESET, Errno::ECONNABORTED, + Errno::EPROTO, Errno::EINVAL + nil + rescue StandardError => ex + msg = "#{ex.class}: #{ex.message}\n\t#{ex.backtrace[0]}" + @logger.error msg + nil end ## @@ -286,6 +294,16 @@ module WEBrick https://github.com/ruby/ruby/blob/trunk/ruby_2_4/lib/webrick/server.rb#L294 @logger.debug "accept: <address unknown>" raise end + if sock.respond_to?(:sync_close=) && @config[:SSLStartImmediately] + WEBrick::Utils.timeout(@config[:RequestTimeout]) do + begin + sock.accept # OpenSSL::SSL::SSLSocket#accept + rescue Errno::ECONNRESET, Errno::ECONNABORTED, + Errno::EPROTO, Errno::EINVAL + Thread.exit + end + end + end call_callback(:AcceptCallback, sock) block ? block.call(sock) : run(sock) rescue Errno::ENOTCONN Index: ruby_2_4/version.h =================================================================== --- ruby_2_4/version.h (revision 61238) +++ ruby_2_4/version.h (revision 61239) @@ -1,6 +1,6 @@ https://github.com/ruby/ruby/blob/trunk/ruby_2_4/version.h#L1 #define RUBY_VERSION "2.4.3" #define RUBY_RELEASE_DATE "2017-12-14" -#define RUBY_PATCHLEVEL 203 +#define RUBY_PATCHLEVEL 204 #define RUBY_RELEASE_YEAR 2017 #define RUBY_RELEASE_MONTH 12 Index: ruby_2_4/test/webrick/test_ssl_server.rb =================================================================== --- ruby_2_4/test/webrick/test_ssl_server.rb (revision 61238) +++ ruby_2_4/test/webrick/test_ssl_server.rb (revision 61239) @@ -2,6 +2,7 @@ require "test/unit" https://github.com/ruby/ruby/blob/trunk/ruby_2_4/test/webrick/test_ssl_server.rb#L2 require "webrick" require "webrick/ssl" require_relative "utils" +require 'timeout' class TestWEBrickSSLServer < Test::Unit::TestCase class Echo < WEBrick::GenericServer @@ -37,4 +38,30 @@ class TestWEBrickSSLServer < Test::Unit: https://github.com/ruby/ruby/blob/trunk/ruby_2_4/test/webrick/test_ssl_server.rb#L38 io.close } end + + def test_slow_connect + poke = lambda do |io, msg| + begin + sock = OpenSSL::SSL::SSLSocket.new(io) + sock.connect + sock.puts(msg) + assert_equal "#{msg}\n", sock.gets, msg + ensure + sock&.close + io.close + end + end + config = { + :SSLEnable => true, + :SSLCertName => "/C=JP/O=www.ruby-lang.org/CN=Ruby", + } + Timeout.timeout(10) do + TestWEBrick.start_server(Echo, config) do |server, addr, port, log| + outer = TCPSocket.new(addr, port) + inner = TCPSocket.new(addr, port) + poke.call(inner, 'fast TLS negotiation') + poke.call(outer, 'slow TLS negotiation') + end + end + end end Index: ruby_2_4 =================================================================== --- ruby_2_4 (revision 61238) +++ ruby_2_4 (revision 61239) Property changes on: ruby_2_4 ___________________________________________________________________ Modified: svn:mergeinfo ## -0,0 +0,1 ## Merged /trunk:r60123,60172,60189,60208,60210-60211 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/