From 97559e9ac36e86fda911a706ac5f4a3f0d49e5c1 Mon Sep 17 00:00:00 2001 From: kares Date: Tue, 3 Nov 2026 19:24:51 +0100 Subject: [PATCH 1/3] [fix] flush buffered alert on failed handshake shutdown (#399) When BCJSSE's unwrap() fails during handshake (e.g. the trust manager rejects the server cert), the fatal alert is queued for output. unwrap() does not throw: saves the exception (ProvSSLEngine.deferredException) and asks for NEED_WRAP. The next wrap() throws the saved exception and writes nothing. doShutdown() stopped at that first exception, so no alert was sent; a server whose peer kept the TCP socket open then blocked in accept indefinitely. regression since 0.19.2 - reduced shutdown to one doShutdown() call --- .../java/org/jruby/ext/openssl/SSLSocket.java | 11 +++++++- test/ssl/test_ssl.rb | 28 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jruby/ext/openssl/SSLSocket.java b/src/main/java/org/jruby/ext/openssl/SSLSocket.java index 6b4e0821..d1840306 100644 --- a/src/main/java/org/jruby/ext/openssl/SSLSocket.java +++ b/src/main/java/org/jruby/ext/openssl/SSLSocket.java @@ -975,7 +975,16 @@ private void doShutdown() throws IOException { try { SSLEngineResult result; do { - result = engine.wrap(EMPTY_DATA.duplicate(), netWriteData); // send close (after engine.closeOutbound) + try { + result = engine.wrap(EMPTY_DATA.duplicate(), netWriteData); // send close (after engine.closeOutbound) + } + catch (SSLException e) { + // BCJSSE re-throws a failed unwrap here (writing nothing); next wrap writes the queued alert + if (engine.isOutboundDone()) throw e; + LOG.debug(getRuntime(), "doShutdown", e); + result = engine.wrap(EMPTY_DATA.duplicate(), netWriteData); + } + if (result.getStatus() == SSLEngineResult.Status.BUFFER_OVERFLOW) { netWriteData = ensureCapacity(netWriteData, engine.getSession().getPacketBufferSize()); continue; diff --git a/test/ssl/test_ssl.rb b/test/ssl/test_ssl.rb index aa7317a7..be93e51f 100644 --- a/test/ssl/test_ssl.rb +++ b/test/ssl/test_ssl.rb @@ -465,6 +465,34 @@ def test_server_verify_peer_allows_missing_client_cert end end + # client rejecting server cert must send an alert, even with TCP socket kept open + def test_client_verify_failure_sends_alert_to_server + [:TLSv1_2, :TLSv1_3].each do |version| + begin + server_ctx = OpenSSL::SSL::SSLContext.new(version) # NOTE: BC-JSSE isn't used for plain "SSL" + server_ctx.cert = @svr_cert + server_ctx.key = @svr_key + tcp_server = TCPServer.new("127.0.0.1", 0) + ssl_server = OpenSSL::SSL::SSLServer.new(tcp_server, server_ctx) + server_thread = Thread.new { ssl_server.accept rescue $! } + + client_ctx = OpenSSL::SSL::SSLContext.new(version) + client_ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER + client_ctx.cert_store = OpenSSL::X509::Store.new # trusts nothing + sock = TCPSocket.new("127.0.0.1", tcp_server.addr[1]) + assert_raise(OpenSSL::SSL::SSLError) { OpenSSL::SSL::SSLSocket.new(sock, client_ctx).connect } + + assert server_thread.join(5), "server accept blocked (version: #{version})" + assert_kind_of OpenSSL::SSL::SSLError, server_thread.value + ensure + sock&.close rescue nil # unblocks a still waiting server + server_thread&.join rescue nil + ssl_server&.close rescue nil + tcp_server&.close rescue nil + end + end + end + def test_unexpected_eof_is_not_accepted_by_default server_ctx = OpenSSL::SSL::SSLContext.new server_ctx.cert = @svr_cert From 20a73fe784ea5513b6b05fb83ac1f427d3baaa2c Mon Sep 17 00:00:00 2001 From: kares Date: Tue, 3 Nov 2026 20:13:51 +0100 Subject: [PATCH 2/3] [compat] report cert verify error on failed handshake (#399) rejected peer certificate raised a bare "certificate verify failed" on SunJSSE and a synthetic "Socket closed" on BCJSSE - in both cases `SSLSocket#verify_result` stayed 0; while MRI reports the verify error in the message and in verify_result trust manager now throws VerifyCertificateException; which carries the verify error, connect/accept find it in the cause chain of the JSSE exception and set the message and verify_result. BCJSSE holds trust failure back until the next wrap(); when unwrap hits end of stream while the engine still asks for NEED_WRAP, doHandshake now runs that wrap, so real failure is raised instead of "Socket closed". --- .../org/jruby/ext/openssl/SSLContext.java | 27 +++++++++++++---- .../java/org/jruby/ext/openssl/SSLSocket.java | 30 +++++++++++++++---- test/ssl/test_ssl.rb | 20 +++++++++++++ 3 files changed, 66 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/jruby/ext/openssl/SSLContext.java b/src/main/java/org/jruby/ext/openssl/SSLContext.java index 65033657..a9e34625 100644 --- a/src/main/java/org/jruby/ext/openssl/SSLContext.java +++ b/src/main/java/org/jruby/ext/openssl/SSLContext.java @@ -1494,12 +1494,11 @@ private void verifyCertChain(final StoreContext storeContext, final boolean rais ok = storeContext.verifyCertificate(); } catch (Exception e) { - internalContext.setLastVerifyResult(storeContext.getError()); - if ( storeContext.getError() == X509Utils.V_OK ) { - internalContext.setLastVerifyResult(X509Utils.V_ERR_CERT_REJECTED); - } + int error = storeContext.getError(); + if ( error == X509Utils.V_OK ) error = X509Utils.V_ERR_CERT_REJECTED; + internalContext.setLastVerifyResult(error); if ( raiseOnFailure ) { - throw new CertificateException("certificate verify failed", e); + throw new VerifyCertificateException(error, e); } LOG.debugStack("verifyCertChain (failure retained in verify_result)", e); return; @@ -1507,9 +1506,25 @@ private void verifyCertChain(final StoreContext storeContext, final boolean rais internalContext.setLastVerifyResult(storeContext.getError()); if ( ok == 0 && raiseOnFailure ) { - throw new CertificateException("certificate verify failed"); + throw new VerifyCertificateException(storeContext.getError(), null); } } } + // trust manager rejection carrying (per handshake) verify error + static final class VerifyCertificateException extends CertificateException { + final int verifyError; + + VerifyCertificateException(final int verifyError, final Throwable cause) { + super(verifyErrorMessage(verifyError), cause); + this.verifyError = verifyError; + } + + private static String verifyErrorMessage(final int verifyError) { + return verifyError == X509Utils.V_OK ? + "certificate verify failed" : + "certificate verify failed (" + X509Utils.verifyCertificateErrorString(verifyError) + ")"; + } + } + }// SSLContext diff --git a/src/main/java/org/jruby/ext/openssl/SSLSocket.java b/src/main/java/org/jruby/ext/openssl/SSLSocket.java index d1840306..e678546b 100644 --- a/src/main/java/org/jruby/ext/openssl/SSLSocket.java +++ b/src/main/java/org/jruby/ext/openssl/SSLSocket.java @@ -148,12 +148,24 @@ private static RaiseException newSSLErrorFromHandshake(Ruby runtime, SSLHandshak // JSSE may abort a handshake with a plain runtime exception (e.g. no protocol left enabled) private static RaiseException newSSLErrorFromException(Ruby runtime, Exception exception) { LOG.debugStack(runtime, null, exception); - if ( "Could not generate DH keypair".equals( exception.getMessage() ) ) { + if ("Could not generate DH keypair".equals(exception.getMessage())) { return SSL.handleCouldNotGenerateDHKeyPairError(runtime, exception); } return SSL.newSSLError(runtime, exception); } + // JSSE wraps a trust manager rejection (e.g. in a fatal alert exception) - report it as MRI + private RaiseException tryExtractVerifyError(Ruby runtime, Exception exception) { + Throwable cause = exception; + for (int i = 0; cause != null && i < 10; i++, cause = cause.getCause()) { + if (cause instanceof SSLContext.VerifyCertificateException) { + verifyResult = ((SSLContext.VerifyCertificateException) cause).verifyError; + return SSL.newSSLError(runtime, (SSLContext.VerifyCertificateException) cause); + } + } + return null; + } + private static CallSite callSite(final CallSite[] sites, final CallSiteIndex index) { return sites[ index.ordinal() ]; } @@ -343,12 +355,14 @@ private IRubyObject connectImpl(final ThreadContext context, final boolean block LOG.debug(context.runtime, "connectImpl", e); // unlike server side, client should close outbound channel even if we have remaining data to be sent forceClose(); - throw newSSLErrorFromHandshake(context.runtime, e); + final RaiseException verifyError = tryExtractVerifyError(context.runtime, e); + throw verifyError != null ? verifyError : newSSLErrorFromHandshake(context.runtime, e); } catch (IOException e) { LOG.debug(context.runtime, "connectImpl", e); forceClose(); - throw newSSLError(context.runtime, e); + final RaiseException verifyError = tryExtractVerifyError(context.runtime, e); + throw verifyError != null ? verifyError : newSSLError(context.runtime, e); } catch (NotYetConnectedException e) { throw newErrnoEPIPEError(context.runtime, "SSL_connect"); @@ -444,11 +458,13 @@ public boolean matches(SNIServerName serverName) { else { LOG.debugStack(context.runtime, null, e); } - throw newSSLErrorFromHandshake(context.runtime, e); + final RaiseException verifyError = tryExtractVerifyError(context.runtime, e); + throw verifyError != null ? verifyError : newSSLErrorFromHandshake(context.runtime, e); } catch (IOException e) { LOG.debugStack(context.runtime, null, e); - throw newSSLError(context.runtime, e); + final RaiseException verifyError = tryExtractVerifyError(context.runtime, e); + throw verifyError != null ? verifyError : newSSLError(context.runtime, e); } catch (RaiseException e) { throw e; @@ -697,6 +713,10 @@ private IRubyObject doHandshake(final boolean blocking, final boolean exception) int unwrapResult = readAndUnwrap(blocking, exception); if (isWouldBlockResult(unwrapResult)) return wouldBlockSymbol(unwrapResult); if (unwrapResult == -1 && handshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED) { + if (handshakeStatus == SSLEngineResult.HandshakeStatus.NEED_WRAP) { + doWrap(blocking); // BCJSSE raises the actual (deferred) handshake failure here + flushData(blocking); + } throw new SSLHandshakeException("Socket closed"); } // during initialHandshake, calling readAndUnwrap that results UNDERFLOW does not mean writable. diff --git a/test/ssl/test_ssl.rb b/test/ssl/test_ssl.rb index be93e51f..da8743b1 100644 --- a/test/ssl/test_ssl.rb +++ b/test/ssl/test_ssl.rb @@ -493,6 +493,26 @@ def test_client_verify_failure_sends_alert_to_server end end + # client rejecting server cert reports the verify error (message and verify_result) like MRI (#399) + def test_client_verify_failure_error_and_verify_result + [nil, :TLSv1_2, :TLSv1_3].each do |version| + start_server(OpenSSL::SSL::VERIFY_NONE, true, ignore_listener_error: true) do |_server, port| + ctx = version ? OpenSSL::SSL::SSLContext.new(version) : OpenSSL::SSL::SSLContext.new + ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER + ctx.cert_store = OpenSSL::X509::Store.new # trusts nothing + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock, ctx) + assert_raise_with_message(OpenSSL::SSL::SSLError, /certificate verify failed \(self.signed certificate in certificate chain\)/) do + ssl.connect + end + assert_equal OpenSSL::X509::V_ERR_SELF_SIGNED_CERT_IN_CHAIN, ssl.verify_result, "version: #{version.inspect}" + ensure + ssl&.close rescue nil + sock&.close rescue nil + end + end + end + def test_unexpected_eof_is_not_accepted_by_default server_ctx = OpenSSL::SSL::SSLContext.new server_ctx.cert = @svr_cert From ffccd116585818224037f5fbc9571e16d9ad1f6c Mon Sep 17 00:00:00 2001 From: kares Date: Tue, 3 Nov 2026 20:12:39 +0100 Subject: [PATCH 3/3] [fix] avoid flushing stale bytes after failed wrap --- src/main/java/org/jruby/ext/openssl/SSLSocket.java | 9 +++++++-- test/ssl/test_ssl.rb | 10 ++++++---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jruby/ext/openssl/SSLSocket.java b/src/main/java/org/jruby/ext/openssl/SSLSocket.java index e678546b..96a71329 100644 --- a/src/main/java/org/jruby/ext/openssl/SSLSocket.java +++ b/src/main/java/org/jruby/ext/openssl/SSLSocket.java @@ -754,8 +754,13 @@ private void doWrap(final boolean blocking) throws IOException { // clear() would silently discard them, corrupting TLS record stream while ( netWriteData.hasRemaining() && flushData(blocking) ) { /* loop */ } netWriteData.clear(); - SSLEngineResult result = engine.wrap(EMPTY_DATA.duplicate(), netWriteData); - netWriteData.flip(); + final SSLEngineResult result; + try { + result = engine.wrap(EMPTY_DATA.duplicate(), netWriteData); + } + finally { // NOTE: failed wrap must not leave (stale) buffer contents to be flushed + netWriteData.flip(); + } handshakeStatus = result.getHandshakeStatus(); status = result.getStatus(); if (handshakeStatus == SSLEngineResult.HandshakeStatus.NEED_TASK && status == SSLEngineResult.Status.OK) { diff --git a/test/ssl/test_ssl.rb b/test/ssl/test_ssl.rb index da8743b1..8959bf98 100644 --- a/test/ssl/test_ssl.rb +++ b/test/ssl/test_ssl.rb @@ -467,23 +467,25 @@ def test_server_verify_peer_allows_missing_client_cert # client rejecting server cert must send an alert, even with TCP socket kept open def test_client_verify_failure_sends_alert_to_server - [:TLSv1_2, :TLSv1_3].each do |version| + [nil, :TLSv1_2, :TLSv1_3].each do |version| begin - server_ctx = OpenSSL::SSL::SSLContext.new(version) # NOTE: BC-JSSE isn't used for plain "SSL" + new_ctx = -> { version ? OpenSSL::SSL::SSLContext.new(version) : OpenSSL::SSL::SSLContext.new } + server_ctx = new_ctx.call # NOTE: BC-JSSE isn't used for plain "SSL" server_ctx.cert = @svr_cert server_ctx.key = @svr_key tcp_server = TCPServer.new("127.0.0.1", 0) ssl_server = OpenSSL::SSL::SSLServer.new(tcp_server, server_ctx) server_thread = Thread.new { ssl_server.accept rescue $! } - client_ctx = OpenSSL::SSL::SSLContext.new(version) + client_ctx = new_ctx.call client_ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER client_ctx.cert_store = OpenSSL::X509::Store.new # trusts nothing sock = TCPSocket.new("127.0.0.1", tcp_server.addr[1]) assert_raise(OpenSSL::SSL::SSLError) { OpenSSL::SSL::SSLSocket.new(sock, client_ctx).connect } - assert server_thread.join(5), "server accept blocked (version: #{version})" + assert server_thread.join(5), "server accept blocked (version: #{version.inspect})" assert_kind_of OpenSSL::SSL::SSLError, server_thread.value + assert_match(/certificate_unknown|unknown ca/, server_thread.value.message) # alert, not garbage ensure sock&.close rescue nil # unblocks a still waiting server server_thread&.join rescue nil