Envoy: [rbac] Envoy doesn't break an existing connection when rbac rules no longer allow traffic

[yanniszark]

Title: Break existing connections when new RBAC rules stop allowing them

This was initially reported as a security vulnerability. Eventually, it was classified as a feature request by @mattklein123, so opening an issue for it.

Problem Statement

Suppose that client is connecting to server. Initially, Istio RBAC allows the connection to happen.
After some time, Istio RBAC is changed to not allow traffic from the client to the server. However,
Envoy doesn't close the TCP connection and the client can still send data.

Reproducer

1. Prepare a K8s cluster with Istio (e.g., MiniKF), disable EnableAutoMtls in the Istio ConfigMap (not sure why this is needed but it is) and enable Istio RBAC (on by default in MiniKF).

2. Apply the attached YAML, which creates a sidecar-enabled namespace with a client and server deployment, as well as necessary Istio RBAC:
   istio-reproducer-rbac.yaml (2).log
   istio-reproducer.yaml (2).log

3. Exec into the server container and start a TCP server on the declared port 8081:

   # server-container
   nc -v -l -p 8081
   

4. Exec into the client container and try to connect to the server:

   # Is not denied by Istio RBAC
   nc -v server.test-istio-rbac.svc.cluster.local 8081
   

5. Send messages from client to server. They are allowed by Istio RBAC.

6. Delete the Istio RBAC that permits the connection:
   istio-reproducer-rbac.yaml (2).log

7. Continue sending messages from the client and see that they are received.

8. Drop client connection, try to connect again. After connecting, send some data. Istio RBAC closes the connection because it's now enforced (on new connections).

Note: You can enable Istio RBAC debug log in the server's Istio sidecar, in order to see the enforcement messages:

kubectl exec -it -n test-istio-rbac <server_pod> -c istio-proxy -- pilot-agent request POST 'logging?rbac=debug'

Proposed Solution

When RBAC rules are updated, Envoy should drop any connections no longer allowed by the new policy.

[mattklein123]

I updated the title to remove Istio and make the feature request in terms of the Envoy RBAC filter. I don't know how Istio does RBAC so Istio details are not relevant here. cc @kyessenov

[kyessenov]

This is interesting since I am not aware of any other config in Envoy that applies retro-actively to old streams. Usually, a stream is 1-1 with a parent config for it, and two configs almost never interact to avoid race conditions. I think the FR also applies to long living HTTP streams, too.

[mattklein123]

Yeah in general today we don't do anything retroactively, but I think it would be neat to consider a model in which we have a way to audit existing streams/connections and do things with them when configuration changes (especially when using ECDS).