Code Review Request, JDK-8207009 SSLEngine#closeInbound mentions SSLException when no close_notify is received

[Xue-Lei Fan]

One more update:
   http://cr.openjdk.java.net/~xuelei/8207009/webrev.05/

It is desired to make a note in SSLSocket and SSLEngine specification, 
so that users have a good sense that an application should close the 
input and output stream always.

Updated for SSLEngine.java and SSLSocket.java only.  No changes in other 
files.

Please let me know your concerns by the end of today.

Thanks,
Xuelei

On 8/10/2018 4:02 PM, Jamil Nimeh wrote:
> I'm good with the changes.
> 
> --Jamil
> 
> On 8/7/2018 5:24 PM, Xuelei Fan wrote:
>> Hi Jamil,
>>
>> Thanks for comments.  Here is the updated webrev:
>>    http://cr.openjdk.java.net/~xuelei/8207009/webrev.04/
>>
>> Thanks,
>> Xuelei
>>
>> On 8/7/2018 3:12 PM, Jamil Nimeh wrote:
>>> Hi Xuelei, mostly small stuff:
>>>
>>>   * SSLEngineImpl.java
>>>       o 717: Nit, inbout --> inbound
>>>   * SSLEngineOutputRecord.java
>>>       o 162, 169: Nit, applicatoin --> application
>>>       o Same section: It looks like the "if" and "else if" clauses take
>>>         the same actions with the same message.  Maybe just do "if
>>>         (isClosed())" ?  Or were you planning to have different messages
>>>         here?
>>>   * SSLSocketOutputRecord.java
>>>       o 58, 97, 204: Typo,  closedd --> closed
>>>   * TLSEngineClosureTest.java
>>>       o 2: Copyright date fix
>>>       o 26: If this is a new test should the bug ID be different than
>>>         8085979?
>>>
>>> --Jamil
>>>
>>>
>>> On 08/07/2018 07:46 AM, Xuelei Fan wrote:
>>>> New webrev:
>>>> http://cr.openjdk.java.net/~xuelei/8207009/webrev.03/
>>>>
>>>> Thanks for a find of Tim Brooks, that the SSLEngine inbound/outbound 
>>>> status is incorrect if closing during handshake. The above webrev is 
>>>> trying to fix the problems. See more in the OpenJDK thread:
>>>> http://mail.openjdk.java.net/pipermail/security-dev/2018-August/017778.html 
>>>>
>>>>
>>>> Please let me know your concerns before this Wednesday.
>>>>
>>>> Thanks,
>>>> Xuelei
>>>>
>>>> On 8/3/2018 1:55 PM, Xuelei Fan wrote:
>>>>> Update: http://cr.openjdk.java.net/~xuelei/8207009/webrev.02/
>>>>>
>>>>> In webrev.01, the socket close may be blocked by super class close 
>>>>> synchronization.  Updated the SSLSocketImpl.java to use handshake 
>>>>> only lock in the startHandshake() implementation.
>>>>>
>>>>> Thanks,
>>>>> Xuelei
>>>>>
>>>>> On 8/1/2018 7:27 PM, Xuelei Fan wrote:
>>>>>> Update: http://cr.openjdk.java.net/~xuelei/8207009/webrev.01/
>>>>>>
>>>>>> Integrated the fix for JDK-8208642, "Server initiated TLSv1.2 
>>>>>> renegotiation fails if Java client allows TLSv1.3". 
>>>>>> SSLHandshake.java is updated to use negotiated version so that TLS 
>>>>>> 1.2 HelloRequest is acceptable in TLS 1.3 client side.
>>>>>>
>>>>>> Thanks,
>>>>>> Xuelei
>>>>>>
>>>>>> On 7/30/2018 10:24 AM, Xuelei Fan wrote:
>>>>>>> <loop in net-dev as well>
>>>>>>> Please let me know your concerns by the end of August 1st, 2018.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Xuelei
>>>>>>>
>>>>>>>
>>>>>>> On 7/30/2018 9:59 AM, Xuelei Fan wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Please review the update for the TLS 1.3 half-close and 
>>>>>>>> synchronization implementation:
>>>>>>>> http://cr.openjdk.java.net/~xuelei/8207009/webrev.00/
>>>>>>>>
>>>>>>>> Unlike TLS 1.2 and prior versions, for TLS 1.3, the close_notify 
>>>>>>>> is use to close the local write side and peer read side only. 
>>>>>>>> After the close_notify get handles, the local read side and peer 
>>>>>>>> write side may still be open.
>>>>>>>>
>>>>>>>> In this update, if an application calls 
>>>>>>>> SSLEngine.closeInbound/Outbound() or 
>>>>>>>> SSLSocket.shutdownInput/Output(), half-close will be used.  For 
>>>>>>>> compatibility, if SSLSocket.close() get called, a duplex close 
>>>>>>>> will be tried.  In order to support duplex close, JDK will use 
>>>>>>>> the user_canceled warning alert even the handshake complete.
>>>>>>>>
>>>>>>>> In practice, an application may only close outbound even it is 
>>>>>>>> intended to close the inbound as well, or close the connection 
>>>>>>>> completely.  It works for TLS 1.2 and prior versions.  But no 
>>>>>>>> more for TLS 1.3 because of the close_notify behavior change in 
>>>>>>>> the TLS 1.3 specification.  The application may be hung and 
>>>>>>>> dead-waiting for read/close.  It could be solved by closing the 
>>>>>>>> inbound explicitly. In order to mitigate the impact, a new 
>>>>>>>> System Property is introduced, "jdk.tls.acknowledgeCloseNotify" 
>>>>>>>> if source code update is not available.   If the System Property 
>>>>>>>> is set to "true", if receiving the close_notify, a close_notify 
>>>>>>>> alert will be responded.  It is a countermeasure of the TLS 1.3 
>>>>>>>> half-close issues.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Xuelei
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>
>