You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by tr...@apache.org on 2007/07/30 02:43:27 UTC

svn commit: r560837 - in /mina: branches/1.0/filter-ssl/src/main/java/org/apache/mina/filter/support/ branches/1.1/filter-ssl/src/main/java/org/apache/mina/filter/support/ trunk/core/src/main/java/org/apache/mina/filter/ssl/

Author: trustin
Date: Sun Jul 29 17:43:25 2007
New Revision: 560837

URL: http://svn.apache.org/viewvc?view=rev&rev=560837
Log:
Fixed infinite loop in SSLHandler.unwrap()

Modified:
    mina/branches/1.0/filter-ssl/src/main/java/org/apache/mina/filter/support/SSLHandler.java
    mina/branches/1.1/filter-ssl/src/main/java/org/apache/mina/filter/support/SSLHandler.java
    mina/trunk/core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java

Modified: mina/branches/1.0/filter-ssl/src/main/java/org/apache/mina/filter/support/SSLHandler.java
URL: http://svn.apache.org/viewvc/mina/branches/1.0/filter-ssl/src/main/java/org/apache/mina/filter/support/SSLHandler.java?view=diff&rev=560837&r1=560836&r2=560837
==============================================================================
--- mina/branches/1.0/filter-ssl/src/main/java/org/apache/mina/filter/support/SSLHandler.java (original)
+++ mina/branches/1.0/filter-ssl/src/main/java/org/apache/mina/filter/support/SSLHandler.java Sun Jul 29 17:43:25 2007
@@ -306,7 +306,7 @@
         if (!initialHandshakeComplete) {
             handshake(nextFilter);
         } else {
-            decrypt();
+            decrypt(nextFilter);
         }
 
         if (isInboundDone()) {
@@ -417,7 +417,7 @@
      *
      * @throws SSLException
      */
-    private void decrypt() throws SSLException {
+    private void decrypt(NextFilter nextFilter) throws SSLException {
 
         if (!initialHandshakeComplete) {
             throw new IllegalStateException();
@@ -431,7 +431,7 @@
             throw new IllegalStateException();
         }
 
-        unwrap();
+        unwrap(nextFilter);
     }
 
     /**
@@ -440,6 +440,14 @@
      */
     private SSLEngineResult.Status checkStatus(SSLEngineResult.Status status)
             throws SSLException {
+        /*
+         * The status may be:
+         * OK - Normal operation
+         * OVERFLOW - Should never happen since the application buffer is
+         *      sized to hold the maximum packet size.
+         * UNDERFLOW - Need to read more data from the socket. It's normal.
+         * CLOSED - The other peer closed the socket. Also normal.
+         */
         if (status != SSLEngineResult.Status.OK
                 && status != SSLEngineResult.Status.CLOSED
                 && status != SSLEngineResult.Status.BUFFER_UNDERFLOW) {
@@ -488,7 +496,7 @@
                     SessionLog.debug(session,
                             "  initialHandshakeStatus=NEED_UNWRAP");
                 }
-                SSLEngineResult.Status status = unwrapHandshake();
+                SSLEngineResult.Status status = unwrapHandshake(nextFilter);
                 if ((initialHandshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED && status == SSLEngineResult.Status.BUFFER_UNDERFLOW)
                         || isInboundDone()) {
                     // We need more data or the session is closed
@@ -587,7 +595,7 @@
         }
     }
 
-    private SSLEngineResult.Status unwrap() throws SSLException {
+    private SSLEngineResult.Status unwrap(NextFilter nextFilter) throws SSLException {
         if (SessionLog.isDebugEnabled(session)) {
             SessionLog.debug(session, " unwrap()");
         }
@@ -597,35 +605,25 @@
         // Prepare the net data for reading.
         inNetBuffer.flip();
 
-        SSLEngineResult res;
-        do {
-            if (SessionLog.isDebugEnabled(session)) {
-                SessionLog.debug(session, "   inNetBuffer: " + inNetBuffer);
-                SessionLog.debug(session, "   appBuffer: " + appBuffer);
-            }
-            res = sslEngine.unwrap(inNetBuffer, appBuffer);
-            if (SessionLog.isDebugEnabled(session)) {
-                SessionLog.debug(session, " Unwrap res:" + res);
-            }
-        } while (res.getStatus() == SSLEngineResult.Status.OK);
+        SSLEngineResult res = unwrap0();
 
         // prepare to be written again
         inNetBuffer.compact();
         // prepare app data to be read
         appBuffer.flip();
+        
+        checkStatus(res.getStatus());
+        
+        if (res.getHandshakeStatus() != SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING) {
+            // Renegotiation required.
+            SessionLog.debug(session, " Renegotiating...");
+            handshake(nextFilter);
+        }
 
-        /*
-         * The status may be:
-         * OK - Normal operation
-         * OVERFLOW - Should never happen since the application buffer is
-         *      sized to hold the maximum packet size.
-         * UNDERFLOW - Need to read more data from the socket. It's normal.
-         * CLOSED - The other peer closed the socket. Also normal.
-         */
-        return checkStatus(res.getStatus());
+        return res.getStatus();
     }
 
-    private SSLEngineResult.Status unwrapHandshake() throws SSLException {
+    private SSLEngineResult.Status unwrapHandshake(NextFilter nextFilter) throws SSLException {
         if (SessionLog.isDebugEnabled(session)) {
             SessionLog.debug(session, " unwrapHandshake()");
         }
@@ -635,20 +633,7 @@
         // Prepare the net data for reading.
         inNetBuffer.flip();
 
-        SSLEngineResult res;
-        do {
-            if (SessionLog.isDebugEnabled(session)) {
-                SessionLog.debug(session, "   inNetBuffer: " + inNetBuffer);
-                SessionLog.debug(session, "   appBuffer: " + appBuffer);
-            }
-            res = sslEngine.unwrap(inNetBuffer, appBuffer);
-            if (SessionLog.isDebugEnabled(session)) {
-                SessionLog.debug(session, " Unwrap res:" + res);
-            }
-
-        } while (res.getStatus() == SSLEngineResult.Status.OK
-                && res.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NEED_UNWRAP);
-
+        SSLEngineResult res = unwrap0();
         initialHandshakeStatus = res.getHandshakeStatus();
 
         // If handshake finished, no data was produced, and the status is still ok,
@@ -657,17 +642,7 @@
                 && appBuffer.position() == 0
                 && res.getStatus() == SSLEngineResult.Status.OK
                 && inNetBuffer.hasRemaining()) {
-            do {
-                if (SessionLog.isDebugEnabled(session)) {
-                    SessionLog.debug(session, "  extra handshake unwrap");
-                    SessionLog.debug(session, "   inNetBuffer: " + inNetBuffer);
-                    SessionLog.debug(session, "   appBuffer: " + appBuffer);
-                }
-                res = sslEngine.unwrap(inNetBuffer, appBuffer);
-                if (SessionLog.isDebugEnabled(session)) {
-                    SessionLog.debug(session, " Unwrap res:" + res);
-                }
-            } while (res.getStatus() == SSLEngineResult.Status.OK);
+            res = unwrap0();
         }
 
         // prepare to be written again
@@ -676,16 +651,27 @@
         // prepare app data to be read
         appBuffer.flip();
 
-        /*
-         * The status may be:
-         * OK - Normal operation
-         * OVERFLOW - Should never happen since the application buffer is
-         *      sized to hold the maximum packet size.
-         * UNDERFLOW - Need to read more data from the socket. It's normal.
-         * CLOSED - The other peer closed the socket. Also normal.
-         */
         //initialHandshakeStatus = res.getHandshakeStatus();
         return checkStatus(res.getStatus());
+    }
+
+    private SSLEngineResult unwrap0() throws SSLException {
+        SSLEngineResult res;
+        do {
+            if (SessionLog.isDebugEnabled(session)) {
+                SessionLog.debug(session, "   inNetBuffer: " + inNetBuffer);
+                SessionLog.debug(session, "   appBuffer: " + appBuffer);
+            }
+            res = sslEngine.unwrap(inNetBuffer, appBuffer);
+            if (SessionLog.isDebugEnabled(session)) {
+                SessionLog.debug(session, " Unwrap res:" + res);
+            }
+
+        } while (res.getStatus() == SSLEngineResult.Status.OK
+                && (initialHandshakeComplete && res.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING
+                        || res.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NEED_UNWRAP));
+        
+        return res;
     }
 
     /**

Modified: mina/branches/1.1/filter-ssl/src/main/java/org/apache/mina/filter/support/SSLHandler.java
URL: http://svn.apache.org/viewvc/mina/branches/1.1/filter-ssl/src/main/java/org/apache/mina/filter/support/SSLHandler.java?view=diff&rev=560837&r1=560836&r2=560837
==============================================================================
--- mina/branches/1.1/filter-ssl/src/main/java/org/apache/mina/filter/support/SSLHandler.java (original)
+++ mina/branches/1.1/filter-ssl/src/main/java/org/apache/mina/filter/support/SSLHandler.java Sun Jul 29 17:43:25 2007
@@ -305,7 +305,7 @@
         if (!initialHandshakeComplete) {
             handshake(nextFilter);
         } else {
-            decrypt();
+            decrypt(nextFilter);
         }
 
         if (isInboundDone()) {
@@ -414,7 +414,7 @@
      *
      * @throws SSLException
      */
-    private void decrypt() throws SSLException {
+    private void decrypt(NextFilter nextFilter) throws SSLException {
 
         if (!initialHandshakeComplete) {
             throw new IllegalStateException();
@@ -428,7 +428,7 @@
             throw new IllegalStateException();
         }
 
-        unwrap();
+        unwrap(nextFilter);
     }
 
     /**
@@ -437,6 +437,14 @@
      */
     private SSLEngineResult.Status checkStatus(SSLEngineResult.Status status)
             throws SSLException {
+        /*
+         * The status may be:
+         * OK - Normal operation
+         * OVERFLOW - Should never happen since the application buffer is
+         *      sized to hold the maximum packet size.
+         * UNDERFLOW - Need to read more data from the socket. It's normal.
+         * CLOSED - The other peer closed the socket. Also normal.
+         */
         if (status != SSLEngineResult.Status.OK
                 && status != SSLEngineResult.Status.CLOSED
                 && status != SSLEngineResult.Status.BUFFER_UNDERFLOW) {
@@ -485,7 +493,7 @@
                     SessionLog.debug(session,
                             "  initialHandshakeStatus=NEED_UNWRAP");
                 }
-                SSLEngineResult.Status status = unwrapHandshake();
+                SSLEngineResult.Status status = unwrapHandshake(nextFilter);
                 if ((initialHandshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED && status == SSLEngineResult.Status.BUFFER_UNDERFLOW)
                         || isInboundDone()) {
                     // We need more data or the session is closed
@@ -579,7 +587,7 @@
         return writeFuture;
     }
 
-    private SSLEngineResult.Status unwrap() throws SSLException {
+    private SSLEngineResult.Status unwrap(NextFilter nextFilter) throws SSLException {
         if (SessionLog.isDebugEnabled(session)) {
             SessionLog.debug(session, " unwrap()");
         }
@@ -589,35 +597,25 @@
         // Prepare the net data for reading.
         inNetBuffer.flip();
 
-        SSLEngineResult res;
-        do {
-            if (SessionLog.isDebugEnabled(session)) {
-                SessionLog.debug(session, "   inNetBuffer: " + inNetBuffer);
-                SessionLog.debug(session, "   appBuffer: " + appBuffer);
-            }
-            res = sslEngine.unwrap(inNetBuffer, appBuffer);
-            if (SessionLog.isDebugEnabled(session)) {
-                SessionLog.debug(session, " Unwrap res:" + res);
-            }
-        } while (res.getStatus() == SSLEngineResult.Status.OK);
+        SSLEngineResult res = unwrap0();
 
         // prepare to be written again
         inNetBuffer.compact();
         // prepare app data to be read
         appBuffer.flip();
+        
+        checkStatus(res.getStatus());
+        
+        if (res.getHandshakeStatus() != SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING) {
+            // Renegotiation required.
+            SessionLog.debug(session, " Renegotiating...");
+            handshake(nextFilter);
+        }
 
-        /*
-         * The status may be:
-         * OK - Normal operation
-         * OVERFLOW - Should never happen since the application buffer is
-         *      sized to hold the maximum packet size.
-         * UNDERFLOW - Need to read more data from the socket. It's normal.
-         * CLOSED - The other peer closed the socket. Also normal.
-         */
-        return checkStatus(res.getStatus());
+        return res.getStatus();
     }
 
-    private SSLEngineResult.Status unwrapHandshake() throws SSLException {
+    private SSLEngineResult.Status unwrapHandshake(NextFilter nextFilter) throws SSLException {
         if (SessionLog.isDebugEnabled(session)) {
             SessionLog.debug(session, " unwrapHandshake()");
         }
@@ -627,20 +625,7 @@
         // Prepare the net data for reading.
         inNetBuffer.flip();
 
-        SSLEngineResult res;
-        do {
-            if (SessionLog.isDebugEnabled(session)) {
-                SessionLog.debug(session, "   inNetBuffer: " + inNetBuffer);
-                SessionLog.debug(session, "   appBuffer: " + appBuffer);
-            }
-            res = sslEngine.unwrap(inNetBuffer, appBuffer);
-            if (SessionLog.isDebugEnabled(session)) {
-                SessionLog.debug(session, " Unwrap res:" + res);
-            }
-
-        } while (res.getStatus() == SSLEngineResult.Status.OK
-                && res.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NEED_UNWRAP);
-
+        SSLEngineResult res = unwrap0();
         initialHandshakeStatus = res.getHandshakeStatus();
 
         // If handshake finished, no data was produced, and the status is still ok,
@@ -649,17 +634,7 @@
                 && appBuffer.position() == 0
                 && res.getStatus() == SSLEngineResult.Status.OK
                 && inNetBuffer.hasRemaining()) {
-            do {
-                if (SessionLog.isDebugEnabled(session)) {
-                    SessionLog.debug(session, "  extra handshake unwrap");
-                    SessionLog.debug(session, "   inNetBuffer: " + inNetBuffer);
-                    SessionLog.debug(session, "   appBuffer: " + appBuffer);
-                }
-                res = sslEngine.unwrap(inNetBuffer, appBuffer);
-                if (SessionLog.isDebugEnabled(session)) {
-                    SessionLog.debug(session, " Unwrap res:" + res);
-                }
-            } while (res.getStatus() == SSLEngineResult.Status.OK);
+            res = unwrap0();
         }
 
         // prepare to be written again
@@ -668,16 +643,27 @@
         // prepare app data to be read
         appBuffer.flip();
 
-        /*
-         * The status may be:
-         * OK - Normal operation
-         * OVERFLOW - Should never happen since the application buffer is
-         *      sized to hold the maximum packet size.
-         * UNDERFLOW - Need to read more data from the socket. It's normal.
-         * CLOSED - The other peer closed the socket. Also normal.
-         */
         //initialHandshakeStatus = res.getHandshakeStatus();
         return checkStatus(res.getStatus());
+    }
+
+    private SSLEngineResult unwrap0() throws SSLException {
+        SSLEngineResult res;
+        do {
+            if (SessionLog.isDebugEnabled(session)) {
+                SessionLog.debug(session, "   inNetBuffer: " + inNetBuffer);
+                SessionLog.debug(session, "   appBuffer: " + appBuffer);
+            }
+            res = sslEngine.unwrap(inNetBuffer, appBuffer);
+            if (SessionLog.isDebugEnabled(session)) {
+                SessionLog.debug(session, " Unwrap res:" + res);
+            }
+
+        } while (res.getStatus() == SSLEngineResult.Status.OK
+                && (initialHandshakeComplete && res.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING
+                        || res.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NEED_UNWRAP));
+        
+        return res;
     }
 
     /**

Modified: mina/trunk/core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java
URL: http://svn.apache.org/viewvc/mina/trunk/core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java?view=diff&rev=560837&r1=560836&r2=560837
==============================================================================
--- mina/trunk/core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java (original)
+++ mina/trunk/core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java Sun Jul 29 17:43:25 2007
@@ -311,7 +311,7 @@
         if (!initialHandshakeComplete) {
             handshake(nextFilter);
         } else {
-            decrypt();
+            decrypt(nextFilter);
         }
 
         if (isInboundDone()) {
@@ -420,7 +420,7 @@
      *
      * @throws SSLException
      */
-    private void decrypt() throws SSLException {
+    private void decrypt(NextFilter nextFilter) throws SSLException {
 
         if (!initialHandshakeComplete) {
             throw new IllegalStateException();
@@ -434,7 +434,7 @@
             throw new IllegalStateException();
         }
 
-        unwrap();
+        unwrap(nextFilter);
     }
 
     /**
@@ -443,6 +443,14 @@
      */
     private SSLEngineResult.Status checkStatus(SSLEngineResult.Status status)
             throws SSLException {
+        /*
+         * The status may be:
+         * OK - Normal operation
+         * OVERFLOW - Should never happen since the application buffer is
+         *      sized to hold the maximum packet size.
+         * UNDERFLOW - Need to read more data from the socket. It's normal.
+         * CLOSED - The other peer closed the socket. Also normal.
+         */
         if (status != SSLEngineResult.Status.OK
                 && status != SSLEngineResult.Status.CLOSED
                 && status != SSLEngineResult.Status.BUFFER_UNDERFLOW) {
@@ -491,7 +499,7 @@
                     SessionLog.debug(session,
                             "  initialHandshakeStatus=NEED_UNWRAP");
                 }
-                SSLEngineResult.Status status = unwrapHandshake();
+                SSLEngineResult.Status status = unwrapHandshake(nextFilter);
                 if ((initialHandshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED && status == SSLEngineResult.Status.BUFFER_UNDERFLOW)
                         || isInboundDone()) {
                     // We need more data or the session is closed
@@ -589,7 +597,7 @@
         }
     }
 
-    private SSLEngineResult.Status unwrap() throws SSLException {
+    private SSLEngineResult.Status unwrap(NextFilter nextFilter) throws SSLException {
         if (SessionLog.isDebugEnabled(session)) {
             SessionLog.debug(session, " unwrap()");
         }
@@ -599,35 +607,25 @@
         // Prepare the net data for reading.
         inNetBuffer.flip();
 
-        SSLEngineResult res;
-        do {
-            if (SessionLog.isDebugEnabled(session)) {
-                SessionLog.debug(session, "   inNetBuffer: " + inNetBuffer);
-                SessionLog.debug(session, "   appBuffer: " + appBuffer);
-            }
-            res = sslEngine.unwrap(inNetBuffer, appBuffer);
-            if (SessionLog.isDebugEnabled(session)) {
-                SessionLog.debug(session, " Unwrap res:" + res);
-            }
-        } while (res.getStatus() == SSLEngineResult.Status.OK);
+        SSLEngineResult res = unwrap0();
 
         // prepare to be written again
         inNetBuffer.compact();
         // prepare app data to be read
         appBuffer.flip();
+        
+        checkStatus(res.getStatus());
+        
+        if (res.getHandshakeStatus() != SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING) {
+            // Renegotiation required.
+            SessionLog.debug(session, " Renegotiating...");
+            handshake(nextFilter);
+        }
 
-        /*
-         * The status may be:
-         * OK - Normal operation
-         * OVERFLOW - Should never happen since the application buffer is
-         *      sized to hold the maximum packet size.
-         * UNDERFLOW - Need to read more data from the socket. It's normal.
-         * CLOSED - The other peer closed the socket. Also normal.
-         */
-        return checkStatus(res.getStatus());
+        return res.getStatus();
     }
 
-    private SSLEngineResult.Status unwrapHandshake() throws SSLException {
+    private SSLEngineResult.Status unwrapHandshake(NextFilter nextFilter) throws SSLException {
         if (SessionLog.isDebugEnabled(session)) {
             SessionLog.debug(session, " unwrapHandshake()");
         }
@@ -637,20 +635,7 @@
         // Prepare the net data for reading.
         inNetBuffer.flip();
 
-        SSLEngineResult res;
-        do {
-            if (SessionLog.isDebugEnabled(session)) {
-                SessionLog.debug(session, "   inNetBuffer: " + inNetBuffer);
-                SessionLog.debug(session, "   appBuffer: " + appBuffer);
-            }
-            res = sslEngine.unwrap(inNetBuffer, appBuffer);
-            if (SessionLog.isDebugEnabled(session)) {
-                SessionLog.debug(session, " Unwrap res:" + res);
-            }
-
-        } while (res.getStatus() == SSLEngineResult.Status.OK
-                && res.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NEED_UNWRAP);
-
+        SSLEngineResult res = unwrap0();
         initialHandshakeStatus = res.getHandshakeStatus();
 
         // If handshake finished, no data was produced, and the status is still ok,
@@ -659,17 +644,7 @@
                 && appBuffer.position() == 0
                 && res.getStatus() == SSLEngineResult.Status.OK
                 && inNetBuffer.hasRemaining()) {
-            do {
-                if (SessionLog.isDebugEnabled(session)) {
-                    SessionLog.debug(session, "  extra handshake unwrap");
-                    SessionLog.debug(session, "   inNetBuffer: " + inNetBuffer);
-                    SessionLog.debug(session, "   appBuffer: " + appBuffer);
-                }
-                res = sslEngine.unwrap(inNetBuffer, appBuffer);
-                if (SessionLog.isDebugEnabled(session)) {
-                    SessionLog.debug(session, " Unwrap res:" + res);
-                }
-            } while (res.getStatus() == SSLEngineResult.Status.OK);
+            res = unwrap0();
         }
 
         // prepare to be written again
@@ -678,16 +653,27 @@
         // prepare app data to be read
         appBuffer.flip();
 
-        /*
-         * The status may be:
-         * OK - Normal operation
-         * OVERFLOW - Should never happen since the application buffer is
-         *      sized to hold the maximum packet size.
-         * UNDERFLOW - Need to read more data from the socket. It's normal.
-         * CLOSED - The other peer closed the socket. Also normal.
-         */
         //initialHandshakeStatus = res.getHandshakeStatus();
         return checkStatus(res.getStatus());
+    }
+
+    private SSLEngineResult unwrap0() throws SSLException {
+        SSLEngineResult res;
+        do {
+            if (SessionLog.isDebugEnabled(session)) {
+                SessionLog.debug(session, "   inNetBuffer: " + inNetBuffer);
+                SessionLog.debug(session, "   appBuffer: " + appBuffer);
+            }
+            res = sslEngine.unwrap(inNetBuffer, appBuffer);
+            if (SessionLog.isDebugEnabled(session)) {
+                SessionLog.debug(session, " Unwrap res:" + res);
+            }
+
+        } while (res.getStatus() == SSLEngineResult.Status.OK
+                && (initialHandshakeComplete && res.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING
+                        || res.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NEED_UNWRAP));
+        
+        return res;
     }
 
     /**