Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

org.eclipse.jetty.client.HttpClientTLSTest#testForcedNonDomainSNI fails on java17 #6624

Closed
gregw opened this issue Aug 16, 2021 · 6 comments · Fixed by #6634 or #6640
Closed

org.eclipse.jetty.client.HttpClientTLSTest#testForcedNonDomainSNI fails on java17 #6624

gregw opened this issue Aug 16, 2021 · 6 comments · Fixed by #6634 or #6640
Labels

Comments

@gregw
Copy link
Contributor

gregw commented Aug 16, 2021

Jetty version(s)
9, 10, 11

Java version/vendor (use: java -version)

openjdk version "17-ea" 2021-09-14

also

openjdk version "17" 2021-09-14
OpenJDK Runtime Environment (build 17+35-2724)
OpenJDK 64-Bit Server VM (build 17+35-2724, mixed mode, sharing)

OS type/version

linux

Description

Test org.eclipse.jetty.client.HttpClientTLSTest#testForcedNonDomainSNI fails on java17:

2021-08-17 09:29:13.894:WARN:oeji.ManagedSelector:client-31: java.lang.IllegalArgumentException: The encoded server name value is invalid
2021-08-17 09:29:13.903:INFO:oejs.AbstractConnector:main: Stopped ServerConnector@6279cee3{SSL, (ssl, http/1.1)}{0.0.0.0:0}

java.util.concurrent.ExecutionException: java.lang.IllegalArgumentException: The encoded server name value is invalid

	at org.eclipse.jetty.client.util.FutureResponseListener.getResult(FutureResponseListener.java:118)
	at org.eclipse.jetty.client.util.FutureResponseListener.get(FutureResponseListener.java:101)
	at org.eclipse.jetty.client.HttpRequest.send(HttpRequest.java:730)
	at org.eclipse.jetty.client.HttpClientTLSTest.testForcedNonDomainSNI(HttpClientTLSTest.java:1022)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:688)
	at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
	at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
	at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:210)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:206)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:131)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:65)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:129)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:127)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:126)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:84)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:143)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:129)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:127)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:126)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:84)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:143)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:129)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:127)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:126)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:84)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:108)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:96)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:75)
	at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:71)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:221)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)
Caused by: java.lang.IllegalArgumentException: The encoded server name value is invalid
	at java.base/javax.net.ssl.SNIHostName.<init>(SNIHostName.java:179)
	at org.eclipse.jetty.util.ssl.SslContextFactory$Client.getSniServerNames(SslContextFactory.java:2287)
	at org.eclipse.jetty.util.ssl.SslContextFactory$Client.access$400(SslContextFactory.java:2181)
	at org.eclipse.jetty.util.ssl.SslContextFactory$Client$SniProvider.lambda$static$0(SslContextFactory.java:2261)
	at org.eclipse.jetty.util.ssl.SslContextFactory$Client.customize(SslContextFactory.java:2217)
	at org.eclipse.jetty.util.ssl.SslContextFactory.newSSLEngine(SslContextFactory.java:1926)
	at org.eclipse.jetty.io.ssl.SslClientConnectionFactory.newConnection(SslClientConnectionFactory.java:129)
	at org.eclipse.jetty.client.AbstractConnectorHttpClientTransport$ClientSelectorManager.newConnection(AbstractConnectorHttpClientTransport.java:180)
	at org.eclipse.jetty.io.ManagedSelector.createEndPoint(ManagedSelector.java:386)
	at org.eclipse.jetty.io.ManagedSelector.access$2100(ManagedSelector.java:65)
	at org.eclipse.jetty.io.ManagedSelector$CreateEndPoint.run(ManagedSelector.java:1069)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:883)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1034)
	at java.base/java.lang.Thread.run(Thread.java:831)
Caused by: java.lang.IllegalArgumentException: Contains non-LDH ASCII characters
	at java.base/java.net.IDN.toASCIIInternal(IDN.java:296)
	at java.base/java.net.IDN.toASCII(IDN.java:122)
	at java.base/javax.net.ssl.SNIHostName.<init>(SNIHostName.java:175)
	... 13 more
@gregw gregw added the Bug For general bugs on Jetty side label Aug 16, 2021
@gregw
Copy link
Contributor Author

gregw commented Aug 16, 2021

It is failing on an IPv6 hostname: host='0:0:0:0:0:0:0:1'

@gregw
Copy link
Contributor Author

gregw commented Aug 16, 2021

Issue is due to a "work around" of JVM host name validation:

        private static List<SNIServerName> getSniServerNames(SSLEngine sslEngine, List<SNIServerName> serverNames)
        {
            if (serverNames.isEmpty())
            {
                String host = sslEngine.getPeerHost();
                if (host != null)
                {
                    // Must use the byte[] constructor, because the character ':' is forbidden when
                    // using the String constructor (but typically present in IPv6 addresses).
                    return Collections.singletonList(new SNIHostName(host.getBytes(StandardCharsets.US_ASCII)));
                }
            }
            return serverNames;
        }

gregw added a commit that referenced this issue Aug 16, 2021
Temp disable of test that is breaking the build.
gregw added a commit that referenced this issue Aug 17, 2021
Temp disable of test that is breaking the build.
gregw added a commit that referenced this issue Aug 17, 2021
Fix flaky test from #6562
Disable ipv6 test for #6624
gregw added a commit that referenced this issue Aug 17, 2021
Temp disable of test that is breaking the build.
@gregw
Copy link
Contributor Author

gregw commented Aug 17, 2021

IPv6 part of the test commented out for now and marked with a TODO

gregw added a commit that referenced this issue Aug 17, 2021
Temp disable of test that is breaking the build.
gregw added a commit that referenced this issue Aug 17, 2021
Temp disable of test that is breaking the build.
sbordet added a commit that referenced this issue Aug 17, 2021
Java 17 only allows letter|digit|hyphen characters for SNI names.

While we could bypass this restriction on the client, when the SNI bytes arrive to the server they will be verified and if not allowed the TLS handshake will fail.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet
Copy link
Contributor

sbordet commented Aug 17, 2021

@gregw I put up another PR with a proper fix.

Unfortunately, Java 17 perform stricter checks on the SNI names.

We could bypass this on the client with a custom SNIServerName subclass, but on the server the SNI bytes are reconstructed into a SNIHostName, which perform the strict checks, so we are out of luck.

@joakime
Copy link
Contributor

joakime commented Aug 17, 2021

Why are we attempting to support IP literals in SNI?

It is not allowed per the specs.

Literal IPv4 and IPv6 addresses are not permitted in "HostName".

@sbordet
Copy link
Contributor

sbordet commented Aug 17, 2021

Why are we attempting to support IP literals in SNI?

Because it was a sponsored change.

@sbordet sbordet linked a pull request Aug 17, 2021 that will close this issue
sbordet added a commit that referenced this issue Aug 18, 2021
Java 17 only allows letter|digit|hyphen characters for SNI names.

While we could bypass this restriction on the client, when the SNI bytes arrive to the server they will be verified and if not allowed the TLS handshake will fail.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this issue Aug 18, 2021
Java 17 only allows letter|digit|hyphen characters for SNI names.

While we could bypass this restriction on the client, when the SNI bytes arrive to the server they will be verified and if not allowed the TLS handshake will fail.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
(cherry picked from commit 693663a)
@sbordet sbordet linked a pull request Aug 25, 2021 that will close this issue
@joakime joakime added Test and removed Bug For general bugs on Jetty side labels Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants