Skip to content

Conversation

@huangqiangxiong
Copy link
Contributor

@huangqiangxiong huangqiangxiong commented Dec 12, 2020

@ejona86 Could you help to review this PR?

Enable this feature by setting the system property in command argument

   -Dio.netty.ssl.masterKeyHandler=true

or in java code

   System.setProperty(SslMasterKeyHandler.SYSTEM_PROP_KEY, "true");

The keys will be written to the log named "io.netty.wireshark" in the warnning level. To export the keys to a file, you can configure log factory like: (log4j.xml for example)

...
<appender name="key-file" class="org.apache.log4j.RollingFileAppender">
	<param name="file" value="d:/keyfile.txt"/>
	<layout class="org.apache.log4j.PatternLayout">
		<param name="ConversionPattern" value="%m%n"/>
	</layout>
</appender>
<category name="io.netty.wireshark">
	<priority value="DEBUG" />
	<appender-ref ref="key-file" />
</category>
...

Wireshark can analyze the messages gRPC over TLS with this key log file.

There is a sample capture and key log file generated by using this commit:

You can refer to Wireshark gRPC wiki page for getting the
person_search_service.proto and addressbook.proto files, and how to set the Wireshark for parsing gRPC network traffic.

And put the path of keylog.txt in the key log preference in Preferences->Protocols->TLS->(Pre)-Master-Secret log filename.

Finally decode traffic on tcp port 60051 and 60052 as TLS.

close #7199

@ericgribkoff ericgribkoff added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Dec 14, 2020
@ericgribkoff ericgribkoff requested a review from ejona86 December 14, 2020 00:29
@grpc-kokoro grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Dec 14, 2020
Copy link
Member

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

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

We shouldn't be using internal classes to netty, even in a test. And in this case we can just use normal java.util.logging, at the only cost of more annoying assertions. See ThreadLocalContextStorageTest.java for an example test that checks java.util.logging logs.

Note that will mean many of my comments may be invalidated because you'll swap to a new approach. That is fine. You may find some of the comments interesting though.

Enable this feature by setting the system property
   -Dio.netty.ssl.masterKeyHandler=true
or
   System.setProperty(SslMasterKeyHandler.SYSTEM_PROP_KEY, "true");
The keys will be written to the log named "io.netty.wireshark" in
the warnning level. To export the keys to a file, you can configure
log factory like: (with log4j.xml for example)
<appender name="key-file" class="org.apache.log4j.RollingFileAppender">
	<param name="file" value="d:/keyfile.txt"/>
	<layout class="org.apache.log4j.PatternLayout">
		<param name="ConversionPattern" value="%m%n"/>
	</layout>
</appender>
<category name="io.netty.wireshark">
	<priority value="DEBUG" />
	<appender-ref ref="key-file" />
</category>

Wireshark can analyze the messages gRPC over TLS with this
key log file.

close grpc#7199
@huangqiangxiong
Copy link
Contributor Author

We shouldn't be using internal classes to netty, even in a test. And in this case we can just use normal java.util.logging, at the only cost of more annoying assertions. See ThreadLocalContextStorageTest.java for an example test that checks java.util.logging logs.

Note that will mean many of my comments may be invalidated because you'll swap to a new approach. That is fine. You may find some of the comments interesting though.

Thank you for your comments. I have updated the commits (test case) according to your comments.

@ejona86 ejona86 requested review from ericgribkoff and voidzcy and removed request for voidzcy December 15, 2020 17:47
@ejona86 ejona86 added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Dec 15, 2020
@grpc-kokoro grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Dec 15, 2020
@ejona86
Copy link
Member

ejona86 commented Jan 8, 2021

@ericgribkoff, could you review? I think you were on support rotation at the time, which is why I added you.

@ericgribkoff ericgribkoff merged commit 9bc05fb into grpc:master Jan 8, 2021
@ericgribkoff
Copy link
Contributor

@huangqiangxiong Sorry for the delay, This is now merged. Thanks for this change!

voidzcy added a commit to voidzcy/grpc-java that referenced this pull request Jan 8, 2021
voidzcy added a commit that referenced this pull request Jan 8, 2021
voidzcy added a commit to voidzcy/grpc-java that referenced this pull request Jan 9, 2021
dfawley pushed a commit to dfawley/grpc-java that referenced this pull request Jan 15, 2021
Enable this feature by setting the system property
   -Dio.netty.ssl.masterKeyHandler=true
or
   System.setProperty(SslMasterKeyHandler.SYSTEM_PROP_KEY, "true");
The keys will be written to the log named "io.netty.wireshark" in
the warnning level. To export the keys to a file, you can configure
log factory like: (with log4j.xml for example)
<appender name="key-file" class="org.apache.log4j.RollingFileAppender">
	<param name="file" value="d:/keyfile.txt"/>
	<layout class="org.apache.log4j.PatternLayout">
		<param name="ConversionPattern" value="%m%n"/>
	</layout>
</appender>
<category name="io.netty.wireshark">
	<priority value="DEBUG" />
	<appender-ref ref="key-file" />
</category>

Wireshark can analyze the messages gRPC over TLS with this
key log file.

close grpc#7199
dfawley pushed a commit to dfawley/grpc-java that referenced this pull request Jan 15, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support SslMasterKeyHandler in NettyServerBuilder

4 participants