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

Jamil Nimeh jamil.j.nimeh at oracle.com
Fri Aug 10 23:02:22 UTC 2018


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
>>>>>>>
>>>>>>>
>>>>>>>
>>




More information about the security-dev mailing list