Author: markt
Date: Tue Aug 8 19:29:55 2017
New Revision: 1804463
URL: http://svn.apache.org/viewvc?rev=1804463&view=rev
Log:
Improve the handling of client disconnections during the TLS renegotiation
handshake.
Modified:
tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
tomcat/trunk/java/org/apache/tomcat/util/net/LocalStrings.properties
tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
tomcat/trunk/java/org/apache/tomcat/util/net/SecureNio2Channel.java
tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java
tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java?rev=1804463&r1=1804462&r2=1804463&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java Tue Aug 8
19:29:55 2017
@@ -371,7 +371,11 @@ public abstract class AbstractProcessor
break;
}
case REQ_SSL_CERTIFICATE: {
- sslReHandShake();
+ try {
+ sslReHandShake();
+ } catch (IOException ioe) {
+ setErrorState(ErrorState.CLOSE_CONNECTION_NOW, ioe);
+ }
break;
}
@@ -645,8 +649,12 @@ public abstract class AbstractProcessor
/**
* Processors that can perform a TLS re-handshake (e.g. HTTP/1.1) should
* override this method and implement the re-handshake.
+ *
+ * @throws IOException If authentication is required then there will be I/O
+ * with the client and this exception will be thrown if
+ * that goes wrong
*/
- protected void sslReHandShake() {
+ protected void sslReHandShake() throws IOException {
// NO-OP
}
Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1804463&r1=1804462&r2=1804463&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Tue Aug 8
19:29:55 2017
@@ -1242,7 +1242,7 @@ public class Http11Processor extends Abs
@Override
- protected final void sslReHandShake() {
+ protected final void sslReHandShake() throws IOException {
if (sslSupport != null) {
// Consume and buffer the request body, so that it does not
// interfere with the client's handshake messages
@@ -1251,8 +1251,17 @@ public class Http11Processor extends Abs
protocol.getMaxSavePostSize());
inputBuffer.addActiveFilter(inputFilters[Constants.BUFFERED_FILTER]);
+ /*
+ * Outside the try/catch because we want I/O errors during
+ * renegotiation to be thrown for the caller to handle since they
+ * will be fatal to the connection.
+ */
+ socketWrapper.doClientAuth(sslSupport);
try {
- socketWrapper.doClientAuth(sslSupport);
+ /*
+ * Errors processing the cert chain do not affect the client
+ * connection so they can be logged and swallowed here.
+ */
Object sslO = sslSupport.getPeerCertificateChain();
if (sslO != null) {
request.setAttribute(SSLSupport.CERTIFICATE_KEY, sslO);
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1804463&r1=1804462&r2=1804463&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Tue Aug 8
19:29:55 2017
@@ -2749,11 +2749,16 @@ public class AprEndpoint extends Abstrac
@Override
- public void doClientAuth(SSLSupport sslSupport) {
+ public void doClientAuth(SSLSupport sslSupport) throws IOException {
long socket = getSocket().longValue();
// Configure connection to require a certificate
- SSLSocket.setVerify(socket, SSL.SSL_CVERIFY_REQUIRE, -1);
- SSLSocket.renegotiate(socket);
+ try {
+ SSLSocket.setVerify(socket, SSL.SSL_CVERIFY_REQUIRE, -1);
+ SSLSocket.renegotiate(socket);
+ } catch (Throwable t) {
+ ExceptionUtils.handleThrowable(t);
+ throw new IOException(sm.getString("socket.sslreneg"), t);
+ }
}
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/LocalStrings.properties
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/LocalStrings.properties?rev=1804463&r1=1804462&r2=1804463&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/LocalStrings.properties
(original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/LocalStrings.properties Tue
Aug 8 19:29:55 2017
@@ -83,6 +83,7 @@ endpoint.nio.timeoutCme=Exception during
endpoint.nio2.exclusiveExecutor=The NIO2 connector requires an exclusive
executor to operate properly on shutdown
channel.nio.interrupted=The current thread was interrupted
+channel.nio.ssl.closeSilentError=As expected, there was an exception trying to
close the connection cleanly.
channel.nio.ssl.notHandshaking=NOT_HANDSHAKING during handshake
channel.nio.ssl.handshakeError=Handshake error
channel.nio.ssl.unexpectedStatusDuringWrap=Unexpected status [{0}] during
handshake WRAP.
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java?rev=1804463&r1=1804462&r2=1804463&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Tue Aug 8
19:29:55 2017
@@ -1589,18 +1589,14 @@ public class Nio2Endpoint extends Abstra
@Override
- public void doClientAuth(SSLSupport sslSupport) {
+ public void doClientAuth(SSLSupport sslSupport) throws IOException {
SecureNio2Channel sslChannel = (SecureNio2Channel) getSocket();
SSLEngine engine = sslChannel.getSslEngine();
if (!engine.getNeedClientAuth()) {
// Need to re-negotiate SSL connection
engine.setNeedClientAuth(true);
- try {
- sslChannel.rehandshake();
- ((JSSESupport) sslSupport).setSession(engine.getSession());
- } catch (IOException ioe) {
- log.warn(sm.getString("socket.sslreneg"), ioe);
- }
+ sslChannel.rehandshake();
+ ((JSSESupport) sslSupport).setSession(engine.getSession());
}
}
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1804463&r1=1804462&r2=1804463&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Tue Aug 8
19:29:55 2017
@@ -1293,18 +1293,14 @@ public class NioEndpoint extends Abstrac
@Override
- public void doClientAuth(SSLSupport sslSupport) {
+ public void doClientAuth(SSLSupport sslSupport) throws IOException {
SecureNioChannel sslChannel = (SecureNioChannel) getSocket();
SSLEngine engine = sslChannel.getSslEngine();
if (!engine.getNeedClientAuth()) {
// Need to re-negotiate SSL connection
engine.setNeedClientAuth(true);
- try {
-
sslChannel.rehandshake(getEndpoint().getConnectionTimeout());
- ((JSSESupport) sslSupport).setSession(engine.getSession());
- } catch (IOException ioe) {
- log.warn(sm.getString("socket.sslreneg",ioe));
- }
+ sslChannel.rehandshake(getEndpoint().getConnectionTimeout());
+ ((JSSESupport) sslSupport).setSession(engine.getSession());
}
}
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/SecureNio2Channel.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/SecureNio2Channel.java?rev=1804463&r1=1804462&r2=1804463&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/SecureNio2Channel.java
(original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/SecureNio2Channel.java Tue Aug
8 19:29:55 2017
@@ -305,8 +305,11 @@ public class SecureNio2Channel extends N
sc.read(netInBuffer, socket,
handshakeReadCompletionHandler);
} else {
try {
-
sc.read(netInBuffer).get(endpoint.getConnectionTimeout(),
- TimeUnit.MILLISECONDS);
+ int read =
sc.read(netInBuffer).get(endpoint.getConnectionTimeout(),
+ TimeUnit.MILLISECONDS).intValue();
+ if (read == -1) {
+ throw new EOFException();
+ }
} catch (InterruptedException | ExecutionException
| TimeoutException e) {
throw new
IOException(sm.getString("channel.nio.ssl.handshakeError"));
}
@@ -449,8 +452,10 @@ public class SecureNio2Channel extends N
}
}
} catch (IOException x) {
+ closeSilently();
throw x;
} catch (Exception cx) {
+ closeSilently();
IOException x = new IOException(cx);
throw x;
}
@@ -576,18 +581,31 @@ public class SecureNio2Channel extends N
closed = (!netOutBuffer.hasRemaining() &&
(handshake.getHandshakeStatus() != HandshakeStatus.NEED_WRAP));
}
+
@Override
public void close(boolean force) throws IOException {
try {
close();
} finally {
- if ( force || closed ) {
+ if (force || closed) {
closed = true;
sc.close();
}
}
}
+
+ private void closeSilently() {
+ try {
+ close(true);
+ } catch (IOException ioe) {
+ // This is expected - swallowing the exception is the reason this
+ // method exists. Log at debug in case someone is interested.
+ log.debug(sm.getString("channel.nio.ssl.closeSilentError"), ioe);
+ }
+ }
+
+
private class FutureRead implements Future<Integer> {
private ByteBuffer dst;
private Future<Integer> integer;
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=1804463&r1=1804462&r2=1804463&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java
(original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java Tue Aug
8 19:29:55 2017
@@ -395,8 +395,10 @@ public class SecureNioChannel extends Ni
}
}
} catch (IOException x) {
+ closeSilently();
throw x;
} catch (Exception cx) {
+ closeSilently();
IOException x = new IOException(cx);
throw x;
} finally {
@@ -526,8 +528,8 @@ public class SecureNioChannel extends Ni
public void close(boolean force) throws IOException {
try {
close();
- }finally {
- if ( force || closed ) {
+ } finally {
+ if (force || closed) {
closed = true;
sc.socket().close();
sc.close();
@@ -535,6 +537,18 @@ public class SecureNioChannel extends Ni
}
}
+
+ private void closeSilently() {
+ try {
+ close(true);
+ } catch (IOException ioe) {
+ // This is expected - swallowing the exception is the reason this
+ // method exists. Log at debug in case someone is interested.
+ log.debug(sm.getString("channel.nio.ssl.closeSilentError"), ioe);
+ }
+ }
+
+
/**
* Reads a sequence of bytes from this channel into the given buffer.
*
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java?rev=1804463&r1=1804462&r2=1804463&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java
(original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java Tue Aug
8 19:29:55 2017
@@ -759,8 +759,12 @@ public abstract class SocketWrapperBase<
* @param sslSupport The SSL/TLS support instance currently being used by
* the connection that may need updating after the client
* authentication
+ *
+ * @throws IOException If authentication is required then there will be I/O
+ * with the client and this exception will be thrown if
+ * that goes wrong
*/
- public abstract void doClientAuth(SSLSupport sslSupport);
+ public abstract void doClientAuth(SSLSupport sslSupport) throws
IOException;
public abstract SSLSupport getSslSupport(String clientCertProvider);
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1804463&r1=1804462&r2=1804463&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Tue Aug 8 19:29:55 2017
@@ -64,6 +64,10 @@
which is the most secure / restrictive option in addition to reporting
the configuration error. (markt)
</fix>
+ <fix>
+ Improve the handling of client disconnections during the TLS
+ renegotiation handshake. (markt)
+ </fix>
</changelog>
</subsection>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
