Support SslMasterKeyHandler in NettyServerBuilder

[nucz]

I have a problem with adding SslMasterKeyHandler#newWireSharkSslMasterKeyHandler on the server side. I tried to do this with channelFactory:

builder.channelFactory(() -> {
          final ServerChannel ch = GrpcUtil.getDefaultServerChannelFactory().newChannel();
          final ChannelPipeline pipeline = ch.pipeline();
          pipeline.addLast(SslMasterKeyHandler.newWireSharkSslMasterKeyHandler());
          return ch;
        })

and later with ProtocolNegotiator:

final SslContext sslContext = GrpcSslContexts.configure(sslClientContextBuilder).build();
final io.grpc.netty.ProtocolNegotiator negotiator = ProtocolNegotiators.serverTls(sslContext);
builder.protocolNegotiator(tlsNegotiator(negotiator);

In the first case, the setting had no effect. In the second case, the server worked incorrectly (it stopped accepting connections, even without a handler, just with a negotiator wrapper).

What should be the proper implementation?

[voidzcy]

I don't think this is the correct usage of WireSharkSslMasterKeyHandler. You would only need to enable it by setting the system property -Dio.netty.ssl.masterKeyHandler=true, no modification for the pipeline is needed. See details in its Javadoc.

[ejona86]

@voidzcy, I don't think that system property would work as-is. That only works if gRPC included SslMasterKeyHandler in the pipeline already.

Changing the channelFactory() didn't work because that's the listening channel factory. It produces children for each connection and those are what you are wanting to modify. The protocolNegotiator() snippet looks pretty mangled, such that it is unclear what it would look like once you actually got it to compile.

I don't think there is a proper implementation currently. You could maybe hack something with the channelFactory(), but it would be a big hack. It isn't really safe for you to change the pipeline without gRPC being aware; we've found Netty pipelines to be very brittle.

[ejona86]

@nucz, would it be fair to call this an enhancement request asking for SslMasterKeyHandler support?

It seems we could check the system property io.netty.ssl.masterKeyHandler just like Netty and if it is true we add the handler to the pipeline. I'd prefer not having it on the pipeline all the time, to avoid any potential performance/behavior confusion. I'm disappointed it doesn't remove itself from the pipeline after the handshake completes; I wonder if it doesn't to allow supporting rekeying (I don't remember what is supported there).

[nucz]

@ejona86 Yes, that would be great, to have SslMasterKeyHandler support. I found the solution using negotiators, but it is far from ideal and is version sensitive.

[Nnadozie]

@ejona86 I would like to take a stab at this, but off the top of your head, please what's your estimate for this unit of work?

[ejona86]

@Nnadozie, should be like 3 lines of code. You'd basically check the system property, and if set, create and add the additional handler within ServerTlsHandler.

[Nnadozie]

Thanks @ejona86! Working on a PR now.

[huangqiangxiong]

@ejona86 @Nnadozie I suggest that this feature can also be used on the client side, because sometimes you can only debug on the client side (because the server is provided by others).

In addition, I recommend to extend .sslContext(SslContext sslContext) to .sslContext(SslContext sslContext, boolean allowExportMasterKey). To do pipeline.addLast(SslMasterKeyHandler.newWireSharkSslMasterKeyHandler()), only while both the allowExportMasterKe and the system property "io.netty.ssl.masterKeyHandler" are true. Because sometimes there are several grpc clients working in one program, we may only want to track/debug on the traffic one of them.