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

[Jamil Nimeh]

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20180807/71299c7f/attachment.htm>