SslMasterKeyHandler.WiresharkSslMasterKeyHandler key log feature failure with TLSv1.3 (exception throws)

[huangqiangxiong]

Hi, @daschl @fzakaria could you help to locate the bug about master key log feature with TLSv1.3?

Expected behavior

Exporting secret likes:

Note following information comes from Wireshark code:

     *   - "CLIENT_EARLY_TRAFFIC_SECRET xxxx yyyy"
     *   - "CLIENT_HANDSHAKE_TRAFFIC_SECRET xxxx yyyy"
     *   - "SERVER_HANDSHAKE_TRAFFIC_SECRET xxxx yyyy"
     *   - "CLIENT_TRAFFIC_SECRET_0 xxxx yyyy"
     *   - "SERVER_TRAFFIC_SECRET_0 xxxx yyyy"
     *   - "EARLY_EXPORTER_SECRET xxxx yyyy"
     *   - "EXPORTER_SECRET xxxx yyyy"
     *     Where xxxx is the client_random from the ClientHello (hex-encoded)
     *     Where yyyy is the secret (hex-encoded) derived from the early,
     *     handshake or master secrets. (This format is introduced with TLS 1.3
     *     and supported by BoringSSL.)

Actual behavior

Throw exception:

java.lang.IllegalArgumentException: An invalid length master key was provided.
	at io.netty.handler.ssl.SslMasterKeyHandler$WiresharkSslMasterKeyHandler.accept(SslMasterKeyHandler.java:184)
	at io.netty.handler.ssl.SslMasterKeyHandler.userEventTriggered(SslMasterKeyHandler.java:140)
	at io.netty.channel.AbstractChannelHandlerContext.invokeUserEventTriggered(AbstractChannelHandlerContext.java:346)
	at io.netty.channel.AbstractChannelHandlerContext.invokeUserEventTriggered(AbstractChannelHandlerContext.java:332)
	at io.netty.channel.AbstractChannelHandlerContext.fireUserEventTriggered(AbstractChannelHandlerContext.java:324)
	at io.netty.handler.ssl.ApplicationProtocolNegotiationHandler.userEventTriggered(ApplicationProtocolNegotiationHandler.java:112)
	at io.netty.channel.AbstractChannelHandlerContext.invokeUserEventTriggered(AbstractChannelHandlerContext.java:346)
	at io.netty.channel.AbstractChannelHandlerContext.invokeUserEventTriggered(AbstractChannelHandlerContext.java:332)
	at io.netty.channel.AbstractChannelHandlerContext.fireUserEventTriggered(AbstractChannelHandlerContext.java:324)
	at io.netty.handler.ssl.SslHandler.setHandshakeSuccess(SslHandler.java:1806)
	at io.netty.handler.ssl.SslHandler.wrapNonAppData(SslHandler.java:991)
	at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1443)
	at io.netty.handler.ssl.SslHandler.decodeNonJdkCompatible(SslHandler.java:1287)
	at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1324)
	at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:501)
	at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:440)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:276)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
	at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
	at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:714)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:650)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:576)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:493)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.lang.Thread.run(Thread.java:748)

This bug also causes grpc/grpc-java#7724 merge (that is for grpc/grpc-java#7199) to be reverted.

Steps to reproduce

Enable TLS (default is TLS1.3 in current version), and enable master key log with SslMasterKeyHandler.newWireSharkSslMasterKeyHandler:

//In Http2Client.java
System.setProperty(SslMasterKeyHandler.SYSTEM_PROP_KEY, Boolean.TRUE.toString());
//...
//In Http2ClientInitializer.java
pipeline.addLast(SslMasterKeyHandler.newWireSharkSslMasterKeyHandler());

The exception will be throwed.

Minimal yet complete reproducer code (or URL to code)

To running the TLS server and client example code (that are modified based on official example):
netty-test.tar.gz

Note that it is a eclipse project with pom.xml.

Run /netty-test/netty-http2-example/io/netty/example/http2/helloworld/server/Http2Server.java first,
and then run /netty-test/netty-http2-example/io/netty/example/http2/helloworld/client/Http2Client.java

You will get the exception.

But if you force to TLS1.2 by

//In Http2Client.java
sslCtx = SslContextBuilder.forClient()
                   .sslProvider(provider)
                   ...
                   .protocols("TLSv1.2")
                   .build();

The feature will work fine.

Since 4.1.52.Final is TLS1.3 enabled by default. Unlike TLS1.2, the length of master key in TLS1.3 seems to be 32 not 48, and the sessionId is also wrong to be empty.

Suggestion: The SSL_CTX_set_keylog_callback is inroduced since openssl 1.1.1 (also in boringssl) that can be used to export the secret materials into key log files (which can be used with Wireshark). And I highly recommend to add method like setKeyLogCallback() or addKeyLogCallback() in SslContextBuilder instead of registering handler in pipeline. Because in some projects using netty like https://github.com/grpc/grpc-java, user can only access SslContextBuilder, but can not access SocketChannel.

Netty version

4.1.52.Final

JVM version (e.g. java -version)

open jdk 1.8

OS version (e.g. uname -a)

win10

[fzakaria]

Hey -- sorry I've been busy. This dropped off my radar.
Your bug report looks solid & you did most of the investigation already :)

Hmm from looking online:

TLS v1.2 https://tools.ietf.org/html/rfc5246

The master secret is always exactly 48 bytes in length.

TLS v1.3 https://tools.ietf.org/id/draft-ietf-tls-tls13-04.html

All master secrets are always exactly 48 bytes in length. The length of the premaster secret will vary depending on key exchange method.

As for the boring-ssl callbacks, I considered that, i would have been way simpler!
Unfortunately Netty has to support older versions of OpenSSL :(

From what I've read they should be both 48 bytes.
Do you have more info ?

[huangqiangxiong]

@fzakaria

Here is an example about go-grpc, that capture according to https://gitlab.com/wireshark/wireshark/-/wikis/How-to-Export-TLS-Master-keys-of-gRPC#golang. That contains message of gRPC over HTTP2 over TLS1.3, and is able to be dissected correctly by wireshark.

The key file format like:

CLIENT_HANDSHAKE_TRAFFIC_SECRET aa... 16...
SERVER_HANDSHAKE_TRAFFIC_SECRET aa... cd...
CLIENT_TRAFFIC_SECRET_0 aa... 7e...
SERVER_TRAFFIC_SECRET_0 aa... 04...

But as far as I know, the TLS of golang is not implemented on openssl or boringssl.

grpc_tls_extend_go_unaryecho_example_tls13.zip

[huangqiangxiong]

@fzakaria And more details about key-file refers to https://gitlab.com/wireshark/wireshark/-/blob/master/epan/dissectors/packet-tls-utils.c (from line.no 5924 to 5960). It seems TLS1.3 does not use format "RSA Session-ID:xxxx Master-Key:yyyy" or "CLIENT_RANDOM xxxx yyyy". But I'm not sure because I'm not know the detail about TLS secret keys.
I hope these information helps.

[normanmaurer]

@fzakaria any ideas ? I think we could also add ifdefs for BoringSSL in the native code if this will help.

[fzakaria]

The easiest solution here at the moment may just be adding more documentation to the key handler; explaining it needs TLSv1 - 1.2.

What do you think @normanmaurer ?

When I wrote this implementation, the NSS keylog format for TLS 1.3 hadn't been published yet.
I see now it has https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/Key_Log_Format
TLSv1.3 requires four different secrets (handshake and traffic secrets).

[fzakaria]

(FWIW, I can't remember the OpenSSL bindings available to netty atm, but if we can get access to the client_random then that is a better emitted log -- ty @huangqiangxiong for the wireshark link)

[huangqiangxiong]

(FWIW, I can't remember the OpenSSL bindings available to netty atm, but if we can get access to the client_random then that is a better emitted log -- ty @huangqiangxiong for the wireshark link)

I don't catch what did you mean. Is this feature blocked by something?

[huangqiangxiong]

I find an alternative way to export TLS key materials in Java that is using jsslkeylog and replacing boringssl sslProvider with JDK built-in security Provider.

I think some people may get help from it, so record it here https://gitlab.com/wireshark/wireshark/-/wikis/How-to-Export-TLS-Master-keys-of-gRPC#using-jsslkeylog

[sprsquish]

I'd like to cast my vote for this suggestion:

The SSL_CTX_set_keylog_callback is introduced since openssl 1.1.1 (also in boringssl) that can be used to export the secret materials into key log files (which can be used with Wireshark). And I highly recommend to add method like setKeyLogCallback() or addKeyLogCallback() in SslContextBuilder instead of registering handler in pipeline.

I'm fighting a decryption bug right now and it'd be very helpful to capture incoming packets and attempt to decrypt them in Wireshark. We're using TLS1.3, though, so I can't do that without the keylog information.

The other way this is handled is with a SSLKEYLOGFILE environment variable that points to a file that the keylogger can write to.