Grpc proxy with tls auth

[Sh4d1]

Hello!
So apparently grpc proxy doesn't work with tls auth. Or at least it doesn't start (I've read the docs). But the question is why couldn't this be allowed? If all clients uses the same cname, it should be able to work right? Could a flag on the proxy be added to allow this usecase?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

[ptabor]

/cc @mitake - looks like you had a lot of contributions around the logic

For the reference docs:

• https://github.com/etcd-io/etcd/blob/master/Documentation/op-guide/authentication.md#using-tls-common-name
• https://chromium.googlesource.com/external/github.com/coreos/etcd/+/949bcbddbe1a59aec089cb33410fb337896f5942/CHANGELOG-3.3.md#v3_3_11-2019_1_11-security_authentication

I spotted the errors by running this and seeing test failures. Generation of server certicate with CN="" helps.

  go test -mod=mod -tags "cluster_proxy" -timeout 30m -run TestCtlV3MemberListClientTLS ./tests/e2e/...

My understanding of the goal and situation:

1. We have following communication schema:
   client --- 1 ---> grpc-proxy --- 2 --- > etcd-server

2. There are 2 sets of flags/certs in grpc proxy [ https://github.com/etcd-io/etcd/blob/master/etcdmain/grpc_proxy.go#L140 ]:
   A. (cert-file, key-file, trusted-ca-file, auto-tls) this are controlling [1] so client to proxy connection and in particular they are describing proxy public identity.
   B. (cert,key, cacert ) - these are controlling [2] so what's the identity that proxy uses to make connections to the etcd-server.

3. If 2 (B.) contains certificate with CN and etcd-server is running with --client-cert-auth=true, the CN can be used as identity of 'client' from service perspective. This is permission escalation, that we should forbid.

4. If 1 (A.) contains certificate with CN - it should be considered perfectly valid. The server can (should) have full identity.

So my intuition is that only --cert flag (and not --cert-file flag) should be validated for empty CN. Will check whether it matches server behavior.

@mitake: Hitoshi: Does it make sense what I wrote above ?
If so - we should check and document the behavior.

[ptabor]

Unfortunately it seems that CN is required for both --cert and --cert-file flag.

It seems that the verification happens in that line:

etcd/etcdmain/grpc_proxy.go

Line 205 in c20cc05

proxyClient := mustNewProxyClient(lg, tlsinfo)

It seems suspicious that to create a proxy server we need to create a proxy-client that speaks to this server for purpose of healhtchecking. At least it should not verify CN="" , but I'm curious why its needed in the first place.

@tangcong - could you, please, comment in context of 0898c5b

[tangcong]

I just want to add a healthcheck for gRPC proxy. gRPC gateway/proxy does not support common name due to security. do you have good advice to optimize it? thanks. @ptabor

[ptabor]

@tangcong: I think that this line:

etcd/etcdmain/grpc_proxy.go

Line 184 in c20cc05

tlsinfo := newTLS(grpcProxyListenCA, grpcProxyListenCert, grpcProxyListenKey)

should pass additional flag to set EmptyCN: False in this context.

---

Offtopic: In general I'm surprised by a practice that /healthcheck handlers are creating a public connection to themselves. It's complicated and has not trivial security implications. Usually /healtcheck is checking internal state, sometimes triggering some 'internal' logic (like Watch from your example), but without doing public connection. The part of 'connection' is already verified by prober being able to reach the server - and there is no reason for another layer of connections.

[tangcong]

I will take a closer look later, this issue(Jun 4) does not seem to be caused by pr 12114 that is submitted in July. I agree with your idea, currently healthcheck is not optimal. @ptabor

[ptabor]

Proposed a fix in 2d0ce9d

[mitake]

@ptabor sorry for my late response, for a few days I cannot allocate a time for taking a look at it. I'll follow this PR this weekend. Could you wait for a while?

[mitake]

@ptabor I followed this issue and PR #12273, and think the proposal is completely valid. As you say non empty cert between client and grpc proxy is acceptable. Thanks for fixing it! I think #12273 fixed the issue, could you close if it's ok?