This looks like a CPU spinning handshake to me.
The operation handshake(true, true); returns an IO interest to be
registered with a selector.
If the client is slow here or misbehaving, you could end up in a end
less loop, and hence we can have introduced a very simple DoS
vulnerability here.
The simplest solution is, would be to use an individual selector.
Register the socket and issue a select() on the thread you are running on.
If you want to use a shared selector (like NIO does for reads and
writes) it requires a bit more logic.
I understand where you are going with this solution, and I can probably
fix this today as I sit on the airplane on my way home.
best
Filip
On 02/25/2011 12:19 PM, ma...@apache.org wrote:
Author: markt
Date: Fri Feb 25 19:19:13 2011
New Revision: 1074675
URL: http://svn.apache.org/viewvc?rev=1074675&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49284
Support SSL re-negotiation in the HTTP NIO connector
There is a fair amount of renaming in this patch. The real work is in the new
rehandshake() method in the SecureNioChannel.
Modified:
tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties
tomcat/trunk/java/org/apache/tomcat/util/net/NioChannel.java
tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java?rev=1074675&r1=1074674&r2=1074675&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java Fri Feb
25 19:19:13 2011
@@ -23,6 +23,8 @@ import java.nio.channels.SelectionKey;
import java.util.Locale;
import java.util.concurrent.Executor;
+import javax.net.ssl.SSLEngine;
+
import org.apache.coyote.ActionCode;
import org.apache.coyote.Request;
import org.apache.coyote.RequestInfo;
@@ -42,7 +44,9 @@ import org.apache.tomcat.util.net.NioCha
import org.apache.tomcat.util.net.NioEndpoint;
import org.apache.tomcat.util.net.NioEndpoint.KeyAttachment;
import org.apache.tomcat.util.net.SSLSupport;
+import org.apache.tomcat.util.net.SecureNioChannel;
import org.apache.tomcat.util.net.SocketStatus;
+import org.apache.tomcat.util.net.jsse.JSSEFactory;
/**
@@ -625,8 +629,25 @@ public class Http11NioProcessor extends
.setLimit(maxSavePostSize);
inputBuffer.addActiveFilter
(inputFilters[Constants.BUFFERED_FILTER]);
+
+ SecureNioChannel sslChannel = (SecureNioChannel) socket;
+ SSLEngine engine = sslChannel.getSslEngine();
+ if (!engine.getNeedClientAuth()&&
!engine.getWantClientAuth()) {
+ // Need to re-negotiate SSL connection
+ engine.setNeedClientAuth(true);
+ try {
+ sslChannel.rehandshake();
+ sslSupport = (new JSSEFactory()).getSSLSupport(
+ engine.getSession());
+ } catch (IOException ioe) {
+
log.warn(sm.getString("http11processor.socket.sslreneg",
+ ioe));
+ }
+ }
try {
- Object sslO = sslSupport.getPeerCertificateChain(true);
+ // use force=false since re-negotiation is handled above
+ // (and it is a NO-OP for NIO anyway)
+ Object sslO = sslSupport.getPeerCertificateChain(false);
if( sslO != null) {
request.setAttribute
(SSLSupport.CERTIFICATE_KEY, sslO);
Modified: tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties?rev=1074675&r1=1074674&r2=1074675&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties Fri Feb
25 19:19:13 2011
@@ -31,6 +31,7 @@ http11processor.request.process=Error pr
http11processor.request.finish=Error finishing request
http11processor.response.finish=Error finishing response
http11processor.socket.info=Exception getting socket information
+http11processor.socket.sslreneg=Exception re-negotiating SSL connection
http11processor.socket.ssl=Exception getting SSL attributes
http11processor.socket.timeout=Error setting socket timeout
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioChannel.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioChannel.java?rev=1074675&r1=1074674&r2=1074675&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/NioChannel.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/NioChannel.java Fri Feb 25
19:19:13 2011
@@ -175,7 +175,7 @@ public class NioChannel implements ByteC
* @return boolean
* TODO Implement this org.apache.tomcat.util.net.SecureNioChannel method
*/
- public boolean isInitHandshakeComplete() {
+ public boolean isHandshakeComplete() {
return true;
}
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java?rev=1074675&r1=1074674&r2=1074675&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java
(original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java Fri Feb
25 19:19:13 2011
@@ -43,8 +43,8 @@ public class SecureNioChannel extends Ni
protected SSLEngine sslEngine;
- protected boolean initHandshakeComplete = false;
- protected HandshakeStatus initHandshakeStatus; //gets set by begin
handshake
+ protected boolean handshakeComplete = false;
+ protected HandshakeStatus handshakeStatus; //gets set by begin handshake
protected boolean closed = false;
protected boolean closing = false;
@@ -82,12 +82,12 @@ public class SecureNioChannel extends Ni
netOutBuffer.limit(0);
netInBuffer.position(0);
netInBuffer.limit(0);
- initHandshakeComplete = false;
+ handshakeComplete = false;
closed = false;
closing = false;
//initiate handshake
sslEngine.beginHandshake();
- initHandshakeStatus = sslEngine.getHandshakeStatus();
+ handshakeStatus = sslEngine.getHandshakeStatus();
}
@Override
@@ -146,35 +146,35 @@ public class SecureNioChannel extends Ni
*/
@Override
public int handshake(boolean read, boolean write) throws IOException {
- if ( initHandshakeComplete ) return 0; //we have done our initial
handshake
+ if ( handshakeComplete ) return 0; //we have done our initial handshake
if (!flush(netOutBuffer)) return SelectionKey.OP_WRITE; //we still
have data to write
SSLEngineResult handshake = null;
- while (!initHandshakeComplete) {
- switch ( initHandshakeStatus ) {
+ while (!handshakeComplete) {
+ switch ( handshakeStatus ) {
case NOT_HANDSHAKING: {
//should never happen
throw new IOException("NOT_HANDSHAKING during handshake");
}
case FINISHED: {
//we are complete if we have delivered the last package
- initHandshakeComplete = !netOutBuffer.hasRemaining();
+ handshakeComplete = !netOutBuffer.hasRemaining();
//return 0 if we are complete, otherwise we still have
data to write
- return initHandshakeComplete?0:SelectionKey.OP_WRITE;
+ return handshakeComplete?0:SelectionKey.OP_WRITE;
}
case NEED_WRAP: {
//perform the wrap function
handshake = handshakeWrap(write);
if ( handshake.getStatus() == Status.OK ){
- if (initHandshakeStatus == HandshakeStatus.NEED_TASK)
- initHandshakeStatus = tasks();
+ if (handshakeStatus == HandshakeStatus.NEED_TASK)
+ handshakeStatus = tasks();
} else {
//wrap should always work with our buffers
throw new IOException("Unexpected status:" +
handshake.getStatus() + " during handshake WRAP.");
}
- if ( initHandshakeStatus != HandshakeStatus.NEED_UNWRAP ||
(!flush(netOutBuffer)) ) {
+ if ( handshakeStatus != HandshakeStatus.NEED_UNWRAP ||
(!flush(netOutBuffer)) ) {
//should actually return OP_READ if we have
NEED_UNWRAP
return SelectionKey.OP_WRITE;
}
@@ -185,26 +185,26 @@ public class SecureNioChannel extends Ni
//perform the unwrap function
handshake = handshakeUnwrap(read);
if ( handshake.getStatus() == Status.OK ) {
- if (initHandshakeStatus == HandshakeStatus.NEED_TASK)
- initHandshakeStatus = tasks();
+ if (handshakeStatus == HandshakeStatus.NEED_TASK)
+ handshakeStatus = tasks();
} else if ( handshake.getStatus() ==
Status.BUFFER_UNDERFLOW ){
//read more data, reregister for OP_READ
return SelectionKey.OP_READ;
} else {
- throw new IOException("Invalid handshake
status:"+initHandshakeStatus+" during handshake UNWRAP.");
+ throw new IOException("Invalid handshake
status:"+handshakeStatus+" during handshake UNWRAP.");
}//switch
break;
}
case NEED_TASK: {
- initHandshakeStatus = tasks();
+ handshakeStatus = tasks();
break;
}
- default: throw new IllegalStateException("Invalid handshake
status:"+initHandshakeStatus);
+ default: throw new IllegalStateException("Invalid handshake
status:"+handshakeStatus);
}//switch
}//while
//return 0 if we are complete, otherwise reregister for any activity
that
//would cause this method to be called again.
- return
initHandshakeComplete?0:(SelectionKey.OP_WRITE|SelectionKey.OP_READ);
+ return
handshakeComplete?0:(SelectionKey.OP_WRITE|SelectionKey.OP_READ);
}
/**
@@ -234,7 +234,7 @@ public class SecureNioChannel extends Ni
//prepare the results to be written
netOutBuffer.flip();
//set the status
- initHandshakeStatus = result.getHandshakeStatus();
+ handshakeStatus = result.getHandshakeStatus();
//optimization, if we do have a writable channel, write it now
if ( doWrite ) flush(netOutBuffer);
return result;
@@ -268,19 +268,42 @@ public class SecureNioChannel extends Ni
//compact the buffer, this is an optional method, wonder what
would happen if we didn't
netInBuffer.compact();
//read in the status
- initHandshakeStatus = result.getHandshakeStatus();
+ handshakeStatus = result.getHandshakeStatus();
if ( result.getStatus() == SSLEngineResult.Status.OK&&
result.getHandshakeStatus() == HandshakeStatus.NEED_TASK ) {
//execute tasks if we need to
- initHandshakeStatus = tasks();
+ handshakeStatus = tasks();
}
//perform another unwrap?
cont = result.getStatus() == SSLEngineResult.Status.OK&&
- initHandshakeStatus == HandshakeStatus.NEED_UNWRAP;
+ handshakeStatus == HandshakeStatus.NEED_UNWRAP;
}while ( cont );
return result;
}
+ public void rehandshake() throws IOException {
+ int readBufLimit = getBufHandler().getReadBuffer().limit();
+ try {
+ // Expand read buffer to maximum to allow handshaking to take place
+ getBufHandler().getReadBuffer().limit(
+ getBufHandler().getReadBuffer().capacity());
+ sslEngine.getSession().invalidate();
+ sslEngine.beginHandshake();
+ handshakeComplete = false;
+ handshakeStatus = sslEngine.getHandshakeStatus();
+ while (!handshakeComplete) {
+ handshake(true, true);
+ if (handshakeStatus == HandshakeStatus.NEED_UNWRAP) {
+ handshakeUnwrap(true);
+ }
+ }
+ } finally {
+ // Restore the pre-handshak value
+ getBufHandler().getReadBuffer().limit(readBufLimit);
+ }
+ }
+
+
/**
* Sends a SSL close message, will not physically close the connection
here.<br>
* To close the connection, you could do something like
@@ -353,7 +376,7 @@ public class SecureNioChannel extends Ni
//are we in the middle of closing or closed?
if ( closing || closed) return -1;
//did we finish our handshake?
- if (!initHandshakeComplete) throw new IllegalStateException("Handshake
incomplete, you must complete handshake before reading data.");
+ if (!handshakeComplete) throw new IllegalStateException("Handshake
incomplete, you must complete handshake before reading data.");
//read from the network
int netread = sc.read(netInBuffer);
@@ -474,8 +497,8 @@ public class SecureNioChannel extends Ni
}
@Override
- public boolean isInitHandshakeComplete() {
- return initHandshakeComplete;
+ public boolean isHandshakeComplete() {
+ return handshakeComplete;
}
@Override
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1074675&r1=1074674&r2=1074675&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Feb 25 19:19:13 2011
@@ -151,6 +151,10 @@
</subsection>
<subsection name="Coyote">
<changelog>
+<add>
+<bug>49284</bug>: Add SSL re-negotiation support to the HTTP NIO
+ connector. (markt)
+</add>
<fix>
<bug>50780</bug>: Fix memory leak in APR implementation of AJP
connector introduced by the refactoring for<bug>49884</bug>. (markt)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
