1berry/ruby
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

openssl: fix possible SEGV on race between SSLSocket#stop and #connect

Browse Source 
* ext/openssl/ossl_ssl.c (ossl_ssl_stop): Don't free the SSL struct
  here. Since some methods such as SSLSocket#connect releases GVL,
  there is a chance of use after free if we free the SSL from another
  thread. SSLSocket#stop was documented as "prepares it for another
  connection" so this is a slightly incompatible change. However when
  this sentence was added (r30090, Add toplevel documentation for
  OpenSSL, 2010-12-06), it didn't actually. The current behavior is
  from r40304 (Correct shutdown behavior w.r.t GC., 2013-04-15).
  [ruby-core:74978] [Bug #12292]

* ext/openssl/lib/openssl/ssl.rb (sysclose): Update doc.

* test/openssl/test_ssl.rb: Test this.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@55100 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
This commit is contained in:
rhe 2016-05-21 07:25:00 +00:00
parent 77d1e6052f
commit 118ee2a734
4 changed files with 46 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
ChangeLog
View File

@ -1,3 +1,19 @@
Sat May 21 16:16:03 2016 Kazuki Yamaguchi <k@rhe.jp>
* ext/openssl/ossl_ssl.c (ossl_ssl_stop): Don't free the SSL struct
here. Since some methods such as SSLSocket#connect releases GVL,
there is a chance of use after free if we free the SSL from another
thread. SSLSocket#stop was documented as "prepares it for another
connection" so this is a slightly incompatible change. However when
this sentence was added (r30090, Add toplevel documentation for
OpenSSL, 2010-12-06), it didn't actually. The current behavior is
from r40304 (Correct shutdown behavior w.r.t GC., 2013-04-15).
[ruby-core:74978] [Bug #12292]
* ext/openssl/lib/openssl/ssl.rb (sysclose): Update doc.
* test/openssl/test_ssl.rb: Test this.
Sat May 21 14:41:14 2016 Kazuki Yamaguchi <k@rhe.jp> Sat May 21 14:41:14 2016 Kazuki Yamaguchi <k@rhe.jp>
* ext/openssl/ossl.c: [DOC] Fix SSL client example. The variable name * ext/openssl/ossl.c: [DOC] Fix SSL client example. The variable name

5
ext/openssl/lib/openssl/ssl.rb
View File

@ -290,7 +290,10 @@ module OpenSSL
# call-seq: # call-seq:
# ssl.sysclose => nil # ssl.sysclose => nil
# #
# Shuts down the SSL connection and prepares it for another connection. # Sends "close notify" to the peer and tries to shut down the SSL
# connection gracefully.
#
# If sync_close is set to +true+, the underlying IO is also closed.
def sysclose def sysclose
return if closed? return if closed?
stop stop

12
ext/openssl/ossl_ssl.c
View File

@ -1601,23 +1601,17 @@ ossl_ssl_write_nonblock(int argc, VALUE *argv, VALUE self)
* call-seq: * call-seq:
* ssl.stop => nil * ssl.stop => nil
* *
* Stops the SSL connection and prepares it for another connection. * Sends "close notify" to the peer and tries to shut down the SSL connection
* gracefully.
*/ */
static VALUE static VALUE
ossl_ssl_stop(VALUE self) ossl_ssl_stop(VALUE self)
{ {
SSL *ssl; SSL *ssl;
/* ossl_ssl_data_get_struct() is not usable here because it may return ossl_ssl_data_get_struct(self, ssl);
* from this function; */
GetSSL(self, ssl);
if (ssl) {
ossl_ssl_shutdown(ssl); ossl_ssl_shutdown(ssl);
SSL_free(ssl);
}
DATA_PTR(self) = NULL;
return Qnil; return Qnil;
} }

22
test/openssl/test_ssl.rb
View File

@ -1169,6 +1169,28 @@ end
} }
end end
def test_close_and_socket_close_while_connecting
# test it doesn't cause a segmentation fault
ctx = OpenSSL::SSL::SSLContext.new
ctx.ciphers = "aNULL"
sock1, sock2 = socketpair
ssl1 = OpenSSL::SSL::SSLSocket.new(sock1, ctx)
ssl2 = OpenSSL::SSL::SSLSocket.new(sock2, ctx)
t = Thread.new { ssl1.connect }
ssl2.accept
ssl1.close
sock1.close
t.value rescue nil
ensure
ssl1.close if ssl1
ssl2.close if ssl2
sock1.close if sock1
sock2.close if sock2
end
def test_get_ephemeral_key def test_get_ephemeral_key
return unless OpenSSL::SSL::SSLSocket.method_defined?(:tmp_key) return unless OpenSSL::SSL::SSLSocket.method_defined?(:tmp_key)
pkey = OpenSSL::PKey pkey = OpenSSL::PKey