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

Wireshark Testing #6060

Merged
merged 14 commits into from
May 29, 2020
Merged

Wireshark Testing #6060

merged 14 commits into from
May 29, 2020

Conversation

yschimke
Copy link
Collaborator

Testing outputting to a key log for assist wireshark decoding of TLS.

@yschimke yschimke requested a review from swankjesse May 17, 2020 17:11
@yschimke
Copy link
Collaborator Author

I think (!) it's working.

wireshark filter = "http2"

image

@yschimke
Copy link
Collaborator Author

yschimke commented May 17, 2020

Damn, just realised afterwards that Netty already did this. Would have saved me time.

netty/netty#8653

@yschimke
Copy link
Collaborator Author

image

@yschimke
Copy link
Collaborator Author

@swankjesse Is it worth landing this as a sample?

@yschimke yschimke marked this pull request as ready for review May 19, 2020 07:56
@swankjesse
Copy link
Member

This is very neat stuff. It’s unfortunate we have to jump through so many hoops to do it.

@yschimke
Copy link
Collaborator Author

This is very neat stuff. It’s unfortunate we have to jump through so many hoops to do it.

Agreed, Conscrypt is the right place to do this. google/conscrypt#847

@yschimke yschimke requested a review from swankjesse May 25, 2020 14:31
@yschimke
Copy link
Collaborator Author

This is very neat stuff. It’s unfortunate we have to jump through so many hoops to do it.

Also, I think this example makes using Wireshark really simple and compelling for debugging HTTP/2 errors. It makes me care a lot less about improving the internal frame logging since Wireshark is obviously the right tool and this works with certificate pinning, with a public webserver, and without MITM proxies.

@yschimke
Copy link
Collaborator Author

@swankjesse ok to land?

Copy link
Member

@swankjesse swankjesse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff. I wonder how much more automatic we can make it. I can imagine us running (or printing) a command like this:

wireshark -o tls.keylog_file:/tmp/secrets.log

Agreed 100% that this is better than frame logging. Just like how Charles is better than request/response logging.

I wonder how much work it is to send keys into Wireshark without any interaction. Ie, if we run the wireshark command above while it’s already running, does “the right thing” happen? Then it could be as simple as installing an event listener and waiting for magic to happen automatically.

/**
* Logs SSL keys to a log file, allowing Wireshark to decode traffic and be examined with http2
* filter. The approach is to hook into JSSE log events for the messages between client and server
* during handshake, and then take the agreed masterSecret from private fields of the session.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doc is 👌

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably improve with a command line

$ wireshark -o tls.keylog_file:/tmp/key.log -Y http2 -k

}
}

override fun secureConnectStart(call: Call) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes OkHttp event listeners look super good


if (tlsVersions.contains(TLS_1_3)) {
println("TLSv1.3 requires an external command run before first traffic is sent")
println("Follow instructions at https://github.com/neykov/extract-tls-secrets for TLSv1.3")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The agent is quite interesting. I wonder if we could detect it automatically so you don’t need to do all the work to extract the PID and log

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The agent is awesome - nice work @neykov

If you are TLSv1.3 - the agent is as automatic as you need since you can just run remotely against any running Java app. OkHttp or otherwise.

But this sample is mainly to make it simple without running additional steps internally. Not sure how to incrementally improve for now. Let's try this out ourselves debugging failures against QNAP or Go based servers :)

println("TLSv1.3 requires an external command run before first traffic is sent")
println("Follow instructions at https://github.com/neykov/extract-tls-secrets for TLSv1.3")
println(
"Pid: ${ProcessHandle.current()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really nice

@yschimke yschimke merged commit 72227df into square:master May 29, 2020
@yschimke yschimke deleted the key_log branch May 29, 2020 07:17
@mrprona92
Copy link

@yschimke im working on Android emulator. How i can do the same thing to export key like that.

@yschimke
Copy link
Collaborator Author

@mrprona92 I don't think you can. This uses non public or stable features of specific JVMs, that don't exist on Android AFAICT.

@mrprona92
Copy link

@yschimke thanks for your response. any suggest for me to do this by another way? i using okhttp with ssl handshake, If i export an keylog file from wireshark. Then read it from android side. is that avaiable?

@yschimke
Copy link
Collaborator Author

Use some sort of MITM proxy? Sorry, not sure. Worth asking in a wider forum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants