Re: RFR: 8282600: SSLSocketImpl should not use user_canceled workaround when not necessary
On Wed, 4 May 2022 19:22:05 GMT, Bradford Wetmore wrote: >> When testing compatibility of jdk TLS implementation with gnutls, I have >> found a problem. The problem is, that gnutls does not like use of >> user_canceled alert when closing TLS-1.3 connection from duplexCloseOutput() >> (used by socket.close() unless shutdownOutput was called explicitly) and >> considers it error. (For more details see: [1]) >> >> As I understand it, usage of user_canceled alert before close is workaround >> for an issue of not being able to cleanly initialize full (duplex) close of >> TLS-1.3 connection (other side is not required to immediately close the >> after receiving close_notify, unlike in earlier TLS versions). Some legacy >> programs could probably hang or something, expecting socket.close to perform >> immediate duplex close. Problem is this is not what user_canceled alert is >> intended for [2] and it is therefore undefined how the other side handles >> this. (JDK itself replies to close_notify preceded by user_canceled alert by >> immediately closing its output [3].) >> >> This fix disables this workaround when it is not necessary (connection is >> already half-closed by the other side). This way it fixes my case (gnutls >> client connected to jdk server initiates close) and it should be safe. (As >> removing workaround altogether could probably reintroduce issues for legacy >> apps... ) >> >> I also ran jdk_security tests locally, which passed for me. >> >> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1918473 >> [2] https://datatracker.ietf.org/doc/html/rfc8446#section-6.1 >> [3] >> https://github.com/openjdk/jdk/blob/b6c35ae44aeb31deb7a15ee2939156ed00b3df52/src/java.base/share/classes/sun/security/ssl/Alert.java#L243 > > Infra + JCK passed. Looks good. @bradfordwetmore Thank you - PR: https://git.openjdk.java.net/jdk/pull/7664
Re: RFR: 8282600: SSLSocketImpl should not use user_canceled workaround when not necessary
On Wed, 2 Mar 2022 19:04:26 GMT, zzambers wrote: > When testing compatibility of jdk TLS implementation with gnutls, I have > found a problem. The problem is, that gnutls does not like use of > user_canceled alert when closing TLS-1.3 connection from duplexCloseOutput() > (used by socket.close() unless shutdownOutput was called explicitly) and > considers it error. (For more details see: [1]) > > As I understand it, usage of user_canceled alert before close is workaround > for an issue of not being able to cleanly initialize full (duplex) close of > TLS-1.3 connection (other side is not required to immediately close the after > receiving close_notify, unlike in earlier TLS versions). Some legacy programs > could probably hang or something, expecting socket.close to perform immediate > duplex close. Problem is this is not what user_canceled alert is intended for > [2] and it is therefore undefined how the other side handles this. (JDK > itself replies to close_notify preceded by user_canceled alert by immediately > closing its output [3].) > > This fix disables this workaround when it is not necessary (connection is > already half-closed by the other side). This way it fixes my case (gnutls > client connected to jdk server initiates close) and it should be safe. (As > removing workaround altogether could probably reintroduce issues for legacy > apps... ) > > I also ran jdk_security tests locally, which passed for me. > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1918473 > [2] https://datatracker.ietf.org/doc/html/rfc8446#section-6.1 > [3] > https://github.com/openjdk/jdk/blob/b6c35ae44aeb31deb7a15ee2939156ed00b3df52/src/java.base/share/classes/sun/security/ssl/Alert.java#L243 Infra + JCK passed. Looks good. - PR: https://git.openjdk.java.net/jdk/pull/7664
Re: RFR: 8282600: SSLSocketImpl should not use user_canceled workaround when not necessary
On Wed, 2 Mar 2022 19:04:26 GMT, zzambers wrote: > When testing compatibility of jdk TLS implementation with gnutls, I have > found a problem. The problem is, that gnutls does not like use of > user_canceled alert when closing TLS-1.3 connection from duplexCloseOutput() > (used by socket.close() unless shutdownOutput was called explicitly) and > considers it error. (For more details see: [1]) > > As I understand it, usage of user_canceled alert before close is workaround > for an issue of not being able to cleanly initialize full (duplex) close of > TLS-1.3 connection (other side is not required to immediately close the after > receiving close_notify, unlike in earlier TLS versions). Some legacy programs > could probably hang or something, expecting socket.close to perform immediate > duplex close. Problem is this is not what user_canceled alert is intended for > [2] and it is therefore undefined how the other side handles this. (JDK > itself replies to close_notify preceded by user_canceled alert by immediately > closing its output [3].) > > This fix disables this workaround when it is not necessary (connection is > already half-closed by the other side). This way it fixes my case (gnutls > client connected to jdk server initiates close) and it should be safe. (As > removing workaround altogether could probably reintroduce issues for legacy > apps... ) > > I also ran jdk_security tests locally, which passed for me. > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1918473 > [2] https://datatracker.ietf.org/doc/html/rfc8446#section-6.1 > [3] > https://github.com/openjdk/jdk/blob/b6c35ae44aeb31deb7a15ee2939156ed00b3df52/src/java.base/share/classes/sun/security/ssl/Alert.java#L243 tier1/tier2 tests pass. Did not try infra or JCK yet. - PR: https://git.openjdk.java.net/jdk/pull/7664
Re: RFR: 8282600: SSLSocketImpl should not use user_canceled workaround when not necessary
On Wed, 2 Mar 2022 19:04:26 GMT, zzambers wrote: > When testing compatibility of jdk TLS implementation with gnutls, I have > found a problem. The problem is, that gnutls does not like use of > user_canceled alert when closing TLS-1.3 connection from duplexCloseOutput() > (used by socket.close() unless shutdownOutput was called explicitly) and > considers it error. (For more details see: [1]) > > As I understand it, usage of user_canceled alert before close is workaround > for an issue of not being able to cleanly initialize full (duplex) close of > TLS-1.3 connection (other side is not required to immediately close the after > receiving close_notify, unlike in earlier TLS versions). Some legacy programs > could probably hang or something, expecting socket.close to perform immediate > duplex close. Problem is this is not what user_canceled alert is intended for > [2] and it is therefore undefined how the other side handles this. (JDK > itself replies to close_notify preceded by user_canceled alert by immediately > closing its output [3].) > > This fix disables this workaround when it is not necessary (connection is > already half-closed by the other side). This way it fixes my case (gnutls > client connected to jdk server initiates close) and it should be safe. (As > removing workaround altogether could probably reintroduce issues for legacy > apps... ) > > I also ran jdk_security tests locally, which passed for me. > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1918473 > [2] https://datatracker.ietf.org/doc/html/rfc8446#section-6.1 > [3] > https://github.com/openjdk/jdk/blob/b6c35ae44aeb31deb7a15ee2939156ed00b3df52/src/java.base/share/classes/sun/security/ssl/Alert.java#L243 Thanks for the extra time to review this change. I'm still wondering if there is better pattern that doesn't use user_canceled, but that doesn't need to delay this fix from going in. - Marked as reviewed by wetmore (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7664
Re: RFR: 8282600: SSLSocketImpl should not use user_canceled workaround when not necessary
On Wed, 2 Mar 2022 19:04:26 GMT, zzambers wrote: > When testing compatibility of jdk TLS implementation with gnutls, I have > found a problem. The problem is, that gnutls does not like use of > user_canceled alert when closing TLS-1.3 connection from duplexCloseOutput() > (used by socket.close() unless shutdownOutput was called explicitly) and > considers it error. (For more details see: [1]) > > As I understand it, usage of user_canceled alert before close is workaround > for an issue of not being able to cleanly initialize full (duplex) close of > TLS-1.3 connection (other side is not required to immediately close the after > receiving close_notify, unlike in earlier TLS versions). Some legacy programs > could probably hang or something, expecting socket.close to perform immediate > duplex close. Problem is this is not what user_canceled alert is intended for > [2] and it is therefore undefined how the other side handles this. (JDK > itself replies to close_notify preceded by user_canceled alert by immediately > closing its output [3].) > > This fix disables this workaround when it is not necessary (connection is > already half-closed by the other side). This way it fixes my case (gnutls > client connected to jdk server initiates close) and it should be safe. (As > removing workaround altogether could probably reintroduce issues for legacy > apps... ) > > I also ran jdk_security tests locally, which passed for me. > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1918473 > [2] https://datatracker.ietf.org/doc/html/rfc8446#section-6.1 > [3] > https://github.com/openjdk/jdk/blob/b6c35ae44aeb31deb7a15ee2939156ed00b3df52/src/java.base/share/classes/sun/security/ssl/Alert.java#L243 Resettting the clock. Sorry for the delay. - PR: https://git.openjdk.java.net/jdk/pull/7664
Re: RFR: 8282600: SSLSocketImpl should not use user_canceled workaround when not necessary
On Wed, 2 Mar 2022 19:04:26 GMT, zzambers wrote: > When testing compatibility of jdk TLS implementation with gnutls, I have > found a problem. The problem is, that gnutls does not like use of > user_canceled alert when closing TLS-1.3 connection from duplexCloseOutput() > (used by socket.close() unless shutdownOutput was called explicitly) and > considers it error. (For more details see: [1]) > > As I understand it, usage of user_canceled alert before close is workaround > for an issue of not being able to cleanly initialize full (duplex) close of > TLS-1.3 connection (other side is not required to immediately close the after > receiving close_notify, unlike in earlier TLS versions). Some legacy programs > could probably hang or something, expecting socket.close to perform immediate > duplex close. Problem is this is not what user_canceled alert is intended for > [2] and it is therefore undefined how the other side handles this. (JDK > itself replies to close_notify preceded by user_canceled alert by immediately > closing its output [3].) > > This fix disables this workaround when it is not necessary (connection is > already half-closed by the other side). This way it fixes my case (gnutls > client connected to jdk server initiates close) and it should be safe. (As > removing workaround altogether could probably reintroduce issues for legacy > apps... ) > > I also ran jdk_security tests locally, which passed for me. > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1918473 > [2] https://datatracker.ietf.org/doc/html/rfc8446#section-6.1 > [3] > https://github.com/openjdk/jdk/blob/b6c35ae44aeb31deb7a15ee2939156ed00b3df52/src/java.base/share/classes/sun/security/ssl/Alert.java#L243 Thanks, been working on another close issue. I should be finishing that shortly, - PR: https://git.openjdk.java.net/jdk/pull/7664
Re: RFR: 8282600: SSLSocketImpl should not use user_canceled workaround when not necessary
On Wed, 9 Mar 2022 21:12:06 GMT, Bradford Wetmore wrote: >> When testing compatibility of jdk TLS implementation with gnutls, I have >> found a problem. The problem is, that gnutls does not like use of >> user_canceled alert when closing TLS-1.3 connection from duplexCloseOutput() >> (used by socket.close() unless shutdownOutput was called explicitly) and >> considers it error. (For more details see: [1]) >> >> As I understand it, usage of user_canceled alert before close is workaround >> for an issue of not being able to cleanly initialize full (duplex) close of >> TLS-1.3 connection (other side is not required to immediately close the >> after receiving close_notify, unlike in earlier TLS versions). Some legacy >> programs could probably hang or something, expecting socket.close to perform >> immediate duplex close. Problem is this is not what user_canceled alert is >> intended for [2] and it is therefore undefined how the other side handles >> this. (JDK itself replies to close_notify preceded by user_canceled alert by >> immediately closing its output [3].) >> >> This fix disables this workaround when it is not necessary (connection is >> already half-closed by the other side). This way it fixes my case (gnutls >> client connected to jdk server initiates close) and it should be safe. (As >> removing workaround altogether could probably reintroduce issues for legacy >> apps... ) >> >> I also ran jdk_security tests locally, which passed for me. >> >> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1918473 >> [2] https://datatracker.ietf.org/doc/html/rfc8446#section-6.1 >> [3] >> https://github.com/openjdk/jdk/blob/b6c35ae44aeb31deb7a15ee2939156ed00b3df52/src/java.base/share/classes/sun/security/ssl/Alert.java#L243 > > I am looking at a few things in this PR. Please ping me before integrating. > > I believe this code is correct, but checking on the overall USER_CANCELED > strategy. @bradfordwetmore, it is ready from my side, but please take your time to do any checks, which you find necessary to make sure there is no problem here. - PR: https://git.openjdk.java.net/jdk/pull/7664
Re: RFR: 8282600: SSLSocketImpl should not use user_canceled workaround when not necessary
On Wed, 2 Mar 2022 19:04:26 GMT, zzambers wrote: > When testing compatibility of jdk TLS implementation with gnutls, I have > found a problem. The problem is, that gnutls does not like use of > user_canceled alert when closing TLS-1.3 connection from duplexCloseOutput() > (used by socket.close() unless shutdownOutput was called explicitly) and > considers it error. (For more details see: [1]) > > As I understand it, usage of user_canceled alert before close is workaround > for an issue of not being able to cleanly initialize full (duplex) close of > TLS-1.3 connection (other side is not required to immediately close the after > receiving close_notify, unlike in earlier TLS versions). Some legacy programs > could probably hang or something, expecting socket.close to perform immediate > duplex close. Problem is this is not what user_canceled alert is intended for > [2] and it is therefore undefined how the other side handles this. (JDK > itself replies to close_notify preceded by user_canceled alert by immediately > closing its output [3].) > > This fix disables this workaround when it is not necessary (connection is > already half-closed by the other side). This way it fixes my case (gnutls > client connected to jdk server initiates close) and it should be safe. (As > removing workaround altogether could probably reintroduce issues for legacy > apps... ) > > I also ran jdk_security tests locally, which passed for me. > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1918473 > [2] https://datatracker.ietf.org/doc/html/rfc8446#section-6.1 > [3] > https://github.com/openjdk/jdk/blob/b6c35ae44aeb31deb7a15ee2939156ed00b3df52/src/java.base/share/classes/sun/security/ssl/Alert.java#L243 I am looking at a few things in this PR. Please ping me before integrating. I believe this code is correct, but checking on the overall USER_CANCELED strategy. - PR: https://git.openjdk.java.net/jdk/pull/7664
Re: RFR: 8282600: SSLSocketImpl should not use user_canceled workaround when not necessary
On Thu, 3 Mar 2022 15:40:31 GMT, Xue-Lei Andrew Fan wrote: >> When testing compatibility of jdk TLS implementation with gnutls, I have >> found a problem. The problem is, that gnutls does not like use of >> user_canceled alert when closing TLS-1.3 connection from duplexCloseOutput() >> (used by socket.close() unless shutdownOutput was called explicitly) and >> considers it error. (For more details see: [1]) >> >> As I understand it, usage of user_canceled alert before close is workaround >> for an issue of not being able to cleanly initialize full (duplex) close of >> TLS-1.3 connection (other side is not required to immediately close the >> after receiving close_notify, unlike in earlier TLS versions). Some legacy >> programs could probably hang or something, expecting socket.close to perform >> immediate duplex close. Problem is this is not what user_canceled alert is >> intended for [2] and it is therefore undefined how the other side handles >> this. (JDK itself replies to close_notify preceded by user_canceled alert by >> immediately closing its output [3].) >> >> This fix disables this workaround when it is not necessary (connection is >> already half-closed by the other side). This way it fixes my case (gnutls >> client connected to jdk server initiates close) and it should be safe. (As >> removing workaround altogether could probably reintroduce issues for legacy >> apps... ) >> >> I also ran jdk_security tests locally, which passed for me. >> >> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1918473 >> [2] https://datatracker.ietf.org/doc/html/rfc8446#section-6.1 >> [3] >> https://github.com/openjdk/jdk/blob/b6c35ae44aeb31deb7a15ee2939156ed00b3df52/src/java.base/share/classes/sun/security/ssl/Alert.java#L243 > > Did you want to add a test? Otherwise, please add a noreg- label in the bug > JDK-8282600. @XueleiFan thank you for review. I have added noreg-external tag and some explanation to Openjdk bug. - PR: https://git.openjdk.java.net/jdk/pull/7664
Re: RFR: 8282600: SSLSocketImpl should not use user_canceled workaround when not necessary
On Wed, 2 Mar 2022 19:04:26 GMT, zzambers wrote: > When testing compatibility of jdk TLS implementation with gnutls, I have > found a problem. The problem is, that gnutls does not like use of > user_canceled alert when closing TLS-1.3 connection from duplexCloseOutput() > (used by socket.close() unless shutdownOutput was called explicitly) and > considers it error. (For more details see: [1]) > > As I understand it, usage of user_canceled alert before close is workaround > for an issue of not being able to cleanly initialize full (duplex) close of > TLS-1.3 connection (other side is not required to immediately close the after > receiving close_notify, unlike in earlier TLS versions). Some legacy programs > could probably hang or something, expecting socket.close to perform immediate > duplex close. Problem is this is not what user_canceled alert is intended for > [2] and it is therefore undefined how the other side handles this. (JDK > itself replies to close_notify preceded by user_canceled alert by immediately > closing its output [3].) > > This fix disables this workaround when it is not necessary (connection is > already half-closed by the other side). This way it fixes my case (gnutls > client connected to jdk server initiates close) and it should be safe. (As > removing workaround altogether could probably reintroduce issues for legacy > apps... ) > > I also ran jdk_security tests locally, which passed for me. > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1918473 > [2] https://datatracker.ietf.org/doc/html/rfc8446#section-6.1 > [3] > https://github.com/openjdk/jdk/blob/b6c35ae44aeb31deb7a15ee2939156ed00b3df52/src/java.base/share/classes/sun/security/ssl/Alert.java#L243 It looks like a nice change to me. Thank you! Did you want to add a test? Otherwise, please add a noreg- label in the bug JDK-8282600. - Marked as reviewed by xuelei (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7664
Re: RFR: 8282600: SSLSocketImpl should not use user_canceled workaround when not necessary
On Thu, 3 Mar 2022 10:45:02 GMT, Severin Gehwolf wrote: >> When testing compatibility of jdk TLS implementation with gnutls, I have >> found a problem. The problem is, that gnutls does not like use of >> user_canceled alert when closing TLS-1.3 connection from duplexCloseOutput() >> (used by socket.close() unless shutdownOutput was called explicitly) and >> considers it error. (For more details see: [1]) >> >> As I understand it, usage of user_canceled alert before close is workaround >> for an issue of not being able to cleanly initialize full (duplex) close of >> TLS-1.3 connection (other side is not required to immediately close the >> after receiving close_notify, unlike in earlier TLS versions). Some legacy >> programs could probably hang or something, expecting socket.close to perform >> immediate duplex close. Problem is this is not what user_canceled alert is >> intended for [2] and it is therefore undefined how the other side handles >> this. (JDK itself replies to close_notify preceded by user_canceled alert by >> immediately closing its output [3].) >> >> This fix disables this workaround when it is not necessary (connection is >> already half-closed by the other side). This way it fixes my case (gnutls >> client connected to jdk server initiates close) and it should be safe. (As >> removing workaround altogether could probably reintroduce issues for legacy >> apps... ) >> >> I also ran jdk_security tests locally, which passed for me. >> >> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1918473 >> [2] https://datatracker.ietf.org/doc/html/rfc8446#section-6.1 >> [3] >> https://github.com/openjdk/jdk/blob/b6c35ae44aeb31deb7a15ee2939156ed00b3df52/src/java.base/share/classes/sun/security/ssl/Alert.java#L243 > > @zzambers I've create https://bugs.openjdk.java.net/browse/JDK-8282600 for > this issue. Please reference it in the PR title. @jerboaa thank you - PR: https://git.openjdk.java.net/jdk/pull/7664
Re: RFR: 8282600: SSLSocketImpl should not use user_canceled workaround when not necessary
On Wed, 2 Mar 2022 19:04:26 GMT, zzambers wrote: > When testing compatibility of jdk TLS implementation with gnutls, I have > found a problem. The problem is, that gnutls does not like use of > user_canceled alert when closing TLS-1.3 connection from duplexCloseOutput() > (used by socket.close() unless shutdownOutput was called explicitly) and > considers it error. (For more details see: [1]) > > As I understand it, usage of user_canceled alert before close is workaround > for an issue of not being able to cleanly initialize full (duplex) close of > TLS-1.3 connection (other side is not required to immediately close the after > receiving close_notify, unlike in earlier TLS versions). Some legacy programs > could probably hang or something, expecting socket.close to perform immediate > duplex close. Problem is this is not what user_canceled alert is intended for > [2] and it is therefore undefined how the other side handles this. (JDK > itself replies to close_notify preceded by user_canceled alert by immediately > closing its output [3].) > > This fix disables this workaround when it is not necessary (connection is > already half-closed by the other side). This way it fixes my case (gnutls > client connected to jdk server initiates close) and it should be safe. (As > removing workaround altogether could probably reintroduce issues for legacy > apps... ) > > I also ran jdk_security tests locally, which passed for me. > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1918473 > [2] https://datatracker.ietf.org/doc/html/rfc8446#section-6.1 > [3] > https://github.com/openjdk/jdk/blob/b6c35ae44aeb31deb7a15ee2939156ed00b3df52/src/java.base/share/classes/sun/security/ssl/Alert.java#L243 @zzambers I've create https://bugs.openjdk.java.net/browse/JDK-8282600 for this issue. Please reference it in the PR title. - PR: https://git.openjdk.java.net/jdk/pull/7664
