From 52ff4b08e2c356115ff56e38b1e1873a8b465a69 Mon Sep 17 00:00:00 2001 From: Scott Ringwelski Date: Mon, 20 Jun 2016 21:34:10 -0700 Subject: [PATCH 1/5] apply changes at https://github.com/ruby-ldap/ruby-net-ldap/pull/273 --- lib/net/ldap/connection.rb | 38 +++++++++++++++++++++++++++--------- test/test_ldap_connection.rb | 2 +- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/lib/net/ldap/connection.rb b/lib/net/ldap/connection.rb index f8ba0b61..3d6aac80 100644 --- a/lib/net/ldap/connection.rb +++ b/lib/net/ldap/connection.rb @@ -31,26 +31,27 @@ def socket_class=(socket_class) @socket_class = socket_class end - def prepare_socket(server) + def prepare_socket(server, timeout = nil) socket = server[:socket] encryption = server[:encryption] @conn = socket - setup_encryption encryption if encryption + setup_encryption(encryption, timeout) if encryption end def open_connection(server) hosts = server[:hosts] encryption = server[:encryption] + timeout = server[:connect_timeout] || DefaultConnectTimeout socket_opts = { - connect_timeout: server[:connect_timeout] || DefaultConnectTimeout, + connect_timeout: timeout, } errors = [] hosts.each do |host, port| begin - prepare_socket(server.merge(socket: @socket_class.new(host, port, socket_opts))) + prepare_socket(server.merge(socket: @socket_class.new(host, port, socket_opts)), timeout) return rescue Net::LDAP::Error, SocketError, SystemCallError, OpenSSL::SSL::SSLError => e @@ -76,7 +77,7 @@ def close end end - def self.wrap_with_ssl(io, tls_options = {}) + def self.wrap_with_ssl(io, tls_options = {}, timeout) raise Net::LDAP::NoOpenSSLError, "OpenSSL is unavailable" unless Net::LDAP::HasOpenSSL ctx = OpenSSL::SSL::SSLContext.new @@ -86,7 +87,26 @@ def self.wrap_with_ssl(io, tls_options = {}) ctx.set_params(tls_options) unless tls_options.empty? conn = OpenSSL::SSL::SSLSocket.new(io, ctx) - conn.connect + + begin + if timeout + conn.connect_nonblock + else + conn.connect + end + rescue IO::WaitReadable + if IO.select([conn], nil, nil, timeout) + retry + else + raise Errno::ETIMEDOUT, "OpenSSL connection read timeout" + end + rescue IO::WaitWritable + if IO.select(nil, [conn], nil, timeout) + retry + else + raise Errno::ETIMEDOUT, "OpenSSL connection write timeout" + end + end # Doesn't work: # conn.sync_close = true @@ -123,11 +143,11 @@ def self.wrap_with_ssl(io, tls_options = {}) # communications, as with simple_tls. Thanks for Kouhei Sutou for # generously contributing the :start_tls path. #++ - def setup_encryption(args) + def setup_encryption(args, timeout = nil) args[:tls_options] ||= {} case args[:method] when :simple_tls - @conn = self.class.wrap_with_ssl(@conn, args[:tls_options]) + @conn = self.class.wrap_with_ssl(@conn, args[:tls_options], timeout) # additional branches requiring server validation and peer certs, etc. # go here. when :start_tls @@ -144,7 +164,7 @@ def setup_encryption(args) end if pdu.result_code.zero? - @conn = self.class.wrap_with_ssl(@conn, args[:tls_options]) + @conn = self.class.wrap_with_ssl(@conn, args[:tls_options], timeout) else raise Net::LDAP::StartTLSError, "start_tls failed: #{pdu.result_code}" end diff --git a/test/test_ldap_connection.rb b/test/test_ldap_connection.rb index 6bb027ac..0d68dc5d 100644 --- a/test/test_ldap_connection.rb +++ b/test/test_ldap_connection.rb @@ -291,7 +291,7 @@ def test_queued_read_setup_encryption_with_start_tls and_return(result2) mock.should_receive(:write) conn = Net::LDAP::Connection.new(:socket => mock) - flexmock(Net::LDAP::Connection).should_receive(:wrap_with_ssl).with(mock, {}). + flexmock(Net::LDAP::Connection).should_receive(:wrap_with_ssl).with(mock, {}, timeout). and_return(mock) conn.next_msgid # simulates ongoing query From b4095948b5b3ed481c53dde537da1ef36fcb22fb Mon Sep 17 00:00:00 2001 From: Scott Ringwelski Date: Mon, 20 Jun 2016 21:49:14 -0700 Subject: [PATCH 2/5] add an ENV to allow turn off --- lib/net/ldap/connection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/net/ldap/connection.rb b/lib/net/ldap/connection.rb index 3d6aac80..27fea883 100644 --- a/lib/net/ldap/connection.rb +++ b/lib/net/ldap/connection.rb @@ -89,7 +89,7 @@ def self.wrap_with_ssl(io, tls_options = {}, timeout) conn = OpenSSL::SSL::SSLSocket.new(io, ctx) begin - if timeout + if timeout && !ENV['FORCE_BLOCKING_LDAP_SOCKET'] == 'true' conn.connect_nonblock else conn.connect From d755caa953cf92c3a620acd01198382c0de8833a Mon Sep 17 00:00:00 2001 From: Scott Ringwelski Date: Mon, 20 Jun 2016 23:08:11 -0700 Subject: [PATCH 3/5] try allowing sync_close for nonblocking and if not disabled --- .travis.yml | 1 + lib/net/ldap/connection.rb | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index fc764963..702296fc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,6 +3,7 @@ rvm: - 2.0.0 - 2.1 - 2.2 + - 2.3.0 # optional - ruby-head - jruby-19mode diff --git a/lib/net/ldap/connection.rb b/lib/net/ldap/connection.rb index 27fea883..c638c0a5 100644 --- a/lib/net/ldap/connection.rb +++ b/lib/net/ldap/connection.rb @@ -88,8 +88,10 @@ def self.wrap_with_ssl(io, tls_options = {}, timeout) conn = OpenSSL::SSL::SSLSocket.new(io, ctx) + use_nonblocking_connect = timeout && !ENV['FORCE_BLOCKING_LDAP_SOCKET'] == 'true' + begin - if timeout && !ENV['FORCE_BLOCKING_LDAP_SOCKET'] == 'true' + if use_nonblocking_connect conn.connect_nonblock else conn.connect @@ -108,8 +110,8 @@ def self.wrap_with_ssl(io, tls_options = {}, timeout) end end - # Doesn't work: - # conn.sync_close = true + # Doesn't work? Or maybe it is needed if connect_nonblock? + conn.sync_close = use_nonblocking_connect && !ENV['DISABLE_SYNC_CLOSE_LDAP_SOCKET'] == 'true' conn.extend(GetbyteForSSLSocket) unless conn.respond_to?(:getbyte) conn.extend(FixSSLSocketSyncClose) From 7be13bf668172926a8a39699ea0fd64ad9ba0a9f Mon Sep 17 00:00:00 2001 From: Scott Ringwelski Date: Mon, 20 Jun 2016 23:15:07 -0700 Subject: [PATCH 4/5] pass in nil for last arg --- test/test_ldap_connection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_ldap_connection.rb b/test/test_ldap_connection.rb index 0d68dc5d..ba6289b3 100644 --- a/test/test_ldap_connection.rb +++ b/test/test_ldap_connection.rb @@ -291,7 +291,7 @@ def test_queued_read_setup_encryption_with_start_tls and_return(result2) mock.should_receive(:write) conn = Net::LDAP::Connection.new(:socket => mock) - flexmock(Net::LDAP::Connection).should_receive(:wrap_with_ssl).with(mock, {}, timeout). + flexmock(Net::LDAP::Connection).should_receive(:wrap_with_ssl).with(mock, {}, nil). and_return(mock) conn.next_msgid # simulates ongoing query From a83d0b29e73c7e671583cae3aa45383be7b183ad Mon Sep 17 00:00:00 2001 From: Scott Ringwelski Date: Mon, 20 Jun 2016 23:18:58 -0700 Subject: [PATCH 5/5] default timeout to nil in wrap-with_ssl --- lib/net/ldap/connection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/net/ldap/connection.rb b/lib/net/ldap/connection.rb index c638c0a5..09f42515 100644 --- a/lib/net/ldap/connection.rb +++ b/lib/net/ldap/connection.rb @@ -77,7 +77,7 @@ def close end end - def self.wrap_with_ssl(io, tls_options = {}, timeout) + def self.wrap_with_ssl(io, tls_options = {}, timeout = nil) raise Net::LDAP::NoOpenSSLError, "OpenSSL is unavailable" unless Net::LDAP::HasOpenSSL ctx = OpenSSL::SSL::SSLContext.new