gRPC with TLS doesn't work with cmux in mixed mode

[jpkrohling]

Describe the bug
I faced open-telemetry/opentelemetry-collector#1227 some time ago, and while reviewing #2337, I noticed that we have the same pattern in Jaeger:

jaeger/cmd/query/app/server.go

Lines 138 to 145 in 02d4567

	// cmux server acts as a reverse-proxy between HTTP and GRPC backends.
	cmuxServer := cmux.New(s.conn)
	grpcListener := cmuxServer.MatchWithWriters(
	cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc"),
	cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc+proto"),
	)
	httpListener := cmuxServer.Match(cmux.Any())

Basically, when gRPC is behind TLS, cmux can't determine the headers before doing the TLS handshake, and the TLS handshake happens at the gRPC layer. The end result is that all calls end up going to the HTTP handler when a client is attempting to establish a gRPC TLS connection.

To Reproduce
Steps to reproduce the behavior:

1. Not reproducible at the moment, as there's a bug preventing the TLS flags from being exposed for gRPC in the Query component.

A possible solution is mentioned here in a follow-up issue at OpenTelemetry Collector: open-telemetry/opentelemetry-collector#1256 (comment)

[rjs211]

I think the Example (RecursiveCmux) from here can easily suit our needs. Basically, create any non-tls handers (if there is any) in the first cmux. If not satisfied, assume it is TLS and use a tls listener as the input listener of the inner cmux with tls enabled servers.

We would need to build the cmux dynamically with some factory method based on whether which of gRPC and / or HTTP tls is enabled.

but one major pitfall would be that we can use only one common pair of certificates for both gRPC and HTTP.

[yurishkuro]

Do you want to try?

[rjs211]

Yes. Will try this first.

[rjs211]

I think #2337 Solves this one as well now. Used only one cmux.
Assumptions:

1. Only one set of server certificate and key (even though both HTTP and GRPC flags are exposed ) Fixed
2. GRPC can handle bad requests. (i.e. if it gets a non-grpc request / random http request, it shouldn't break.)

[jpkrohling]

Only one set of server certificate and key (even though both HTTP and GRPC flags are exposed )

Then we should have only one set of flags. I can't see a way to have both gRPC and HTTP under one single port, each with its own cert...

[rjs211]

Should be fixed now.

[rjs211]

This causes another problem. There can be only one ClientCA for both GRPC and HTTP server. But they might very well get their client-certificates from two different CAs.

Can i change the tlsCfg.Options.ClientCAPath from string to StringSlice (defined in https://github.com/jaegertracing/jaeger/blob/master/pkg/config/string_slice.go) ? this would enable multiple ClientCAs for any single TLSCfg

[rjs211]

Aah, one certificate file would contain multiple certificates. We dont ahve to do that.

[yurishkuro]

Aah, one certificate file would contain multiple certificates.

it may be worth mentioning this in the corresponding TLS config option.

[jpkrohling]

@rjs211 I think you fixed this one already, haven't you?

[rjs211]

@jpkrohling Yes. This should be fixed when #2387 is merged.

[rjs211]

Just to clarify, This is solved (i.e. a working solution) only when the user descides to use two separate ports. i.e. when query.host-port is not defined and at least one of query.grpc-server.host-port or query.http-server.host-port is defined. This actual feature would be a part if #2337. I am resuming working on that now.