align failed handshake behavior

src/main/java/org/jruby/ext/openssl/SSLContext.java

@@ -1494,22 +1494,37 @@ 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;
             }
 
             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

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.
@@ -734,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) {
@@ -975,7 +1000,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;

test/ssl/test_ssl.rb

@@ -465,6 +465,56 @@ 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
+    [nil, :TLSv1_2, :TLSv1_3].each do |version|
+      begin
+        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 = 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.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
+        ssl_server&.close rescue nil
+        tcp_server&.close rescue nil
+      end
+    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