Protection gap when changing workload security policies

[holyspectral]

Is there an existing issue for this?

• I have searched the existing issues

Environment

- OS: Ubuntu
- Architecture: amd64
- Cluster: kind

Issue Description

• Expected behavior:
  • When changing workload security policies CR, there should be no protection gap.
• Current behavior:
  • Internally, when a tracing policy is changed, Tetragon deletes the existing security policies and creates a new one. This behavior leads a protection gap when a policy is changed.
• Steps to reproduce:
  • Modify the WorkloadSecurityPolicy CR and observe the behavior of protected containers.

Desired solution

The current flow from the Tetragon perspective:

• Delete the existing TracingPolicyNamespaced.
• Create a new TracingPolicyNamespaced with the updated allowed list (adding/removing the executable).

An alternative flow could be:

• Create a new TracingPolicyNamespaced with the updated allowed list (adding/removing the executable). This policy should have the same podSelector of the existing one and actually a different name (because names must be unique in the namespace, not sure how to call it).
• Delete the old TracingPolicyNamespaced.

[holyspectral]

I think having two policies would work to some degree, but to fix this properly we may need to dig into Tetragon.
One problem that I can see is that, it's an eventually-consistent/asynchronous flow for a TracingPolicyNamespaced to be applied and takes effect. That means, while we can create two policies, we don't really know if the new one already takes effect.

Some options that I can think of:

• Tetragon should at least update the status of the TracingPolicyNamespaced CR.
• Or we do this logic in Tetragon level.

Here is where the tracing policy is handled.

[Andreagit97]

This is actually what we are doing when we switch the policy from mode: monitor -> mode: protect.

We update the tetragon TracingPolicyNamespaced and tetragon under the hood calls the update flow https://github.com/cilium/tetragon/blob/9dea41615eae8a59b0433fa29fa01cf590dda41f/pkg/watcher/crdwatcher/tracingpolicy.go#L173
So it destroys the collection (ebpf progs/maps) and recreates it, without destroying the TracingPolicyNamespaced CR. So it seems the CR remains always alive, but under the hood, we lose the protection

Our first approach could be to create a new policy and then delete the old one. We can also ask upstream what is the suggested option to do that.

What I would like to understand is if this "create and then delete" approach is actually technically feasible or not. I don't see any blocker if I look at the code, but maybe there is something I'm missing, and it is worth trying it in practice.

The first steps that I see for this ticket are:

• reaching upstream asking what they suggest to keep the protection "alive" without disruption
• test in practice if the "create/delete" is doable, or if there are some hidden blockers

P.S: I would classify this more as an enhancement than a bug

[holyspectral]

One thing that I'd like to add is that, it seems like inside the Tetragon, a TracingPolicy is read only. I guess that's because a change of TracingPolicy would potentially lead to a complete reload of ebpf progs/maps. Say if we change a kprobe to a lsm in the same TracingPolicy, then only a complete reload can handle this.

Similarly, some optimization is also done based on this assumption, e.g., the size of string map is allocated to be matched the size of argSelector defined in TracingPolicy.

For completeness, I think we have two directions to handle this:

• Two TracingPolicy CRs or internal ebpf collections. This what we went through above. I think this is the way to go because it's much easier than the other approach.
• Define what field of TracingPolicy is immutable inside Tetragon, so we know what field policy change can be safely reloaded.
  • I think this is the normal way to handle this in kubernetes, but it will involve more changes and it could get complex pretty easily. Not sure if we should start from this.

[Andreagit97]

One thing that I'd like to add is that, it seems like inside the Tetragon, a TracingPolicy is read only.

More than read-only, I would say that every change causes a redeploy of the whole collection. When we switch from monitor to protect, we apply the Override MatchAction and everything is redeployed following this flow https://github.com/Andreagit97/tetragon/blob/657f19eb85569de48314875d3e0f9f63d81ecc90/pkg/watcher/crdwatcher/tracingpolicy.go#L94, the only check that they do to avoid the redeploy of the collection is on the resourceVersion https://github.com/Andreagit97/tetragon/blob/657f19eb85569de48314875d3e0f9f63d81ecc90/pkg/watcher/crdwatcher/tracingpolicy.go#L124

[holyspectral]

More than read-only, I would say that every change causes a redeploy of the whole collection.

yeah I mean, since it's read-only, the only way they support right now is to delete and re-create. The grpc API doesn't have a method to modify a tracing policy as well.

https://github.com/cilium/tetragon/blob/32acaa4f172a188d408b2fdb3fe3189a270b3542/pkg/server/server.go#L41

[holyspectral]

BTW, I'd recommend adding this to cilium/design-cfps#80 if you haven't, because avoiding the protection gap is also a valid use case of cilium/design-cfps#80.

[Andreagit97]

BTW, I'd recommend adding this to cilium/design-cfps#80 if you haven't, because avoiding the protection gap is also a valid use case of cilium/design-cfps#80.

Yep, in that scenario, we can swap the map with values in an atomic way without detaching the eBPF progs. i didn't highlight it in the CFP, great point!

[Andreagit97]

Opened an issue upstream to ask about this protection gap cilium/tetragon#4322