From 505860ee9c6b3e231f936fe860697183f15fa757 Mon Sep 17 00:00:00 2001 From: Ben Wagner Date: Tue, 1 Apr 2025 10:44:43 -0500 Subject: [PATCH 1/2] Added CCC and close notify capabilities --- .../org/apache/mina/filter/ssl/SslFilter.java | 178 ++++++++++++------ .../apache/mina/filter/ssl/SslHandler.java | 121 +++++++++--- .../apache/mina/filter/ssl/SslFilterTest.java | 33 +++- 3 files changed, 248 insertions(+), 84 deletions(-) diff --git a/mina-core/src/main/java/org/apache/mina/filter/ssl/SslFilter.java b/mina-core/src/main/java/org/apache/mina/filter/ssl/SslFilter.java index 8b25d8109..9ff6129d2 100644 --- a/mina-core/src/main/java/org/apache/mina/filter/ssl/SslFilter.java +++ b/mina-core/src/main/java/org/apache/mina/filter/ssl/SslFilter.java @@ -137,7 +137,7 @@ public class SslFilter extends IoFilterAdapter { /** An attribute containing the next filter */ private static final AttributeKey NEXT_FILTER = new AttributeKey(SslFilter.class, "nextFilter"); - private static final AttributeKey SSL_HANDLER = new AttributeKey(SslFilter.class, "handler"); + static final AttributeKey SSL_HANDLER = new AttributeKey(SslFilter.class, "handler"); /** The SslContext used */ /* No qualifier */final SSLContext sslContext; @@ -335,8 +335,60 @@ public WriteFuture stopSsl(IoSession session) throws SSLException { return future; } + /** + * Marks the handler as CCC being enabled + * + * @param session + * the {@link IoSession} to initiate TLS closure + * + * @throws IllegalArgumentException + * if this filter is not managing the specified session + */ + public void enableCCC(IoSession session) { + SslHandler handler = getSslSessionHandler(session); + handler.setCCCEnabled(true); + } + + /** + * Stops the SSL filter handling of messages, but does not send a + * CLOSE_NOTIFY to the client. This basically disables the filter and + * proceeds with passing through the messages assumed to be plain text. This + * is added to support clients that do not wish to honor the close_notify + * negotiation. + * + * @param session + * the {@link IoSession} to initiate TLS closure + * @throws IllegalArgumentException + * if this filter is not managing the specified session + */ + public void stopSslWithoutCloseNotify(IoSession session) { + SslHandler handler = getSslSessionHandler(session); + handler.setDisabled(true); + } + + /** + * Similar to the isSslStarted() method, however this will additionally + * check to see if the handler is disabled, which can happen when ssl is + * disabled without sending a CLOSE_NOTIFY + * + * @param session + * the SSL session to check + * @return whether or not ssl is currently active + */ + public boolean isSslActive(IoSession session) { + SslHandler handler = (SslHandler) session.getAttribute(SSL_HANDLER); + + if (handler == null) { + return false; + } + + synchronized (handler) { + return !handler.isOutboundDone() && !handler.isDisabled(); + } + } + /** - * @return true if the engine is set to use client mode + * @return true if the engine is set to use client mode * when handshaking. */ public boolean isUseClientMode() { @@ -520,62 +572,63 @@ public void messageReceived(NextFilter nextFilter, IoSession session, Object mes SslHandler sslHandler = getSslSessionHandler(session); AtomicBoolean canPushMessage = new AtomicBoolean( false ); - // The SslHandler instance is *guaranteed* to nit be null here + // The SslHandler instance is *guaranteed* to not be null here + synchronized (sslHandler.getCccLock()) { + synchronized (sslHandler) { + if ((sslHandler.isOutboundDone() && sslHandler.isInboundDone()) || sslHandler.isDisabled()) { + // We aren't handshaking here. Let's push the message to the next filter - synchronized (sslHandler) { - if (sslHandler.isOutboundDone() && sslHandler.isInboundDone()) { - // We aren't handshaking here. Let's push the message to the next filter - - // Note: we can push the message to the queue immediately, - // but don't do so in the synchronized block. We use a protected - // flag to do so. - canPushMessage.set( true ); - } else { - canPushMessage.set( false ); - IoBuffer buf = (IoBuffer) message; - - try { - if (sslHandler.isOutboundDone()) { - sslHandler.destroy(); - throw new SSLException("Outbound done"); - } - - // forward read encrypted data to SSL handler - sslHandler.messageReceived(nextFilter, buf.buf()); - - // Handle data to be forwarded to application or written to net - handleSslData(nextFilter, sslHandler); - - if (sslHandler.isInboundDone()) { - if (sslHandler.isOutboundDone()) { + // Note: we can push the message to the queue immediately, + // but don't do so in the synchronized block. We use a protected + // flag to do so. + canPushMessage.set( true ); + } else { + canPushMessage.set( false ); + IoBuffer buf = (IoBuffer) message; + + try { + if (sslHandler.isOutboundDone() && !sslHandler.isCCCEnabled()) { sslHandler.destroy(); - } else { - initiateClosure(nextFilter, session); + throw new SSLException("Outbound done"); + } + + // forward read encrypted data to SSL handler + sslHandler.messageReceived(nextFilter, buf.buf()); + + // Handle data to be forwarded to application or written to net + handleSslData(nextFilter, sslHandler); + + if (sslHandler.isInboundDone()) { + if (sslHandler.isOutboundDone()) { + sslHandler.destroy(); + } else { + initiateClosure(nextFilter, session); + } + + if (buf.hasRemaining()) { + // Forward the data received after closure. + sslHandler.scheduleMessageReceived(nextFilter, buf); + } } - - if (buf.hasRemaining()) { - // Forward the data received after closure. - sslHandler.scheduleMessageReceived(nextFilter, buf); + } catch (SSLException ssle) { + if (!sslHandler.isHandshakeComplete()) { + SSLException newSsle = new SSLHandshakeException("SSL handshake failed."); + newSsle.initCause(ssle); + ssle = newSsle; + + // Close the session immediately, the handshake has failed + session.closeNow(); + } else { + // Free the SSL Handler buffers + sslHandler.release(); } + + throw ssle; } - } catch (SSLException ssle) { - if (!sslHandler.isHandshakeComplete()) { - SSLException newSsle = new SSLHandshakeException("SSL handshake failed."); - newSsle.initCause(ssle); - ssle = newSsle; - - // Close the session immediately, the handshake has failed - session.closeNow(); - } else { - // Free the SSL Handler buffers - sslHandler.release(); - } - - throw ssle; } } } - + if (canPushMessage.get()) { nextFilter.messageReceived(session, message); } else { @@ -672,6 +725,9 @@ else if (session.containsAttribute(DISABLE_ENCRYPTION_ONCE)) { // Remove the marker attribute because it is temporary. session.removeAttribute(DISABLE_ENCRYPTION_ONCE); sslHandler.scheduleFilterWrite(nextFilter, writeRequest); + } + else if(sslHandler.isDisabled()) { + sslHandler.scheduleFilterWrite(nextFilter,writeRequest); } else { // Otherwise, encrypt the buffer. IoBuffer buf = (IoBuffer) writeRequest.getMessage(); @@ -722,12 +778,7 @@ public void filterClose(final NextFilter nextFilter, final IoSession session) th synchronized (sslHandler) { if (isSslStarted(session)) { future = initiateClosure(nextFilter, session); - future.addListener(new IoFutureListener() { - @Override - public void operationComplete(IoFuture future) { - nextFilter.filterClose(session); - } - }); + future.addListener(future1 -> nextFilter.filterClose(session)); } sslHandler.flushFilterWrite(); } @@ -890,7 +941,7 @@ public String toString() { private final IoBuffer encryptedMessage; // The original message - private WriteRequest parentRequest; + private final WriteRequest parentRequest; /** * Create a new instance of an EncryptedWriteRequest @@ -926,4 +977,17 @@ public WriteFuture getFuture() { return parentRequest.getFuture(); } } + + /** + * Returns the ccc lock from the underlying ssl handler for synchronizing + * ccc logic when proper ssl termination is bypassed + * + * @param session + * the iosession + * @return the object to lock on + */ + public Object getCccLock(IoSession session) { + SslHandler handler = getSslSessionHandler(session); + return handler.getCccLock(); + } } diff --git a/mina-core/src/main/java/org/apache/mina/filter/ssl/SslHandler.java b/mina-core/src/main/java/org/apache/mina/filter/ssl/SslHandler.java index aaae15066..713147cd1 100644 --- a/mina-core/src/main/java/org/apache/mina/filter/ssl/SslHandler.java +++ b/mina-core/src/main/java/org/apache/mina/filter/ssl/SslHandler.java @@ -118,15 +118,31 @@ class SslHandler { * for data being produced during the handshake). */ private boolean writingEncryptedData; + /** + * Whether or not this ssl handler is disabled + */ + private boolean disabled = false; + + /** + * The lock used for CCC (clear command channel) negotiation + */ + private final Object cccLock; + + /** + * Whether or not CCC is enabled + */ + private boolean cccEnabled = false; + /** * Create a new SSL Handler, and initialize it. * - * @param sslContext - * @throws SSLException + * @param sslFilter the {@code SslFilter} + * @param session the {@code IoSession} implementation */ /* no qualifier */SslHandler(SslFilter sslFilter, IoSession session) { this.sslFilter = sslFilter; this.session = session; + this.cccLock = new Object(); } /** @@ -196,6 +212,8 @@ class SslHandler { // set the flags accordingly firstSSLNegociation = true; handshakeComplete = false; + disabled = false; + cccEnabled = false; if (LOGGER.isDebugEnabled()) { LOGGER.debug("{} SSL Handler Initialization done.", sslFilter.getSessionInfo(session)); @@ -509,6 +527,53 @@ class SslHandler { return true; } + /** + * Sets whether or not this handler has CCC enabled + * + * @param cccEnabled + * whether or not this handler is has CCC enabled + */ + void setCCCEnabled(boolean cccEnabled) { + this.cccEnabled = cccEnabled; + } + + /** + * Returns whether or not this handler has CCC enabled + * + * @return whether or not this handler has CCC enabled + */ + boolean isCCCEnabled() { + return cccEnabled; + } + + /** + * Sets whether or not this handler is disabled + * + * @param disabled + * whether or not this handler is disabled + */ + void setDisabled(boolean disabled) { + this.disabled = disabled; + } + + /** + * Returns whether or not this handler is disabled + * + * @return whether or not this handler is disabled + */ + boolean isDisabled() { + return disabled; + } + + /** + * Returns the CCC lock + * + * @return the CCC lock object + */ + Object getCccLock() { + return cccLock; + } + /** * @param res * @throws SSLException @@ -530,28 +595,31 @@ private void checkStatus(SSLEngineResult res) throws SSLException { throw new SSLException("SSLEngine error during decrypt: " + status + " inNetBuffer: " + inNetBuffer + "appBuffer: " + appBuffer); case CLOSED: - Exception exception =new RuntimeIoException("SSL/TLS close_notify received"); - - // Empty the Ssl queue - for (IoFilterEvent event:filterWriteEventQueue) { - EncryptedWriteRequest writeRequest = (EncryptedWriteRequest)event.getParameter(); - WriteFuture writeFuture = writeRequest.getParentRequest().getFuture(); - writeFuture.setException(exception); - writeFuture.notifyAll(); - } - - // Empty the session queue - WriteRequestQueue queue = session.getWriteRequestQueue(); - WriteRequest request = null; - - while ((request = queue.poll(session)) != null) { - WriteFuture writeFuture = request.getFuture(); - writeFuture.setException(exception); - writeFuture.notifyAll(); - } - - // We *must* shutdown session - session.closeNow(); + // Adding this if check so that we don't close the connection completely if we are using CCC + if (!cccEnabled) { + Exception exception =new RuntimeIoException("SSL/TLS close_notify received"); + + // Empty the Ssl queue + for (IoFilterEvent event:filterWriteEventQueue) { + EncryptedWriteRequest writeRequest = (EncryptedWriteRequest)event.getParameter(); + WriteFuture writeFuture = writeRequest.getParentRequest().getFuture(); + writeFuture.setException(exception); + writeFuture.notifyAll(); + } + + // Empty the session queue + WriteRequestQueue queue = session.getWriteRequestQueue(); + WriteRequest request = null; + + while ((request = queue.poll(session)) != null) { + WriteFuture writeFuture = request.getFuture(); + writeFuture.setException(exception); + writeFuture.notifyAll(); + } + + // We *must* shutdown session + session.closeNow(); + } break; default: break; @@ -588,7 +656,10 @@ private void checkStatus(SSLEngineResult res) throws SSLException { } if (inNetBuffer != null && inNetBuffer.hasRemaining()) { - LOGGER.debug("pos: " + inNetBuffer.position() + ", lim: " + inNetBuffer.limit() + ", cap: " + inNetBuffer.capacity()); + LOGGER.debug("pos: {}, lim: {}, cap: {}", + inNetBuffer.position(), + inNetBuffer.limit(), + inNetBuffer.capacity()); inNetBuffer.flip(); SSLEngineResult res = unwrap(); diff --git a/mina-core/src/test/java/org/apache/mina/filter/ssl/SslFilterTest.java b/mina-core/src/test/java/org/apache/mina/filter/ssl/SslFilterTest.java index a5aad0176..82416a449 100644 --- a/mina-core/src/test/java/org/apache/mina/filter/ssl/SslFilterTest.java +++ b/mina-core/src/test/java/org/apache/mina/filter/ssl/SslFilterTest.java @@ -21,13 +21,17 @@ package org.apache.mina.filter.ssl; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import javax.net.ssl.SSLContext; import javax.net.ssl.SSLException; import org.apache.mina.core.filterchain.IoFilter.NextFilter; @@ -74,14 +78,14 @@ public String toString() { }; /** - * A test for DIRMINA-1019 + * Tests for the {@code SslFilter} * @author Apache MINA Project */ public class SslFilterTest { SslHandler test_class; @Before - public void init() throws SSLException { + public void init() { test_class = new SslHandler(null, new DummySession()); } @@ -139,4 +143,29 @@ public void filterWrite(IoSession session, WriteRequest writeRequest) { } assertEquals(1, message_received_messages.size()); assertEquals(1, filter_write_requests.size()); } + + @Test + public void testIsSslActiveScenarios() throws NoSuchAlgorithmException, SSLException { + final SslFilter filter = new SslFilter(SSLContext.getDefault()); + final IoSession dummySession = new DummySession(); + + // Scenario 1: No SSL handler attribute + assertFalse(filter.isSslActive(dummySession)); + + // Scenario 2: SSL handler present but disabled + final SslHandler disabledHandler = new SslHandler(filter, dummySession); + dummySession.setAttribute(SslFilter.SSL_HANDLER, disabledHandler); + disabledHandler.setDisabled(true); + assertFalse(filter.isSslActive(dummySession)); + + // Scenario 3: SSL handler present, enabled, and initialized + final SslHandler enabledHandler = new SslHandler(filter, dummySession); + enabledHandler.init(); + dummySession.setAttribute(SslFilter.SSL_HANDLER, enabledHandler); + assertTrue(filter.isSslActive(dummySession)); + + // Scenario 4: SSL handler removed + dummySession.removeAttribute(SslFilter.SSL_HANDLER); + assertFalse(filter.isSslActive(dummySession)); + } } From 2aba909c4ae6484fc76f23ee1f29fc87d1c7b32f Mon Sep 17 00:00:00 2001 From: Ben Wagner Date: Wed, 2 Apr 2025 14:44:54 -0500 Subject: [PATCH 2/2] Reverting Javadoc that was accidentally changed --- .../src/main/java/org/apache/mina/filter/ssl/SslFilter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mina-core/src/main/java/org/apache/mina/filter/ssl/SslFilter.java b/mina-core/src/main/java/org/apache/mina/filter/ssl/SslFilter.java index 9ff6129d2..e72c8efcd 100644 --- a/mina-core/src/main/java/org/apache/mina/filter/ssl/SslFilter.java +++ b/mina-core/src/main/java/org/apache/mina/filter/ssl/SslFilter.java @@ -388,7 +388,7 @@ public boolean isSslActive(IoSession session) { } /** - * @return true if the engine is set to use client mode + * @return true if the engine is set to use client mode * when handshaking. */ public boolean isUseClientMode() {