OpenJDK11u: Backward incompatible behavior

Sat Feb 29 00:24:16 UTC 2020

 > Webrev JDK11: http://cr.openjdk.java.net/~bae/8239788/webrev.v3/

 > getSession() method is implemented identically to JDK8, so it's
 > behaviour is backward compatible to JDK8
I know, but I would like to see if there is really a compatibility 
impact if the getSession() is consistent with other IO operations.  We 
could fix the problem later if there is a need.

 > I may try to catch the InterruptedIOException, rather than handle it
 > in the IOException catching block or the Exception catching block.

try {
    ...
} catch (Exception e) {
    if (e instanceof ... ) {
        ...
    } else (e instanceof ...) {
        ...
    }
}

vs
try {
} catch (AException e) {
    ...
} catch (IOException e) {
    ...
} catch (Exception e) {
    ...
}

the later is the common coding style

SSLSocketInputRecord:
   52     private byte[] inputBuffer = new byte[maxRecordSize];

Would you mind consider an improvement to use less memory?

If you have webrev for JDK 15, I could help to run more testing for you.

Thanks,
Xuelei

On 2/27/2020 9:05 AM, Alexey Bakhtin wrote:
> Hello Xuelei,
> 
> You are right, SSLSocketInputRecord.read() method could be interrupted 
> before all requested data is received.
> 
> I have updated  SSLSocketInputRecord class to use single buffer for 
> incoming data. Also I’ve updated read() method to take into account 
> every chunk of incoming data. It should prevent possible loss of data if 
> socket timed out.
> 
> Webrev JDK11: http://cr.openjdk.java.net/~bae/8239788/webrev.v3/
> 
> Tested with sun/security/ssl and javax/net/ssl jtreg tests
> 
> Thank you
> Alexey
> 
>> On 27 Feb 2020, at 01:04, Xuelei Fan <xuelei.fan at oracle.com 
>> <mailto:xuelei.fan at oracle.com>> wrote:
>>
>> > Webrev for JDK15 : http://cr.openjdk.java.net/~yan/8239788/webrev.2/
>> For the SSLSocketInputRecord.java update, is it possible that the 
>> read() implementation interrupted before reading the exact specified 
>> bytes of data?  The received data may be lost if it is interrupted.
>>
>> BTW, it might not be effective to use three fields to store the data, 
>> temporary, header and inputBuffer.  Is it possible to use just one 
>> buffer (temporary, for example) and one integer to remember the 
>> received data length?
>>
>> Thanks,
>> Xuelei
>>
>>
>> On 2/26/2020 4:22 AM, Alexey Bakhtin wrote:
>>> Hello Xuelei,
>>> Thank you for review.
>>> getSession() method is implemented identically to JDK8, so it's 
>>> behaviour is backward compatible to JDK8
>>> I have updated review with some modifications:
>>> 1) Enabled sun/security/ssl/SSLSocketImpl/ClientTimeout.java jtreg 
>>> test. This test emulates socket timeout during handshake and app data 
>>> transfer.
>>> 2) I have added cache for incoming data received from socket 
>>> (sun.security.ssl.SSLSocketInputRecord class). Socket timeout could 
>>> happen while reading single handshake message by small chunks. It is 
>>> implemented similarly to JDK8 and allows to pass 
>>> sun/security/ssl/SSLSocketImpl/ClientTimeout test
>>> 3) I have added SocketTimeoutException handling into 
>>> sun/security/ssl/SSLSocketImpl/SSLExceptionForIOIssue.java jtreg 
>>> test. This test also verifies SocketExceptions during handshake.
>>> Webrev for JDK11 : http://cr.openjdk.java.net/~yan/8239788/webrev.1/
>>> Webrev for JDK15 : http://cr.openjdk.java.net/~yan/8239788/webrev.2/
>>> Tested with sun/security/ssl and javax/net/ssl jtreg tests
>>> Thank you
>>> Alexey
>>>> On 25 Feb 2020, at 19:42, Xuelei Fan <xuelei.fan at oracle.com 
>>>> <mailto:xuelei.fan at oracle.com>> wrote:
>>>>
>>>>> JDK11 webrev: http://cr.openjdk.java.net/~yan/8239788/webrev.0/
>>>> Maybe, the getSession() could throw InterruptedIOException as well.
>>>>
>>>> I may try to catch the InterruptedIOException, rather than handle it 
>>>> in the IOException catching block.
>>>>
>>>> Thanks,
>>>> Xuelei
>>>>
>>>> On 2/24/2020 11:04 AM, Alexey Bakhtin wrote:
>>>>> Hi Xuelei,
>>>>> Thank you. It would be glad if you can review this fix.
>>>>> The patch almost cleanly applied to JDK15.
>>>>> Also, As Kumar mentioned, the patch does not include regression test.
>>>>> I’m going to add regression test and patch for JDK15 tomorrow.
>>>>> Thank you.
>>>>> Alexey
>>>>>> On 24 Feb 2020, at 21:41, Xuelei Fan <xuelei.fan at oracle.com 
>>>>>> <mailto:xuelei.fan at oracle.com>> wrote:
>>>>>>
>>>>>> Hi Alexey,
>>>>>>
>>>>>> Thanks for working on this issue.  Do you plan to fix it for JDK 
>>>>>> 15, the current JDK reposiroty?
>>>>>>
>>>>>> I need more time for evaluate the fix.  For example, I'm not sure 
>>>>>> if we could always throw InterruptedIOException to applications, 
>>>>>> even for getSession().
>>>>>>
>>>>>> Regards,
>>>>>> Xuelei
>>>>>>
>>>>>> On 2/24/2020 7:58 AM, Alexey Bakhtin wrote:
>>>>>>> Hello,
>>>>>>> I have been working on this issue for some time already.
>>>>>>> The patch below adds  java.net.SocketTimeoutException handling 
>>>>>>> during TLS handshake negotiation. This functionality seems to 
>>>>>>> have been missed during the TLSv1.3 implementation ( JDK-8196584 )
>>>>>>> Tested with JDK11 and higher.
>>>>>>> JDK11 webrev: http://cr.openjdk.java.net/~yan/8239788/webrev.0/
>>>>>>> jbs: https://bugs.openjdk.java.net/browse/JDK-8239798 and 
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8239788
>>>>>>> Thank you,
>>>>>>> Alexey
>>>>>>>> On 22 Feb 2020, at 15:00, security-dev-request at openjdk.java.net 
>>>>>>>> <mailto:security-dev-request at openjdk.java.net> 
>>>>>>>> <mailto:security-dev-request at openjdk.java.net> wrote:
>>>>>>>>
>>>>>>>> I will look into the issue.
>>>>>>>>
>>>>>>>> BTW, I closed JDK-8239788 as duplicate of JDK-8239798.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Xuelei
>>>>>>>>
>>>>>>>> On 2/21/2020 9:24 AM, Kumar Srinivasan wrote:
>>>>>>>>> Hi security-folk,
>>>>>>>>>
>>>>>>>>> At VMware while upgrading our application to OpenJDK11u, we have
>>>>>>>>> encountered what seems to be a serious behavior issue.
>>>>>>>>> The issue AFAICT seems to have stemmed from the work for TLS1.3 and
>>>>>>>>> JDK-8196584 <https://bugs.openjdk.java.net/browse/JDK-8196584>.
>>>>>>>>>
>>>>>>>>> Overview:
>>>>>>>>> With OpenJDK11 the end-points are closed immediately with TLS 
>>>>>>>>> alerts
>>>>>>>>> raised when an exception is received.
>>>>>>>>> This is not the case with JDK8 the socket is not closed 
>>>>>>>>> allowing retries.
>>>>>>>>>
>>>>>>>>> I have filed: JDK-8239798 (with a reproducer), this issue was also
>>>>>>>>> reported ?to Azul and they have filed: JDK-8239788.
>>>>>>>>>
>>>>>>>>> Can you please evaluate this at the earliest, this is a serious 
>>>>>>>>> show
>>>>>>>>> stopper for VMware.
>>>>>>>>>
>>>>>>>>> Thank
>>>>>>>>> Kumar Srinivasan
>>>>>>>>> VMware
>>>>>>>>
> 