Code Review Request, JDK-8208166, Still unable to use custom SSLEngine with default TrustManagerFactory after JDK-8207029

Xuelei Fan xuelei.fan at oracle.com
Mon Jul 30 20:23:38 UTC 2018


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.maurer at googlemail.com 
>> <mailto:norman.maurer at 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.maurer at googlemail.com <mailto:norman.maurer at googlemail.com>> 
>>> wrote:
>>>
>>> Will do and report back as soon as possible.
>>>
>>> Thanks
>>> Norman
>>>
>>>
>>>> On 30. Jul 2018, at 19:57, Xuelei Fan <xuelei.fan at oracle.com 
>>>> <mailto:xuelei.fan at 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.fan at oracle.com 
>>>>>> <mailto:xuelei.fan at oracle.com> <mailto:xuelei.fan at 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
>>>
>>
> 


More information about the security-dev mailing list