After digging more this morning I noticed the test code did made some wrong assumptions which just worked out of luck before. After fixing the test everything passes now.
So +1 from me on the patch :) Also sorry for the false-alarm. Niorman > On 30. Jul 2018, at 22:23, Xuelei Fan <xuelei....@oracle.com> wrote: > > Would you mind send me the debug log (-Djavax.net.debug=all) and the > exception stacks? The "renegotiation" in TLS 1.3 is different from TLS 1.2 > and prior specifications. It would be helpful to me to find the cause of the > test failure. > > Thanks, > Xuelei > > On 7/30/2018 1:11 PM, Norman Maurer wrote: >> Sorry but I just noticed we still have a another integration test failing >> which tests that client SSL renegotiation is failing. This seems to be not >> the case anymore with java11 + your patch (it was in ea20 tho). >> https://github.com/netty/netty/blob/netty-4.1.28.Final/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketSslClientRenegotiateTest.java >> Let me know if I need to dig more into it. >> Bye >> Norman >>> On 30. Jul 2018, at 21:54, Norman Maurer <norman.mau...@googlemail.com >>> <mailto:norman.mau...@googlemail.com>> wrote: >>> >>> Hey Xuelei, >>> >>> I just re-ran our testsuite with your patch and everything pass except two >>> tests. After digging a bit I found that we needed to add explicit calls to >>> `SSLEngine.setUSeClientMode(false)` now in these test where we did not need >>> to do this before. >>> >>> The tests in question are: >>> >>> https://github.com/netty/netty/blob/netty-4.1.28.Final/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java#L400 >>> https://github.com/netty/netty/blob/netty-4.1.28.Final/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java#L418 >>> >>> Here we use SslContext.getDefault().createSSLEngine() and did not set the >>> mode explicitly before. With the following patch to netty all works when >>> using your patch: >>> >>> diff --git a/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java >>> b/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java >>> index e982b6a63..40d6e7b59 100644 >>> --- a/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java >>> +++ b/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java >>> @@ -398,7 +398,9 @@ public class SslHandlerTest { >>> @Test >>> public void testCloseFutureNotified() throws Exception { >>> - SslHandler handler = new >>> SslHandler(SSLContext.getDefault().createSSLEngine()); >>> + SSLEngine engine = SSLContext.getDefault().createSSLEngine(); >>> + engine.setUseClientMode(false); >>> + SslHandler handler = new SslHandler(engine); >>> EmbeddedChannel ch = new EmbeddedChannel(handler); >>> ch.close(); >>> @@ -417,6 +419,7 @@ public class SslHandlerTest { >>> @Test(timeout = 5000) >>> public void testEventsFired() throws Exception { >>> SSLEngine engine = SSLContext.getDefault().createSSLEngine(); >>> + engine.setUseClientMode(false); >>> final BlockingQueue<SslCompletionEvent> events = new >>> LinkedBlockingQueue<SslCompletionEvent>(); >>> EmbeddedChannel channel = new EmbeddedChannel(new >>> SslHandler(engine), new ChannelInboundHandlerAdapter() { >>> @Override >>> >>> >>> The exception we see without the patch is: >>> >>> java.lang.IllegalStateException: Client/Server mode has not yet been set. >>> at >>> java.base/sun.security.ssl.SSLEngineImpl.beginHandshake(SSLEngineImpl.java:98) >>> at io.netty.handler.ssl.SslHandler.handshake(SslHandler.java:1731) >>> at >>> io.netty.handler.ssl.SslHandler.startHandshakeProcessing(SslHandler.java:1644) >>> at io.netty.handler.ssl.SslHandler.handlerAdded(SslHandler.java:1634) >>> at >>> io.netty.channel.DefaultChannelPipeline.callHandlerAdded0(DefaultChannelPipeline.java:637) >>> at >>> io.netty.channel.DefaultChannelPipeline.addLast(DefaultChannelPipeline.java:235) >>> at >>> io.netty.channel.DefaultChannelPipeline.addLast(DefaultChannelPipeline.java:409) >>> at >>> io.netty.channel.DefaultChannelPipeline.addLast(DefaultChannelPipeline.java:396) >>> at >>> io.netty.channel.embedded.EmbeddedChannel$2.initChannel(EmbeddedChannel.java:203) >>> at >>> io.netty.channel.ChannelInitializer.initChannel(ChannelInitializer.java:115) >>> at >>> io.netty.channel.ChannelInitializer.handlerAdded(ChannelInitializer.java:107) >>> at >>> io.netty.channel.DefaultChannelPipeline.callHandlerAdded0(DefaultChannelPipeline.java:637) >>> at >>> io.netty.channel.DefaultChannelPipeline.access$000(DefaultChannelPipeline.java:46) >>> at >>> io.netty.channel.DefaultChannelPipeline$PendingHandlerAddedTask.execute(DefaultChannelPipeline.java:1487) >>> at >>> io.netty.channel.DefaultChannelPipeline.callHandlerAddedForAllHandlers(DefaultChannelPipeline.java:1161) >>> at >>> io.netty.channel.DefaultChannelPipeline.invokeHandlerAddedIfNeeded(DefaultChannelPipeline.java:686) >>> at >>> io.netty.channel.AbstractChannel$AbstractUnsafe.register0(AbstractChannel.java:510) >>> at >>> io.netty.channel.AbstractChannel$AbstractUnsafe.register(AbstractChannel.java:476) >>> at >>> io.netty.channel.embedded.EmbeddedChannel$EmbeddedUnsafe$1.register(EmbeddedChannel.java:773) >>> at >>> io.netty.channel.embedded.EmbeddedEventLoop.register(EmbeddedEventLoop.java:130) >>> at >>> io.netty.channel.embedded.EmbeddedEventLoop.register(EmbeddedEventLoop.java:124) >>> at io.netty.channel.embedded.EmbeddedChannel.setup(EmbeddedChannel.java:208) >>> at >>> io.netty.channel.embedded.EmbeddedChannel.<init>(EmbeddedChannel.java:167) >>> at >>> io.netty.channel.embedded.EmbeddedChannel.<init>(EmbeddedChannel.java:148) >>> at >>> io.netty.channel.embedded.EmbeddedChannel.<init>(EmbeddedChannel.java:135) >>> at >>> io.netty.channel.embedded.EmbeddedChannel.<init>(EmbeddedChannel.java:100) >>> at >>> io.netty.handler.ssl.SslHandlerTest.testCloseFutureNotified(SslHandlerTest.java:404) >>> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native >>> Method) >>> at >>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) >>> at >>> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) >>> at java.base/java.lang.reflect.Method.invoke(Method.java:566) >>> at >>> org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50) >>> at >>> org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) >>> at >>> org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47) >>> at >>> org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) >>> at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325) >>> at >>> org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78) >>> at >>> org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57) >>> at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290) >>> at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71) >>> at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288) >>> at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58) >>> at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268) >>> at org.junit.runners.ParentRunner.run(ParentRunner.java:363) >>> at org.junit.runner.JUnitCore.run(JUnitCore.java:137) >>> at >>> com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68) >>> at >>> com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47) >>> at >>> com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242) >>> at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70) >>> >>> >>> So I have no problem to patch our test-case but I wondered if this may >>> break others in other cases and so is a regression. >>> >>> Let me know what you think. >>> Norman >>> >>>> On 30. Jul 2018, at 20:06, Norman Maurer <norman.mau...@googlemail.com >>>> <mailto:norman.mau...@googlemail.com>> wrote: >>>> >>>> Will do and report back as soon as possible. >>>> >>>> Thanks >>>> Norman >>>> >>>> >>>>> On 30. Jul 2018, at 19:57, Xuelei Fan <xuelei....@oracle.com >>>>> <mailto:xuelei....@oracle.com>> wrote: >>>>> >>>>> Hi Norman, >>>>> >>>>> Would you mind look at the code I posted in the following thread: >>>>> http://mail.openjdk.java.net/pipermail/security-dev/2018-July/017708.html >>>>> >>>>> I appreciate if you could have a test by the end of this week. >>>>> >>>>> Note that with this update, a complete TLS connection should close both >>>>> inbound and outbound explicitly. However, existing applications may not >>>>> did this way. If the source code update is not available, please >>>>> consider to use the "jdk.tls.acknowledgeCloseNotify" as a workaround. >>>>> >>>>> Thanks, >>>>> Xuelei >>>>> >>>>> On 7/25/2018 11:22 PM, Norman Maurer wrote: >>>>>> Just FYI… I tested this patch via the netty ssl tests and we no longer >>>>>> see the class-cast-exception problems I reported before dso I think this >>>>>> solves the issue. >>>>>> That said we still encounter a few test-failures for tests that test >>>>>> behaviour of closing outbound of the SSLEngine but I think these are >>>>>> more related to >>>>>> http://mail.openjdk.java.net/pipermail/security-dev/2018-July/017633.html >>>>>> and >>>>>> http://mail.openjdk.java.net/pipermail/security-dev/2018-July/017566.html >>>>>> . >>>>>> Bye >>>>>> Norman >>>>>>> On 25. Jul 2018, at 20:30, Xuelei Fan <xuelei....@oracle.com >>>>>>> <mailto:xuelei....@oracle.com> <mailto:xuelei....@oracle.com>> wrote: >>>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> Please review the update for JDK-8208166: >>>>>>> http://cr.openjdk.java.net/~xuelei/8208166/webrev.00/ >>>>>>> <http://cr.openjdk.java.net/%7Exuelei/8208166/webrev.00/> >>>>>>> <http://cr.openjdk.java.net/%7Exuelei/8208166/webrev.00/> >>>>>>> https://bugs.openjdk.java.net/browse/JDK-8208166 >>>>>>> >>>>>>> Thanks, >>>>>>> Xuelei >>>> >>>
