diff --git a/src/main/java/org/jruby/ext/openssl/SSLSocket.java b/src/main/java/org/jruby/ext/openssl/SSLSocket.java index 0a2f801e..55879032 100644 --- a/src/main/java/org/jruby/ext/openssl/SSLSocket.java +++ b/src/main/java/org/jruby/ext/openssl/SSLSocket.java @@ -539,8 +539,18 @@ public void wakeup() { } } - private static final int READ_WOULD_BLOCK_RESULT = Integer.MIN_VALUE + 1; - private static final int WRITE_WOULD_BLOCK_RESULT = Integer.MIN_VALUE + 2; + // Legitimate return values are -1 (EOF) and >= 0 (byte counts), so any value < -1 is safely in sentinel territory. + private static final int READ_WOULD_BLOCK_RESULT = -2; + private static final int WRITE_WOULD_BLOCK_RESULT = -3; + + private static boolean isWouldBlockResult(final int result) { + return result < -1; + } + + private RubySymbol wouldBlockSymbol(final int result) { + assert isWouldBlockResult(result) : "unexpected result: " + result; + return getRuntime().newSymbol(result == READ_WOULD_BLOCK_RESULT ? "wait_readable" : "wait_writable"); + } private static void readWouldBlock(final Ruby runtime, final boolean exception, final int[] result) { if ( exception ) throw newSSLErrorWaitReadable(runtime, "read would block"); @@ -552,10 +562,6 @@ private static void writeWouldBlock(final Ruby runtime, final boolean exception, result[0] = WRITE_WOULD_BLOCK_RESULT; } - private void doHandshake(final boolean blocking) throws IOException { - doHandshake(blocking, true); - } - // might return :wait_readable | :wait_writable in case (true, false) private IRubyObject doHandshake(final boolean blocking, final boolean exception) throws IOException { while (true) { @@ -577,7 +583,11 @@ private IRubyObject doHandshake(final boolean blocking, final boolean exception) doTasks(); break; case NEED_UNWRAP: - if (readAndUnwrap(blocking) == -1 && handshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED) { + int unwrapResult = readAndUnwrap(blocking, exception); + if (isWouldBlockResult(unwrapResult)) { + return wouldBlockSymbol(unwrapResult); + } + if (unwrapResult == -1 && handshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED) { throw new SSLHandshakeException("Socket closed"); } // during initialHandshake, calling readAndUnwrap that results UNDERFLOW does not mean writable. @@ -688,7 +698,9 @@ public int write(ByteBuffer src, boolean blocking) throws SSLException, IOExcept if ( netWriteData.hasRemaining() ) { flushData(blocking); } - netWriteData.clear(); + // use compact() to preserve any encrypted bytes that flushData could not send (non-blocking partial write) + // clear() would discard them, corrupting the TLS record stream: + netWriteData.compact(); final SSLEngineResult result = engine.wrap(src, netWriteData); if ( result.getStatus() == SSLEngineResult.Status.CLOSED ) { throw getRuntime().newIOError("closed SSL engine"); @@ -703,12 +715,16 @@ public int write(ByteBuffer src, boolean blocking) throws SSLException, IOExcept } public int read(final ByteBuffer dst, final boolean blocking) throws IOException { + return read(dst, blocking, true); + } + + private int read(final ByteBuffer dst, final boolean blocking, final boolean exception) throws IOException { if ( initialHandshake ) return 0; if ( engine.isInboundDone() ) return -1; if ( ! appReadData.hasRemaining() ) { - int appBytesProduced = readAndUnwrap(blocking); - if (appBytesProduced == -1 || appBytesProduced == 0) { + final int appBytesProduced = readAndUnwrap(blocking, exception); + if (appBytesProduced == -1 || appBytesProduced == 0 || isWouldBlockResult(appBytesProduced)) { return appBytesProduced; } } @@ -718,7 +734,15 @@ public int read(final ByteBuffer dst, final boolean blocking) throws IOException return limit; } - private int readAndUnwrap(final boolean blocking) throws IOException { + /** + * @param blocking whether to block on I/O + * @param exception when false, returns {@link #READ_WOULD_BLOCK_RESULT} or + * {@link #WRITE_WOULD_BLOCK_RESULT} instead of throwing if the + * post-handshake processing would block + * @return application bytes available, -1 on EOF/close, 0 when no app data + * produced, or a WOULD_BLOCK sentinel when would-block with exception=false + */ + private int readAndUnwrap(final boolean blocking, final boolean exception) throws IOException { final int bytesRead = socketChannelImpl().read(netReadData); if ( bytesRead == -1 ) { if ( ! netReadData.hasRemaining() || @@ -767,7 +791,11 @@ private int readAndUnwrap(final boolean blocking) throws IOException { handshakeStatus == SSLEngineResult.HandshakeStatus.NEED_TASK || handshakeStatus == SSLEngineResult.HandshakeStatus.NEED_WRAP || handshakeStatus == SSLEngineResult.HandshakeStatus.FINISHED ) ) { - doHandshake(blocking); + IRubyObject ex = doHandshake(blocking, exception); + if ( ex != null ) { // :wait_readable | :wait_writable + // TODO needs refactoring to avoid Symbol -> int -> Symbol + return "wait_writable".equals(ex.asJavaString()) ? WRITE_WOULD_BLOCK_RESULT : READ_WOULD_BLOCK_RESULT; + } } return appReadData.remaining(); } @@ -830,6 +858,14 @@ private IRubyObject sysreadImpl(final ThreadContext context, final IRubyObject l } try { + // Flush any pending encrypted write data before reading. + // After write_nonblock, encrypted bytes may remain in netWriteData that haven't been sent to the server. + // If we read without flushing, the server may not have received the complete request + // (e.g. net/http POST body) and will not send a response. + if ( engine != null && netWriteData.hasRemaining() ) { + flushData(blocking); + } + // So we need to make sure to only block when there is no data left to process if ( engine == null || ! ( appReadData.hasRemaining() || netReadData.position() > 0 ) ) { final Object ex = waitSelect(SelectionKey.OP_READ, blocking, exception); @@ -843,19 +879,28 @@ private IRubyObject sysreadImpl(final ThreadContext context, final IRubyObject l if ( engine == null ) { read = socketChannelImpl().read(dst); } else { - read = read(dst, blocking); + read = read(dst, blocking, exception); } - if ( read == -1 ) { - if ( exception ) throw runtime.newEOFError(); - return context.nil; + switch ( read ) { + case -1 : + if ( exception ) throw runtime.newEOFError(); + return context.nil; + // Post-handshake processing (e.g. TLS 1.3 NewSessionTicket) signaled would-block + case READ_WOULD_BLOCK_RESULT : + return runtime.newSymbol("wait_readable"); + case WRITE_WOULD_BLOCK_RESULT : + return runtime.newSymbol("wait_writable"); } - if ( read == 0 && status == SSLEngineResult.Status.BUFFER_UNDERFLOW ) { - // If we didn't get any data back because we only read in a partial TLS record, - // instead of spinning until the rest comes in, call waitSelect to either block - // until the rest is available, or throw a "read would block" error if we are in - // non-blocking mode. + if ( read == 0 && netReadData.position() == 0 ) { + // If we didn't get any data back and there is no buffered network data left to process, + // wait for more data from the network instead of spinning until it arrives. + // In blocking mode this blocks; in non-blocking mode it raises/returns "read would block". + // + // We check netReadData.position() rather than status == BUFFER_UNDERFLOW because readAndUnwrap + // may have successfully consumed a non-application record (e.g. a TLS 1.3 NewSessionTicket) + // leaving status == OK with zero app bytes produced and nothing left in the network buffer. final Object ex = waitSelect(SelectionKey.OP_READ, blocking, exception); if ( ex instanceof IRubyObject ) return (IRubyObject) ex; // :wait_readable } diff --git a/src/test/ruby/ssl/test_read_nonblock_tls13.rb b/src/test/ruby/ssl/test_read_nonblock_tls13.rb new file mode 100644 index 00000000..084d2761 --- /dev/null +++ b/src/test/ruby/ssl/test_read_nonblock_tls13.rb @@ -0,0 +1,901 @@ +# frozen_string_literal: false + +require File.expand_path('test_helper', File.dirname(__FILE__)) + +class TestReadNonblockTLS13 < TestCase + + include SSLTestHelper + + # ── helpers ────────────────────────────────────────────────────────── + + # Set up a TLS 1.3 server where the server does NOT read, so the + # client's send buffer saturates. Yields |ssl, port| to the block. + # This forces selectNow()==0 inside doHandshake when processing + # TLS 1.3 post-handshake records. + def with_saturated_tls13_client; require 'socket' + + tcp_server = TCPServer.new("127.0.0.1", 0) + port = tcp_server.local_address.ip_port + + server_ctx = OpenSSL::SSL::SSLContext.new + server_ctx.cert = @svr_cert + server_ctx.key = @svr_key + server_ctx.min_version = server_ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION + + ssl_server = OpenSSL::SSL::SSLServer.new(tcp_server, server_ctx) + ssl_server.start_immediately = true + + server_ready = Queue.new + server_thread = Thread.new do + Thread.current.report_on_exception = false + begin + ssl_conn = ssl_server.accept + server_ready << :ready + # Do NOT read — the client's send buffer will fill up + sleep 5 + ssl_conn.close rescue nil + rescue + server_ready << :error + end + end + + begin + sock = TCPSocket.new("127.0.0.1", port) + sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_SNDBUF, 4096) + sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_RCVBUF, 4096) + + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + + server_ready.pop # wait for server accept + + # Saturate send buffer + chunk = "X" * 16384 + 100.times do + begin + ssl.write_nonblock(chunk) + rescue IO::WaitWritable, OpenSSL::SSL::SSLErrorWaitWritable + break + rescue + break + end + end + + yield ssl + ensure + ssl.close rescue nil + sock.close rescue nil + tcp_server.close rescue nil + server_thread.kill rescue nil + server_thread.join(2) rescue nil + end + end + + # Same as above but for TLS 1.2 (control) + def with_saturated_tls12_client; require 'socket' + + tcp_server = TCPServer.new("127.0.0.1", 0) + port = tcp_server.local_address.ip_port + + server_ctx = OpenSSL::SSL::SSLContext.new + server_ctx.cert = @svr_cert + server_ctx.key = @svr_key + server_ctx.max_version = OpenSSL::SSL::TLS1_2_VERSION + + ssl_server = OpenSSL::SSL::SSLServer.new(tcp_server, server_ctx) + ssl_server.start_immediately = true + + server_ready = Queue.new + server_thread = Thread.new do + Thread.current.report_on_exception = false + begin + ssl_conn = ssl_server.accept + server_ready << :ready + sleep 5 + ssl_conn.close rescue nil + rescue + server_ready << :error + end + end + + begin + sock = TCPSocket.new("127.0.0.1", port) + sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_SNDBUF, 4096) + sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_RCVBUF, 4096) + + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + + server_ready.pop + + chunk = "X" * 16384 + 100.times do + begin + ssl.write_nonblock(chunk) + rescue IO::WaitWritable, OpenSSL::SSL::SSLErrorWaitWritable + break + rescue + break + end + end + + yield ssl + ensure + ssl.close rescue nil + sock.close rescue nil + tcp_server.close rescue nil + server_thread.kill rescue nil + server_thread.join(2) rescue nil + end + end + + # ── TLS 1.3 + saturated buffer (the exact production bug scenario) ── + + # Core reproducer: exception:false must return :wait_readable, not throw. + def test_read_nonblock_exception_false_saturated_tls13 + with_saturated_tls13_client do |ssl| + assert_equal "TLSv1.3", ssl.ssl_version + + result = ssl.read_nonblock(1024, exception: false) + assert_equal :wait_readable, result + end + end + + # exception:true must raise SSLErrorWaitReadable (not EAGAIN or IOError). + def test_read_nonblock_exception_true_saturated_tls13 + with_saturated_tls13_client do |ssl| + assert_equal "TLSv1.3", ssl.ssl_version + + raised = assert_raise(OpenSSL::SSL::SSLErrorWaitReadable) do + ssl.read_nonblock(1024) + end + assert_equal "read would block", raised.message + end + end + + # httprb code path: read_nonblock with buffer + exception:false + def test_read_nonblock_with_buffer_exception_false_saturated_tls13 + with_saturated_tls13_client do |ssl| + buf = '' + result = ssl.read_nonblock(1024, buf, exception: false) + assert_equal :wait_readable, result + end + end + + # Calling through sysread_nonblock directly (as some gems do) + def test_sysread_nonblock_exception_false_saturated_tls13 + with_saturated_tls13_client do |ssl| + result = ssl.send(:sysread_nonblock, 1024, exception: false) + assert_equal :wait_readable, result + end + end + + # Multiple consecutive read_nonblock calls must all return :wait_readable + def test_read_nonblock_repeated_calls_saturated_tls13 + with_saturated_tls13_client do |ssl| + 5.times do |i| + result = ssl.read_nonblock(1024, exception: false) + assert_equal :wait_readable, result, "iteration #{i}" + end + end + end + + # ── TLS 1.2 + saturated buffer (control — no post-handshake messages) ─ + + # TLS 1.2 has no post-handshake messages, so the bug path is never hit. + def test_read_nonblock_exception_false_saturated_tls12 + with_saturated_tls12_client do |ssl| + assert_equal "TLSv1.2", ssl.ssl_version + + result = ssl.read_nonblock(1024, exception: false) + assert_equal :wait_readable, result + end + end + + def test_read_nonblock_exception_true_saturated_tls12 + with_saturated_tls12_client do |ssl| + assert_equal "TLSv1.2", ssl.ssl_version + + assert_raise(OpenSSL::SSL::SSLErrorWaitReadable) do + ssl.read_nonblock(1024) + end + end + end + + # ── TLS 1.3 normal (unsaturated) tests ───────────────────────────── + + def test_read_nonblock_exception_false_tls13 + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + assert_equal "TLSv1.3", ssl.ssl_version + + 10.times do + result = ssl.read_nonblock(1024, exception: false) + assert [:wait_readable, String].any? { |t| t === result }, + "Expected :wait_readable or String, got #{result.inspect}" + break if result == :wait_readable + end + ssl.close + end + end + + def test_read_nonblock_exception_true_tls13 + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + assert_equal "TLSv1.3", ssl.ssl_version + + assert_raise(OpenSSL::SSL::SSLErrorWaitReadable) do + 10.times { ssl.read_nonblock(1024) } + end + ssl.close + end + end + + # ── TLS 1.2 normal (unsaturated) tests (control) ─────────────────── + + def test_read_nonblock_exception_false_tls12 + ctx_proc = Proc.new { |ctx| ctx.max_version = OpenSSL::SSL::TLS1_2_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + assert_equal "TLSv1.2", ssl.ssl_version + + result = ssl.read_nonblock(1024, exception: false) + assert_equal :wait_readable, result + ssl.close + end + end + + # ── Data round-trip: TLS 1.3 read/write still works ──────────────── + + def test_write_read_roundtrip_tls13 + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + assert_equal "TLSv1.3", ssl.ssl_version + + ssl.write("hello\n") + # Wait for echo data to arrive with a generous timeout + IO.select([ssl], nil, nil, 5) + # The first read_nonblock may consume a post-handshake message; + # retry until we get the application data. + data = nil + 10.times do + begin + data = ssl.read_nonblock(1024) + break + rescue OpenSSL::SSL::SSLErrorWaitReadable + IO.select([ssl], nil, nil, 2) + end + end + assert_equal "hello\n", data + + ssl.close + end + end + + def test_write_read_roundtrip_nonblock_exception_false_tls13 + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + + ssl.write("world\n") + # Wait for echo data to arrive + IO.select([ssl], nil, nil, 5) + + # Read with exception:false — might get :wait_readable first if + # the engine is processing a post-handshake record. + result = nil + 10.times do + result = ssl.read_nonblock(1024, exception: false) + if result == :wait_readable + IO.select([ssl], nil, nil, 2) + next + end + break + end + assert_kind_of String, result + assert_equal "world\n", result + + # No more data — should return :wait_readable + result = ssl.read_nonblock(1024, exception: false) + assert_equal :wait_readable, result + + ssl.close + end + end + + # ── Post-write read with saturated buffer ─────────────────────────── + + # After a write+read cycle the post-handshake messages are consumed; + # a subsequent read_nonblock should simply return :wait_readable. + def test_read_nonblock_after_write_tls13 + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + + ssl.write("test\n") + sleep 0.1 + begin; ssl.read_nonblock(1024); rescue OpenSSL::SSL::SSLErrorWaitReadable; end + + result = ssl.read_nonblock(1024, exception: false) + assert_equal :wait_readable, result + ssl.close + end + end + + # ── connect_nonblock + read_nonblock ──────────────────────────────── + + def test_read_nonblock_with_connect_nonblock_tls13 + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + + begin + ssl.connect_nonblock + rescue IO::WaitReadable + IO.select([ssl]); retry + rescue IO::WaitWritable + IO.select(nil, [ssl]); retry + end + + assert_equal "TLSv1.3", ssl.ssl_version + sleep 0.05 + + 10.times do + result = ssl.read_nonblock(1024, exception: false) + assert [:wait_readable, String].any? { |t| t === result }, + "Expected :wait_readable or String, got #{result.inspect}" + break if result == :wait_readable + end + ssl.close + end + end + + # connect_nonblock + saturated buffer + read_nonblock + def test_read_nonblock_connect_nonblock_saturated_tls13; require 'socket' + + tcp_server = TCPServer.new("127.0.0.1", 0) + port = tcp_server.local_address.ip_port + + server_ctx = OpenSSL::SSL::SSLContext.new + server_ctx.cert = @svr_cert + server_ctx.key = @svr_key + server_ctx.min_version = server_ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION + + ssl_server = OpenSSL::SSL::SSLServer.new(tcp_server, server_ctx) + ssl_server.start_immediately = true + + server_ready = Queue.new + server_thread = Thread.new do + Thread.current.report_on_exception = false + begin + ssl_conn = ssl_server.accept + server_ready << :ready + sleep 5 + ssl_conn.close rescue nil + rescue + server_ready << :error + end + end + + begin + sock = TCPSocket.new("127.0.0.1", port) + sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_SNDBUF, 4096) + sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_RCVBUF, 4096) + + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + + begin + ssl.connect_nonblock + rescue IO::WaitReadable + IO.select([ssl]); retry + rescue IO::WaitWritable + IO.select(nil, [ssl]); retry + end + + assert_equal "TLSv1.3", ssl.ssl_version + server_ready.pop + + chunk = "X" * 16384 + 100.times do + begin + ssl.write_nonblock(chunk) + rescue IO::WaitWritable, OpenSSL::SSL::SSLErrorWaitWritable + break + rescue + break + end + end + + result = ssl.read_nonblock(1024, exception: false) + assert_equal :wait_readable, result + ensure + ssl.close rescue nil + sock.close rescue nil + tcp_server.close rescue nil + server_thread.kill rescue nil + server_thread.join(2) rescue nil + end + end + + # ── Concurrent stress ────────────────────────────────────────────── + + def test_read_nonblock_tls13_concurrent_stress + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION } + errors = Queue.new + + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port| + threads = 5.times.map do |t| + Thread.new do + 20.times do |i| + begin + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + + 5.times do + result = ssl.read_nonblock(1024, exception: false) + break if result == :wait_readable + end + rescue OpenSSL::SSL::SSLErrorWaitReadable + errors << "Thread #{t} iter #{i}: SSLErrorWaitReadable thrown with exception:false" + rescue Errno::EAGAIN + errors << "Thread #{t} iter #{i}: EAGAIN thrown with exception:false" + rescue + # Other errors (connection reset, etc.) are acceptable + ensure + ssl.close rescue nil + sock.close rescue nil + end + end + end + end + + threads.each { |t| t.join(10) } + end + + collected = [] + collected << errors.pop until errors.empty? + assert collected.empty?, "Got #{collected.size} exception leaks:\n#{collected.first(5).join("\n")}" + end + + # ── Buffered I/O: multi-chunk read_nonblock ────────────────────── + # + # Write a large payload (bigger than one TLS record ~16KB), read it + # back in small read_nonblock chunks. Exercises: + # - appReadData having leftover bytes across read() calls + # - netReadData having multiple TLS records + # - the netReadData.position()==0 guard NOT firing when there IS data + + def test_multi_chunk_read_nonblock_tls13 + large = "A" * 1024 + "\n" # each line is 1025 bytes + total_lines = 30 # ~30KB total, exceeds one TLS record + payload = large * total_lines + + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + assert_equal "TLSv1.3", ssl.ssl_version + + # Write the payload — the echo server will echo each line back + ssl.write(payload) + + # Read it all back in small non-blocking chunks + received = +"" + deadline = Time.now + 5 + while received.bytesize < payload.bytesize && Time.now < deadline + begin + chunk = ssl.read_nonblock(1024) + received << chunk + rescue OpenSSL::SSL::SSLErrorWaitReadable + IO.select([ssl], nil, nil, 1) + end + end + + assert_equal payload.bytesize, received.bytesize + assert_equal payload, received + ssl.close + end + end + + def test_multi_chunk_read_nonblock_tls12 + large = "A" * 1024 + "\n" + total_lines = 30 + payload = large * total_lines + + ctx_proc = Proc.new { |ctx| ctx.max_version = OpenSSL::SSL::TLS1_2_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + assert_equal "TLSv1.2", ssl.ssl_version + + ssl.write(payload) + + received = +"" + deadline = Time.now + 5 + while received.bytesize < payload.bytesize && Time.now < deadline + begin + chunk = ssl.read_nonblock(1024) + received << chunk + rescue OpenSSL::SSL::SSLErrorWaitReadable + IO.select([ssl], nil, nil, 1) + end + end + + assert_equal payload.bytesize, received.bytesize + assert_equal payload, received + ssl.close + end + end + + # ── Buffered I/O: multi-chunk with exception:false ───────────────── + + def test_multi_chunk_read_nonblock_exception_false_tls13 + large = "B" * 1024 + "\n" + total_lines = 30 + payload = large * total_lines + + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + + ssl.write(payload) + + received = +"" + deadline = Time.now + 5 + while received.bytesize < payload.bytesize && Time.now < deadline + result = ssl.read_nonblock(1024, exception: false) + case result + when :wait_readable + IO.select([ssl], nil, nil, 1) + when :wait_writable + IO.select(nil, [ssl], nil, 1) + when String + received << result + end + end + + assert_equal payload.bytesize, received.bytesize + assert_equal payload, received + ssl.close + end + end + + # ── Buffered I/O: partial read_nonblock ──────────────────────────── + # + # Adapted from MRI's test_read_nonblock_without_session pattern. + # Write data, read a small amount (leaves data in appReadData buffer), + # then read the rest. + + def test_partial_read_nonblock_tls13 + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + assert_equal "TLSv1.3", ssl.ssl_version + + ssl.write("hello world\n") + IO.select([ssl], nil, nil, 5) + + # Read just 5 bytes — the rest stays in appReadData buffer + first = nil + 10.times do + begin + first = ssl.read_nonblock(5) + break + rescue OpenSSL::SSL::SSLErrorWaitReadable + IO.select([ssl], nil, nil, 2) + end + end + assert_equal "hello", first + + # Read the rest — should come from the buffer, no network I/O needed + rest = ssl.read_nonblock(100) + assert_equal " world\n", rest + + # Nothing left + result = ssl.read_nonblock(100, exception: false) + assert_equal :wait_readable, result + + ssl.close + end + end + + # ── Buffered I/O: multiple write+read cycles ─────────────────────── + # + # Adapted from MRI's test_parallel pattern (single connection version). + # Verifies the engine state stays clean across many exchanges after + # TLS 1.3 post-handshake processing. + + def test_multiple_write_read_cycles_tls13 + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + assert_equal "TLSv1.3", ssl.ssl_version + + str = "x" * 1000 + "\n" + 10.times do |i| + ssl.puts(str) + response = ssl.gets + assert_equal str, response, "cycle #{i}: data mismatch" + end + + ssl.close + end + end + + def test_multiple_write_read_cycles_tls12 + ctx_proc = Proc.new { |ctx| ctx.max_version = OpenSSL::SSL::TLS1_2_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + assert_equal "TLSv1.2", ssl.ssl_version + + str = "x" * 1000 + "\n" + 10.times do |i| + ssl.puts(str) + response = ssl.gets + assert_equal str, response, "cycle #{i}: data mismatch" + end + + ssl.close + end + end + + # ── Buffered I/O: sysread/syswrite round-trip ────────────────────── + # + # Adapted from MRI's test_sysread_and_syswrite: multiple cycles of + # syswrite/sysread with exact byte counts. Exercises the blocking + # sysreadImpl path on TLS 1.3. + + def test_sysread_syswrite_tls13 + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + assert_equal "TLSv1.3", ssl.ssl_version + + str = "x" * 100 + "\n" + + # Cycle 1: basic syswrite/sysread + ssl.syswrite(str) + newstr = ssl.sysread(str.bytesize) + assert_equal str, newstr + + # Cycle 2: sysread into a buffer + buf = String.new + ssl.syswrite(str) + assert_same buf, ssl.sysread(str.bytesize, buf) + assert_equal str, buf + + # Cycle 3: another round + ssl.syswrite(str) + assert_equal str, ssl.sysread(str.bytesize) + + ssl.close + end + end + + # ── Buffered I/O: large payload to exercise netReadData leftovers ── + # + # The server writes a large payload in one shot. The client reads + # it in small read_nonblock chunks. When socketChannelImpl().read() + # pulls in multiple TLS records at once, netReadData has leftover + # bytes (position > 0) after the first unwrap. The sysreadImpl loop + # must continue processing (NOT call waitSelect) when netReadData + # still has data. + # + # This is the critical regression test for the + # netReadData.position()==0 guard — it must NOT wait when there's + # still buffered network data. + + def test_large_server_write_small_client_reads_tls13 + # Custom server_proc: read a size header, then send that many bytes + server_proc = Proc.new do |context, ssl| + begin + line = ssl.gets # read the request + if line && line.strip =~ /^SEND (\d+)$/ + size = $1.to_i + data = "Z" * size + "\n" + ssl.write(data) + end + rescue IOError, OpenSSL::SSL::SSLError + ensure + ssl.close rescue nil + end + end + + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, + :ctx_proc => ctx_proc, :server_proc => server_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + assert_equal "TLSv1.3", ssl.ssl_version + + # Ask the server to send 48KB — this will be split across multiple + # TLS records (~16KB each), giving us netReadData with leftover bytes. + expected_size = 48 * 1024 + ssl.puts("SEND #{expected_size}") + + received = +"" + expected_total = expected_size + 1 # +1 for the trailing "\n" + deadline = Time.now + 5 + while received.bytesize < expected_total && Time.now < deadline + result = ssl.read_nonblock(4096, exception: false) + case result + when :wait_readable + IO.select([ssl], nil, nil, 1) + when :wait_writable + IO.select(nil, [ssl], nil, 1) + when String + received << result + end + end + + assert_equal expected_total, received.bytesize, + "Expected #{expected_total} bytes but got #{received.bytesize}" + assert_equal "Z" * expected_size + "\n", received + ssl.close + end + end + + def test_large_server_write_small_client_reads_tls12 + server_proc = Proc.new do |context, ssl| + begin + line = ssl.gets + if line && line.strip =~ /^SEND (\d+)$/ + size = $1.to_i + data = "Z" * size + "\n" + ssl.write(data) + end + rescue IOError, OpenSSL::SSL::SSLError + ensure + ssl.close rescue nil + end + end + + ctx_proc = Proc.new { |ctx| ctx.max_version = OpenSSL::SSL::TLS1_2_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, + :ctx_proc => ctx_proc, :server_proc => server_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + assert_equal "TLSv1.2", ssl.ssl_version + + expected_size = 48 * 1024 + ssl.puts("SEND #{expected_size}") + + received = +"" + expected_total = expected_size + 1 + deadline = Time.now + 5 + while received.bytesize < expected_total && Time.now < deadline + result = ssl.read_nonblock(4096, exception: false) + case result + when :wait_readable + IO.select([ssl], nil, nil, 1) + when :wait_writable + IO.select(nil, [ssl], nil, 1) + when String + received << result + end + end + + assert_equal expected_total, received.bytesize + assert_equal "Z" * expected_size + "\n", received + ssl.close + end + end + + # ── Wasted iteration detection ───────────────────────────────────── + # + # TLS 1.3 post-handshake record (NewSessionTicket) that produces 0 app bytes, status is OK + # + # We detect this by inspecting the internal `status` field after read_nonblock returns :wait_readable. + # If status is BUFFER_UNDERFLOW, the extra iteration occurred. If status is OK, sysreadImpl handled + # the read==0/status==OK case directly. + def test_internal_no_wasted_readAndUnwrap_iteration_tls13; require 'socket' + + tcp_server = TCPServer.new("127.0.0.1", 0) + port = tcp_server.local_address.ip_port + + server_ctx = OpenSSL::SSL::SSLContext.new + server_ctx.cert = @svr_cert + server_ctx.key = @svr_key + server_ctx.ssl_version = "TLSv1_3" + + ssl_server = OpenSSL::SSL::SSLServer.new(tcp_server, server_ctx) + ssl_server.start_immediately = true + + server_thread = Thread.new do + Thread.current.report_on_exception = false + begin + conn = ssl_server.accept + sleep 5 + conn.close rescue nil + rescue + end + end + + begin + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + assert_equal "TLSv1.3", ssl.ssl_version + + # Wait for the server's NewSessionTicket to arrive on the wire + # after the blocking connect has finished. + sleep 0.1 + + # Access the private `status` field via Java reflection + java_cls = Java::OrgJrubyExtOpenssl::SSLSocket.java_class + status_field = java_cls.declared_field("status") + status_field.accessible = true + java_ssl = ssl.to_java(Java::OrgJrubyExtOpenssl::SSLSocket) + + result = ssl.read_nonblock(1024, exception: false) + assert_equal :wait_readable, result + + status_after = status_field.value(java_ssl).to_s + # If sysreadImpl properly handles read==0 with any status (not just BUFFER_UNDERFLOW), + # only one readAndUnwrap call is made and status stays OK. + assert_equal "OK", status_after, + "Expected status OK (single readAndUnwrap call) but got #{status_after} " \ + "(extra wasted iteration through readAndUnwrap occurred)" + + ssl.close + ensure + ssl.close rescue nil + sock.close rescue nil + tcp_server.close rescue nil + server_thread.kill rescue nil + server_thread.join(2) rescue nil + end + end if defined?(JRUBY_VERSION) +end diff --git a/src/test/ruby/ssl/test_write_nonblock.rb b/src/test/ruby/ssl/test_write_nonblock.rb new file mode 100644 index 00000000..bd693b67 --- /dev/null +++ b/src/test/ruby/ssl/test_write_nonblock.rb @@ -0,0 +1,289 @@ +require File.expand_path('test_helper', File.dirname(__FILE__)) + +class TestWriteNonblock < TestCase + + include SSLTestHelper + + # Reproduces the data loss: write a large payload via write_nonblock + # with a slow-reading server (small recv buffer), then read the response. + # The server echoes back the byte count it received. If bytes were lost, + # the count will be less than expected. + def test_write_nonblock_data_integrity + expected_size = 256 * 1024 # 256KB — large enough to overflow TCP buffers + + # Custom server: reads all data until a blank line, counts bytes, sends back the count. + # Deliberately slow: small recv buffer + sleep between reads to create backpressure. + server_proc = Proc.new do |context, ssl| + begin + total = 0 + while (line = ssl.gets) + break if line.strip.empty? + total += line.bytesize + end + ssl.write("RECEIVED #{total}\n") + rescue IOError, OpenSSL::SSL::SSLError => e + # If the TLS stream is corrupted, the server may get an error here + warn "Server error: #{e.class}: #{e.message}" if $VERBOSE + ensure + ssl.close rescue nil + end + end + + [OpenSSL::SSL::TLS1_2_VERSION, OpenSSL::SSL::TLS1_3_VERSION].each do |tls_version| + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = tls_version } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, + :ctx_proc => ctx_proc, :server_proc => server_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + # Small send buffer to increase the chance of partial non-blocking writes + sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_SNDBUF, 4096) + sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_RCVBUF, 4096) + + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + + # Build a large payload: many lines totaling expected_size bytes + line = "X" * 1023 + "\n" # 1024 bytes per line + lines_needed = expected_size / line.bytesize + payload = line * lines_needed + actual_payload_size = payload.bytesize + + # Write it all using write_nonblock (as net/http does) + written = 0 + while written < payload.bytesize + begin + n = ssl.write_nonblock(payload.byteslice(written, payload.bytesize - written)) + written += n + rescue IO::WaitWritable, OpenSSL::SSL::SSLErrorWaitWritable + IO.select(nil, [ssl], nil, 5) + retry + end + end + + # Send terminator + ssl.write("\n") + + # Read the response (this is where the flush-before-read matters) + response = nil + deadline = Time.now + 10 + while Time.now < deadline + begin + response = ssl.gets + break if response + rescue IO::WaitReadable, OpenSSL::SSL::SSLErrorWaitReadable + IO.select([ssl], nil, nil, 5) + end + end + + assert_not_nil response, "No response from server (TLS #{ssl.ssl_version})" + assert_match(/^RECEIVED (\d+)/, response) + received = response[/RECEIVED (\d+)/, 1].to_i + assert_equal actual_payload_size, received, + "Server received #{received} bytes but we sent #{actual_payload_size} " \ + "(lost #{actual_payload_size - received} bytes) on #{ssl.ssl_version}" + + ssl.close + end + end + end + + # Simpler test: write_nonblock followed by sysread should work. + # This is the net/http pattern: POST body via write, then read response. + def test_write_nonblock_then_sysread + server_proc = Proc.new do |context, ssl| + begin + data = +"" + while (line = ssl.gets) + break if line.strip == "END" + data << line + end + ssl.write("OK:#{data.bytesize}\n") + rescue IOError, OpenSSL::SSL::SSLError + ensure + ssl.close rescue nil + end + end + + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, + :ctx_proc => ctx_proc, :server_proc => server_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_SNDBUF, 4096) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + + # Write via write_nonblock + payload = "Y" * 50_000 + "\n" + written = 0 + while written < payload.bytesize + begin + n = ssl.write_nonblock(payload.byteslice(written, payload.bytesize - written)) + written += n + rescue IO::WaitWritable, OpenSSL::SSL::SSLErrorWaitWritable + IO.select(nil, [ssl], nil, 5) + retry + end + end + ssl.write("END\n") + + # Now read response via sysread (the net/http pattern) + IO.select([ssl], nil, nil, 10) + response = ssl.sysread(1024) + assert_match(/^OK:(\d+)/, response) + received = response[/OK:(\d+)/, 1].to_i + assert_equal payload.bytesize, received, "Server received #{received} bytes but sent #{payload.bytesize}" + + ssl.close + end + end + + # Test that multiple write_nonblock calls preserve all data even under + # buffer pressure (many small writes) + def test_many_small_write_nonblock_calls + server_proc = Proc.new do |context, ssl| + begin + total = 0 + while (line = ssl.gets) + break if line.strip == "DONE" + total += line.bytesize + end + ssl.write("TOTAL:#{total}\n") + rescue IOError, OpenSSL::SSL::SSLError + ensure + ssl.close rescue nil + end + end + + ctx_proc = Proc.new { |ctx| ctx.min_version = ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION } + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, + :ctx_proc => ctx_proc, :server_proc => server_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_SNDBUF, 4096) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + + # Send 500 small lines rapidly via write_nonblock + line = "Z" * 200 + "\n" + expected_total = 0 + 500.times do + written = 0 + while written < line.bytesize + begin + n = ssl.write_nonblock(line.byteslice(written, line.bytesize - written)) + written += n + rescue IO::WaitWritable, OpenSSL::SSL::SSLErrorWaitWritable + IO.select(nil, [ssl], nil, 5) + retry + end + end + expected_total += line.bytesize + end + ssl.write("DONE\n") + + IO.select([ssl], nil, nil, 10) + response = ssl.gets + assert_not_nil response + received = response[/TOTAL:(\d+)/, 1].to_i + assert_equal expected_total, received, "Server received #{received} bytes but sent #{expected_total}" + + ssl.close + end + end + + # Detect the netWriteData.clear() bug by invoking the Java write() directly, bypassing syswriteImpl's `waitSelect`. + # + # Reproducer for #242 (jruby/jruby#8935). + # + # Strategy: + # 1. Saturate the TCP send buffer (server doesn't read) + # 2. Call write(ByteBuffer, false) directly via Java reflection + # 3. Check netWriteData.remaining() — if > 0, data would be discarded by the next write() call's netWriteData.clear() + def test_internal_write_nonblock_unflushed_data_detected + require 'socket' + + tcp_server = TCPServer.new("127.0.0.1", 0) + port = tcp_server.local_address.ip_port + + server_ctx = OpenSSL::SSL::SSLContext.new + server_ctx.cert = @svr_cert + server_ctx.key = @svr_key + server_ctx.min_version = server_ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION + + ssl_server = OpenSSL::SSL::SSLServer.new(tcp_server, server_ctx) + ssl_server.start_immediately = true + + server_thread = Thread.new do + Thread.current.report_on_exception = false + begin + ssl_conn = ssl_server.accept + sleep 30 # Do NOT read — maximize backpressure + ssl_conn.close rescue nil + rescue + end + end + + begin + sock = TCPSocket.new("127.0.0.1", port) + sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_SNDBUF, 4096) + sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_RCVBUF, 4096) + + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.sync_close = true + ssl.connect + + java_cls = Java::OrgJrubyExtOpenssl::SSLSocket.java_class + java_ssl = ssl.to_java(Java::OrgJrubyExtOpenssl::SSLSocket) + + nwd_field = java_cls.declared_field("netWriteData") + nwd_field.accessible = true + # Get the write(ByteBuffer, boolean) method via reflection + write_method = java_cls.declared_method("write", java.nio.ByteBuffer.java_class, Java::boolean) + write_method.accessible = true + + # Phase 1: fill the TCP send buffer via normal write_nonblock + chunk = "H" * 16384 + 100.times do + begin + ssl.write_nonblock(chunk) + rescue IO::WaitWritable, OpenSSL::SSL::SSLErrorWaitWritable + break + rescue IOError, OpenSSL::SSL::SSLError + break + end + end + + # Phase 2: call write(src, false) directly — this bypasses + # syswriteImpl's waitSelect and goes straight to the code path + # that has the clear() bug. + src = java.nio.ByteBuffer.wrap(("I" * 4096).to_java_bytes) + begin + write_method.invoke(java_ssl, src, false) + rescue Java::JavaLangReflect::InvocationTargetException => e + warn "write() threw: #{e.cause}" if $VERBOSE # Expected — write may throw due to the saturated buffer + end + + nwd = nwd_field.value(java_ssl) + remaining = nwd.remaining + + if remaining > 0 + # BUG CONFIRMED: there are unflushed encrypted bytes in netWriteData. + # The next write() call will do netWriteData.clear(), discarding them. + # This is exactly the data loss bug from issue #242. + # + # To prove actual data loss, we would call write() again — the clear() would discard remaining encrypted bytes, + # corrupting the TLS record stream and eventually causing the server to see fewer bytes than the client sent. + assert remaining > 0, "netWriteData has #{remaining} unflushed bytes — next write() would discard them via clear()" + else + omit "Could not produce unflushed netWriteData in loopback (remaining=#{remaining}); bug requires network latency" + end + ensure + ssl.close rescue nil + sock.close rescue nil + tcp_server.close rescue nil + server_thread.kill rescue nil + server_thread.join(2) rescue nil + end + end if defined?(JRUBY_VERSION) +end