Re: OpenJDK11u: Backward incompatible behavior

2020-03-17 Thread Alexey Bakhtin
Hi Kumar, I have no OpenJDK Author account yet, so I had to ask another authors to publish my patches. All patches still available, but on different locations: [1] JDK11 patch: http://cr.openjdk.java.net/~yan/8239788/webrev.0/ [2] JDK11 patch: http://cr.openjdk.java.net/~yan/8239788/webrev.1/ [3

Re: OpenJDK11u: Backward incompatible behavior

2020-03-16 Thread Kumar Srinivasan
Hi Alexey - I was trying to understand the fix for the "Unexpected number of plaintext bytes” issue. But it appears that the earlier iterations of the webrevs have disappeared, only webrev.5 is available in [1] In the future it would be a good practice, to retain all the webrevs for sometime.

Re: OpenJDK11u: Backward incompatible behavior

2020-03-11 Thread Xuelei Fan
Hi Alexey, I had run the testing for you, no surprise. Please commit to JDK 15, and backport accordingly. Thanks, Xuelei On 3/11/2020 7:16 AM, Alexey Bakhtin wrote: Hello Xuelei, Thank you for review. Can I commit it to JDK15 and create backports to JDK 14, 13 and 11 ? Thank you Alexey

Re: OpenJDK11u: Backward incompatible behavior

2020-03-11 Thread Alexey Bakhtin
Hello Xuelei, Thank you for review. Can I commit it to JDK15 and create backports to JDK 14, 13 and 11 ? Thank you Alexey > On 10 Mar 2020, at 20:59, Xuelei Fan wrote: > > Looks fine to me. > > Thanks, > Xuelei > > On 3/5/2020 8:50 AM, Alexey Bakhtin wrote: >> Hello Xuelei, >> I have renamed

Re: OpenJDK11u: Backward incompatible behavior

2020-03-10 Thread Xuelei Fan
Looks fine to me. Thanks, Xuelei On 3/5/2020 8:50 AM, Alexey Bakhtin wrote: Hello Xuelei, I have renamed inputBuffer to recordBody. Also, as you suggested, recordBody is not removed but used for multiple records. So, it should be better for performance. JDK15 webrev: http://cr.openjdk.java.n

Re: OpenJDK11u: Backward incompatible behavior

2020-03-05 Thread Alexey Bakhtin
Hello Xuelei, I have renamed inputBuffer to recordBody. Also, as you suggested, recordBody is not removed but used for multiple records. So, it should be better for performance. JDK15 webrev: http://cr.openjdk.java.net/~dcherepanov/8239788/webrev.v5/ Regards Alexey > On 4 Mar 2020, at 21:23, X

Re: OpenJDK11u: Backward incompatible behavior

2020-03-04 Thread Xuelei Fan
> http://cr.openjdk.java.net/~bae/8239788/webrev.v4/ SSLSocketInputRecord: 54 // Cache for incomplete input record. 55 private ByteBuffer inputBuffer = null; This variable is used for record body, I may use a instinctive name, for example recordBody. Otherwise, looks good to me. I

Re: OpenJDK11u: Backward incompatible behavior

2020-03-02 Thread Alexey Bakhtin
Hello Xuelei, Could you please review new version of the patch : http://cr.openjdk.java.net/~bae/8239788/webrev.v4/ I did not find any reasons for such getSession() behaviour. This code seems exists since initial TLSv1.2 implementation. As yo

Re: OpenJDK11u: Backward incompatible behavior

2020-03-01 Thread Xuelei Fan
On 2/28/2020 4:24 PM, Xuelei Fan wrote: SSLSocketInputRecord:   52 private byte[] inputBuffer = new byte[maxRecordSize]; Would you mind consider an improvement to use less memory? I think more about it. You previous idea may be better, which uses an additional buffer for caching. Maybe,

Re: OpenJDK11u: Backward incompatible behavior

2020-02-28 Thread Xuelei Fan
> 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 I

Re: OpenJDK11u: Backward incompatible behavior

2020-02-27 Thread Alexey Bakhtin
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 s

Re: OpenJDK11u: Backward incompatible behavior

2020-02-26 Thread Xuelei Fan
> 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

Re: OpenJDK11u: Backward incompatible behavior

2020-02-26 Thread Alexey Bakhtin
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

Re: OpenJDK11u: Backward incompatible behavior

2020-02-25 Thread Xuelei Fan
> 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 wr

Re: OpenJDK11u: Backward incompatible behavior

2020-02-25 Thread Kumar Srinivasan
Hi Alexey, Good catch, this kinda answers my question wrt. "Escape from Alcatraz” comment. I vaguely remembered some discussions around this in the Mantis/Tiger era in the Deployment team. https://bugs.openjdk.java.net/browse/JDK-4836493 The reason I was asking for a regression test, are all t

Re: OpenJDK11u: Backward incompatible behavior

2020-02-25 Thread Alexey Bakhtin
Hi Alan, It seems this issue could be related to jtreg test jdk/sun/security/ssl/SSLSocketImpl/ClientTimeout.java This test passed on JDK8 but fails on JDK11. The test was disabled by JDK-8196584 ( TLS 1.3 Implementation ) With patch provided this test works better but still fails. I’m trying to

Re: OpenJDK11u: Backward incompatible behavior

2020-02-25 Thread Alan Bateman
On 24/02/2020 16:32, Kumar Srinivasan wrote: Hi Alexey, Thanks for the update. I glossed over the changes. Observation: I don’t see a regression test in your webrev, is this still a work in progress ? I am very surprised, how a simple socket feature such as this, escape the JSN-test dragnet

Re: OpenJDK11u: Backward incompatible behavior

2020-02-24 Thread Alexey Bakhtin
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

Re: OpenJDK11u: Backward incompatible behavior

2020-02-24 Thread Xuelei Fan
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/202

Re: OpenJDK11u: Backward incompatible behavior

2020-02-24 Thread Kumar Srinivasan
Hi Alexey, Thanks for the update. I glossed over the changes. Observation: I don’t see a regression test in your webrev, is this still a work in progress ? I am very surprised, how a simple socket feature such as this, escape the JSN-test dragnet in the first place ? Kumar On Feb 24, 2020

Re: OpenJDK11u: Backward incompatible behavior

2020-02-24 Thread Alexey Bakhtin
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 web

Re: OpenJDK11u: Backward incompatible behavior

2020-02-21 Thread Xuelei Fan
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