agent: rewrite checks with proxy address, not local service address (#7518)

Exposing checks is supposed to allow a Consul agent bound to a different
IP address (e.g., in a different Kubernetes pod) to access healthchecks
through the proxy while the underlying service binds to localhost. This
is an important security feature that makes sure no external traffic
reaches the service except through the proxy.

However, as far as I can tell, this is subtly broken in the case where
the Consul agent cannot reach the proxy over localhost.

If a proxy is configured with: `{ LocalServiceAddress: "127.0.0.1",
Checks: true }`, as is typical with a sidecar proxy, the Consul checks
are currently rewritten to `127.0.0.1:<random port>`. A Consul agent
that does not share the loopback address cannot reach this address. Just
to make sure I was not misunderstanding, I tried configuring the proxy
with `{ LocalServiceAddress: "<pod ip>", Checks: true }`. In this case,
while the checks are rewritten as expected and the agent can reach the
dynamic port, the proxy can no longer reach its backend because the
traffic is no longer on the loopback interface.

I think rewriting the checks to use `proxy.Address`, the proxy's own
address, is more correct in this case. That is the IP where the proxy
can be reached, both by other proxies and by a Consul agent running on
a different IP. The local service address should continue to use
`127.0.0.1` in most cases.
This commit is contained in:
Andy Lindeman 2020-04-02 03:35:43 -04:00 committed by GitHub
parent c1cb18c648
commit fb0a990e4d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 3 additions and 3 deletions

View File

@ -2539,7 +2539,7 @@ func (a *Agent) addServiceInternal(req *addServiceRequest, snap map[structs.Chec
psid.Init(service.Proxy.DestinationServiceID, &service.EnterpriseMeta) psid.Init(service.Proxy.DestinationServiceID, &service.EnterpriseMeta)
if service.Proxy.Expose.Checks { if service.Proxy.Expose.Checks {
err := a.rerouteExposedChecks(psid, service.Proxy.LocalServiceAddress) err := a.rerouteExposedChecks(psid, service.Address)
if err != nil { if err != nil {
a.logger.Warn("failed to reroute L7 checks to exposed proxy listener") a.logger.Warn("failed to reroute L7 checks to exposed proxy listener")
} }
@ -2983,7 +2983,7 @@ func (a *Agent) addCheck(check *structs.HealthCheck, chkType *structs.CheckType,
) )
return err return err
} }
http.ProxyHTTP = httpInjectAddr(http.HTTP, proxy.Proxy.LocalServiceAddress, port) http.ProxyHTTP = httpInjectAddr(http.HTTP, proxy.Address, port)
} }
http.Start() http.Start()
@ -3052,7 +3052,7 @@ func (a *Agent) addCheck(check *structs.HealthCheck, chkType *structs.CheckType,
) )
return err return err
} }
grpc.ProxyGRPC = grpcInjectAddr(grpc.GRPC, proxy.Proxy.LocalServiceAddress, port) grpc.ProxyGRPC = grpcInjectAddr(grpc.GRPC, proxy.Address, port)
} }
grpc.Start() grpc.Start()