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 <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> 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#L400> > https://github.com/netty/netty/blob/netty-4.1.28.Final/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java#L418 > > <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 >>> <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>> 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/> >>>>> https://bugs.openjdk.java.net/browse/JDK-8208166 >>>>> >>>>> Thanks, >>>>> Xuelei >> >
