This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push: new acf6076 Include failed TLS handshakes in the access log acf6076 is described below commit acf6076d7118571ebc881984b96792f861b72bb2 Author: Mark Thomas <ma...@apache.org> AuthorDate: Wed Jul 31 16:49:35 2019 +0100 Include failed TLS handshakes in the access log Failed TLS handshakes are logged with the following information: - status 400 - start time close to the connection failure - duration 0 All other fields apart from network related fields (e.g. IP address etc) obtained from the SocketWrapper are empty. --- java/org/apache/coyote/AbstractProcessor.java | 17 ++++++++++++++++- java/org/apache/coyote/AbstractProcessorLight.java | 17 ++++++++++++++++- java/org/apache/coyote/http11/Http11Processor.java | 10 ++++++++-- java/org/apache/coyote/http2/Http2UpgradeHandler.java | 1 + java/org/apache/tomcat/util/net/AprEndpoint.java | 2 ++ java/org/apache/tomcat/util/net/Nio2Endpoint.java | 1 + java/org/apache/tomcat/util/net/NioEndpoint.java | 1 + java/org/apache/tomcat/util/net/SocketEvent.java | 11 ++++++++++- .../tomcat/websocket/server/WsHttpUpgradeHandler.java | 1 + .../http11/upgrade/TestUpgradeInternalHandler.java | 1 + webapps/docs/changelog.xml | 4 ++++ 11 files changed, 61 insertions(+), 5 deletions(-) diff --git a/java/org/apache/coyote/AbstractProcessor.java b/java/org/apache/coyote/AbstractProcessor.java index e7909ba..d94a8e4 100644 --- a/java/org/apache/coyote/AbstractProcessor.java +++ b/java/org/apache/coyote/AbstractProcessor.java @@ -149,7 +149,7 @@ public abstract class AbstractProcessor extends AbstractProcessorLight implement * Set the socket wrapper being used. * @param socketWrapper The socket wrapper */ - protected final void setSocketWrapper(SocketWrapperBase<?> socketWrapper) { + protected void setSocketWrapper(SocketWrapperBase<?> socketWrapper) { this.socketWrapper = socketWrapper; } @@ -944,6 +944,7 @@ public abstract class AbstractProcessor extends AbstractProcessorLight implement */ protected abstract boolean flushBufferedWrite() throws IOException ; + /** * Perform any necessary clean-up processing if the dispatch resulted in the * completion of processing for the current request. @@ -955,4 +956,18 @@ public abstract class AbstractProcessor extends AbstractProcessorLight implement * request */ protected abstract SocketState dispatchEndRequest() throws IOException; + + + @Override + protected final void logAccess(SocketWrapperBase<?> socketWrapper) throws IOException { + // Set the socket wrapper so the access log can read the socket related + // information (e.g. client IP) + setSocketWrapper(socketWrapper); + // Setup the minimal request information + request.setStartTime(System.currentTimeMillis()); + // Setup the minimal response information + response.setStatus(400); + response.setError(); + getAdapter().log(request, response, 0); + } } diff --git a/java/org/apache/coyote/AbstractProcessorLight.java b/java/org/apache/coyote/AbstractProcessorLight.java index 7a46c79..7d0b6e0 100644 --- a/java/org/apache/coyote/AbstractProcessorLight.java +++ b/java/org/apache/coyote/AbstractProcessorLight.java @@ -62,8 +62,10 @@ public abstract class AbstractProcessorLight implements Processor { } else if (status == SocketEvent.OPEN_WRITE) { // Extra write event likely after async, ignore state = SocketState.LONG; - } else if (status == SocketEvent.OPEN_READ){ + } else if (status == SocketEvent.OPEN_READ) { state = service(socketWrapper); + } else if (status == SocketEvent.CONNECT_FAIL) { + logAccess(socketWrapper); } else { // Default to closing the socket if the SocketEvent passed in // is not consistent with the current state of the Processor @@ -130,6 +132,19 @@ public abstract class AbstractProcessorLight implements Processor { /** + * Add an entry to the access log for a failed connection attempt. + * + * @param socketWrapper The connection to process + * + * @throws IOException If an I/O error occurs during the processing of the + * request + */ + protected void logAccess(SocketWrapperBase<?> socketWrapper) throws IOException { + // NO-OP by default + } + + + /** * Service a 'standard' HTTP request. This method is called for both new * requests and for requests that have partially read the HTTP request line * or HTTP headers. Once the headers have been fully read this method is not diff --git a/java/org/apache/coyote/http11/Http11Processor.java b/java/org/apache/coyote/http11/Http11Processor.java index b707291..1ba4625 100644 --- a/java/org/apache/coyote/http11/Http11Processor.java +++ b/java/org/apache/coyote/http11/Http11Processor.java @@ -276,8 +276,6 @@ public class Http11Processor extends AbstractProcessor { // Setting up the I/O setSocketWrapper(socketWrapper); - inputBuffer.init(socketWrapper); - outputBuffer.init(socketWrapper); // Flags keepAlive = true; @@ -505,6 +503,14 @@ public class Http11Processor extends AbstractProcessor { } + @Override + protected final void setSocketWrapper(SocketWrapperBase<?> socketWrapper) { + super.setSocketWrapper(socketWrapper); + inputBuffer.init(socketWrapper); + outputBuffer.init(socketWrapper); + } + + private Request cloneRequest(Request source) throws IOException { Request dest = new Request(); diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java index 71a5c1b..41c620d 100644 --- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java +++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java @@ -373,6 +373,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH case ERROR: case TIMEOUT: case STOP: + case CONNECT_FAIL: close(); break; } diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java index 1caa5e7..f9bed26 100644 --- a/java/org/apache/tomcat/util/net/AprEndpoint.java +++ b/java/org/apache/tomcat/util/net/AprEndpoint.java @@ -1982,6 +1982,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB getConnectionTimeout(), Poll.APR_POLLIN); } else { // Close socket and pool + getHandler().process(socket, SocketEvent.CONNECT_FAIL); closeSocket(socket.getSocket().longValue()); socket = null; } @@ -1989,6 +1990,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB // Process the request from this socket if (!setSocketOptions(socket)) { // Close socket and pool + getHandler().process(socket, SocketEvent.CONNECT_FAIL); closeSocket(socket.getSocket().longValue()); socket = null; return; diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java b/java/org/apache/tomcat/util/net/Nio2Endpoint.java index 66f9e20..4b59c9b 100644 --- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java +++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java @@ -1682,6 +1682,7 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS launch = true; } } else if (handshake == -1 ) { + getHandler().process(socketWrapper, SocketEvent.CONNECT_FAIL); socketWrapper.close(); } } catch (VirtualMachineError vme) { diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java index 85bfa9d..0df3b0a 100644 --- a/java/org/apache/tomcat/util/net/NioEndpoint.java +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java @@ -1590,6 +1590,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> poller.cancelledKey(key, socketWrapper); } } else if (handshake == -1 ) { + getHandler().process(socketWrapper, SocketEvent.CONNECT_FAIL); poller.cancelledKey(key, socketWrapper); } else if (handshake == SelectionKey.OP_READ){ socketWrapper.registerReadInterest(); diff --git a/java/org/apache/tomcat/util/net/SocketEvent.java b/java/org/apache/tomcat/util/net/SocketEvent.java index 9df1116..9d86028 100644 --- a/java/org/apache/tomcat/util/net/SocketEvent.java +++ b/java/org/apache/tomcat/util/net/SocketEvent.java @@ -60,5 +60,14 @@ public enum SocketEvent { * during Servlet 3.0 asynchronous processing.</li> * </ul> */ - ERROR + ERROR, + + /** + * A client attempted to establish a connection but failed. Examples of + * where this is used include: + * <ul> + * <li>TLS handshake failures</li> + * </ul> + */ + CONNECT_FAIL } diff --git a/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java b/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java index 437a33c..a1b82ea 100644 --- a/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java +++ b/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java @@ -178,6 +178,7 @@ public class WsHttpUpgradeHandler implements InternalHttpUpgradeHandler { //$FALL-THROUGH$ case DISCONNECT: case TIMEOUT: + case CONNECT_FAIL: return SocketState.CLOSED; } diff --git a/test/org/apache/coyote/http11/upgrade/TestUpgradeInternalHandler.java b/test/org/apache/coyote/http11/upgrade/TestUpgradeInternalHandler.java index 841989a..aaee563 100644 --- a/test/org/apache/coyote/http11/upgrade/TestUpgradeInternalHandler.java +++ b/test/org/apache/coyote/http11/upgrade/TestUpgradeInternalHandler.java @@ -252,6 +252,7 @@ public class TestUpgradeInternalHandler extends TomcatBaseTest { case DISCONNECT: case ERROR: case TIMEOUT: + case CONNECT_FAIL: return SocketState.CLOSED; } return SocketState.UPGRADED; diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 2393a89..d656546 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -129,6 +129,10 @@ Fix h2spec test suite failure. It is an error if a Huffman encoded string literal contains the EOS symbol. (jfclere) </fix> + <add> + Connections that fail the TLS handshake will now appear in the access + logs with a 400 status code. (markt) + </add> </changelog> </subsection> <subsection name="Cluster"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org