Introduce TlsMasterKeyHandler by fzakaria · Pull Request #8653 · netty/netty

fzakaria · 2018-12-13T06:33:35Z

Motivation

Debugging SSL/TLS connections through wireshark is a pain -- if the cipher used involves Diffie-Hellman then it is essentially impossible unless you can have the client dump out the master key [1]

This is a work-in-progress change (tests & comments to come!) that introduces a new handler you can set on the SslContext to receive the master key & session id. I'm hoping to get feedback if a change in this vein would be welcomed.

An implementation that conforms to Wireshark's NSS key log[2] file is also included.

Depending on feedback on the PR going forward I am planning to "clean it up" by adding documentation, example server & tests. Implementation will need to be finished as well for retrieving the master key from the OpenSSL context.

[1] https://jimshaver.net/2015/02/11/decrypting-tls-browser-traffic-with-wireshark-the-easy-way/
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/Key_Log_Format

Modification

I've wired up a new functional handler that gets passed eventually to SSLHandler to be called with the SSL session id & master key.

An implementation of the handler that conforms to Wireshark's key log format is included.

Result

I was able to set the handler to log to a file, which when provided to Wireshark, decrypted the TLS flow.

I did a quick test by modifying HttpHelloWorldServer

            sslCtx = SslContextBuilder
                    .forServer(ssc.certificate(), ssc.privateKey())
                    .sslProvider(SslProvider.JDK)
                    .sslMasterKeyHandler(TlsMasterKeyHandler.WiresharkTlsMasterKeyHandler.INSTANCE)
                    .build();

cat ~/key.log
RSA Session-ID:5c11f6c920488f4dc9891771ae001bd645a16791ab47b337f89136e0deba3cdb Master-Key:6a3aed11200f26d115de4fb7c6aaa01222532520592e93b45ae1e243d4ec06099f4057f3f75616ea6e741c9318f36a30

Fixes #8663

netty-bot · 2018-12-13T06:34:03Z

Can one of the admins verify this patch?

normanmaurer · 2018-12-19T11:20:03Z

@fzakaria will check soon. Just a bit busy atm

fzakaria · 2018-12-20T00:32:26Z

Sounds good @normanmaurer

handler/src/main/java/io/netty/handler/ssl/SslHandler.java

fzakaria · 2018-12-20T18:36:26Z

@normanmaurer thanks for the feedback.

What do you think overall about the motivation of the PR with respect to having it accepted? I'd like to propose having it submitting in two parts: support for JDK engine (the majority of this pull-request) and a latter commit that introduces it for OpenSSL
(I'm not sure yet on the work needed for netty-tcnative)

If that sounds good, I'd like to address your feedback + clean it up (add comments, tests etc..) to get it ready for approval.

This change accompanies the broader pull-request introduced by netty/netty#8653 to allow access to the SSL session master key for debugging purposes.

normanmaurer · 2018-12-22T07:10:29Z

@fzakaria this sounds good yes :)

handler/src/main/java/io/netty/handler/ssl/TlsMasterKeyBiConsumer.java

handler/src/main/java/io/netty/handler/ssl/TlsMasterKeyHandler.java

normanmaurer · 2018-12-27T13:16:47Z

handler/src/main/java/io/netty/handler/ssl/TlsMasterKeyHandler.java

Should we use PlatformDependent (Which will use usafe) to access it ? This way it will also work with jigsaw.

I'm not familiar with this class: do I just need to access the fieldOffset then get the object ?
(no need to set Accessible on the field?)

Sorry I meant to say use ReflectionUtil 🤦‍♂️

handler/src/main/java/io/netty/handler/ssl/TlsMasterKeyHandler.java

normanmaurer

left some comments that should be addressed

Motivation: This change accompanies the broader pull-request introduced by netty/netty#8653 to allow access to the SSL session master key for debugging purposes. For instance, you would be able to log the master key to decrypt the SSL session in Wireshark. Modifications: Exported two new functions through the JNI interface that allow access to the necessary information to decrypt the SSL session. Result: After this change, you can easily decrypt SSL sessions that use Diffie-Hellman key exchange (perfect forward secrecy) with tools like Wireshark or decrypt the payloads in Java following the example outlined https://github.com/netty/netty/pull/8699/files#diff-7a4cbf2f526e2066184fb9ef098cd815R1127 Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>

normanmaurer · 2019-03-08T08:07:12Z

handler/src/main/java/io/netty/handler/ssl/TlsMasterKeyHandler.java

should re really lookup the property every time ?

I've left this in for now -- I'm open to making it once lookup.
I imagined this is largely a test driven handler anyways not to be included in typical pipelines?

normanmaurer

Can you also add a test ?

handler/src/main/java/io/netty/handler/ssl/TlsMasterKeyHandler.java

normanmaurer · 2019-03-08T08:11:17Z

handler/src/main/java/io/netty/handler/ssl/TlsMasterKeyHandler.java

(see note above)

normanmaurer · 2019-03-08T08:12:06Z

handler/src/main/java/io/netty/handler/ssl/TlsMasterKeyHandler.java

maybe not expose the class directly but just add a static method that returns this instance ?

fzakaria · 2019-03-08T18:14:03Z

@normanmaurer do you have a good idea for a test or other tests I can draw inspiration from ?
I would need some sort of "TeeClient" that stores the encrypted TLS message (without TLS record header) so that I could then go and decrypt them with the logged master key ?
or
Are we just looking for a more surface-level test for TlsMasterKeyHandler that verifies it gets a non-null key material ?
(leaving the more functional test to the one I introduced in #8699)

normanmaurer · 2019-03-22T11:03:06Z

@fzakaria no idea for a test yet :(

Motivation: I've introduced netty/netty-tcnative#421 that introduced exposing OpenSSL master key & client/server random values with the purpose of allowing someone to log them to debug the traffic via auxiliary tools like Wireshark (see also netty#8653) Modification: Augmented OpenSslEngineTest to include a test which manually decrypts the TLS ciphertext after exposing the masterkey + client/server random. This acts as proof that the tc-native new methods work correctly! Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>

Motivation: I've introduced netty/netty-tcnative#421 that introduced exposing OpenSSL master key & client/server random values with the purpose of allowing someone to log them to debug the traffic via auxiliary tools like Wireshark (see also #8653) Modification: Augmented OpenSslEngineTest to include a test which manually decrypts the TLS ciphertext after exposing the masterkey + client/server random. This acts as proof that the tc-native new methods work correctly! Result: More tests Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>

handler/src/main/java/io/netty/handler/ssl/TlsMasterKeyHandler.java

normanmaurer

Sorry not done yet and did miss this before .

handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java

normanmaurer · 2019-07-01T19:51:05Z

@netty-bot test this please

handler/src/main/java/io/netty/handler/ssl/TlsMasterKeyHandler.java

fzakaria · 2019-07-01T19:55:04Z

That's a good point. Let me do that.

handler/src/main/java/io/netty/handler/ssl/TlsMasterKeyHandler.java

fzakaria · 2019-07-01T20:28:07Z

For those following this pull-request a demo logback.xml would be (just the raw message and new line):

  <appender name="FILE" class="ch.qos.logback.core.FileAppender">
    <file>key.log</file>
    <encoder>
      <pattern>%m%n</pattern>
    </encoder>
  </appender>

   <logger name="io.netty.wireshark" addivity="false">
    <appender-ref ref="FILE" />
  </logger>

Don't forget to also -Dio.netty.ssl.masterKeyHandler=true

normanmaurer · 2019-07-02T06:39:08Z

@netty-bot test this please

fzakaria · 2019-07-02T19:27:22Z

Yes it passed :)
6 months in the making this PR 👍

handler/src/main/java/io/netty/handler/ssl/SslMasterKeyHandler.java

handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java

Debugging SSL/TLS connections through wireshark is a pain -- if the cipher used involves Diffie-Hellman then it is essentially impossible unless you can have the client dump out the master key. Introduce a new TLS handler which accepts the master key of the TLS session. An implementation that conforms to Wireshark's NSS key log format is included. Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>

fzakaria · 2019-07-03T18:02:53Z

@carl-mastrangelo responded to your feedback.

handler/src/main/java/io/netty/handler/ssl/SslMasterKeyHandler.java

carl-mastrangelo

LGTM

fzakaria · 2019-07-10T02:40:15Z

@normanmaurer good to go ? :P

normanmaurer · 2019-07-10T07:25:05Z

@netty-bot test this please

normanmaurer · 2019-07-10T10:02:55Z

@fzakaria thanks a lot!

Motivation Debugging SSL/TLS connections through wireshark is a pain -- if the cipher used involves Diffie-Hellman then it is essentially impossible unless you can have the client dump out the master key [1] This is a work-in-progress change (tests & comments to come!) that introduces a new handler you can set on the SslContext to receive the master key & session id. I'm hoping to get feedback if a change in this vein would be welcomed. An implementation that conforms to Wireshark's NSS key log[2] file is also included. Depending on feedback on the PR going forward I am planning to "clean it up" by adding documentation, example server & tests. Implementation will need to be finished as well for retrieving the master key from the OpenSSL context. [1] https://jimshaver.net/2015/02/11/decrypting-tls-browser-traffic-with-wireshark-the-easy-way/ [2] https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/Key_Log_Format Modification - Added SslMasterKeyHandler - An implementation of the handler that conforms to Wireshark's key log format is included. Result: Be able to debug SSL / TLS connections more easily. Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>

normanmaurer requested changes Dec 20, 2018

View reviewed changes

handler/src/main/java/io/netty/handler/ssl/SslHandler.java Outdated Show resolved Hide resolved

fzakaria mentioned this pull request Dec 21, 2018

Introduce a new native method to access the master key netty/netty-tcnative#421

Merged

normanmaurer reviewed Dec 27, 2018

View reviewed changes

handler/src/main/java/io/netty/handler/ssl/TlsMasterKeyBiConsumer.java Outdated Show resolved Hide resolved

normanmaurer reviewed Dec 27, 2018

View reviewed changes

handler/src/main/java/io/netty/handler/ssl/TlsMasterKeyHandler.java Outdated Show resolved Hide resolved