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