Project

General

Profile

Bug #14005

[PATCH] webrick: do not hang acceptor on slow TLS connections

Added by normalperson (Eric Wong) over 1 year ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
[ruby-core:83221]

Description

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

Files


Related issues

Related to Ruby trunk - Bug #14011: Appveyor failure - svn 60172 - ‘webrick: do not hang acceptor on slow TLS connections’RejectedActions

Associated revisions

Revision feafe078
Added by normal over 1 year ago

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]

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

Revision 60172
Added by normalperson (Eric Wong) over 1 year ago

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]

Revision 60172
Added by normal over 1 year ago

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]

Revision 60172
Added by normal over 1 year ago

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]

Revision 525ebb86
Added by normal over 1 year ago

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]

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

Revision 60189
Added by normalperson (Eric Wong) over 1 year ago

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]

Revision 60189
Added by normal over 1 year ago

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]

Revision 60189
Added by normal over 1 year ago

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]

Revision 3b1db7d3
Added by normal over 1 year ago

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]

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

Revision 60208
Added by normalperson (Eric Wong) over 1 year ago

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]

Revision 60208
Added by normal over 1 year ago

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]

Revision 60208
Added by normal over 1 year ago

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]

Revision 60210
Added by normalperson (Eric Wong) over 1 year ago

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

Revision 60211
Added by normalperson (Eric Wong) over 1 year ago

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

Revision 2e728d51
Added by nagachika (Tomoyuki Chikanaga) over 1 year ago

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

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

Revision 61239
Added by nagachika (Tomoyuki Chikanaga) over 1 year ago

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

Revision 1beda297
Added by usa (Usaku NAKAMURA) over 1 year ago

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

Revision 61240
Added by usa (Usaku NAKAMURA) over 1 year ago

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

History

#1

Updated by Anonymous over 1 year ago

  • Status changed from Open to Closed

Applied in changeset trunk|r60172.


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]
#2

Updated by nagachika (Tomoyuki Chikanaga) over 1 year ago

  • Related to Bug #14011: Appveyor failure - svn 60172 - ‘webrick: do not hang acceptor on slow TLS connections’ added

Updated by normalperson (Eric Wong) over 1 year ago

  • Status changed from Closed to Open

needs r60211, too :x

#4

Updated by normalperson (Eric Wong) over 1 year ago

  • Status changed from Open to Closed

Updated by nagachika (Tomoyuki Chikanaga) over 1 year ago

  • Backport changed from 2.3: REQUIRED, 2.4: REQUIRED to 2.3: REQUIRED, 2.4: DONE

ruby_2_4 r61239 merged revision(s) 60123,60172,60189,60208,60210,60211.

Updated by usa (Usaku NAKAMURA) over 1 year ago

  • Backport changed from 2.3: REQUIRED, 2.4: DONE to 2.3: DONE, 2.4: DONE

ruby_2_3 r61240 merged revision(s) 60172,60189,60208,60210,60211.

Also available in: Atom PDF