From 68ac33a511929268ff5b0e05601b855a3a031b31 Mon Sep 17 00:00:00 2001 From: akr Date: Thu, 29 May 2014 10:32:19 +0000 Subject: [PATCH] * ext/openssl/lib/openssl/ssl.rb (SSLServer#accept): Close a socket if any exception occur. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@46223 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 5 +++ ext/openssl/lib/openssl/ssl.rb | 8 +++- test/openssl/test_partial_record_read.rb | 21 ++++++--- test/openssl/test_ssl.rb | 57 ++++++++++++++++++------ test/openssl/utils.rb | 2 - 5 files changed, 69 insertions(+), 24 deletions(-) diff --git a/ChangeLog b/ChangeLog index 452a7adc4c..d4e6a36deb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +Thu May 29 19:31:10 2014 Tanaka Akira + + * ext/openssl/lib/openssl/ssl.rb (SSLServer#accept): Close a socket + if any exception occur. + Thu May 29 05:05:29 2014 Eric Wong * include/ruby/ruby.h: Hide Symbol internals. diff --git a/ext/openssl/lib/openssl/ssl.rb b/ext/openssl/lib/openssl/ssl.rb index bb5db9b5d3..fb1045381f 100644 --- a/ext/openssl/lib/openssl/ssl.rb +++ b/ext/openssl/lib/openssl/ssl.rb @@ -234,8 +234,12 @@ module OpenSSL ssl.sync_close = true ssl.accept if @start_immediately ssl - rescue SSLError => ex - sock.close + rescue Exception => ex + if ssl + ssl.close + else + sock.close + end raise ex end end diff --git a/test/openssl/test_partial_record_read.rb b/test/openssl/test_partial_record_read.rb index ce42131cd7..f3d83c6973 100644 --- a/test/openssl/test_partial_record_read.rb +++ b/test/openssl/test_partial_record_read.rb @@ -8,17 +8,26 @@ if defined?(OpenSSL) start_server(port, OpenSSL::SSL::VERIFY_NONE, true, :server_proc => Proc.new do |server_ctx, server_ssl| - server_ssl.io.write("\x01") # the beginning of a TLS record - sleep 6 # do not finish prematurely before the read by the client is attempted + begin + server_ssl.io.write("\x01") # the beginning of a TLS record + sleep 6 # do not finish prematurely before the read by the client is attempted + ensure + server_ssl.close + end end ) do |server, port| sock = TCPSocket.new("127.0.0.1", port) ssl = OpenSSL::SSL::SSLSocket.new(sock) - ssl.connect - sleep 3 # wait is required for the (incomplete) TLS record to arrive at the client socket + ssl.sync_close = true + begin + ssl.connect + sleep 3 # wait is required for the (incomplete) TLS record to arrive at the client socket - # Should raise a IO::WaitReadable since a full TLS record is not available for reading. - assert_raise(IO::WaitReadable) { ssl.read_nonblock(1) } + # Should raise a IO::WaitReadable since a full TLS record is not available for reading. + assert_raise(IO::WaitReadable) { ssl.read_nonblock(1) } + ensure + ssl.close + end end end diff --git a/test/openssl/test_ssl.rb b/test/openssl/test_ssl.rb index e99d3d9786..bd9879cffb 100644 --- a/test/openssl/test_ssl.rb +++ b/test/openssl/test_ssl.rb @@ -125,7 +125,12 @@ class OpenSSL::TestSSL < OpenSSL::SSLTestCase assert_raise(OpenSSL::SSL::SSLError, Errno::ECONNRESET){ sock = TCPSocket.new("127.0.0.1", port) ssl = OpenSSL::SSL::SSLSocket.new(sock) - ssl.connect + ssl.sync_close = true + begin + ssl.connect + ensure + ssl.close + end } ctx = OpenSSL::SSL::SSLContext.new @@ -239,8 +244,13 @@ class OpenSSL::TestSSL < OpenSSL::SSLTestCase ctx = OpenSSL::SSL::SSLContext.new ctx.set_params ssl = OpenSSL::SSL::SSLSocket.new(sock, ctx) - assert_raise(OpenSSL::SSL::SSLError){ ssl.connect } - assert_equal(OpenSSL::X509::V_ERR_SELF_SIGNED_CERT_IN_CHAIN, ssl.verify_result) + ssl.sync_close = true + begin + assert_raise(OpenSSL::SSL::SSLError){ ssl.connect } + assert_equal(OpenSSL::X509::V_ERR_SELF_SIGNED_CERT_IN_CHAIN, ssl.verify_result) + ensure + ssl.close + end sock = TCPSocket.new("127.0.0.1", port) ctx = OpenSSL::SSL::SSLContext.new @@ -251,8 +261,13 @@ class OpenSSL::TestSSL < OpenSSL::SSLTestCase end ) ssl = OpenSSL::SSL::SSLSocket.new(sock, ctx) - ssl.connect - assert_equal(OpenSSL::X509::V_OK, ssl.verify_result) + ssl.sync_close = true + begin + ssl.connect + assert_equal(OpenSSL::X509::V_OK, ssl.verify_result) + ensure + ssl.close + end sock = TCPSocket.new("127.0.0.1", port) ctx = OpenSSL::SSL::SSLContext.new @@ -263,8 +278,13 @@ class OpenSSL::TestSSL < OpenSSL::SSLTestCase end ) ssl = OpenSSL::SSL::SSLSocket.new(sock, ctx) - assert_raise(OpenSSL::SSL::SSLError){ ssl.connect } - assert_equal(OpenSSL::X509::V_ERR_APPLICATION_VERIFICATION, ssl.verify_result) + ssl.sync_close = true + begin + assert_raise(OpenSSL::SSL::SSLError){ ssl.connect } + assert_equal(OpenSSL::X509::V_ERR_APPLICATION_VERIFICATION, ssl.verify_result) + ensure + ssl.close + end } end @@ -279,12 +299,16 @@ class OpenSSL::TestSSL < OpenSSL::SSLTestCase end ) ssl = OpenSSL::SSL::SSLSocket.new(sock, ctx) - OpenSSL::TestUtils.silent do - # SSLError, not RuntimeError - assert_raise(OpenSSL::SSL::SSLError) { ssl.connect } + ssl.sync_close = true + begin + OpenSSL::TestUtils.silent do + # SSLError, not RuntimeError + assert_raise(OpenSSL::SSL::SSLError) { ssl.connect } + end + assert_equal(OpenSSL::X509::V_ERR_CERT_REJECTED, ssl.verify_result) + ensure + ssl.close end - assert_equal(OpenSSL::X509::V_ERR_CERT_REJECTED, ssl.verify_result) - ssl.close } end @@ -301,8 +325,13 @@ class OpenSSL::TestSSL < OpenSSL::SSLTestCase assert(ciphers_names.all?{|v| /ADH/ !~ v }) assert(ciphers_versions.all?{|v| /SSLv2/ !~ v }) ssl = OpenSSL::SSL::SSLSocket.new(sock, ctx) - assert_raise(OpenSSL::SSL::SSLError){ ssl.connect } - assert_equal(OpenSSL::X509::V_ERR_SELF_SIGNED_CERT_IN_CHAIN, ssl.verify_result) + ssl.sync_close = true + begin + assert_raise(OpenSSL::SSL::SSLError){ ssl.connect } + assert_equal(OpenSSL::X509::V_ERR_SELF_SIGNED_CERT_IN_CHAIN, ssl.verify_result) + ensure + ssl.close + end } end diff --git a/test/openssl/utils.rb b/test/openssl/utils.rb index f4509f2980..4df14056e4 100644 --- a/test/openssl/utils.rb +++ b/test/openssl/utils.rb @@ -311,7 +311,6 @@ AQjjxMXhwULlmuR/K+WwlaZPiLIBYalLAZQ7ZbOPeVkJ8ePao0eLAgEC if (server) server.join(5) if server.alive? - server.kill server.join flunk("TCPServer was closed and SSLServer is still alive") unless $! end @@ -322,7 +321,6 @@ AQjjxMXhwULlmuR/K+WwlaZPiLIBYalLAZQ7ZbOPeVkJ8ePao0eLAgEC end ensure threads.each {|th| - th.kill th.join } end