From 505860ee9c6b3e231f936fe860697183f15fa757 Mon Sep 17 00:00:00 2001 From: Ben Wagner Date: Tue, 1 Apr 2025 10:44:43 -0500 Subject: [PATCH 1/3] 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/3] 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() { From a21445867c89f41f89be8384296620ca94314e74 Mon Sep 17 00:00:00 2001 From: "maarten.bosteels" Date: Fri, 25 Sep 2026 09:20:24 +0200 Subject: [PATCH 3/3] Added CCC and close notify capabilities to SslFilter --- .../apache/mina/filter/ssl/SslFilterTest.java | 264 +++++++++++++++++- .../mina/filter/ssl/SslHandlerTest.java | 90 ++++++ .../org/apache/mina/filter/ssl/SslTest.java | 4 + 3 files changed, 355 insertions(+), 3 deletions(-) create mode 100644 mina-core/src/test/java/org/apache/mina/filter/ssl/SslHandlerTest.java 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 82416a449..441dbb1e1 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 @@ -22,6 +22,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import java.security.NoSuchAlgorithmException; @@ -34,6 +35,7 @@ import javax.net.ssl.SSLContext; import javax.net.ssl.SSLException; +import org.apache.mina.core.buffer.IoBuffer; import org.apache.mina.core.filterchain.IoFilter.NextFilter; import org.apache.mina.core.session.DummySession; import org.apache.mina.core.session.IdleStatus; @@ -83,18 +85,20 @@ public String toString() { */ public class SslFilterTest { SslHandler test_class; - + @Before public void init() { test_class = new SslHandler(null, new DummySession()); + test_class.setCCCEnabled(true); + test_class.setDisabled(false); } - + @Test public void testFlushRaceCondition() { final ExecutorService executor = Executors.newFixedThreadPool(1); final List message_received_messages = new ArrayList<>(); final List filter_write_requests = new ArrayList<>(); - + final AbstractNextFilter write_filter = new AbstractNextFilter() { @Override @@ -168,4 +172,258 @@ public void testIsSslActiveScenarios() throws NoSuchAlgorithmException, SSLExcep dummySession.removeAttribute(SslFilter.SSL_HANDLER); assertFalse(filter.isSslActive(dummySession)); } + + // ------------------------------------------------------------------------ + // Tests for the SslFilter CCC / close-notify public API + // ------------------------------------------------------------------------ + + /** + * {@link SslFilter#enableCCC(IoSession)} must mark the underlying handler + * as CCC enabled. + */ + @Test + public void testEnableCCC() throws NoSuchAlgorithmException { + final SslFilter filter = new SslFilter(SSLContext.getDefault()); + final IoSession session = new DummySession(); + final SslHandler handler = new SslHandler(filter, session); + session.setAttribute(SslFilter.SSL_HANDLER, handler); + + assertFalse(handler.isCCCEnabled()); + + filter.enableCCC(session); + + assertTrue(handler.isCCCEnabled()); + } + + /** + * {@link SslFilter#stopSslWithoutCloseNotify(IoSession)} must disable the + * underlying handler, which in turn makes SSL inactive. + */ + @Test + public void testStopSslWithoutCloseNotify() throws NoSuchAlgorithmException, SSLException { + final SslFilter filter = new SslFilter(SSLContext.getDefault()); + final IoSession session = new DummySession(); + final SslHandler handler = new SslHandler(filter, session); + handler.init(); + session.setAttribute(SslFilter.SSL_HANDLER, handler); + + assertTrue("SSL should be active before it is stopped", filter.isSslActive(session)); + + filter.stopSslWithoutCloseNotify(session); + + assertTrue("The handler must be disabled", handler.isDisabled()); + assertFalse("SSL must no longer be active once disabled", filter.isSslActive(session)); + } + + /** + * {@link SslFilter#getCccLock(IoSession)} must return the exact same lock + * object owned by the session's handler. + */ + @Test + public void testGetCccLockReturnsHandlerLock() throws NoSuchAlgorithmException { + final SslFilter filter = new SslFilter(SSLContext.getDefault()); + final IoSession session = new DummySession(); + final SslHandler handler = new SslHandler(filter, session); + session.setAttribute(SslFilter.SSL_HANDLER, handler); + + assertSame(handler.getCccLock(), filter.getCccLock(session)); + } + + /** + * The CCC / close-notify API methods rely on + * {@code getSslSessionHandler(IoSession)}, which must throw an + * {@link IllegalStateException} when there is no handler attached to the + * session. + */ + @Test(expected = IllegalStateException.class) + public void testEnableCCCWithoutHandlerThrows() throws NoSuchAlgorithmException { + final SslFilter filter = new SslFilter(SSLContext.getDefault()); + + filter.enableCCC(new DummySession()); + } + + /** + * When the session's handler belongs to a different filter, the CCC API + * must reject the call with an {@link IllegalArgumentException}. + */ + @Test(expected = IllegalArgumentException.class) + public void testGetCccLockWithForeignHandlerThrows() throws NoSuchAlgorithmException { + final SslFilter filter = new SslFilter(SSLContext.getDefault()); + final SslFilter otherFilter = new SslFilter(SSLContext.getDefault()); + final IoSession session = new DummySession(); + + // The handler is managed by 'otherFilter', not 'filter' + final SslHandler foreignHandler = new SslHandler(otherFilter, session); + session.setAttribute(SslFilter.SSL_HANDLER, foreignHandler); + + filter.getCccLock(session); + } + + /** + * The {@code checkStatus} handling of a {@code close_notify} must not close + * the session while CCC is enabled. We assert the behavioral contract via + * the handler flags exposed by the commit. + */ + @Test + public void testCccEnabledKeepsHandlerActive() throws NoSuchAlgorithmException, SSLException { + final SslFilter filter = new SslFilter(SSLContext.getDefault()); + final IoSession session = new DummySession(); + final SslHandler handler = new SslHandler(filter, session); + handler.init(); + session.setAttribute(SslFilter.SSL_HANDLER, handler); + + filter.enableCCC(session); + + assertTrue("CCC enabled handler is still considered active", filter.isSslActive(session)); + assertTrue(handler.isCCCEnabled()); + assertFalse(handler.isDisabled()); + } + + // ------------------------------------------------------------------------ + // Regression tests: messageReceived / filterWrite behaviour must be + // unchanged when CCC is NOT enabled (and the handler is not disabled). + // ------------------------------------------------------------------------ + + /** + * Builds a {@link NextFilter} that records every {@code filterWrite} request + * and every {@code messageReceived} message it is handed. + */ + private static NextFilter recordingNextFilter(final List received, final List written) { + return new AbstractNextFilter() { + @Override + public void messageReceived(IoSession session, Object message) { + received.add(message); + } + + @Override + public void filterWrite(IoSession session, WriteRequest writeRequest) { + written.add(writeRequest); + } + }; + } + + /** + * When SSL has not been started yet (no {@code SSLEngine} created) and CCC + * is not enabled, {@link SslFilter#filterWrite} must forward the plaintext + * write request straight to the next filter, exactly as before the commit. + */ + @Test + public void testFilterWriteForwardsWhenSslNotStarted() throws NoSuchAlgorithmException, SSLException { + final SslFilter filter = new SslFilter(SSLContext.getDefault()); + final IoSession session = new DummySession(); + // Not initialized => SSL not started, not disabled, CCC not enabled + final SslHandler handler = new SslHandler(filter, session); + session.setAttribute(SslFilter.SSL_HANDLER, handler); + + assertFalse(handler.isCCCEnabled()); + assertFalse(handler.isDisabled()); + assertFalse(filter.isSslStarted(session)); + + final List received = new ArrayList<>(); + final List written = new ArrayList<>(); + final NextFilter next = recordingNextFilter(received, written); + + final WriteRequest request = new DefaultWriteRequest(IoBuffer.wrap(new byte[] { 1, 2, 3 })); + + filter.filterWrite(next, session, request); + + // Unchanged behaviour: the write is passed through untouched. + assertEquals(1, written.size()); + assertSame(request, written.get(0)); + } + + /** + * When {@link SslFilter#DISABLE_ENCRYPTION_ONCE} is set and CCC is not + * enabled, {@link SslFilter#filterWrite} must forward the request without + * encrypting it and clear the temporary marker attribute afterwards. This + * is the StartTLS bypass path that must remain unchanged. + */ + @Test + public void testFilterWriteBypassesWithDisableEncryptionOnce() throws NoSuchAlgorithmException, SSLException { + final SslFilter filter = new SslFilter(SSLContext.getDefault()); + final IoSession session = new DummySession(); + final SslHandler handler = new SslHandler(filter, session); + handler.init(); // SSL started + session.setAttribute(SslFilter.SSL_HANDLER, handler); + session.setAttribute(SslFilter.DISABLE_ENCRYPTION_ONCE, Boolean.TRUE); + + assertFalse(handler.isCCCEnabled()); + assertFalse(handler.isDisabled()); + + final List received = new ArrayList<>(); + final List written = new ArrayList<>(); + final NextFilter next = recordingNextFilter(received, written); + + final WriteRequest request = new DefaultWriteRequest(IoBuffer.wrap(new byte[] { 1, 2, 3 })); + + filter.filterWrite(next, session, request); + + // Unchanged behaviour: forwarded unencrypted and the marker is removed. + assertEquals(1, written.size()); + assertSame(request, written.get(0)); + assertFalse(session.containsAttribute(SslFilter.DISABLE_ENCRYPTION_ONCE)); + } + + /** + * While the handshake is still in progress and CCC is not enabled, + * {@link SslFilter#filterWrite} must queue the write as a pre-handshake + * request instead of forwarding it to the next filter. This deferral must + * remain unchanged by the CCC changes. + */ + @Test + public void testFilterWriteDefersDuringHandshake() throws NoSuchAlgorithmException, SSLException { + final SslFilter filter = new SslFilter(SSLContext.getDefault()); + final IoSession session = new DummySession(); + final SslHandler handler = new SslHandler(filter, session); + handler.init(); // SSL started, handshake not complete + + session.setAttribute(SslFilter.SSL_HANDLER, handler); + + assertFalse(handler.isHandshakeComplete()); + assertFalse(handler.isCCCEnabled()); + assertFalse(handler.isDisabled()); + + final List received = new ArrayList<>(); + final List written = new ArrayList<>(); + final NextFilter next = recordingNextFilter(received, written); + + final WriteRequest request = new DefaultWriteRequest(IoBuffer.wrap(new byte[] { 1, 2, 3 })); + + filter.filterWrite(next, session, request); + + // Unchanged behaviour: nothing is forwarded until the handshake completes. + assertTrue(written.isEmpty()); + } + + /** + * Once the engine is fully closed (inbound and outbound done) and CCC is not + * enabled, {@link SslFilter#messageReceived} must push the raw message to + * the next filter as-is, without attempting to decrypt it. This pass-through + * path must remain unchanged by the CCC changes. + */ + @Test + public void testMessageReceivedForwardsWhenSessionClosed() throws NoSuchAlgorithmException, SSLException { + final SslFilter filter = new SslFilter(SSLContext.getDefault()); + final IoSession session = new DummySession(); + // Not initialized => sslEngine null => both inbound and outbound "done" + final SslHandler handler = new SslHandler(filter, session); + session.setAttribute(SslFilter.SSL_HANDLER, handler); + + assertTrue(handler.isInboundDone()); + assertTrue(handler.isOutboundDone()); + assertFalse(handler.isCCCEnabled()); + assertFalse(handler.isDisabled()); + + final List received = new ArrayList<>(); + final List written = new ArrayList<>(); + final NextFilter next = recordingNextFilter(received, written); + + final IoBuffer message = IoBuffer.wrap(new byte[] { 1, 2, 3 }); + + filter.messageReceived(next, session, message); + + // Unchanged behaviour: the raw message is forwarded untouched. + assertEquals(1, received.size()); + assertSame(message, received.get(0)); + } } diff --git a/mina-core/src/test/java/org/apache/mina/filter/ssl/SslHandlerTest.java b/mina-core/src/test/java/org/apache/mina/filter/ssl/SslHandlerTest.java new file mode 100644 index 000000000..2cf1293ef --- /dev/null +++ b/mina-core/src/test/java/org/apache/mina/filter/ssl/SslHandlerTest.java @@ -0,0 +1,90 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ + +package org.apache.mina.filter.ssl; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertTrue; + +import java.security.NoSuchAlgorithmException; + +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLException; + +import org.apache.mina.core.session.DummySession; +import org.junit.Test; + +/** + * Tests for the {@code SslHandler}. These tests only exercise the + * {@link SslHandler} directly, without going through the {@link SslFilter}. + * + * @author Apache MINA Project + */ +public class SslHandlerTest { + + /** + * A freshly created {@link SslHandler} must have both the {@code disabled} + * and {@code cccEnabled} flags set to {@code false} by default. + */ + @Test + public void testSslHandlerFlagsDefaults() { + final SslHandler handler = new SslHandler(null, new DummySession()); + + assertFalse("A new handler should not be disabled", handler.isDisabled()); + assertFalse("A new handler should not have CCC enabled", handler.isCCCEnabled()); + } + + /** + * Each {@link SslHandler} owns a non-null CCC lock, and different handlers + * must not share the same lock instance. + */ + @Test + public void testSslHandlerCccLock() { + final SslHandler handler1 = new SslHandler(null, new DummySession()); + final SslHandler handler2 = new SslHandler(null, new DummySession()); + assertNotNull("The CCC lock must never be null", handler1.getCccLock()); + // Distinct handlers use distinct lock objects + assertNotSame(handler1.getCccLock(), handler2.getCccLock()); + } + + /** + * {@link SslHandler#init()} must reset the {@code disabled} and + * {@code cccEnabled} flags back to {@code false}. + */ + @Test + public void testInitResetsFlags() throws NoSuchAlgorithmException, SSLException { + // A SslFilter is only needed here to provide the SSLContext used by init() + final SslFilter filter = new SslFilter(SSLContext.getDefault()); + final SslHandler handler = new SslHandler(filter, new DummySession()); + + handler.setDisabled(true); + handler.setCCCEnabled(true); + + handler.init(); + + assertFalse("init() must reset the disabled flag", handler.isDisabled()); + assertFalse("init() must reset the cccEnabled flag", handler.isCCCEnabled()); + } +} + + + diff --git a/mina-core/src/test/java/org/apache/mina/filter/ssl/SslTest.java b/mina-core/src/test/java/org/apache/mina/filter/ssl/SslTest.java index e61bad654..22eb0899c 100644 --- a/mina-core/src/test/java/org/apache/mina/filter/ssl/SslTest.java +++ b/mina-core/src/test/java/org/apache/mina/filter/ssl/SslTest.java @@ -34,6 +34,7 @@ import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.TrustManagerFactory; +import org.apache.log4j.Level; import org.apache.mina.core.filterchain.DefaultIoFilterChainBuilder; import org.apache.mina.core.service.IoHandlerAdapter; import org.apache.mina.core.session.IoSession; @@ -176,6 +177,8 @@ private static SSLContext createSSLContext() throws IOException, GeneralSecurity @Test public void testSSL() throws Exception { + // It's easier when we see the logs + org.apache.log4j.Logger.getRootLogger().setLevel(Level.DEBUG); try { startServer(); @@ -202,6 +205,7 @@ public void run() { @Test public void unsecureClientTryToConnectoToSecureServer() throws Exception { + org.apache.log4j.Logger.getRootLogger().setLevel(Level.DEBUG); try { startServer(); // Start Server with SSLFilter