mirror/ruby
1
0
Fork 0
mirror of https://github.com/ruby/ruby.git synced 2026-01-26 20:19:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

[ruby/openssl] ssl: fix SSLSocket#sysread leaking locktmp String on timeout

Browse Source 
Commit https://github.com/ruby/openssl/commit/3bbf5178a90e made blocking methods on SSLSocket follow the
IO#timeout= value. The commit changed io_wait_readable() to potentially
raise an exception without unlocking the String.

The String is currently locked for the entire duration of a #sysread
method call. This does not seem to be necessary, as SSL_read() does not
require that the same buffer is specified when retrying. Locking the
String during each SSL_read() call should be sufficient.

https://github.com/ruby/openssl/commit/8f791d73f5
This commit is contained in:
Kazuki Yamaguchi 2024-12-22 00:35:03 +09:00 committed by git
parent 4df16051be
commit 72480389d1
2 changed files with 16 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
ext/openssl/ossl_ssl.c
View File

@ -1881,9 +1881,10 @@ ossl_ssl_read_internal(int argc, VALUE *argv, VALUE self, int nonblock)
VALUE io = rb_attr_get(self, id_i_io);
rb_str_locktmp(str);
for (;;) {
rb_str_locktmp(str);
int nread = SSL_read(ssl, RSTRING_PTR(str), ilen);
rb_str_unlocktmp(str);
cb_state = rb_attr_get(self, ID_callback_state);
if (!NIL_P(cb_state)) {
@ -1894,32 +1895,27 @@ ossl_ssl_read_internal(int argc, VALUE *argv, VALUE self, int nonblock)
switch (ssl_get_error(ssl, nread)) {
case SSL_ERROR_NONE:
rb_str_unlocktmp(str);
rb_str_set_len(str, nread);
return str;
case SSL_ERROR_ZERO_RETURN:
rb_str_unlocktmp(str);
if (no_exception_p(opts)) { return Qnil; }
rb_eof_error();
case SSL_ERROR_WANT_WRITE:
if (nonblock) {
rb_str_unlocktmp(str);
if (no_exception_p(opts)) { return sym_wait_writable; }
write_would_block(nonblock);
}
io_wait_writable(io);
continue;
break;
case SSL_ERROR_WANT_READ:
if (nonblock) {
rb_str_unlocktmp(str);
if (no_exception_p(opts)) { return sym_wait_readable; }
read_would_block(nonblock);
}
io_wait_readable(io);
continue;
break;
case SSL_ERROR_SYSCALL:
if (!ERR_peek_error()) {
rb_str_unlocktmp(str);
if (errno)
rb_sys_fail(0);
else {
@ -1936,9 +1932,13 @@ ossl_ssl_read_internal(int argc, VALUE *argv, VALUE self, int nonblock)
}
/* fall through */
default:
rb_str_unlocktmp(str);
ossl_raise(eSSLError, "SSL_read");
}
// Ensure the buffer is not modified during io_wait_*able()
rb_str_modify(str);
if (rb_str_capacity(str) < (size_t)ilen)
rb_raise(eSSLError, "read buffer was modified");
}
}

9
test/openssl/test_ssl.rb
View File

@ -255,11 +255,16 @@ class OpenSSL::TestSSL < OpenSSL::SSLTestCase
ssl.syswrite(str)
assert_equal(str, ssl.sysread(str.bytesize))
ssl.timeout = 1
assert_raise(IO::TimeoutError) {ssl.read(1)}
ssl.timeout = 0.1
assert_raise(IO::TimeoutError) { ssl.sysread(1) }
ssl.syswrite(str)
assert_equal(str, ssl.sysread(str.bytesize))
buf = "orig".b
assert_raise(IO::TimeoutError) { ssl.sysread(1, buf) }
assert_equal("orig", buf)
assert_nothing_raised { buf.clear }
end
end
end