merge revision(s) 60172,60189,60208,60210,60211: [Backport #14005]

	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

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

4 files changed, 156 insertions, 15 deletions

diff --git a/ChangeLog b/ChangeLog
index 395f11c9f9..af30f685b2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,66 @@
+Thu Dec 14 22:29:04 2017  Eric Wong  <normalperson@yhbt.net>
+
+	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#acc
+ept
+	* 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
+
 Thu Nov 30 23:37:08 2017  Nobuyoshi Nakada  <nobu@ruby-lang.org>
 
 	parse.y: fix line in rescue
diff --git a/lib/webrick/server.rb b/lib/webrick/server.rb
index e2e99bfc5f..3c19f32bf4 100644
--- a/lib/webrick/server.rb
+++ b/lib/webrick/server.rb
@@ -258,18 +258,26 @@ module WEBrick
     # 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
 
     ##
@@ -292,6 +300,16 @@ module WEBrick
             @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
diff --git a/test/webrick/test_ssl_server.rb b/test/webrick/test_ssl_server.rb
new file mode 100644
index 0000000000..04a6f9e70a
--- /dev/null
+++ b/test/webrick/test_ssl_server.rb
@@ -0,0 +1,60 @@
+require "test/unit"
+require "webrick"
+require "webrick/ssl"
+require_relative "utils"
+require 'timeout'
+
+class TestWEBrickSSLServer < Test::Unit::TestCase
+  class Echo < WEBrick::GenericServer
+    def run(sock)
+      while line = sock.gets
+        sock << line
+      end
+    end
+  end
+
+  def test_self_signed_cert_server
+    assert_self_signed_cert(
+      :SSLEnable => true,
+      :SSLCertName => [["C", "JP"], ["O", "www.ruby-lang.org"], ["CN", "Ruby"]],
+    )
+  end
+
+  def assert_self_signed_cert(config)
+    TestWEBrick.start_server(Echo, config){|server, addr, port, log|
+      io = TCPSocket.new(addr, port)
+      sock = OpenSSL::SSL::SSLSocket.new(io)
+      sock.connect
+      sock.puts(server.ssl_context.cert.subject.to_s)
+      assert_equal("/C=JP/O=www.ruby-lang.org/CN=Ruby\n", sock.gets, log.call)
+      sock.close
+      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
diff --git a/version.h b/version.h
index 53257f61ce..48fc535f86 100644
--- a/version.h
+++ b/version.h
@@ -1,10 +1,10 @@
 #define RUBY_VERSION "2.3.6"
-#define RUBY_RELEASE_DATE "2017-11-30"
-#define RUBY_PATCHLEVEL 380
+#define RUBY_RELEASE_DATE "2017-12-14"
+#define RUBY_PATCHLEVEL 381
 
 #define RUBY_RELEASE_YEAR 2017
-#define RUBY_RELEASE_MONTH 11
-#define RUBY_RELEASE_DAY 30
+#define RUBY_RELEASE_MONTH 12
+#define RUBY_RELEASE_DAY 14
 
 #include "ruby/version.h"