RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v20]

[Xue-Lei Andrew Fan]

On Thu, 17 Oct 2024 20:11:06 GMT, Artur Barashev <abarashev at openjdk.org> wrote:

> We want to address just this particular issue and not to cause any collateral damage by simplifying things. For example: if plaintext alert comes after handshake we want to throw an exception.

I think I have a better sense of your ideas.  Thank you.  Let's look at the scenarios that failed:
1. client send client_hello;
2. server received client_hello and send back handshake messages.
3. client wait for server hello, but timed out.
4. client would like to close the connection, for example call sslSocket.close(), sending user_canceled alert and close_notify alert.
5. server received the user_canceled, and failed to decrypt it
6. server throws an exception.

It looks like you want to have the following behavior:
1. client send client_hello;
2. server received client_hello and send back handshake messages.
3. client wait for server hello, but timed out.
4. client would like to close the connection, for example call sslSocket.close(), sending user_canceled alert and close_notify alert.
5. server received the user_canceled, and checked that it might be an plaintext alert, handle it as an user_canceled alert.
6. server received the close_notify, and checked that it might be an plaintext alert, handle it as an close_notify alert.
7. server close the inbound and outbound.
8. the connection get closed gracefully.

The scenarios may work if both client and server TLS vendor works like SunJSSE.  However, some vendor may not use user_canceled alert for connection close.  Then, if receiving close_notify during handshaking, an exception will be thrown as well (See AlertConsumer.consume()).  I may add a test to cover the case.

I was just wondering, what's the value to close the connection gracefully for server side?  The client and server want to establish a connection, and then use the connection for something else.  If the connection cannot be established, even if the close is graceful and silently, the using of the connection may fail immediately after while reading or writing, or checking the connection states.

I think the following code may be typical in practice (example got from JSSE Reference Guides):

        InputStream sslIS = sslSocket.getInputStream();
        sslIS.read();

```        
The sslIS.read() will throw SocketException("Connection or inbound has closed").  Per the message, the developer may want to look for calls to the close() method. Which is not the right direction definitely.   I don't think gracefully closing is helpful here.

"Insufficient buffer remaining ..." exception is hard to understand.   This is not a normal case for either client or server side.   I don't think we want to make it a normal process.  Maybe, using a user-friendly exception message should be sufficient, for example "handshake failed caused by unexpected alert ...".

If an exception will be thrown for the case, I think it the logic could be clearer/consistent and the fix could be easier, and both client and server sides timeout can be handled, without breaking TLS specifications.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/21043#issuecomment-2421323000