From 726709e6572b35829738f296d13dbad942050342 Mon Sep 17 00:00:00 2001 From: kares Date: Mon, 20 Apr 2026 14:34:13 +0200 Subject: [PATCH 1/7] [release] bump version to 0.16.0 SNAPSHOT --- lib/jopenssl/version.rb | 2 +- pom.xml | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/jopenssl/version.rb b/lib/jopenssl/version.rb index 2608d80e..2aee7ee9 100644 --- a/lib/jopenssl/version.rb +++ b/lib/jopenssl/version.rb @@ -1,5 +1,5 @@ module JOpenSSL - VERSION = '0.15.8.dev' + VERSION = '0.16.0.dev' BOUNCY_CASTLE_VERSION = '1.84' end diff --git a/pom.xml b/pom.xml index f9222af6..d395ff42 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ DO NOT MODIFY - GENERATED CODE 4.0.0 rubygems jruby-openssl - 0.15.8.dev-SNAPSHOT + 0.16.0.dev-SNAPSHOT gem JRuby OpenSSL JRuby-OpenSSL is an add-on gem for JRuby that emulates the Ruby OpenSSL native library. @@ -67,8 +67,8 @@ DO NOT MODIFY - GENERATED CODE -W0 9.2.19.0 9.2.19.0 - 2.0.2 - 3.0.0 + 3.0.0 + 2.0.2 pom.xml false src/test/ruby/**/test_*.rb @@ -98,7 +98,7 @@ DO NOT MODIFY - GENERATED CODE org.jruby jruby-core - 9.2.0.0 + 9.2.1.0 provided From 06692c706faa006677ef0820029d3570eaaf2f14 Mon Sep 17 00:00:00 2001 From: Joao Duarte Date: Mon, 16 Mar 2026 22:36:44 +0000 Subject: [PATCH 2/7] [fix] SSL write loss on non-blocking partial flush (#242) write() unconditionally called netWriteData.clear() after a non-blocking flushData() that may not have flushed all encrypted bytes; discarding unflushed TLS records, corrupting the encrypted stream and causing 'Broken pipe' or 'Connection reset' errors on subsequent writes (most commonly seen during 'gem push' over https of large artifacts) additionally, sysread needs to also flush pending writes before reading since after write_nonblock, encrypted bytes could remain unsent; without flushing first the server would never receive the complete request body (e.g. net/http POST), causing it to time out or reset --- .../java/org/jruby/ext/openssl/SSLSocket.java | 16 +- src/test/ruby/ssl/test_write_flush.rb | 171 ++++++++++++++++++ 2 files changed, 183 insertions(+), 4 deletions(-) create mode 100644 src/test/ruby/ssl/test_write_flush.rb diff --git a/src/main/java/org/jruby/ext/openssl/SSLSocket.java b/src/main/java/org/jruby/ext/openssl/SSLSocket.java index ebc9cb03..df8946d0 100644 --- a/src/main/java/org/jruby/ext/openssl/SSLSocket.java +++ b/src/main/java/org/jruby/ext/openssl/SSLSocket.java @@ -689,7 +689,9 @@ public int write(ByteBuffer src, boolean blocking) throws SSLException, IOExcept if ( netWriteData.hasRemaining() ) { flushData(blocking); } - netWriteData.clear(); + // compact() to preserve encrypted bytes 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"); @@ -727,9 +729,8 @@ private int readAndUnwrap(final boolean blocking) throws IOException { closeInbound(); return -1; } - // inbound channel has been already closed but closeInbound() must - // be defered till the last engine.unwrap() call. - // peerNetData could not be empty. + // inbound channel has been already closed but closeInbound() must be defered till + // the last engine.unwrap() call; peerNetData could not be empty } appReadData.clear(); netReadData.flip(); @@ -831,6 +832,13 @@ 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 the buffer that haven't been sent, if we read wout flushing, + // server may not have received the complete request (e.g. net/http POST body) and will not respond + 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); diff --git a/src/test/ruby/ssl/test_write_flush.rb b/src/test/ruby/ssl/test_write_flush.rb new file mode 100644 index 00000000..8b809ff5 --- /dev/null +++ b/src/test/ruby/ssl/test_write_flush.rb @@ -0,0 +1,171 @@ +# frozen_string_literal: false +require File.expand_path('test_helper', File.dirname(__FILE__)) + +class TestSSLWriteFlush < TestCase + + include SSLTestHelper + + # Exercises the write_nonblock -> read transition used by net/http for POST + # requests. The bug (clear() instead of compact()) loses encrypted bytes that + # remain in netWriteData after a partial flushData on the *last* write_nonblock. + # + # We run multiple request/response rounds on the same TLS connection with + # varying payload sizes to increase the probability that at least one round + # triggers a partial flush at the write->read boundary. + # + # NOTE: On localhost the loopback interface rarely causes partial socket writes, + # so this test may not reliably catch regressions to clear(). The definitive + # coverage is in the Java-level SSLSocketTest which can control buffer state + # directly. This test serves as an integration smoke test for the write->read + # data path. + # + # https://github.com/jruby/jruby-openssl/issues/242 + def test_write_nonblock_data_integrity + # Payload sizes chosen to exercise different alignments with the TLS record + # layer (~16 KB records) and socket send buffer. Primes avoid lucky alignment. + payload_sizes = [ + 8_191, # just under 8 KB — fits in one TLS record + 16_381, # just under 16 KB — nearly one full TLS record + 65_521, # ~64 KB — several TLS records, common chunk size + 262_139, # ~256 KB — large payload, many partial flushes likely + ] + + # The server reads a 4-byte big-endian length prefix, then that many bytes + # of payload, and responds with "OK:" where hex_digest is the + # SHA-256 of the received payload. This is repeated for each payload size. + server_proc = proc { |ctx, ssl| + begin + payload_sizes.length.times do + # read 4-byte length prefix + header = read_exactly(ssl, 4) + break unless header && header.bytesize == 4 + expected_len = header.unpack('N')[0] + + # read payload + payload = read_exactly(ssl, expected_len) + break unless payload && payload.bytesize == expected_len + + digest = OpenSSL::Digest::SHA256.hexdigest(payload) + response = "OK:#{digest}" + ssl.write(response) + end + ensure + ssl.close rescue nil + end + } + + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, + server_proc: server_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + # Constrain the send buffer to make partial flushes more likely. + # The kernel may round this up, but even a modest reduction helps. + sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_SNDBUF, 2048) + sock.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) + + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.connect + ssl.sync_close = true + + payload_sizes.each do |size| + data = generate_test_data(size) + expected_digest = OpenSSL::Digest::SHA256.hexdigest(data) + + # Send length-prefixed payload via write_nonblock + message = [size].pack('N') + data + write_nonblock_all(ssl, message) + + # Immediately switch to reading — this is where the bug manifests: + # if compact() was replaced with clear(), residual encrypted bytes + # from the last write_nonblock are lost and the server never + # receives the complete payload. + response = read_with_timeout(ssl, 5) + + assert_equal "OK:#{expected_digest}", response, + "Data integrity failure for #{size}-byte payload: " \ + "server did not receive the complete payload or it was corrupted" + end + + ssl.close + end + end + + private + + # Generate non-trivial test data that won't compress well in TLS. + # Uses a seeded PRNG so failures are reproducible, and avoids + # OpenSSL::Random which has a per-call size limit in some BC versions. + def generate_test_data(size) + rng = Random.new(size) # seeded for reproducibility + (0...size).map { rng.rand(256).chr }.join.b + end + + # Write all of +data+ via write_nonblock, retrying on WaitWritable. + # Does NOT do any extra flushing after the last write — this is critical + # for exercising the bug where clear() loses the tail of encrypted data. + def write_nonblock_all(ssl, data) + remaining = data + while remaining.bytesize > 0 + begin + written = ssl.write_nonblock(remaining) + remaining = remaining.byteslice(written..-1) + rescue IO::WaitWritable + IO.select(nil, [ssl]) + retry + end + end + end + + # Read a complete response from the SSL socket with a timeout. + # Returns the accumulated data, or fails the test on timeout. + def read_with_timeout(ssl, timeout_sec) + response = "" + deadline = Time.now + timeout_sec + loop do + remaining = deadline - Time.now + if remaining <= 0 + flunk "Timed out after #{timeout_sec}s waiting for server response " \ + "(got #{response.bytesize} bytes so far: #{response.inspect[0, 80]})" + end + if IO.select([ssl], nil, nil, [remaining, 0.5].min) + begin + chunk = ssl.read_nonblock(16384, exception: false) + case chunk + when :wait_readable then next + when nil then break # EOF + else + response << chunk + # Our protocol responses are short ("OK:<64 hex chars>"), so if + # we've received a plausible amount we can stop. + break if response.include?("OK:") && response.bytesize >= 67 + end + rescue IO::WaitReadable + # Can occur despite exception: false and IO.select — TLS protocol + # data (e.g. post-handshake messages) made the socket look readable + # but no application data is available yet. + next + rescue EOFError + break + end + end + end + response + end + + # Read exactly +n+ bytes from an SSL socket, retrying partial reads. + def self.read_exactly(ssl, n) + buf = "" + while buf.bytesize < n + chunk = ssl.readpartial(n - buf.bytesize) + buf << chunk + end + buf + rescue EOFError + buf + end + + # Instance method wrapper for use in server_proc + def read_exactly(ssl, n) + self.class.read_exactly(ssl, n) + end + +end From a5d4fe1425e63fb570411872d8fe32a974b159ab Mon Sep 17 00:00:00 2001 From: kares Date: Tue, 31 Mar 2026 11:30:55 +0200 Subject: [PATCH 3/7] [fix] read_nonblock(exception: false) raising SSLErrorWaitReadable `readAndUnwrap()` called doHandshake(blocking) assuming `exception = true` When a post-handshake TLS event (e.g. TLS 1.3 NewSessionTicket) triggered handshake during a non-blocking read, waitSelect raised SSLErrorWaitReadable instead of returning :wait_readable. --- .../java/org/jruby/ext/openssl/SSLSocket.java | 18 +++++++++++------- src/test/ruby/ssl/test_write_flush.rb | 5 ----- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/jruby/ext/openssl/SSLSocket.java b/src/main/java/org/jruby/ext/openssl/SSLSocket.java index df8946d0..813ae005 100644 --- a/src/main/java/org/jruby/ext/openssl/SSLSocket.java +++ b/src/main/java/org/jruby/ext/openssl/SSLSocket.java @@ -553,10 +553,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) { @@ -706,11 +702,15 @@ 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); + int appBytesProduced = readAndUnwrap(blocking, exception); if (appBytesProduced == -1 || appBytesProduced == 0) { return appBytesProduced; } @@ -722,6 +722,10 @@ public int read(final ByteBuffer dst, final boolean blocking) throws IOException } private int readAndUnwrap(final boolean blocking) throws IOException { + return readAndUnwrap(blocking, true); + } + + private int readAndUnwrap(final boolean blocking, final boolean exception) throws IOException { final int bytesRead = socketChannelImpl().read(netReadData); if ( bytesRead == -1 ) { if ( ! netReadData.hasRemaining() || @@ -769,7 +773,7 @@ private int readAndUnwrap(final boolean blocking) throws IOException { handshakeStatus == SSLEngineResult.HandshakeStatus.NEED_TASK || handshakeStatus == SSLEngineResult.HandshakeStatus.NEED_WRAP || handshakeStatus == SSLEngineResult.HandshakeStatus.FINISHED ) ) { - doHandshake(blocking); + doHandshake(blocking, exception); } return appReadData.remaining(); } @@ -852,7 +856,7 @@ 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 ) { diff --git a/src/test/ruby/ssl/test_write_flush.rb b/src/test/ruby/ssl/test_write_flush.rb index 8b809ff5..f3bd7bd1 100644 --- a/src/test/ruby/ssl/test_write_flush.rb +++ b/src/test/ruby/ssl/test_write_flush.rb @@ -138,11 +138,6 @@ def read_with_timeout(ssl, timeout_sec) # we've received a plausible amount we can stop. break if response.include?("OK:") && response.bytesize >= 67 end - rescue IO::WaitReadable - # Can occur despite exception: false and IO.select — TLS protocol - # data (e.g. post-handshake messages) made the socket look readable - # but no application data is available yet. - next rescue EOFError break end From 06da113401dea031b1be25a00d88d482f9830b05 Mon Sep 17 00:00:00 2001 From: kares Date: Mon, 30 Mar 2026 13:56:33 +0200 Subject: [PATCH 4/7] [test] introduce a helper for loading in JUnit --- .../org/jruby/ext/openssl/OpenSSLHelper.java | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 src/test/java/org/jruby/ext/openssl/OpenSSLHelper.java diff --git a/src/test/java/org/jruby/ext/openssl/OpenSSLHelper.java b/src/test/java/org/jruby/ext/openssl/OpenSSLHelper.java new file mode 100644 index 00000000..5c2bf19d --- /dev/null +++ b/src/test/java/org/jruby/ext/openssl/OpenSSLHelper.java @@ -0,0 +1,70 @@ +package org.jruby.ext.openssl; + +import org.jruby.Ruby; +import org.jruby.runtime.ThreadContext; + +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +abstract class OpenSSLHelper { + + protected Ruby runtime; + + void setUpRuntime() throws ClassNotFoundException { + runtime = Ruby.newInstance(); + loadOpenSSL(runtime); + } + + void tearDownRuntime() { + if (runtime != null) runtime.tearDown(false); + } + + protected void loadOpenSSL(final Ruby runtime) throws ClassNotFoundException { + // prepend lib/ so openssl.rb + jopenssl/ are loaded instead of bundled OpenSSL in jruby-stdlib + final String libDir = new File("lib").getAbsolutePath(); + runtime.evalScriptlet("$LOAD_PATH.unshift '" + libDir + "'"); + runtime.evalScriptlet("require 'openssl'"); + + // sanity: verify openssl was loaded from the project, not jruby-stdlib : + final String versionFile = new File(libDir, "jopenssl/version.rb").getAbsolutePath(); + final String expectedVersion = runtime.evalScriptlet( + "File.read('" + versionFile + "').match( /.*\\sVERSION\\s*=\\s*['\"](.*)['\"]/ )[1]") + .toString(); + final String loadedVersion = runtime.evalScriptlet("JOpenSSL::VERSION").toString(); + assertEquals("OpenSSL must be loaded from project (got version " + loadedVersion + + "), not from jruby-stdlib", expectedVersion, loadedVersion); + + // Also check the Java extension classes were resolved from the project, not jruby-stdlib : + final String classOrigin = runtime.getJRubyClassLoader() + .loadClass("org.jruby.ext.openssl.OpenSSL") + .getProtectionDomain().getCodeSource().getLocation().toString(); + assertTrue("OpenSSL.class (via JRuby classloader) come from project, got: " + classOrigin, + classOrigin.endsWith("/pkg/classes/")); + } + + // HELPERS + + public ThreadContext currentContext() { + return runtime.getCurrentContext(); + } + + public static String readResource(final String resource) { + int n; + try (InputStream in = SSLSocketTest.class.getResourceAsStream(resource)) { + if (in == null) throw new IllegalArgumentException(resource + " not found on classpath"); + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buf = new byte[8192]; + while ((n = in.read(buf)) != -1) out.write(buf, 0, n); + return new String(out.toByteArray(), StandardCharsets.UTF_8); + } catch (IOException e) { + throw new IllegalStateException("failed to load" + resource, e); + } + } +} From 5f8c201f3d20031aad8e7b4efdb915b4728b1ca8 Mon Sep 17 00:00:00 2001 From: kares Date: Mon, 20 Apr 2026 14:42:09 +0200 Subject: [PATCH 5/7] [test] introduce SSLSocketTest (unit test) - writeNonblockDataIntegrity: approximates the gem push scenario (#242) large payload via write_nonblock loop, then read server's byte count response, assert data integrity (no bytes lost) - writeNonblockNetWriteDataState: saturates TCP buffer, then accesses the package-private netWriteData field directly to verify buffer consistency after the compact() fix --- Mavenfile | 5 +- pom.xml | 6 + .../java/org/jruby/ext/openssl/SSLSocket.java | 6 +- .../org/jruby/ext/openssl/OpenSSLHelper.java | 12 +- .../org/jruby/ext/openssl/SSLSocketTest.java | 176 ++++++++++++++++++ src/test/resources/start_ssl_server.rb | 37 ++++ 6 files changed, 232 insertions(+), 10 deletions(-) create mode 100644 src/test/java/org/jruby/ext/openssl/SSLSocketTest.java create mode 100644 src/test/resources/start_ssl_server.rb diff --git a/Mavenfile b/Mavenfile index 3308406a..22f30d30 100644 --- a/Mavenfile +++ b/Mavenfile @@ -82,10 +82,13 @@ plugin :clean do 'failOnError' => 'false' ) end -jar 'org.jruby:jruby-core', '9.2.1.0', :scope => :provided +jruby_compile_compat = '9.2.1.0' +jar 'org.jruby:jruby-core', jruby_compile_compat, :scope => :provided # for invoker generated classes we need to add javax.annotation when on Java > 8 jar 'javax.annotation:javax.annotation-api', '1.3.1', :scope => :compile jar 'org.junit.jupiter:junit-jupiter', '5.11.4', :scope => :test +# a test dependency to provide digest and other stdlib bits, needed when loading OpenSSL in Java unit tests +jar 'org.jruby:jruby-stdlib', jruby_compile_compat, :scope => :test plugin :surefire, '3.5.5' diff --git a/pom.xml b/pom.xml index d395ff42..d32de57f 100644 --- a/pom.xml +++ b/pom.xml @@ -113,6 +113,12 @@ DO NOT MODIFY - GENERATED CODE 5.11.4 test + + org.jruby + jruby-stdlib + 9.2.1.0 + test + diff --git a/src/main/java/org/jruby/ext/openssl/SSLSocket.java b/src/main/java/org/jruby/ext/openssl/SSLSocket.java index 813ae005..39d5a609 100644 --- a/src/main/java/org/jruby/ext/openssl/SSLSocket.java +++ b/src/main/java/org/jruby/ext/openssl/SSLSocket.java @@ -145,9 +145,9 @@ private static CallSite callSite(final CallSite[] sites, final CallSiteIndex ind private SSLEngine engine; private RubyIO io; - private ByteBuffer appReadData; - private ByteBuffer netReadData; - private ByteBuffer netWriteData; + ByteBuffer appReadData; + ByteBuffer netReadData; + ByteBuffer netWriteData; private final ByteBuffer dummy = ByteBuffer.allocate(0); // could be static private boolean initialHandshake = false; diff --git a/src/test/java/org/jruby/ext/openssl/OpenSSLHelper.java b/src/test/java/org/jruby/ext/openssl/OpenSSLHelper.java index 5c2bf19d..17489836 100644 --- a/src/test/java/org/jruby/ext/openssl/OpenSSLHelper.java +++ b/src/test/java/org/jruby/ext/openssl/OpenSSLHelper.java @@ -9,8 +9,8 @@ import java.io.InputStream; import java.nio.charset.StandardCharsets; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; abstract class OpenSSLHelper { @@ -37,15 +37,15 @@ protected void loadOpenSSL(final Ruby runtime) throws ClassNotFoundException { "File.read('" + versionFile + "').match( /.*\\sVERSION\\s*=\\s*['\"](.*)['\"]/ )[1]") .toString(); final String loadedVersion = runtime.evalScriptlet("JOpenSSL::VERSION").toString(); - assertEquals("OpenSSL must be loaded from project (got version " + loadedVersion + - "), not from jruby-stdlib", expectedVersion, loadedVersion); + assertEquals(expectedVersion, loadedVersion, "OpenSSL must be loaded from project " + + "(got version " + loadedVersion + "), not from jruby-stdlib"); // Also check the Java extension classes were resolved from the project, not jruby-stdlib : final String classOrigin = runtime.getJRubyClassLoader() .loadClass("org.jruby.ext.openssl.OpenSSL") .getProtectionDomain().getCodeSource().getLocation().toString(); - assertTrue("OpenSSL.class (via JRuby classloader) come from project, got: " + classOrigin, - classOrigin.endsWith("/pkg/classes/")); + assertTrue(classOrigin.endsWith("/pkg/classes/"), "OpenSSL.class (via JRuby classloader) " + + "come from project, got: " + classOrigin); } // HELPERS diff --git a/src/test/java/org/jruby/ext/openssl/SSLSocketTest.java b/src/test/java/org/jruby/ext/openssl/SSLSocketTest.java new file mode 100644 index 00000000..83d01fdc --- /dev/null +++ b/src/test/java/org/jruby/ext/openssl/SSLSocketTest.java @@ -0,0 +1,176 @@ +package org.jruby.ext.openssl; + +import java.nio.ByteBuffer; + +import org.jruby.RubyArray; +import org.jruby.RubyFixnum; +import org.jruby.RubyInteger; +import org.jruby.RubyString; +import org.jruby.exceptions.RaiseException; +import org.jruby.runtime.builtin.IRubyObject; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +public class SSLSocketTest extends OpenSSLHelper { + + /** Loads the ssl_pair.rb script that creates a connected SSL socket pair. */ + private String start_ssl_server_rb() { return readResource("/start_ssl_server.rb"); } + + @BeforeEach + public void setUp() throws Exception { + setUpRuntime(); + } + + @AfterEach + public void tearDown() { + tearDownRuntime(); + } + + /** + * Real-world scenario: {@code gem push} sends a large POST body via {@code syswrite_nonblock}, + * then reads the HTTP response via {@code sysread}. + * + * Approximates the {@code gem push} scenario: + *
    + *
  1. Write 256KB via {@code syswrite_nonblock} in a loop (the net/http POST pattern)
    2. + *
  2. Server reads via {@code sysread} and counts bytes
    3. + *
  3. Assert: server received exactly what client sent
    4. + *
+ * + * With the old {@code clear()} bug, encrypted bytes were silently + * discarded during partial non-blocking writes, so the server would + * receive fewer bytes than sent. + */ + @org.junit.jupiter.api.Test + public void syswriteNonblockDataIntegrity() throws Exception { + final RubyArray pair = (RubyArray) runtime.evalScriptlet(start_ssl_server_rb()); + SSLSocket client = (SSLSocket) pair.entry(0).toJava(SSLSocket.class); + SSLSocket server = (SSLSocket) pair.entry(1).toJava(SSLSocket.class); + + try { + // Server: read all data in a background thread, counting bytes + final long[] serverReceived = { 0 }; + Thread serverReader = startServerReader(server, serverReceived); + + // Client: write 256KB in 4KB chunks via syswrite_nonblock + byte[] chunk = new byte[4096]; + java.util.Arrays.fill(chunk, (byte) 'P'); // P for POST body + RubyString payload = RubyString.newString(runtime, chunk); + + long totalSent = 0; + for (int i = 0; i < 64; i++) { // 64 * 4KB = 256KB + try { + IRubyObject written = client.syswrite_nonblock(currentContext(), payload); + totalSent += ((RubyInteger) written).getLongValue(); + } catch (RaiseException e) { + if ("OpenSSL::SSL::SSLErrorWaitWritable".equals(e.getException().getMetaClass().getName())) { + System.out.println("syswrite_nonblock expected: " + e.getMessage()); + // Expected: non-blocking write would block — retry as blocking + IRubyObject written = client.syswrite(currentContext(), payload); + totalSent += ((RubyInteger) written).getLongValue(); + } else { + System.err.println("syswrite_nonblock unexpected: " + e.getMessage()); + throw e; + } + } + } + assertTrue(totalSent > 0, "should have sent data"); + + // Close client to signal EOF, let server finish reading + client.callMethod(currentContext(), "close"); + serverReader.join(10_000); + + assertEquals(totalSent, serverReceived[0], + "server must receive exactly what client sent — mismatch means encrypted bytes were lost!" + ); + } finally { + closeQuietly(pair); + } + } + + private Thread startServerReader(final SSLSocket server, final long[] serverReceived) { + Thread serverReader = new Thread(() -> { + try { + RubyFixnum len = RubyFixnum.newFixnum(runtime, 8192); + while (true) { + IRubyObject data = server.sysread(currentContext(), len); + serverReceived[0] += ((RubyString) data).getByteList().getRealSize(); + } + } catch (RaiseException e) { + String errorName = e.getException().getMetaClass().getName(); + if ("EOFError".equals(errorName) || "IOError".equals(errorName)) { // client closes connection + System.out.println("server-reader expected: " + e.getMessage()); + } else { + System.err.println("server-reader unexpected: " + e.getMessage()); + e.printStackTrace(System.err); + throw e; + } + } + }); + serverReader.start(); + return serverReader; + } + + /** + * After saturating the TCP send buffer with {@code syswrite_nonblock}, + * inspect {@code netWriteData} to verify the buffer is consistent. + */ + @Test + public void syswriteNonblockNetWriteDataConsistency() { + final RubyArray pair = (RubyArray) runtime.evalScriptlet(start_ssl_server_rb()); + SSLSocket client = (SSLSocket) pair.entry(0).toJava(SSLSocket.class); + + try { + assertNotNull(client.netWriteData, "netWriteData initialized after handshake"); + + // Saturate: server is not reading yet, so backpressure builds + byte[] chunk = new byte[16384]; + java.util.Arrays.fill(chunk, (byte) 'S'); + RubyString payload = RubyString.newString(runtime, chunk); + + int successWrites = 0; + for (int i = 0; i < 200; i++) { + try { + client.syswrite_nonblock(currentContext(), payload); + successWrites++; + } catch (RaiseException e) { + if ("OpenSSL::SSL::SSLErrorWaitWritable".equals(e.getException().getMetaClass().getName())) { + System.out.println("saturate-loop expected: " + e.getMessage()); + break; // buffer saturated — expected + } + System.err.println("saturate-loop unexpected: " + e.getMessage()); + throw e; + } + } + assertTrue(successWrites > 0, "at least one write should succeed"); + + ByteBuffer netWriteData = client.netWriteData; + assertTrue(netWriteData.position() <= netWriteData.limit(), "position <= limit"); + assertTrue(netWriteData.limit() <= netWriteData.capacity(), "limit <= capacity"); + + // If there are unflushed bytes, compact() preserved them + if (netWriteData.remaining() > 0) { + // The bytes should be valid TLS record data, not zeroed memory + byte b = netWriteData.get(netWriteData.position()); + assertNotEquals(0, b, "preserved bytes should be TLS data, not zeroed"); + } + + } finally { + closeQuietly(pair); + } + } + + private void closeQuietly(final RubyArray sslPair) { + for (int i = 0; i < sslPair.getLength(); i++) { + final IRubyObject elem = sslPair.entry(i); + try { elem.callMethod(currentContext(), "close"); } + catch (RaiseException e) { // already closed? + System.err.println("close raised (" + elem.inspect() + ") : " + e.getMessage()); + } + } + } +} diff --git a/src/test/resources/start_ssl_server.rb b/src/test/resources/start_ssl_server.rb new file mode 100644 index 00000000..2e30eb96 --- /dev/null +++ b/src/test/resources/start_ssl_server.rb @@ -0,0 +1,37 @@ +# Creates a connected SSL socket pair for Java unit tests. +# Returns [client_ssl, server_ssl] +# +# OpenSSL extension is loaded by SSLSocketTest.setUp via OpenSSL.load(runtime). + +require 'socket' + +key = OpenSSL::PKey::RSA.new(2048) +cert = OpenSSL::X509::Certificate.new +cert.version = 2 +cert.serial = 1 +cert.subject = cert.issuer = OpenSSL::X509::Name.parse('/CN=Test') +cert.public_key = key.public_key +cert.not_before = Time.now +cert.not_after = Time.now + 3600 +cert.sign(key, OpenSSL::Digest::SHA256.new) + +tcp_server = TCPServer.new('127.0.0.1', 0) +port = tcp_server.local_address.ip_port +ctx = OpenSSL::SSL::SSLContext.new +ctx.cert = cert +ctx.key = key +ssl_server = OpenSSL::SSL::SSLServer.new(tcp_server, ctx) +ssl_server.start_immediately = true + +server_ssl = nil +server_thread = Thread.new { server_ssl = ssl_server.accept } + +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) +client_ssl = OpenSSL::SSL::SSLSocket.new(sock) +client_ssl.sync_close = true +client_ssl.connect +server_thread.join(5) + +[client_ssl, server_ssl] From b765238270f776b9e0418f66f4fbcacb7d9090ac Mon Sep 17 00:00:00 2001 From: kares Date: Mon, 20 Apr 2026 14:43:55 +0200 Subject: [PATCH 6/7] [fix] return wait_writable on non-blocking reads --- .../java/org/jruby/ext/openssl/SSLSocket.java | 31 ++++---- .../org/jruby/ext/openssl/SSLSocketTest.java | 76 ++++++++++++++++++- 2 files changed, 89 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/jruby/ext/openssl/SSLSocket.java b/src/main/java/org/jruby/ext/openssl/SSLSocket.java index 39d5a609..c1beef88 100644 --- a/src/main/java/org/jruby/ext/openssl/SSLSocket.java +++ b/src/main/java/org/jruby/ext/openssl/SSLSocket.java @@ -141,7 +141,7 @@ private static CallSite callSite(final CallSite[] sites, final CallSiteIndex ind return sites[ index.ordinal() ]; } - private SSLContext sslContext; + SSLContext sslContext; private SSLEngine engine; private RubyIO io; @@ -209,7 +209,7 @@ private IRubyObject fallback_set_io_nonblock_checked(ThreadContext context, Ruby private static final String SESSION_SOCKET_ID = "socket_id"; - private SSLEngine ossl_ssl_setup(final ThreadContext context, final boolean server) { + SSLEngine ossl_ssl_setup(final ThreadContext context, final boolean server) { SSLEngine engine = this.engine; if ( engine != null ) return engine; @@ -574,7 +574,7 @@ private IRubyObject doHandshake(final boolean blocking, final boolean exception) doTasks(); break; case NEED_UNWRAP: - if (readAndUnwrap(blocking) == -1 && handshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED) { + if (readAndUnwrap(blocking, exception) == -1 && handshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED) { throw new SSLHandshakeException("Socket closed"); } // during initialHandshake, calling readAndUnwrap that results UNDERFLOW does not mean writable. @@ -721,10 +721,6 @@ private int read(final ByteBuffer dst, final boolean blocking, final boolean exc return limit; } - private int readAndUnwrap(final boolean blocking) throws IOException { - return readAndUnwrap(blocking, true); - } - private int readAndUnwrap(final boolean blocking, final boolean exception) throws IOException { final int bytesRead = socketChannelImpl().read(netReadData); if ( bytesRead == -1 ) { @@ -813,10 +809,10 @@ private void doShutdown() throws IOException { } /** - * @return the (@link RubyString} buffer or :wait_readable / :wait_writeable {@link RubySymbol} + * @return the {@link RubyString} buffer or :wait_readable / :wait_writeable {@link RubySymbol} */ - private IRubyObject sysreadImpl(final ThreadContext context, final IRubyObject len, final IRubyObject buff, - final boolean blocking, final boolean exception) { + private IRubyObject sysreadImpl(final ThreadContext context, + final IRubyObject len, final IRubyObject buff, final boolean blocking, final boolean exception) { final Ruby runtime = context.runtime; final int length = RubyNumeric.fix2int(len); @@ -836,11 +832,12 @@ 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 the buffer that haven't been sent, if we read wout flushing, - // server may not have received the complete request (e.g. net/http POST body) and will not respond + // Flush pending write data before reading (after write_nonblock encrypted bytes may still be buffered) if ( engine != null && netWriteData.hasRemaining() ) { - flushData(blocking); + if ( flushData(blocking) && ! blocking ) { + if ( exception ) throw newSSLErrorWaitWritable(runtime, "write would block"); + return runtime.newSymbol("wait_writable"); + } } // So we need to make sure to only block when there is no data left to process @@ -851,7 +848,7 @@ private IRubyObject sysreadImpl(final ThreadContext context, final IRubyObject l final ByteBuffer dst = ByteBuffer.allocate(length); int read = -1; - // ensure >0 bytes read; sysread is blocking read. + // ensure > 0 bytes read; sysread is blocking read while ( read <= 0 ) { if ( engine == null ) { read = socketChannelImpl().read(dst); @@ -1238,7 +1235,7 @@ public IRubyObject ssl_version(ThreadContext context) { return context.runtime.newString( engine.getSession().getProtocol() ); } - private transient SocketChannelImpl socketChannel; + transient SocketChannelImpl socketChannel; private SocketChannelImpl socketChannelImpl() { if ( socketChannel != null ) return socketChannel; @@ -1253,7 +1250,7 @@ private SocketChannelImpl socketChannelImpl() { throw new IllegalStateException("unknow channel impl: " + channel + " of type " + channel.getClass().getName()); } - private interface SocketChannelImpl { + interface SocketChannelImpl { boolean isOpen() ; diff --git a/src/test/java/org/jruby/ext/openssl/SSLSocketTest.java b/src/test/java/org/jruby/ext/openssl/SSLSocketTest.java index 83d01fdc..3c1e97d2 100644 --- a/src/test/java/org/jruby/ext/openssl/SSLSocketTest.java +++ b/src/test/java/org/jruby/ext/openssl/SSLSocketTest.java @@ -1,9 +1,15 @@ package org.jruby.ext.openssl; +import java.io.IOException; import java.nio.ByteBuffer; +import java.nio.channels.SelectionKey; +import java.nio.channels.Selector; +import javax.net.ssl.SSLEngine; +import org.jruby.Ruby; import org.jruby.RubyArray; import org.jruby.RubyFixnum; +import org.jruby.RubyHash; import org.jruby.RubyInteger; import org.jruby.RubyString; import org.jruby.exceptions.RaiseException; @@ -45,7 +51,7 @@ public void tearDown() { * discarded during partial non-blocking writes, so the server would * receive fewer bytes than sent. */ - @org.junit.jupiter.api.Test + @Test public void syswriteNonblockDataIntegrity() throws Exception { final RubyArray pair = (RubyArray) runtime.evalScriptlet(start_ssl_server_rb()); SSLSocket client = (SSLSocket) pair.entry(0).toJava(SSLSocket.class); @@ -173,4 +179,72 @@ private void closeQuietly(final RubyArray sslPair) { } } } + + // ---------- + + /** + * MRI's ossl_ssl_read_internal returns :wait_writable (or raises SSLErrorWaitWritable / "write would block") + * when SSL_read hits SSL_ERROR_WANT_WRITE. Pending netWriteData is JRuby's equivalent state. + */ + @Test + public void sysreadNonblockReturnsWaitWritableWhenPendingEncryptedBytesRemain() { + final SSLSocket socket = newSSLSocket(runtime, partialWriteChannel(1)); + final SSLEngine engine = socket.ossl_ssl_setup(currentContext(), false); + engine.setUseClientMode(true); + + socket.netWriteData = ByteBuffer.wrap(new byte[] { 1, 2 }); + + final RubyHash opts = RubyHash.newKwargs(runtime, "exception", runtime.getFalse()); // exception: false + final IRubyObject result = socket.sysread_nonblock(currentContext(), runtime.newFixnum(1), opts); + + assertEquals("wait_writable", result.asJavaString()); + assertEquals(1, socket.netWriteData.remaining()); + } + + @Test + public void sysreadNonblockRaisesWaitWritableWhenPendingEncryptedBytesRemain() { + final SSLSocket socket = newSSLSocket(runtime, partialWriteChannel(1)); + final SSLEngine engine = socket.ossl_ssl_setup(currentContext(), false); + engine.setUseClientMode(true); + + socket.netWriteData = ByteBuffer.wrap(new byte[] { 1, 2 }); + + try { + socket.sysread_nonblock(currentContext(), runtime.newFixnum(1)); + fail("expected SSLErrorWaitWritable"); + } + catch (RaiseException ex) { + assertEquals("OpenSSL::SSL::SSLErrorWaitWritable", ex.getException().getMetaClass().getName()); + assertTrue(ex.getMessage().contains("write would block")); + assertEquals(1, socket.netWriteData.remaining()); + } + } + + private static SSLSocket newSSLSocket(final Ruby runtime, final SSLSocket.SocketChannelImpl socketChannel) { + final SSLContext sslContext = new SSLContext(runtime); + sslContext.doSetup(runtime.getCurrentContext()); + final SSLSocket sslSocket = new SSLSocket(runtime, runtime.getObject()); + sslSocket.sslContext = sslContext; + sslSocket.socketChannel = socketChannel; + return sslSocket; + } + + private static SSLSocket.SocketChannelImpl partialWriteChannel(final int bytesPerWrite) { + return new SSLSocket.SocketChannelImpl() { + public boolean isOpen() { return true; } + public int read(final ByteBuffer dst) { return 0; } + public int write(final ByteBuffer src) { + final int written = Math.min(bytesPerWrite, src.remaining()); + src.position(src.position() + written); + return written; + } + public int getRemotePort() { return 443; } + public boolean isSelectable() { return false; } + public boolean isBlocking() { return false; } + public void configureBlocking(final boolean block) { } + public SelectionKey register(final Selector selector, final int ops) throws IOException { + throw new UnsupportedOperationException(); + } + }; + } } From 523bbe30adc0f98a131ae63b64bd57b757cdd2fa Mon Sep 17 00:00:00 2001 From: kares Date: Mon, 30 Mar 2026 14:01:47 +0200 Subject: [PATCH 7/7] [refactor] share empty read-only byte-buffer --- src/main/java/org/jruby/ext/openssl/SSLSocket.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jruby/ext/openssl/SSLSocket.java b/src/main/java/org/jruby/ext/openssl/SSLSocket.java index c1beef88..5d71cc87 100644 --- a/src/main/java/org/jruby/ext/openssl/SSLSocket.java +++ b/src/main/java/org/jruby/ext/openssl/SSLSocket.java @@ -141,6 +141,8 @@ private static CallSite callSite(final CallSite[] sites, final CallSiteIndex ind return sites[ index.ordinal() ]; } + private static final ByteBuffer EMPTY_DATA = ByteBuffer.allocate(0).asReadOnlyBuffer(); + SSLContext sslContext; private SSLEngine engine; private RubyIO io; @@ -148,7 +150,6 @@ private static CallSite callSite(final CallSite[] sites, final CallSiteIndex ind ByteBuffer appReadData; ByteBuffer netReadData; ByteBuffer netWriteData; - private final ByteBuffer dummy = ByteBuffer.allocate(0); // could be static private boolean initialHandshake = false; private transient long initializeTime; @@ -610,7 +611,7 @@ private IRubyObject doHandshake(final boolean blocking, final boolean exception) private void doWrap(final boolean blocking) throws IOException { netWriteData.clear(); - SSLEngineResult result = engine.wrap(dummy, netWriteData); + SSLEngineResult result = engine.wrap(EMPTY_DATA.duplicate(), netWriteData); netWriteData.flip(); handshakeStatus = result.getHandshakeStatus(); status = result.getStatus(); @@ -794,7 +795,7 @@ private void doShutdown() throws IOException { } netWriteData.clear(); try { - engine.wrap(dummy, netWriteData); // send close (after sslEngine.closeOutbound) + engine.wrap(EMPTY_DATA.duplicate(), netWriteData); // send close (after sslEngine.closeOutbound) } catch (SSLException e) { debug(getRuntime(), "SSLSocket.doShutdown", e);