On Thu, 7 Jan 2021 21:04:48 GMT, Clive Verghese <[email protected]> wrote:
> While looking into the TrustTrustedCert.java test, we found out this change > may cause the test to fails intermittently. This is because it's is not > expecting a SocketException in Line 134. The root cause for this intermittent > failure seems to be that when the server processes the the ClientHello > message, it iterates through a list of producers to create the response > message. These response messages are written to the socket by each producer, > and the client can start processing these messages before all of them are > sent. > > If one of the earlier messages causes the client to reject the TLS handshake > (in this example, a bad certificate), the client will send a TLS fatal alert > and close the socket. In most scenarios, by the time the TLS fatal alert is > received by the server, all the pending TLS handshake messages would´ve been > written into the socket, but if the client response is fast enough, this TLS > alert can reach the server before it has finished sending all the messages. > The server will not wait to check if it has received a fatal alert from the > client and it will attempt to send the next message anyway. > > Because in this scenario the socket has already been closed by the client, a > SocketException (Broken Pipe) will be generated, instead of the more > appropriate SSLHandshakeException. Before this patch, those SocketExceptions > were at least being wrapped as SSLException, but now, the SocketException > will go all the way up. Most of the time, this might not be a problem, but > there are scenarios that come to mind where this could become relevant. The > simpler one to explain is MTLS, where a very similar scenario can happen but > this time in the client side if the server sends the fatal while the client > is still writing (for example, rejecting the client certificate before this > has sent the Client Key Exchange). Again, instead of an SSLHanshakeException, > we have SSLException before this patch and SocketException after. In this > case, seeing a SocketException the client may decide to retry the connection > (while for an SSLException or SSLHandshakeException, most likely it > wouldn't). How bad thi s is, and how bad the potential for an extra retry is compared to the current situation when retries are not happening for transient errors, we'll leave for the discussion. > > Additionally, although the step between Certificate and Server/Client Key > Exchange is the most likely to trigger this, every time multiple producers > are being called there is a chance for this issue to arise. The good news is > the issue requires round-trip communication between server and client to be > faster than processing all the producers in a batch. The aforementioned test > triggers this behavior quite often as both sides of the TLS connection are on > the same machine. Maybe, sending the fatal alter before throw the socket exception could be a safer update. ------------- PR: https://git.openjdk.java.net/jdk/pull/1968
