On Thu, 7 Jan 2021 23:39:34 GMT, Xue-Lei Andrew Fan <[email protected]> wrote:
>> Clive Verghese has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Shutdown sockets to prevent leak
>
> src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java line 1678:
>
>> 1676: // Don't close the Socket in case of timeouts, interrupts or
>> SocketException.
>> 1677: if (cause instanceof InterruptedIOException ||
>> 1678: cause instanceof SocketException) {
>
> Maybe we still need to shutdown the connection with a fatal alter for socket
> exception, otherwise there might be socket leaks. Instead, the socket
> exception could be thrown after the fatal alert.
Updated to shutdown the socket in case of a SocketException.
> src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java line 452:
>
>> 450: } catch (SocketException se) {
>> 451: // don't change exception in case of SocketException
>> 452: throw se;
>
> Maybe, the fatal alter could be sent before thrown the socket exception.
The client is sending the fatal,
However, the server, since it's producing the message, It's not reading from
the socket to see that the client sent the `bad_certificate`
SERVER CLIENT
* <------------
CLIENT_HELLO
CLIENT_HELLO_CONSUMER
SERVER_HELLO_PRODUCER ------------->
SERVER_HELLO_CONSUMER
CERTIFICATE_PRODUCER ------------->
CERTIFICATE_CONSUMER
CERTIFICATE_STATUS -------------> Still in
CERTIFICATE_CONSUMER
START SERVER_KEY_EXCHANGE
* <-------------
CERTIFICATE_CONSUMER sends bad_certificate alert
* <-------------
CLIENT_CLOSES_SOCKET
SERVER_KEY_EXCHANGE
attempts to write to socket --------||||
(broken_pipe exception)
Server throws a SocketException(broken_pipe) exception instead of
SSLException(bad_certificate) or SSLHandshakeException(bad_certificate)
When in the producer, the server does not read from the socket, and hence does
not process the bad_certificate alert from the client
The SERVER_KEY_EXCHANGE produce then attempts to write to the socket, which
encounters the broken pipe.
We could, in the SSLSocketImpl::handleException, attempt to check if there is a
message available in the socket. If so, read the message and throw the
appropriate exception.
I could open a follow up JBS issue to address this.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1968
