Bug #13921: buffered read_nonblock doesn't work as expected using SSLSocket

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: 2.4.2
• Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN

[chucke (Tiago Cardoso)]

I have something similar to the following code that handles both tcp as well as ssl sockets. The idea is to use the #read_nonblock API which uses a buffer when passed in the argument (instead of creating a string every time).

class A
  def initialize
    @buffer = String.new("", encoding: Encoding::BINARY)
  end

  def read(io)
     io.read_nonblock(16_384, @buffer, exception: false)
     # do stuff...
     @buffer.clear
  end

For plain tcp sockets, it works as expected (buffers payload). However, when passed an ssl socket, it buffers the whole SSL payload, which SSLSocket's usually handle transparently. Of course, my logic fails after that.

My current workaround is to resort to the unbuffered API:

buffer = io.read_nonblock(16_384, exception: false)

however, the call mentioned above should have worked transparently.

[chucke (Tiago Cardoso)]

Just confirmed that the problem only happens because I pass exception: false.

[normalperson (Eric Wong)]

Reading the implementation of read_nonblock and consume_rbuff in
ext/openssl/lib/openssl/buffering.rb and I'm not seeing a
problem in those pure Ruby methods...

Are you using sysread or sysread_nonblock directly, anywhere?
That would cause problems because it's not respectin @rbuffer.

[chucke (Tiago Cardoso)]

Are you using sysread or sysread_nonblock directly, anywhere?

I'm not. But can you reproduce the error as well?

[normalperson (Eric Wong)]

[e-mail] wrote:

Issue #13921 has been updated by chucke (Tiago Cardoso).

Are you using sysread or sysread_nonblock directly, anywhere?

I'm not. But can you reproduce the error as well?

I haven't tried directly for this issue, but I've had code
running using SSLSocket#read_nonblock in yahns(*) for over a
year without noticing problems; though admittedly it doesn't
handle many uploads...

Do you have a standalone reproduction?

(*) git clone git://yhbt.net/yahns
I use it as an HTTPS server and reverse proxy.

[normalperson (Eric Wong)]

I've setup some test cases with yahns on https://80x24.org:8443/
to handle large uploads and return the MD5

curl -v -T large_file https://80x24.org:8443/
curl -v -T large_file https://80x24.org:8443/gets_read_mix
curl -v -T large_file https://80x24.org:8443/each

Where large_file is <= 1GB

For each of the endpoints, the MD5 hexdigest matches
the output of md5sum large_file.

All of these match for me with random-sized buffers at the Rack
app level. yahns always uses the buffer arg for
SSLSocket#read_nonblock

I run yahns with the following command and config files below:

/path/to/bin/yahns -c /path/to/yahns.conf.rb

==> /path/to/yahns.conf.rb <==

disclaimer: I don't know if this HTTPS configuration is correct

require 'openssl'
ctx = OpenSSL::SSL::SSLContext.new
ctx.cert = OpenSSL::X509::Certificate.new(IO.read(
'/path/to/ssl/certs/example.com.crt'))
ctx.extra_chain_cert = [ OpenSSL::X509::Certificate.new(IO.read(
'/path/to/ssl/certs/example.com.chain.crt')) ]
ctx.key = OpenSSL::PKey::RSA.new(IO.read(
'/path/to/ssl/private/example.com.key'))
ctx.set_params(verify_mode: OpenSSL::SSL::VERIFY_NONE)
ctx.session_cache_mode = OpenSSL::SSL::SSLContext::SESSION_CACHE_SERVER

app(:rack, '/path/to/config.ru', preload: true) do
listen 8443, ssl_ctx: ctx
listen '[::]:8443', ipv6only: true, ssl_ctx: ctx
client_max_body_size(1024 * 1024 * 1024)
client_timeout 5
output_buffering false
input_buffering false
end

==> /path/to/config.ru <==
map('http://80x24.org/') do
use Rack::Head
run(lambda do |env|
case env['REQUEST_METHOD']
when 'PUT'
cap = 0x1000000
/\A100-continue\z/i =~ env['HTTP_EXPECT'] and return [ 100, {}, [] ]
digest = Digest::MD5.new
input = env['rack.input']
case env["PATH_INFO"]
when "/gets_read_mix"
if buf = input.gets
begin
digest.update(buf)
end while input.read(rand(cap), buf)
end
when "/each"
input.each do |b|
digest.update(b)
b.clear
end
else
if buf = input.read(rand(cap))
begin
raise "#{buf.size} > #{cap}" if buf.size > cap
digest.update(buf)
end while input.read(rand(cap), buf)
end
end
buf&.clear

[ 200, {
'Content-Type' => -'text/plain',
'Content-Length' => -'33',
}, [ "#{digest.hexdigest}\n" ] ]
else
[ 405, {
'Content-Type' => -'text/plain',
'Content-Length' => -'3'
}, [ "no\n" ] ]
end
end)
end

[rhenium (Kazuki Yamaguchi)]

chucke (Tiago Cardoso) wrote:

I have something similar to the following code that handles both tcp as well as ssl sockets. The idea is to use the #read_nonblock API which uses a buffer when passed in the argument (instead of creating a string every time).

class A
  def initialize
    @buffer = String.new("", encoding: Encoding::BINARY)
  end

  def read(io)
     io.read_nonblock(16_384, @buffer, exception: false)
     # do stuff...
     @buffer.clear
  end

For plain tcp sockets, it works as expected (buffers payload). However, when passed an ssl socket, it buffers the whole SSL payload, which SSLSocket's usually handle transparently. Of course, my logic fails after that.

chucke (Tiago Cardoso) wrote:

Just confirmed that the problem only happens because I pass exception: false.

I don't understand what you mean by "it buffers the whole SSL payload". Passing exception:false to SSLSocket#read_nonblock makes it not raise IO::Wait{Readable,Writable} or EOFError and instead return a symbol or nil, but does nothing else.

But I can see the code snippet isn't checking the return value of SSLSocket#read_nonblock. While reviewing the implementation of SSLSocket#sysread{,_nonblock}, I discovered that they leave uninitialized data in the destination buffer String (which may be provided by the caller, as the argument, as in your code) on error paths. Are you referring to this?

https://github.com/ruby/openssl/pull/153

[chucke (Tiago Cardoso)]

rhenium, thank you for the link to the openssl issue, I just tested, and I can confirm that it solves the issue! Indeed I had problems with data left in the destination buffer passed as argument, as my example above shows.

But I still have to keep my workaround for openssl < 2.0.6, as those versions won't go away anytime soon.

But there should be a warning in the documentation for #read_nonblock saying that passing a buffer as argument is unsafe for ssl sockets.

You can close this issue.

[rhenium (Kazuki Yamaguchi)]

• Status changed from Open to Closed

chucke (Tiago Cardoso) wrote:

rhenium, thank you for the link to the openssl issue, I just tested, and I can confirm that it solves the issue! Indeed I had problems with data left in the destination buffer passed as argument, as my example above shows.

But I still have to keep my workaround for openssl < 2.0.6, as those versions won't go away anytime soon.

But there should be a warning in the documentation for #read_nonblock saying that passing a buffer as argument is unsafe for ssl sockets.

Thanks for the confirmation. Backport request for Ruby 2.3 and 2.4 is [Bug #13935].

I suggest you check the return value from *#read_nonblock. It happens only in error cases where read_nonblock could not read anything.