On Fri, 18 Oct 2024 04:37:46 GMT, Xue-Lei Andrew Fan <xue...@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.
Thanks for the detailed clarification, it definitely makes sense. Looks like
the main issue not the exception per se but incorrect exception. I'll make the
changes.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/21043#issuecomment-2422469575
