[ruby-core:75594] [Ruby trunk Bug#12292][Assigned] Race between OpenSSL::SSL::SSLSocket#stop and #connect can cause a segmentation fault

Issue #12292 has been updated by Shyouhei Urabe.

Status changed from Open to Assigned
Assignee set to Kazuki Yamaguchi

Kazuki, I think it's OK for you to fix a segmentation fault now.

----------------------------------------
Bug #12292: Race between OpenSSL::SSL::SSLSocket#stop and #connect can cause a segmentation fault
https://bugs.ruby-lang.org/issues/12292#change-58729

* Author: Kazuki Yamaguchi
* Status: Assigned
* Priority: Normal
* Assignee: Kazuki Yamaguchi
* ruby -v: ruby 2.4.0dev (2016-04-15 trunk 54594) [x86_64-linux]
* Backport: 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN
----------------------------------------
The following code will demonstrate the issue:

~~~ruby
require "openssl"
require "socket"

ctx = OpenSSL::SSL::SSLContext.new
ctx.ciphers = "aNULL"

sock1, sock2 = UNIXSocket.pair
ssl1 = OpenSSL::SSL::SSLSocket.new(sock1, ctx)
ssl2 = OpenSSL::SSL::SSLSocket.new(sock2, ctx)

t = Thread.new { ssl1.connect } # => segmentation fault
ssl2.accept

ssl1.close # calls #stop (private method)
sock1.close

t.value
~~~

The SSL (OpenSSL land object) can be freed by SSLSocket#stop (#close) while SSLSocket#connect is still using it. This happens because SSLSocket#connect releases GVL while waiting for the peer.

There are two ways to resolve this:

- Check that the SSL object is still set every time after reacquiring GVL
- Change SSLSocket#stop not to free the SSL object

The latter introduces an incompatibility (#stop is currently documented as "prepares it for another connection").
I however prefer this because similar bugs can be introduced in future if we choose an ad-hoc way, and I don't think anyone reuses the SSLSocket object (keep in mind that we don't have interface to replace the underlying IO object).


Anyway I attach fixes in both way.

- 0001-ext-openssl-check-that-the-SSL-object-is-still-set-a.patch: the fix in the first way
- 0001-ext-openssl-make-OpenSSL-SSL-SSLSocket-non-reusable.patch: the fix in the second way
- 0002-ext-openssl-some-trivial-cleanups.patch: some minor cleanups, not actually related to this issue (what's the most desirable way to submit such trivial patches?)

---Files--------------------------------
0001-ext-openssl-check-that-the-SSL-object-is-still-set-a.patch (2.97 KB)
0001-ext-openssl-make-OpenSSL-SSL-SSLSocket-non-reusable.patch (3.41 KB)
0002-ext-openssl-some-trivial-cleanups.patch (2.47 KB)


-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:[e-mail]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>