From 8ed81ff4b0a893376f949c006942fea8f7fba8c3 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Mon, 4 Sep 2017 17:19:58 +0900 Subject: [PATCH 01/14] test/test_pair: fix test_write_nonblock{,_no_exceptions} When the previous SSLSocket#write_nonblock call does not finish writing the complete contents, SSL_shutdown() which is called through SSLSocket#close will not send a close_notify alert. As of commit e3a305063675 ssl_pair no longer uses the sync_close feature. Do not expect that SSL_read() would get ECONNRESET. --- test/test_pair.rb | 58 +++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/test/test_pair.rb b/test/test_pair.rb index 2fb7726de..7e2a4a633 100644 --- a/test/test_pair.rb +++ b/test/test_pair.rb @@ -238,44 +238,42 @@ def test_read_nonblock_no_exception } end - def write_nonblock(socket, meth, str) - ret = socket.send(meth, str) - ret.is_a?(Symbol) ? 0 : ret - end - - def write_nonblock_no_ex(socket, str) - ret = socket.write_nonblock str, exception: false - ret.is_a?(Symbol) ? 0 : ret - end - def test_write_nonblock ssl_pair {|s1, s2| - n = 0 - begin - n += write_nonblock s1, :write_nonblock, "a" * 100000 - n += write_nonblock s1, :write_nonblock, "b" * 100000 - n += write_nonblock s1, :write_nonblock, "c" * 100000 - n += write_nonblock s1, :write_nonblock, "d" * 100000 - n += write_nonblock s1, :write_nonblock, "e" * 100000 - n += write_nonblock s1, :write_nonblock, "f" * 100000 - rescue IO::WaitWritable + assert_equal 3, s1.write_nonblock("foo") + assert_equal "foo", s2.read(3) + + data = "x" * 16384 + written = 0 + while true + begin + written += s1.write_nonblock(data) + rescue IO::WaitWritable, IO::WaitReadable + break + end end - s1.close - assert_equal(n, s2.read.length) + assert written > 0 + assert_equal written, s2.read(written).bytesize } end def test_write_nonblock_no_exceptions ssl_pair {|s1, s2| - n = 0 - n += write_nonblock_no_ex s1, "a" * 100000 - n += write_nonblock_no_ex s1, "b" * 100000 - n += write_nonblock_no_ex s1, "c" * 100000 - n += write_nonblock_no_ex s1, "d" * 100000 - n += write_nonblock_no_ex s1, "e" * 100000 - n += write_nonblock_no_ex s1, "f" * 100000 - s1.close - assert_equal(n, s2.read.length) + assert_equal 3, s1.write_nonblock("foo", exception: false) + assert_equal "foo", s2.read(3) + + data = "x" * 16384 + written = 0 + while true + case ret = s1.write_nonblock(data, exception: false) + when :wait_readable, :wait_writable + break + else + written += ret + end + end + assert written > 0 + assert_equal written, s2.read(written).bytesize } end From d7984397f8fa6afc836175cc5ce1ca17ddeb630e Mon Sep 17 00:00:00 2001 From: nobu Date: Thu, 7 Sep 2017 03:24:08 +0000 Subject: [PATCH 02/14] ruby.h: unnormalized Fixnum value * include/ruby/ruby.h (ST2FIX): fix unnormalized Fixnum value bug on mingw/mswin. [ruby-core:82687] [Bug #13877] git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@59765 b2dd03c8-39d4-4d8f-98ff-823fe69b080e [ky: add ST2FIX() definition to ext/openssl/ruby_missing.h, and adapt the test case to the 2.0 branch.] Sync-with-trunk: r59765 --- ext/openssl/ossl_bn.c | 2 +- ext/openssl/ruby_missing.h | 10 ++++++++-- test/test_bn.rb | 1 + 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/ext/openssl/ossl_bn.c b/ext/openssl/ossl_bn.c index aa0f2c605..29dc1a216 100644 --- a/ext/openssl/ossl_bn.c +++ b/ext/openssl/ossl_bn.c @@ -953,7 +953,7 @@ ossl_bn_hash(VALUE self) ossl_raise(eBNError, NULL); } - hash = INT2FIX(rb_memhash(buf, len)); + hash = ST2FIX(rb_memhash(buf, len)); xfree(buf); return hash; diff --git a/ext/openssl/ruby_missing.h b/ext/openssl/ruby_missing.h index 8dacc8266..5b1481aea 100644 --- a/ext/openssl/ruby_missing.h +++ b/ext/openssl/ruby_missing.h @@ -15,9 +15,15 @@ #define FPTR_TO_FD(fptr) ((fptr)->fd) +/* Ruby 2.4 */ #ifndef RB_INTEGER_TYPE_P -/* for Ruby 2.3 compatibility */ -#define RB_INTEGER_TYPE_P(obj) (RB_FIXNUM_P(obj) || RB_TYPE_P(obj, T_BIGNUM)) +# define RB_INTEGER_TYPE_P(obj) (RB_FIXNUM_P(obj) || RB_TYPE_P(obj, T_BIGNUM)) +#endif + +/* Ruby 2.5 */ +#ifndef ST2FIX +# define RB_ST2FIX(h) LONG2FIX((long)(h)) +# define ST2FIX(h) RB_ST2FIX(h) #endif #endif /* _OSSL_RUBY_MISSING_H_ */ diff --git a/test/test_bn.rb b/test/test_bn.rb index 37ba5e559..5f3ae2b46 100644 --- a/test/test_bn.rb +++ b/test/test_bn.rb @@ -55,6 +55,7 @@ def test_cmp assert_equal(false, bn1.eql?(bn3)) assert_equal(bn1.hash, bn2.hash) assert_not_equal(bn3.hash, bn1.hash) + assert_instance_of(String, bn1.hash.to_s) end end From a09d8c78dd30482b6422ad1aea5b802cf879fa98 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Fri, 8 Sep 2017 22:02:02 +0900 Subject: [PATCH 03/14] test/test_ssl: suppress warning in test_alpn_protocol_selection_cancel Suppress "using default DH parameters" message. --- test/test_ssl.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_ssl.rb b/test/test_ssl.rb index 46509f5e9..e6368943d 100644 --- a/test/test_ssl.rb +++ b/test/test_ssl.rb @@ -884,6 +884,7 @@ def test_alpn_protocol_selection_cancel ctx1 = OpenSSL::SSL::SSLContext.new ctx1.cert = @svr_cert ctx1.key = @svr_key + ctx1.tmp_dh_callback = proc { Fixtures.pkey_dh("dh1024") } ctx1.alpn_select_cb = -> (protocols) { nil } ssl1 = OpenSSL::SSL::SSLSocket.new(sock1, ctx1) From de965374ee85eb9b8475e619bd76307c779d2ba9 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Fri, 8 Sep 2017 22:24:05 +0900 Subject: [PATCH 04/14] test/test_pair: disable compression The test cases added by commit 8ed81ff4b0a8 ("test/test_pair: fix test_write_nonblock{,_no_exceptions}", 2017-09-04) can consume much memory and time if the OpenSSL supports SSL/TLS compression. Disable it explicitly. --- test/test_pair.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_pair.rb b/test/test_pair.rb index 7e2a4a633..7daa92881 100644 --- a/test/test_pair.rb +++ b/test/test_pair.rb @@ -24,6 +24,7 @@ def ssl_pair sctx.cert = @svr_cert sctx.key = @svr_key sctx.tmp_dh_callback = proc { OpenSSL::TestUtils::Fixtures.pkey_dh("dh1024") } + sctx.options |= OpenSSL::SSL::OP_NO_COMPRESSION ssls = OpenSSL::SSL::SSLServer.new(tcps, sctx) ns = ssls.accept ssls.close From b7591c2383335b3fe884f576ba2b77ddd12328a6 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sun, 13 Aug 2017 23:24:48 +0900 Subject: [PATCH 05/14] test/test_ssl: skip tmp_ecdh_callback test for LibreSSL >= 2.6.1 LibreSSL 2.6.1 has SSL_CTX_set_tmp_ecdh_callback() function, but it does not work. --- test/test_ssl.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/test_ssl.rb b/test/test_ssl.rb index e6368943d..017fd357d 100644 --- a/test/test_ssl.rb +++ b/test/test_ssl.rb @@ -1111,6 +1111,8 @@ def test_tmp_ecdh_callback pend "EC is disabled" unless defined?(OpenSSL::PKey::EC) pend "tmp_ecdh_callback is not supported" unless \ OpenSSL::SSL::SSLContext.method_defined?(:tmp_ecdh_callback) + pend "LibreSSL 2.6 has broken SSL_CTX_set_tmp_ecdh_callback()" \ + if libressl?(2, 6, 1) EnvUtil.suppress_warning do # tmp_ecdh_callback is deprecated (2016-05) called = false From a2ed156cc9f1f7c3cb3ed89b8092a42a8522c22d Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sun, 13 Aug 2017 23:22:36 +0900 Subject: [PATCH 06/14] test/test_ssl: do not run NPN tests for LibreSSL >= 2.6.1 Similar to the previous one, LibreSSL 2.6.1 has relevant functions such as SSL_CTX_set_next_proto_select_cb(), but they are broken and do nothing. --- test/test_ssl.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/test_ssl.rb b/test/test_ssl.rb index 017fd357d..7bb32adf9 100644 --- a/test/test_ssl.rb +++ b/test/test_ssl.rb @@ -911,6 +911,7 @@ def test_npn_protocol_selection_ary pend "TLS 1.2 is not supported" unless tls12_supported? pend "NPN is not supported" unless \ OpenSSL::SSL::SSLContext.method_defined?(:npn_select_cb) + pend "LibreSSL 2.6 has broken NPN functions" if libressl?(2, 6, 1) advertised = ["http/1.1", "spdy/2"] ctx_proc = proc { |ctx| ctx.npn_protocols = advertised } @@ -931,6 +932,7 @@ def test_npn_protocol_selection_enum pend "TLS 1.2 is not supported" unless tls12_supported? pend "NPN is not supported" unless \ OpenSSL::SSL::SSLContext.method_defined?(:npn_select_cb) + pend "LibreSSL 2.6 has broken NPN functions" if libressl?(2, 6, 1) advertised = Object.new def advertised.each @@ -955,6 +957,7 @@ def test_npn_protocol_selection_cancel pend "TLS 1.2 is not supported" unless tls12_supported? pend "NPN is not supported" unless \ OpenSSL::SSL::SSLContext.method_defined?(:npn_select_cb) + pend "LibreSSL 2.6 has broken NPN functions" if libressl?(2, 6, 1) ctx_proc = Proc.new { |ctx| ctx.npn_protocols = ["http/1.1"] } start_server_version(:TLSv1_2, ctx_proc) { |port| @@ -968,6 +971,7 @@ def test_npn_advertised_protocol_too_long pend "TLS 1.2 is not supported" unless tls12_supported? pend "NPN is not supported" unless \ OpenSSL::SSL::SSLContext.method_defined?(:npn_select_cb) + pend "LibreSSL 2.6 has broken NPN functions" if libressl?(2, 6, 1) ctx_proc = Proc.new { |ctx| ctx.npn_protocols = ["a" * 256] } start_server_version(:TLSv1_2, ctx_proc) { |port| @@ -981,6 +985,7 @@ def test_npn_selected_protocol_too_long pend "TLS 1.2 is not supported" unless tls12_supported? pend "NPN is not supported" unless \ OpenSSL::SSL::SSLContext.method_defined?(:npn_select_cb) + pend "LibreSSL 2.6 has broken NPN functions" if libressl?(2, 6, 1) ctx_proc = Proc.new { |ctx| ctx.npn_protocols = ["http/1.1"] } start_server_version(:TLSv1_2, ctx_proc) { |port| From 6652ad7c639d71cec93c4d21030ff9020a4e9717 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Fri, 15 Sep 2017 12:29:52 +0900 Subject: [PATCH 07/14] tool/ruby-openssl-docker: update - Upgrade to latest Ubuntu LTS. - Remove unnecessary packages. - Update OpenSSL, LibreSSL, and Ruby versions. Notably, LibreSSL 2.6 is added. Accordingly, .travis.yml is also updated to use that. --- .travis.yml | 1 + tool/ruby-openssl-docker/Dockerfile | 42 ++++++++++++++--------------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/.travis.yml b/.travis.yml index 99cd60d96..1342da6d7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -25,6 +25,7 @@ matrix: - env: RUBY_VERSION=ruby-2.4 OPENSSL_VERSION=libressl-2.3 - env: RUBY_VERSION=ruby-2.4 OPENSSL_VERSION=libressl-2.4 - env: RUBY_VERSION=ruby-2.4 OPENSSL_VERSION=libressl-2.5 + - env: RUBY_VERSION=ruby-2.4 OPENSSL_VERSION=libressl-2.6 - language: ruby rvm: ruby-head before_install: diff --git a/tool/ruby-openssl-docker/Dockerfile b/tool/ruby-openssl-docker/Dockerfile index 0bafbaae2..b8ed4bca1 100644 --- a/tool/ruby-openssl-docker/Dockerfile +++ b/tool/ruby-openssl-docker/Dockerfile @@ -1,22 +1,16 @@ -FROM ubuntu:14.04 +FROM ubuntu:16.04 RUN apt-get update && apt-get install -y --no-install-recommends \ autoconf \ bison \ build-essential \ - bzip2 \ ca-certificates \ - cpio \ curl \ - file \ - git \ gzip \ libreadline-dev \ - make \ patch \ pkg-config \ sed \ - xz-utils \ zlib1g-dev # Supported OpenSSL versions: 1.0.1- @@ -35,15 +29,15 @@ RUN curl -s https://www.openssl.org/source/openssl-1.0.1u.tar.gz | tar -C /build shared linux-x86_64 && \ make && make install_sw -RUN curl -s https://www.openssl.org/source/openssl-1.0.2k.tar.gz | tar -C /build/openssl -xzf - && \ - cd /build/openssl/openssl-1.0.2k && \ +RUN curl -s https://www.openssl.org/source/openssl-1.0.2l.tar.gz | tar -C /build/openssl -xzf - && \ + cd /build/openssl/openssl-1.0.2l && \ ./Configure \ --openssldir=/opt/openssl/openssl-1.0.2 \ shared linux-x86_64 && \ make && make install_sw -RUN curl -s https://www.openssl.org/source/openssl-1.1.0e.tar.gz | tar -C /build/openssl -xzf - && \ - cd /build/openssl/openssl-1.1.0e && \ +RUN curl -s https://www.openssl.org/source/openssl-1.1.0f.tar.gz | tar -C /build/openssl -xzf - && \ + cd /build/openssl/openssl-1.1.0f && \ ./Configure \ --prefix=/opt/openssl/openssl-1.1.0 \ enable-crypto-mdebug enable-crypto-mdebug-backtrace \ @@ -51,36 +45,42 @@ RUN curl -s https://www.openssl.org/source/openssl-1.1.0e.tar.gz | tar -C /build make && make install_sw # Supported libressl versions: 2.3- -RUN curl -s http://ftp.openbsd.org/pub/OpenBSD/LibreSSL/libressl-2.3.10.tar.gz | tar -C /build/openssl -xzf - -RUN cd /build/openssl/libressl-2.3.10 && \ +RUN curl -s http://ftp.openbsd.org/pub/OpenBSD/LibreSSL/libressl-2.3.10.tar.gz | tar -C /build/openssl -xzf - && \ + cd /build/openssl/libressl-2.3.10 && \ ./configure \ --prefix=/opt/openssl/libressl-2.3 && \ make && make install -RUN curl -s http://ftp.openbsd.org/pub/OpenBSD/LibreSSL/libressl-2.4.5.tar.gz | tar -C /build/openssl -xzf - -RUN cd /build/openssl/libressl-2.4.5 && \ +RUN curl -s http://ftp.openbsd.org/pub/OpenBSD/LibreSSL/libressl-2.4.5.tar.gz | tar -C /build/openssl -xzf - && \ + cd /build/openssl/libressl-2.4.5 && \ ./configure \ --prefix=/opt/openssl/libressl-2.4 && \ make && make install -RUN curl -s http://ftp.openbsd.org/pub/OpenBSD/LibreSSL/libressl-2.5.4.tar.gz | tar -C /build/openssl -xzf - -RUN cd /build/openssl/libressl-2.5.4 && \ +RUN curl -s http://ftp.openbsd.org/pub/OpenBSD/LibreSSL/libressl-2.5.5.tar.gz | tar -C /build/openssl -xzf - && \ + cd /build/openssl/libressl-2.5.5 && \ ./configure \ --prefix=/opt/openssl/libressl-2.5 && \ make && make install +RUN curl -s http://ftp.openbsd.org/pub/OpenBSD/LibreSSL/libressl-2.6.1.tar.gz | tar -C /build/openssl -xzf - && \ + cd /build/openssl/libressl-2.6.1 && \ + ./configure \ + --prefix=/opt/openssl/libressl-2.6 && \ + make && make install + # Supported Ruby versions: 2.3- RUN mkdir -p /build/ruby -RUN curl -s https://cache.ruby-lang.org/pub/ruby/2.3/ruby-2.3.4.tar.gz | tar -C /build/ruby -xzf - && \ - cd /build/ruby/ruby-2.3.4 && \ +RUN curl -s https://cache.ruby-lang.org/pub/ruby/2.3/ruby-2.3.5.tar.gz | tar -C /build/ruby -xzf - && \ + cd /build/ruby/ruby-2.3.5 && \ autoconf && ./configure \ --without-openssl \ --prefix=/opt/ruby/ruby-2.3 \ --disable-install-doc && \ make && make install -RUN curl -s https://cache.ruby-lang.org/pub/ruby/2.4/ruby-2.4.1.tar.gz | tar -C /build/ruby -xzf - && \ - cd /build/ruby/ruby-2.4.1 && \ +RUN curl -s https://cache.ruby-lang.org/pub/ruby/2.4/ruby-2.4.2.tar.gz | tar -C /build/ruby -xzf - && \ + cd /build/ruby/ruby-2.4.2 && \ autoconf && ./configure \ --without-openssl \ --prefix=/opt/ruby/ruby-2.4 \ From 6c5e6b3ba0363ca496ea0b464edd1f2a235e8bf2 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Fri, 22 Sep 2017 18:05:20 +0900 Subject: [PATCH 08/14] test/test_pair: replace sleep with IO.select The sleep was to ensure that the SSLSocket#read_nonblock will get close_notify alert. A simple IO.select will suffice. --- test/test_pair.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_pair.rb b/test/test_pair.rb index 7daa92881..cbb985dda 100644 --- a/test/test_pair.rb +++ b/test/test_pair.rb @@ -218,7 +218,7 @@ def test_read_nonblock assert_nothing_raised("[ruby-core:20298]") { ret = s2.read_nonblock(10) } assert_equal("def\n", ret) s1.close - sleep 0.1 + IO.select([s2]) assert_raise(EOFError) { s2.read_nonblock(10) } } end @@ -234,7 +234,7 @@ def test_read_nonblock_no_exception assert_nothing_raised("[ruby-core:20298]") { ret = s2.read_nonblock(10, exception: false) } assert_equal("def\n", ret) s1.close - sleep 0.1 + IO.select([s2]) assert_equal(nil, s2.read_nonblock(10, exception: false)) } end From 6ff7844ea13ded27241fed9c641a20081b8ff402 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sat, 23 Sep 2017 03:04:48 +0900 Subject: [PATCH 09/14] ssl: prevent SSLSocket#sysread* from leaking uninitialized data Set the length of the buffer string to 0 first, and adjust to the size successfully read by the SSL_read() call later. This is needed because the buffer string may be provided by the caller. --- ext/openssl/ossl_ssl.c | 22 +++++++++++++--------- test/test_pair.rb | 24 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index bf40c5b12..aa2dbbc8f 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -1688,20 +1688,26 @@ ossl_ssl_read_internal(int argc, VALUE *argv, VALUE self, int nonblock) } ilen = NUM2INT(len); - if(NIL_P(str)) str = rb_str_new(0, ilen); - else{ - StringValue(str); - rb_str_modify(str); - rb_str_resize(str, ilen); + if (NIL_P(str)) + str = rb_str_new(0, ilen); + else { + StringValue(str); + if (RSTRING_LEN(str) >= ilen) + rb_str_modify(str); + else + rb_str_modify_expand(str, ilen - RSTRING_LEN(str)); } - if(ilen == 0) return str; + OBJ_TAINT(str); + rb_str_set_len(str, 0); + if (ilen == 0) + return str; GetSSL(self, ssl); io = rb_attr_get(self, id_i_io); GetOpenFile(io, fptr); if (ssl_started(ssl)) { for (;;){ - nread = SSL_read(ssl, RSTRING_PTR(str), RSTRING_LENINT(str)); + nread = SSL_read(ssl, RSTRING_PTR(str), ilen); switch(ssl_get_error(ssl, nread)){ case SSL_ERROR_NONE: goto end; @@ -1751,8 +1757,6 @@ ossl_ssl_read_internal(int argc, VALUE *argv, VALUE self, int nonblock) end: rb_str_set_len(str, nread); - OBJ_TAINT(str); - return str; } diff --git a/test/test_pair.rb b/test/test_pair.rb index cbb985dda..ea5f0dcf9 100644 --- a/test/test_pair.rb +++ b/test/test_pair.rb @@ -239,6 +239,30 @@ def test_read_nonblock_no_exception } end + def test_read_with_outbuf + ssl_pair { |s1, s2| + s1.write("abc\n") + buf = "" + ret = s2.read(2, buf) + assert_same ret, buf + assert_equal "ab", ret + + buf = "garbage" + ret = s2.read(2, buf) + assert_same ret, buf + assert_equal "c\n", ret + + buf = "garbage" + assert_equal :wait_readable, s2.read_nonblock(100, buf, exception: false) + assert_equal "", buf + + s1.close + buf = "garbage" + assert_equal nil, s2.read(100, buf) + assert_equal "", buf + } + end + def test_write_nonblock ssl_pair {|s1, s2| assert_equal 3, s1.write_nonblock("foo") From 7dd6c28a335b5a590be89c7e782e53e4112f4bb5 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Wed, 20 Sep 2017 11:27:10 +0900 Subject: [PATCH 10/14] ossl.c: use struct CRYPTO_dynlock_value for non-dynamic locks In preparation for making the mutexes reentrant. It is common to the non-dynamic and the dynamic locking callbacks. --- ext/openssl/ossl.c | 54 ++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/ext/openssl/ossl.c b/ext/openssl/ossl.c index 4eacc64a4..7d3ed6c4a 100644 --- a/ext/openssl/ossl.c +++ b/ext/openssl/ossl.c @@ -484,40 +484,41 @@ print_mem_leaks(VALUE self) /** * Stores locks needed for OpenSSL thread safety */ -static rb_nativethread_lock_t *ossl_locks; +struct CRYPTO_dynlock_value { + rb_nativethread_lock_t lock; +}; static void -ossl_lock_unlock(int mode, rb_nativethread_lock_t *lock) +ossl_lock_init(struct CRYPTO_dynlock_value *l) { - if (mode & CRYPTO_LOCK) { - rb_nativethread_lock_lock(lock); - } else { - rb_nativethread_lock_unlock(lock); - } + rb_nativethread_lock_initialize(&l->lock); } static void -ossl_lock_callback(int mode, int type, const char *file, int line) +ossl_lock_unlock(int mode, struct CRYPTO_dynlock_value *l) { - ossl_lock_unlock(mode, &ossl_locks[type]); + if (mode & CRYPTO_LOCK) { + rb_nativethread_lock_lock(&l->lock); + } else { + rb_nativethread_lock_unlock(&l->lock); + } } -struct CRYPTO_dynlock_value { - rb_nativethread_lock_t lock; -}; - static struct CRYPTO_dynlock_value * ossl_dyn_create_callback(const char *file, int line) { - struct CRYPTO_dynlock_value *dynlock = (struct CRYPTO_dynlock_value *)OPENSSL_malloc((int)sizeof(struct CRYPTO_dynlock_value)); - rb_nativethread_lock_initialize(&dynlock->lock); + /* Do not use xmalloc() here, since it may raise NoMemoryError */ + struct CRYPTO_dynlock_value *dynlock = + OPENSSL_malloc(sizeof(struct CRYPTO_dynlock_value)); + if (dynlock) + ossl_lock_init(dynlock); return dynlock; } static void ossl_dyn_lock_callback(int mode, struct CRYPTO_dynlock_value *l, const char *file, int line) { - ossl_lock_unlock(mode, &l->lock); + ossl_lock_unlock(mode, l); } static void @@ -541,21 +542,22 @@ static unsigned long ossl_thread_id(void) } #endif +static struct CRYPTO_dynlock_value *ossl_locks; + +static void +ossl_lock_callback(int mode, int type, const char *file, int line) +{ + ossl_lock_unlock(mode, &ossl_locks[type]); +} + static void Init_ossl_locks(void) { int i; int num_locks = CRYPTO_num_locks(); - if ((unsigned)num_locks >= INT_MAX / (int)sizeof(VALUE)) { - rb_raise(rb_eRuntimeError, "CRYPTO_num_locks() is too big: %d", num_locks); - } - ossl_locks = (rb_nativethread_lock_t *) OPENSSL_malloc(num_locks * (int)sizeof(rb_nativethread_lock_t)); - if (!ossl_locks) { - rb_raise(rb_eNoMemError, "CRYPTO_num_locks() is too big: %d", num_locks); - } - for (i = 0; i < num_locks; i++) { - rb_nativethread_lock_initialize(&ossl_locks[i]); - } + ossl_locks = ALLOC_N(struct CRYPTO_dynlock_value, num_locks); + for (i = 0; i < num_locks; i++) + ossl_lock_init(&ossl_locks[i]); #ifdef HAVE_CRYPTO_THREADID_PTR CRYPTO_THREADID_set_callback(ossl_threadid_func); From b2b2fb868745f4069df959cbb0972be033347f28 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Wed, 20 Sep 2017 23:40:25 +0900 Subject: [PATCH 11/14] ossl.c: make legacy locking callbacks reentrant Although it's not documented explicitly that the locking callbacks must provide reentrant mutexes, it seems to be required. Specifically, the session_remove_cb callback function of an SSL_CTX is called in a critical section for CRYPTO_LOCK_SSL_CTX, which is shared across the library. This leads, if the callback function calls another OpenSSL function that will attempt to lock CRYPTO_LOCK_SSL_CTX, to deadlock. SSL_CTX_free() is one example of such a function. http://ci.rvm.jp/results/trunk@P895/64001 --- ext/openssl/ossl.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/ext/openssl/ossl.c b/ext/openssl/ossl.c index 7d3ed6c4a..88bb8f210 100644 --- a/ext/openssl/ossl.c +++ b/ext/openssl/ossl.c @@ -486,21 +486,33 @@ print_mem_leaks(VALUE self) */ struct CRYPTO_dynlock_value { rb_nativethread_lock_t lock; + rb_nativethread_id_t owner; + size_t count; }; static void ossl_lock_init(struct CRYPTO_dynlock_value *l) { rb_nativethread_lock_initialize(&l->lock); + l->count = 0; } static void ossl_lock_unlock(int mode, struct CRYPTO_dynlock_value *l) { if (mode & CRYPTO_LOCK) { + /* TODO: rb_nativethread_id_t is not necessarily compared with ==. */ + rb_nativethread_id_t tid = rb_nativethread_self(); + if (l->count && l->owner == tid) { + l->count++; + return; + } rb_nativethread_lock_lock(&l->lock); + l->owner = tid; + l->count = 1; } else { - rb_nativethread_lock_unlock(&l->lock); + if (!--l->count) + rb_nativethread_lock_unlock(&l->lock); } } From 8c4bf53262d9a2d21641f1bb6eaf352424f648b7 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sun, 24 Sep 2017 12:57:39 +0900 Subject: [PATCH 12/14] test/test_engine: suppress stderr Use ignore_stderr option of assert_separately instead of $stderr.reopen which may not work if the OpenSSL library uses a different stdio. Reference: https://github.com/ruby/openssl/issues/154 --- test/test_engine.rb | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/test/test_engine.rb b/test/test_engine.rb index 634350465..ee1ebb33e 100644 --- a/test/test_engine.rb +++ b/test/test_engine.rb @@ -52,32 +52,22 @@ def test_openssl_engine_digest_sha1 end def test_openssl_engine_cipher_rc4 - with_openssl <<-'end;' - begin - engine = get_engine - algo = "RC4" #AES is not supported by openssl Engine (<=1.0.0e) - data = "a" * 1000 - key = OpenSSL::Random.random_bytes(16) - # suppress message from openssl Engine's RC4 cipher [ruby-core:41026] - err_back = $stderr.dup - $stderr.reopen(IO::NULL) - encrypted = crypt_data(data, key, :encrypt) { engine.cipher(algo) } - decrypted = crypt_data(encrypted, key, :decrypt) { OpenSSL::Cipher.new(algo) } - assert_equal(data, decrypted) - ensure - if err_back - $stderr.reopen(err_back) - err_back.close - end - end + with_openssl(<<-'end;', ignore_stderr: true) + engine = get_engine + algo = "RC4" #AES is not supported by openssl Engine (<=1.0.0e) + data = "a" * 1000 + key = OpenSSL::Random.random_bytes(16) + encrypted = crypt_data(data, key, :encrypt) { engine.cipher(algo) } + decrypted = crypt_data(encrypted, key, :decrypt) { OpenSSL::Cipher.new(algo) } + assert_equal(data, decrypted) end; end private # this is required because OpenSSL::Engine methods change global state - def with_openssl(code) - assert_separately([{ "OSSL_MDEBUG" => nil }, "-ropenssl"], <<~"end;") + def with_openssl(code, **opts) + assert_separately([{ "OSSL_MDEBUG" => nil }, "-ropenssl"], <<~"end;", **opts) require #{__FILE__.dump} include OpenSSL::TestEngine::Utils #{code} From 45bec388132a156471be5fae9713bcc7ea65530b Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sun, 24 Sep 2017 12:59:54 +0900 Subject: [PATCH 13/14] test/test_engine: check if RC4 is supported Skip test_openssl_engine_cipher_rc4 which will fail without RC4 support. It may be disabled by 'no-rc4' configure option of the OpenSSL library. Reference: https://github.com/ruby/openssl/issues/154 --- test/test_engine.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/test_engine.rb b/test/test_engine.rb index ee1ebb33e..801d1c8e1 100644 --- a/test/test_engine.rb +++ b/test/test_engine.rb @@ -52,9 +52,15 @@ def test_openssl_engine_digest_sha1 end def test_openssl_engine_cipher_rc4 + begin + OpenSSL::Cipher.new("rc4") + rescue OpenSSL::Cipher::CipherError + pend "RC4 is not supported" + end + with_openssl(<<-'end;', ignore_stderr: true) engine = get_engine - algo = "RC4" #AES is not supported by openssl Engine (<=1.0.0e) + algo = "RC4" data = "a" * 1000 key = OpenSSL::Random.random_bytes(16) encrypted = crypt_data(data, key, :encrypt) { engine.cipher(algo) } From 14e116554b56b722337b285adfc30481155dd1de Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sun, 24 Sep 2017 14:53:33 +0900 Subject: [PATCH 14/14] Ruby/OpenSSL 2.0.6 --- History.md | 20 ++++++++++++++++++++ ext/openssl/ossl_version.h | 2 +- openssl.gemspec | 2 +- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/History.md b/History.md index 8baa0208e..d592bc6a1 100644 --- a/History.md +++ b/History.md @@ -1,3 +1,23 @@ +Version 2.0.6 +============= + +Bug fixes +--------- + +* The session_remove_cb set to an OpenSSL::SSL::SSLContext is no longer called + during GC. +* A possible deadlock in OpenSSL::SSL::SSLSocket#sysread is fixed. + [[GitHub #139]](https://github.com/ruby/openssl/pull/139) +* OpenSSL::BN#hash could return an unnormalized fixnum value on Windows. + [[Bug #13877]](https://bugs.ruby-lang.org/issues/13877) +* OpenSSL::SSL::SSLSocket#sysread and #sysread_nonblock set the length of the + destination buffer String to 0 on error. + [[GitHub #153]](https://github.com/ruby/openssl/pull/153) +* Possible deadlock is fixed. This happened only when built with older versions + of OpenSSL (before 1.1.0) or LibreSSL. + [[GitHub #155]](https://github.com/ruby/openssl/pull/155) + + Version 2.0.5 ============= diff --git a/ext/openssl/ossl_version.h b/ext/openssl/ossl_version.h index be3eebe1f..7725bc052 100644 --- a/ext/openssl/ossl_version.h +++ b/ext/openssl/ossl_version.h @@ -10,6 +10,6 @@ #if !defined(_OSSL_VERSION_H_) #define _OSSL_VERSION_H_ -#define OSSL_VERSION "2.0.5" +#define OSSL_VERSION "2.0.6" #endif /* _OSSL_VERSION_H_ */ diff --git a/openssl.gemspec b/openssl.gemspec index 1a3059039..34cef6f18 100644 --- a/openssl.gemspec +++ b/openssl.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |spec| spec.name = "openssl" - spec.version = "2.0.5" + spec.version = "2.0.6" spec.authors = ["Martin Bosslet", "SHIBATA Hiroshi", "Zachary Scott", "Kazuki Yamaguchi"] spec.email = ["ruby-core@ruby-lang.org"] spec.summary = %q{OpenSSL provides SSL, TLS and general purpose cryptography.}