clj-commons / aleph

Asynchronous streaming communication for Clojure - web server, web client, and raw TCP/UDP
http://aleph.io
MIT License
2.54k stars 241 forks source link

`test-failed-ssl-handshake` broken on Java 8 #647

Closed KingMob closed 1 year ago

KingMob commented 1 year ago
lein test :only aleph.tcp-ssl-test/test-failed-ssl-handshake

ERROR in (test-failed-ssl-handshake) (Alert.java:131)
Uncaught exception, not in assertion.
expected: nil
  actual: javax.net.ssl.SSLHandshakeException: Received fatal alert: handshake_failure
 at sun.security.ssl.Alert.createSSLException (Alert.java:131)
    sun.security.ssl.Alert.createSSLException (Alert.java:117)
    sun.security.ssl.TransportContext.fatal (TransportContext.java:311)
    sun.security.ssl.Alert$AlertConsumer.consume (Alert.java:293)
    sun.security.ssl.TransportContext.dispatch (TransportContext.java:185)
    sun.security.ssl.SSLTransport.decode (SSLTransport.java:152)
    sun.security.ssl.SSLEngineImpl.decode (SSLEngineImpl.java:588)
    sun.security.ssl.SSLEngineImpl.readRecord (SSLEngineImpl.java:544)
    sun.security.ssl.SSLEngineImpl.unwrap (SSLEngineImpl.java:411)
    sun.security.ssl.SSLEngineImpl.unwrap (SSLEngineImpl.java:390)
    javax.net.ssl.SSLEngine.unwrap (SSLEngine.java:626)
    io.netty.handler.ssl.SslHandler$SslEngineType$3.unwrap (SslHandler.java:296)
    io.netty.handler.ssl.SslHandler.unwrap (SslHandler.java:1343)
    io.netty.handler.ssl.SslHandler.decodeJdkCompatible (SslHandler.java:1236)
    io.netty.handler.ssl.SslHandler.decode (SslHandler.java:1285)
    io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection (ByteToMessageDecoder.java:519)
    io.netty.handler.codec.ByteToMessageDecoder.callDecode (ByteToMessageDecoder.java:458)
    io.netty.handler.codec.ByteToMessageDecoder.channelRead (ByteToMessageDecoder.java:280)
    io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead (AbstractChannelHandlerContext.java:379)
    io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead (AbstractChannelHandlerContext.java:365)
    io.netty.channel.AbstractChannelHandlerContext.fireChannelRead (AbstractChannelHandlerContext.java:357)
    io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead (DefaultChannelPipeline.java:1410)
    io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead (AbstractChannelHandlerContext.java:379)
    io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead (AbstractChannelHandlerContext.java:365)
    io.netty.channel.DefaultChannelPipeline.fireChannelRead (DefaultChannelPipeline.java:919)
    io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read (AbstractNioByteChannel.java:166)
    io.netty.channel.nio.NioEventLoop.processSelectedKey (NioEventLoop.java:788)
    io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized (NioEventLoop.java:724)
    io.netty.channel.nio.NioEventLoop.processSelectedKeys (NioEventLoop.java:650)
    io.netty.channel.nio.NioEventLoop.run (NioEventLoop.java:562)
    io.netty.util.concurrent.SingleThreadEventExecutor$4.run (SingleThreadEventExecutor.java:997)
    io.netty.util.internal.ThreadExecutorMap$2.run (ThreadExecutorMap.java:74)
    manifold.executor$thread_factory$reify__1252$f__1253.invoke (executor.clj:70)
    clojure.lang.AFn.run (AFn.java:22)
    io.netty.util.concurrent.FastThreadLocalRunnable.run (FastThreadLocalRunnable.java:30)
    java.lang.Thread.run (Thread.java:750)

Ran 1 tests containing 1 assertions.
0 failures, 1 errors.
Subprocess failed (exit code: 1)
KingMob commented 1 year ago

@DerGuteMoritz I believe you're the most familiar with this, can you take a look?

It appears the new CircleCI img defaults to JDK 17. We should also update the image name with the openjdk tag to use Java 8: https://hub.docker.com/r/cimg/clojure

KingMob commented 1 year ago

(Docker image would be cimg/clojure:1.11.1-openjdk-8.0)

DerGuteMoritz commented 1 year ago

Thanks, I'll have a look!

DerGuteMoritz commented 1 year ago

OK, turns out this is caused by the fact that with the JDK 8, Netty defaults to TLSv1.2 only. You can see it in the log output when netty is loaded:

DEBUG io.netty.handler.ssl.JdkSslContext - Default protocols (JDK): [TLSv1.2] 

With JDK 11 and 17 we get this instead:

DEBUG io.netty.handler.ssl.JdkSslContext - Default protocols (JDK): [TLSv1.3, TLSv1.2] 

Now, this matters because the handshake protocol has changed a lot between these versions which results in a different kind of error behavior. Quoting from my recent commit on the issue:

Unfortunately, since TLSv1.3 the connection may still fail with a handshake error even after the handshake has seemingly succeeded. This is because only an invalid certificate is explicitly rejected while a valid certificate will be implicitly accepted. Thus, it's only possible to determine whether the handshake has fully succeeded after having succeessfully read some data. See [1] and [2] for more details. [...]

1: https://github.com/netty/netty/issues/10502 2: https://stackoverflow.com/questions/62459802/tls-1-3-client-does-not-report-failed-handshake-when-client-certificate-verifica/62465859#62465859

The test case is written against the delayed TLSv1.3 error behavior while on JDK 8 we now see the (more eager) pre-TLSv1.3 error behavior which doesn't match the test assertions. One way to solve this would be to make it use TLSv1.3 even on JDK 8 which is indeed supported, just not enabled by default (probably because it was an early version). WDYT?

DerGuteMoritz commented 1 year ago

Given this kind of diverging behavior is possible: Perhaps we should also think about running the test suite against all LTS JDK versions supported by Clojure itself in CI?