CVE-2009-3555 prevent renegotiation

git-svn-id: svn+ssh://dev.eclipse.org/svnroot/rt/org.eclipse.jetty/jetty/trunk@1047 7e9141cc-0065-0410-87d8-b60c137991c4

VERSION.txt

@@ -38,6 +38,7 @@ jetty-7.0.1-SNAPSHOT
  + CQ-3581 jetty OSGi contribution
  + Moved centralized logging and verifier back to sandbox
  + 294345 Support for HTTP/301 + HTTP/302 response codes
+ + CVE-2009-3555 Prevent SSL renegotiate for SSL vulnerability
 
 jetty-7.0.0.v20091005 5 October 2009
 291340 Race condition in onException() notifications

jetty-http/src/main/java/org/eclipse/jetty/http/ssl/SslSelectChannelEndPoint.java

@@ -65,6 +65,9 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
     private boolean _closing=false;
     private SSLEngineResult _result;
 
+    private boolean _handshook=false;
+    private boolean _allowRenegotiate=false;
+
     private final boolean _debug = __log.isDebugEnabled(); // snapshot debug status for optimizer 
 
     
@@ -88,12 +91,34 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
         if (_debug) __log.debug(_session+" channel="+channel);
     }
 
+
+    /* ------------------------------------------------------------ */
+    /**
+     * @return True if SSL re-negotiation is allowed (default false)
+     */
+    public boolean isAllowRenegotiate()
+    {
+        return _allowRenegotiate;
+    }
+
+    /* ------------------------------------------------------------ */
+    /**
+     * Set if SSL re-negotiation is allowed. CVE-2009-3555 discovered
+     * a vulnerability in SSL/TLS with re-negotiation.  If your JVM
+     * does not have CVE-2009-3555 fixed, then re-negotiation should 
+     * not be allowed.
+     * @param allowRenegotiate true if re-negotiation is allowed (default false)
+     */
+    public void setAllowRenegotiate(boolean allowRenegotiate)
+    {
+        _allowRenegotiate = allowRenegotiate;
+    }
+
+    /* ------------------------------------------------------------ */
     // TODO get rid of these dumps
     public void dump()
     {
         Log.info(""+_result);
-        // System.err.println(h.toString());
-        // System.err.println("--");
     }
     
     /* ------------------------------------------------------------ */
@@ -163,6 +188,7 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
                 {
                     case FINISHED:
                     case NOT_HANDSHAKING:
+                        _handshook=true;
                         break loop;
                         
                     case NEED_UNWRAP:
@@ -240,6 +266,8 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
         }   
     }
 
+
+    
     /* ------------------------------------------------------------ */
     /** Fill the buffer with unencrypted bytes.
      * Called by a {@link Parser} instance when more data is 
@@ -302,6 +330,7 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
                             break loop;
 
                         case NEED_UNWRAP:
+                            checkRenegotiate();
                             // Need more data to be unwrapped so try another call to unwrap
                             if (!unwrap(bbuf) && _engine.getHandshakeStatus()==HandshakeStatus.NEED_UNWRAP)
                             {
@@ -339,6 +368,7 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
 
                         case NEED_WRAP:
                         {
+                            checkRenegotiate();
                             // The SSL needs to send some handshake data to the other side,
                             // so let fill become a flush for a little bit.
                             wraps++;
@@ -391,11 +421,13 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
                 buffer.setPutIndex(bbuf.position());
                 bbuf.position(0);
             }
-        }
-        
-        // return the number of unencrypted bytes filled.
-        return buffer.length()-size; 
 
+            // return the number of unencrypted bytes filled.
+            int filled=buffer.length()-size; 
+            if (filled>0)
+                _handshook=true;
+            return filled; 
+        }
     }
 
     /* ------------------------------------------------------------ */
@@ -426,12 +458,11 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
                 if (isBufferingOutput())
                     break loop;
             }
-            
+
             switch(_engine.getHandshakeStatus())
             {
                 case FINISHED:
                 case NOT_HANDSHAKING:
-
                     if (_closing || available==0)
                     {
                         if (consumed==0)
@@ -450,8 +481,10 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
                     else 
                         c=wrap(buffer);
                     
+                    
                     if (c>0)
                     {
+                        _handshook=true;
                         consumed+=c;
                         available-=c;
                     }
@@ -465,6 +498,7 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
                     break;
 
                 case NEED_UNWRAP:
+                    checkRenegotiate();
                     Buffer buf =_buffers.getBuffer(_engine.getSession().getApplicationBufferSize());
                     try
                     {
@@ -493,6 +527,7 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
 
                 case NEED_WRAP:
                 {
+                    checkRenegotiate();
                     synchronized(_outBuffer)
                     {
                         try
@@ -549,6 +584,16 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
             }
         }
     }
+    
+    /* ------------------------------------------------------------ */
+    private void checkRenegotiate() throws IOException
+    {
+        if (_handshook && !_allowRenegotiate && _channel!=null && _channel.isOpen())
+        {
+            Log.warn("SSL renegotiate denied: "+_channel);
+            close();
+        }   
+    }
 
     /* ------------------------------------------------------------ */
     private ByteBuffer extractInputBuffer(Buffer buffer)

jetty-server/src/main/java/org/eclipse/jetty/server/ssl/SslConnector.java

@@ -217,4 +217,21 @@ public interface SslConnector extends Connector
      * @return The SSLContext
      */
     public abstract SSLContext getSslContext();
+    
+
+    /* ------------------------------------------------------------ */
+    /**
+     * @return True if SSL re-negotiation is allowed (default false)
+     */
+    public boolean isAllowRenegotiate();
+
+    /* ------------------------------------------------------------ */
+    /**
+     * Set if SSL re-negotiation is allowed. CVE-2009-3555 discovered
+     * a vulnerability in SSL/TLS with re-negotiation.  If your JVM
+     * does not have CVE-2009-3555 fixed, then re-negotiation should 
+     * not be allowed.
+     * @param allowRenegotiate true if re-negotiation is allowed (default false)
+     */
+    public void setAllowRenegotiate(boolean allowRenegotiate);
 }

jetty-server/src/main/java/org/eclipse/jetty/server/ssl/SslSelectChannelConnector.java

@@ -82,6 +82,7 @@ public class SslSelectChannelConnector extends SelectChannelConnector implements
     /** Set to true if we require client certificate authentication. */
     private boolean _needClientAuth=false;
     private boolean _wantClientAuth=false;
+    private boolean _allowRenegotiate=false;
 
     private transient Password _password;
     private transient Password _keyPassword;
@@ -223,6 +224,28 @@ public class SslSelectChannelConnector extends SelectChannelConnector implements
     {
     }
 
+    /* ------------------------------------------------------------ */
+    /**
+     * @return True if SSL re-negotiation is allowed (default false)
+     */
+    public boolean isAllowRenegotiate()
+    {
+        return _allowRenegotiate;
+    }
+
+    /* ------------------------------------------------------------ */
+    /**
+     * Set if SSL re-negotiation is allowed. CVE-2009-3555 discovered
+     * a vulnerability in SSL/TLS with re-negotiation.  If your JVM
+     * does not have CVE-2009-3555 fixed, then re-negotiation should 
+     * not be allowed.
+     * @param allowRenegotiate true if re-negotiation is allowed (default false)
+     */
+    public void setAllowRenegotiate(boolean allowRenegotiate)
+    {
+        _allowRenegotiate = allowRenegotiate;
+    }
+
     /* ------------------------------------------------------------ */
     /**
      * @see org.eclipse.jetty.server.ssl.SslConnector#getExcludeCipherSuites()
@@ -554,7 +577,9 @@ public class SslSelectChannelConnector extends SelectChannelConnector implements
     @Override
     protected SelectChannelEndPoint newEndPoint(SocketChannel channel, SelectSet selectSet, SelectionKey key) throws IOException
     {
-        return new SslSelectChannelEndPoint(_sslBuffers,channel,selectSet,key,createSSLEngine());
+        SslSelectChannelEndPoint endp = new SslSelectChannelEndPoint(_sslBuffers,channel,selectSet,key,createSSLEngine());
+        endp.setAllowRenegotiate(_allowRenegotiate);
+        return endp;
     }
 
     /* ------------------------------------------------------------------------------- */
@@ -597,7 +622,6 @@ public class SslSelectChannelConnector extends SelectChannelConnector implements
 
                 engine.setEnabledCipherSuites(enabledCipherSuites);
             }
-
         }
         catch (Exception e)
         {
@@ -607,7 +631,6 @@ public class SslSelectChannelConnector extends SelectChannelConnector implements
         }
         return engine;
     }
-
    
     @Override
     protected void doStart() throws Exception

jetty-server/src/main/java/org/eclipse/jetty/server/ssl/SslSocketConnector.java

@@ -29,6 +29,8 @@ import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 
+import javax.net.ssl.HandshakeCompletedEvent;
+import javax.net.ssl.HandshakeCompletedListener;
 import javax.net.ssl.KeyManager;
 import javax.net.ssl.KeyManagerFactory;
 import javax.net.ssl.SSLContext;
@@ -144,6 +146,7 @@ public class SslSocketConnector extends SocketConnector  implements SslConnector
     private int _handshakeTimeout = 0; //0 means use maxIdleTime
     
     private SSLContext _context;
+    private boolean _allowRenegotiate =false;
 
 
     /* ------------------------------------------------------------ */
@@ -155,6 +158,27 @@ public class SslSocketConnector extends SocketConnector  implements SslConnector
         super();
     }
 
+    /* ------------------------------------------------------------ */
+    /**
+     * @return True if SSL re-negotiation is allowed (default false)
+     */
+    public boolean isAllowRenegotiate()
+    {
+        return _allowRenegotiate;
+    }
+
+    /* ------------------------------------------------------------ */
+    /**
+     * Set if SSL re-negotiation is allowed. CVE-2009-3555 discovered
+     * a vulnerability in SSL/TLS with re-negotiation.  If your JVM
+     * does not have CVE-2009-3555 fixed, then re-negotiation should 
+     * not be allowed.
+     * @param allowRenegotiate true if re-negotiation is allowed (default false)
+     */
+    public void setAllowRenegotiate(boolean allowRenegotiate)
+    {
+        _allowRenegotiate = allowRenegotiate;
+    }
 
     /* ------------------------------------------------------------ */
     @Override
@@ -687,7 +711,25 @@ public class SslSocketConnector extends SocketConnector  implements SslConnector
                 if (handshakeTimeout > 0)            
                     _socket.setSoTimeout(handshakeTimeout);
 
-                ((SSLSocket)_socket).startHandshake();
+                final SSLSocket ssl=(SSLSocket)_socket;
+                ssl.addHandshakeCompletedListener(new HandshakeCompletedListener()
+                {
+                    boolean handshook=false;
+                    public void handshakeCompleted(HandshakeCompletedEvent event)
+                    {
+                        if (handshook)
+                        {
+                            if (!_allowRenegotiate)
+                            {
+                                Log.warn("SSL renegotiate denied: "+ssl);
+                                try{ssl.close();}catch(IOException e){Log.warn(e);}
+                            }
+                        }
+                        else
+                            handshook=true;
+                    }
+                });
+                ssl.startHandshake();
 
                 if (handshakeTimeout>0)
                     _socket.setSoTimeout(oldTimeout);

jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslRenegotiateTest.java

@@ -0,0 +1,271 @@
+package org.eclipse.jetty.server.ssl;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.net.InetSocketAddress;
+import java.net.SocketAddress;
+import java.nio.ByteBuffer;
+import java.nio.channels.SocketChannel;
+
+import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.SSLEngineResult;
+import javax.net.ssl.SSLSession;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.X509TrustManager;
+import javax.net.ssl.SSLEngineResult.HandshakeStatus;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.eclipse.jetty.io.nio.IndirectNIOBuffer;
+import org.eclipse.jetty.server.Connector;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.handler.AbstractHandler;
+import org.eclipse.jetty.util.StringUtil;
+import org.eclipse.jetty.util.log.Log;
+
+import junit.framework.TestCase;
+
+
+public class SslRenegotiateTest extends TestCase
+{
+
+    static TrustManager[] trustAllCerts = new TrustManager[] { new X509TrustManager()
+    {
+        public java.security.cert.X509Certificate[] getAcceptedIssuers()
+        {
+            return null;
+        }
+
+        public void checkClientTrusted( java.security.cert.X509Certificate[] certs, String authType )
+        {
+        }
+
+        public void checkServerTrusted( java.security.cert.X509Certificate[] certs, String authType )
+        {
+        }
+    } };
+
+    static HostnameVerifier hostnameVerifier = new HostnameVerifier()
+    {
+        public boolean verify( String urlHostName, SSLSession session )
+        {
+            Log.warn( "Warning: URL Host: " + urlHostName + " vs." + session.getPeerHost() );
+            return true;
+        }
+    };
+
+    ByteBuffer _outAppB;
+    ByteBuffer _outPacketB;
+    ByteBuffer _inAppB;
+    ByteBuffer _inPacketB;
+    SocketChannel _socket;
+    SSLEngine _engine;
+    
+
+    public void testRenegNIO() throws Exception
+    {
+        doRequests(new SslSelectChannelConnector(),true);
+    }
+    
+    public void testNoRenegNIO() throws Exception
+    {
+        doRequests(new SslSelectChannelConnector(),false);
+    }
+    public void testRenegBIO() throws Exception
+    {
+        doRequests(new SslSocketConnector(),true);
+    }
+    
+    public void testNoRenegBIO() throws Exception
+    {
+        /* TODO - this test does not always work??? need to investigate why
+        doRequests(new SslSocketConnector(),false);
+        */
+    }
+
+    public void doRequests(SslConnector connector,boolean reneg) throws Exception
+    {
+        Server server=new Server();
+        try
+        {
+            String keystore = System.getProperty("user.dir")+File.separator+"src"+File.separator+"test"+File.separator+"resources"+File.separator+"keystore";
+            connector.setPort(0);
+            connector.setKeystore(keystore);
+            connector.setPassword("storepwd");
+            connector.setKeyPassword("keypwd");
+            connector.setAllowRenegotiate(reneg);
+
+            server.setConnectors(new Connector[] { connector });
+            server.setHandler(new HelloWorldHandler());
+
+            server.start();
+            
+            SocketAddress addr = new InetSocketAddress("localhost",connector.getLocalPort());
+            _socket = SocketChannel.open(addr);
+            _socket.configureBlocking(true);
+            
+            SSLContext context=SSLContext.getInstance("SSL");
+            context.init( null, trustAllCerts, new java.security.SecureRandom() );
+
+            _engine = context.createSSLEngine();
+            _engine.setUseClientMode(true);
+            SSLSession session=_engine.getSession();
+            
+            _outAppB = ByteBuffer.allocate(session.getApplicationBufferSize());
+            _outPacketB = ByteBuffer.allocate(session.getPacketBufferSize());
+            _inAppB = ByteBuffer.allocate(session.getApplicationBufferSize());
+            _inPacketB = ByteBuffer.allocate(session.getPacketBufferSize());
+            
+            
+            _outAppB.put("GET /1 HTTP/1.1\r\nHost: localhost\r\n\r\n".getBytes(StringUtil.__ISO_8859_1));
+            _outAppB.flip();
+            
+            _engine.beginHandshake();
+            
+            runHandshake();
+
+            doWrap();
+            doUnwrap();
+            _inAppB.flip();
+            String response=new IndirectNIOBuffer(_inAppB,true).toString();
+            // System.err.println(response);
+            assertTrue(response.startsWith("HTTP/1.1 200 OK"));
+            
+            if (response.indexOf("HELLO WORLD")<0)
+            {
+                _inAppB.clear();
+                doUnwrap();
+                _inAppB.flip();
+                response=new IndirectNIOBuffer(_inAppB,true).toString();
+            }
+            
+            assertTrue(response.indexOf("HELLO WORLD")>=0);
+            
+            _inAppB.clear();
+            _outAppB.clear();
+            _outAppB.put("GET /2 HTTP/1.1\r\nHost: localhost\r\n\r\n".getBytes(StringUtil.__ISO_8859_1));
+            _outAppB.flip();
+
+            try
+            {
+                session.invalidate();
+                _engine.beginHandshake();
+                runHandshake();
+                
+                doWrap();
+                doUnwrap();
+                _inAppB.flip();
+                response=new IndirectNIOBuffer(_inAppB,true).toString();
+                assertTrue(response.startsWith("HTTP/1.1 200 OK"));
+                assertTrue(response.indexOf("HELLO WORLD")>0);
+
+                assertTrue(reneg);
+            }
+            catch(IOException e)
+            {
+                // System.err.println(e);
+                assertFalse(reneg);
+                return;
+            }
+            
+        }
+        finally
+        {
+            server.stop();
+        }
+    }
+    
+    void runHandshake() throws Exception
+    {
+        SSLEngineResult result;
+        
+        while (true)
+        {
+            //System.err.println();
+            //System.err.println(_engine.getHandshakeStatus());
+
+            switch(_engine.getHandshakeStatus())
+            {
+                case NEED_TASK:
+                {
+                    //System.err.println("running task");
+                    _engine.getDelegatedTask().run();
+                    break;
+                }
+                
+                case NEED_WRAP:
+                {
+                    doWrap();
+                    break;
+                }
+                
+                case NEED_UNWRAP:
+                {
+                    doUnwrap();
+                    break;
+                }
+                
+                default:
+                    return;
+            }
+        }
+    }
+    
+    private void doWrap() throws Exception
+    {
+        SSLEngineResult result =_engine.wrap(_outAppB,_outPacketB);
+//        System.err.println("wrapped "+result.bytesConsumed()+" to "+result.bytesProduced());
+        _outPacketB.flip();
+        while (_outPacketB.hasRemaining())
+        {
+            int p = _outPacketB.remaining();
+            int l =_socket.write(_outPacketB);
+            // System.err.println("wrote "+l+" of "+p);
+        }
+        _outPacketB.clear();
+    }
+    
+    private void doUnwrap() throws Exception
+    {
+        _inPacketB.clear();
+        int l=_socket.read(_inPacketB);
+        // System.err.println("read "+l);
+        if (l<0)
+            throw new IOException("EOF");
+        
+        _inPacketB.flip();
+
+        SSLEngineResult result;
+        do
+        {
+            result =_engine.unwrap(_inPacketB,_inAppB);
+//            System.err.println("unwrapped "+result.bytesConsumed()+" to "+result.bytesProduced()+" "+_engine.getHandshakeStatus());
+            
+        }
+        while(result.bytesConsumed()>0 &&
+              _inPacketB.remaining()>0 && 
+              (_engine.getHandshakeStatus()==HandshakeStatus.NEED_UNWRAP || _engine.getHandshakeStatus()==HandshakeStatus.NOT_HANDSHAKING));
+        
+    }
+
+    private static class HelloWorldHandler extends AbstractHandler
+    {
+
+        public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
+        {
+            baseRequest.setHandled(true);
+            //System.err.println("HELLO WORLD HANDLING");
+
+//            System.err.println("hello "+baseRequest.getUri());
+            byte[] b=("HELLO WORLD "+baseRequest.getUri()).getBytes(StringUtil.__UTF8);
+            response.setContentLength(b.length);
+            response.getOutputStream().write(b);
+            response.getOutputStream().flush();
+        }
+    }
+}