Skip to content

Commit

Permalink
Include failed TLS handshakes in the access log
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
markt-asf committed Aug 2, 2019
1 parent 22301d2 commit acf6076
Show file tree
Hide file tree
Showing 11 changed files with 61 additions and 5 deletions.
17 changes: 16 additions & 1 deletion java/org/apache/coyote/AbstractProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public Adapter getAdapter() {
* 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;
}

Expand Down Expand Up @@ -944,6 +944,7 @@ protected boolean isTrailerFieldsSupported() {
*/
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.
Expand All @@ -955,4 +956,18 @@ protected boolean isTrailerFieldsSupported() {
* 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);
}
}
17 changes: 16 additions & 1 deletion java/org/apache/coyote/AbstractProcessorLight.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,10 @@ public SocketState process(SocketWrapperBase<?> socketWrapper, SocketEvent statu
} 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
Expand Down Expand Up @@ -129,6 +131,19 @@ protected void clearDispatches() {
}


/**
* 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
Expand Down
10 changes: 8 additions & 2 deletions java/org/apache/coyote/http11/Http11Processor.java
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,6 @@ public SocketState service(SocketWrapperBase<?> socketWrapper)

// Setting up the I/O
setSocketWrapper(socketWrapper);
inputBuffer.init(socketWrapper);
outputBuffer.init(socketWrapper);

// Flags
keepAlive = true;
Expand Down Expand Up @@ -505,6 +503,14 @@ public SocketState service(SocketWrapperBase<?> socketWrapper)
}


@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();

Expand Down
1 change: 1 addition & 0 deletions java/org/apache/coyote/http2/Http2UpgradeHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ public SocketState upgradeDispatch(SocketEvent status) {
case ERROR:
case TIMEOUT:
case STOP:
case CONNECT_FAIL:
close();
break;
}
Expand Down
2 changes: 2 additions & 0 deletions java/org/apache/tomcat/util/net/AprEndpoint.java
Original file line number Diff line number Diff line change
Expand Up @@ -1982,13 +1982,15 @@ public void run() {
getConnectionTimeout(), Poll.APR_POLLIN);
} else {
// Close socket and pool
getHandler().process(socket, SocketEvent.CONNECT_FAIL);
closeSocket(socket.getSocket().longValue());
socket = null;
}
} else {
// 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;
Expand Down
1 change: 1 addition & 0 deletions java/org/apache/tomcat/util/net/Nio2Endpoint.java
Original file line number Diff line number Diff line change
Expand Up @@ -1682,6 +1682,7 @@ protected void doRun() {
launch = true;
}
} else if (handshake == -1 ) {
getHandler().process(socketWrapper, SocketEvent.CONNECT_FAIL);
socketWrapper.close();
}
} catch (VirtualMachineError vme) {
Expand Down
1 change: 1 addition & 0 deletions java/org/apache/tomcat/util/net/NioEndpoint.java
Original file line number Diff line number Diff line change
Expand Up @@ -1590,6 +1590,7 @@ protected void doRun() {
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();
Expand Down
11 changes: 10 additions & 1 deletion java/org/apache/tomcat/util/net/SocketEvent.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ public SocketState upgradeDispatch(SocketEvent status) {
//$FALL-THROUGH$
case DISCONNECT:
case TIMEOUT:
case CONNECT_FAIL:
return SocketState.CLOSED;

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ public SocketState upgradeDispatch(SocketEvent status) {
case DISCONNECT:
case ERROR:
case TIMEOUT:
case CONNECT_FAIL:
return SocketState.CLOSED;
}
return SocketState.UPGRADED;
Expand Down
4 changes: 4 additions & 0 deletions webapps/docs/changelog.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down

0 comments on commit acf6076

Please sign in to comment.