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 6b4e0821..96a71329 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. @@ -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; diff --git a/test/ssl/test_ssl.rb b/test/ssl/test_ssl.rb index aa7317a7..8959bf98 100644 --- a/test/ssl/test_ssl.rb +++ b/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