bpfman / bpfman/bpfman-operator
Namespaced BpfApplication pod selectors are not scoped to the application's namespace
- Dominant language
- Go
- Stars
- 38
- Forks
- 27
- PR merge metrics
- No merged PRs in 30d
Description
A namespaced `BpfApplication` should only resolve its pod and network-namespace selectors inside the namespace that owns it. It does not: it resolves them against every namespace on the node. As a result an application in namespace A can attach its eBPF programs to a pod in namespace B, as long as B has a pod on the same node whose labels match the selector. For a CRD whose entire purpose is to give a tenant a namespace-confined slice of bpfman, that is an isolation hole.
We came across this while auditing for other instances of the bug that #539 fixed (BPFMAN-45). That one was in the agent's `BpfApplicationState` lookup: the agent listed state objects without a namespace, so two same-named applications in different namespaces fought over one state object. Fixing it prompted the obvious question -- where else does the namespaced path drop the namespace? -- and this is the answer. Same class of mistake, different mechanism, and this one has a nastier consequence.
## Where it goes wrong
The namespaced reconcilers share `NsProgramReconcilerCommon`, which carries a `namespace` field (`controllers/bpfman-agent/ns_application_program.go`):
```go
type NsProgramReconcilerCommon struct {
currentProgram *bpfmaniov1alpha1.BpfApplicationProgram
currentProgramState *bpfmaniov1alpha1.BpfApplicationProgramState
namespace string
}
```
`getNamespace()` hands that field to the container lookup, and the lookup lists pods with it (`controllers/bpfman-agent/containers.go`):
```go
podList, err := c.clientSet.CoreV1().Pods(selectorNamespace).List(ctx, listOptions)
```
`Pods("").List()` in client-go is a list across all namespaces, filtered only by the label selector and the `spec.nodeName` field selector. And `selectorNamespace` is empty, because nothing ever sets the field. The four constructions in `getProgramReconciler` set `currentProgram` and `currentProgramState` and stop:
```go
case bpfmaniov1alpha1.ProgTypeTC:
rec = &NsTcProgramReconciler{
ReconcilerCommon: r.ReconcilerCommon,
NsProgramReconcilerCommon: NsProgramReconcilerCommon{
currentProgram: prog,
currentProgramState: progState,
// namespace is never set, so it stays ""
},
}
```
So `getNamespace()` has returned `""` since the field was born. The cluster-scoped path, by contrast, is correct: `ClContainerSelector` has an explicit `Namespace` field, and there an empty value legitimately means "all namespaces". The namespaced path has no such field for the user to set -- the namespace has to come from the owning application -- which is exactly why the omission produces no error and no warning. It just silently widens the search.
This shared lookup backs every namespaced program type: TC, TCX, XDP, uprobe and uretprobe (the last two share `NsUprobeProgramReconciler`), so all of them are affected. The reproducer below demonstrates it with TC.
## It worked before the load/attach split
This is a regression, not a feature that never landed. The field arrived with the load/attach split, 7c11035a (PR #347, "bpfman-operator: Support Load/Attach Split", 2024-12-09). Before that commit the namespaced reconcilers derived the namespace live from the object they were reconciling. In the commit just before it:
```go
func (r *TcNsProgramReconciler) getNamespace() string {
return r.currentTcNsProgram.Namespace
}
```
The split collapsed the per-type namespaced reconcilers into `NsProgramReconcilerCommon` and swapped that live read for a struct field -- then never wired the field up. `git log -S 'r.namespace'` over `controllers/bpfman-agent/` returns only 7c11035a, and there is no commit anywhere that assigns the field. It has been `""` for the whole life of the current namespaced controller.
For reference, the state-lookup sibling was 9423825c ("Scope BpfApplicationState lookup to the application namespace", #539); this issue is the pod-selection counterpart that audit turned up.
## Reproduce it
You need a cluster running bpfman-operator and the agent from `main` (tip 1e5dc2ba as of 2026-07-14; the bug dates to the 2024 load/attach split, so any `main` since then has it). `make run-on-kind` in the bpfman-operator repo brings one up on a single-node kind cluster, which is where the node container name and the `docker exec` calls below come from. A single node also keeps the link count deterministic without pinning pods; on a multi-node cluster put both target pods on the same node and inspect that node's agent, otherwise the two pods land on different nodes and no single agent sees both.
The setup is two namespaces, an identically-labelled pod in each, and a namespaced `BpfApplication` in one of them with a TC network-namespace selector:
```yaml
apiVersion: v1
kind: Namespace
metadata:
name: tenant-a
---
apiVersion: v1
kind: Namespace
metadata:
name: tenant-b
---
# Target in the application's own namespace.
apiVersion: v1
kind: Pod
metadata:
name: target-a
namespace: tenant-a
labels:
app: demo-target
spec:
tolerations:
- operator: Exists
containers:
- name: target
image: quay.io/quay/busybox:latest
command: ["sleep", "3600"]
---
# Target in a DIFFERENT namespace, same labels, different name.
apiVersion: v1
kind: Pod
metadata:
name: target-b
namespace: tenant-b
labels:
app: demo-target
spec:
tolerations:
- operator: Exists
containers:
- name: target
image: quay.io/quay/busybox:latest
command: ["sleep", "3600"]
---
apiVersion: bpfman.io/v1alpha1
kind: BpfApplication
metadata:
name: demo
namespace: tenant-a
spec:
nodeSelector: {}
byteCode:
image:
url: quay.io/bpfman-bytecode/go-app-counter:latest
programs:
- name: stats
type: TC
tc:
links:
- interfaceSelector:
interfaces:
- eth0
priority: 55
direction: Ingress
networkNamespaces:
pods:
matchLabels:
app: demo-target
```
Save that manifest as `repro.yaml`. The `crictl` calls further down run inside the kind node container -- the container runtime and the pods live there -- so they go through `docker exec`. For the default bpfman kind cluster the node container is `bpfman-deployment-control-plane`; adjust the name if your cluster differs.
```console
$ kubectl apply -f repro.yaml
$ kubectl -n tenant-a wait --for=condition=Ready pod/target-a --timeout=90s
$ kubectl -n tenant-b wait --for=condition=Ready pod/target-b --timeout=90s
$ kubectl -n tenant-a wait --for=condition=Success bpfapplication/demo --timeout=300s
bpfapplication.bpfman.io/demo condition met
```
Wait for that `Success` condition before reading anything into the numbers below. The application has to pull and load the go-app-counter bytecode before it expands any links, and the first pull can take a couple of minutes (cosign verification is slow). Until it finishes there is no `BpfApplicationState` at all, and the link-count query returns `0` -- which looks exactly like "not affected". Do not mistake "not loaded yet" for a clean result.
The application lives in `tenant-a`, and exactly one pod in that namespace matches its selector, so a correctly scoped application attaches exactly one TC link. On current `main` it attaches two, and the second is a pod in `tenant-b`:
```console
$ kubectl -n tenant-a get pods -l app=demo-target
NAME READY STATUS RESTARTS AGE
target-a 1/1 Running 0 10s
$ kubectl -n tenant-a get bpfapplicationstate -o json | jq '[.items[].status.programs[].tc.links[]] | length'
2
$ kubectl -n tenant-a get bpfapplicationstate -o json | jq -r '.items[].status.programs[].tc.links[].netnsPath'
/host/proc/23122/ns/net
/host/proc/23154/ns/net
$ docker exec bpfman-deployment-control-plane crictl inspect "$(docker exec bpfman-deployment-control-plane crictl ps --name target --label io.kubernetes.pod.namespace=tenant-a -q)" | jq .info.pid
23122
$ docker exec bpfman-deployment-control-plane crictl inspect "$(docker exec bpfman-deployment-control-plane crictl ps --name target --label io.kubernetes.pod.namespace=tenant-b -q)" | jq .info.pid
23154
```
You do not have to take the count on faith. `tenant-a` holds a single matching pod (`target-a`), yet the state lists two links, and the second link's netns PID (`23154`) is the container PID of `tenant-b/target-b`. A namespaced application has attached an eBPF program into the network namespace of a pod in another namespace. The general test for any cluster: if a namespaced application's link count exceeds the number of pods matching its selector in its own namespace, the agent is reaching across namespaces.
For contrast, the same manifest against a build with the namespace scoped correctly attaches one link, the `tenant-a` pod (`24537`), and leaves `tenant-b/target-b` (`24570`) untouched:
```console
$ kubectl -n tenant-a get pods -l app=demo-target
NAME READY STATUS RESTARTS AGE
target-a 1/1 Running 0 10s
$ kubectl -n tenant-a get bpfapplicationstate -o json | jq '[.items[].status.programs[].tc.links[]] | length'
1
$ kubectl -n tenant-a get bpfapplicationstate -o json | jq -r '.items[].status.programs[].tc.links[].netnsPath'
/host/proc/24537/ns/net
$ docker exec bpfman-deployment-control-plane crictl inspect "$(docker exec bpfman-deployment-control-plane crictl ps --name target --label io.kubernetes.pod.namespace=tenant-a -q)" | jq .info.pid
24537
$ docker exec bpfman-deployment-control-plane crictl inspect "$(docker exec bpfman-deployment-control-plane crictl ps --name target --label io.kubernetes.pod.namespace=tenant-b -q)" | jq .info.pid
24570
```
### Use distinct pod names, or you will hide the bug from yourself
The two target pods share labels but have different names, and that matters. After matching pods, the netns-based program types (TC, TCX, XDP) reduce them to one container per pod -- a pod has a single network namespace, so one container is enough to find it. That reduction, `GetOneContainerPerPod`, decides which containers belong to the same pod by pod name alone:
```go
func GetOneContainerPerPod(containers *[]ContainerInfo) *[]ContainerInfo {
uniquePods := make(map[string]bool)
uniqueContainers := []ContainerInfo{}
for _, container := range *containers {
if _, ok := uniquePods[container.podName]; !ok {
uniquePods[container.podName] = true
uniqueContainers = append(uniqueContainers, container)
}
}
return &uniqueContainers
}
```
Here is why that hides the leak. Pod names are unique within a namespace but not across them. So the moment the selector match crosses namespaces -- which is the bug itself -- two different pods that happen to share a name look like one pod to this dedup, and it keeps only the first. The pod it drops is the foreign one, so the very evidence of the leak is what gets erased. Two namespace-blind steps compound: the listing ignores namespace and pulls in the other tenant's pod, then the dedup ignores namespace and merges it away. Distinct names give the two pods distinct keys, nothing is merged, and the second attachment becomes visible.
Give both targets the same name and the same buggy agent reports one link:
```console
$ kubectl -n tenant-a get pods -l app=demo-target -o custom-columns=NS:.metadata.namespace,NAME:.metadata.name
NS NAME
tenant-a target
$ kubectl -n tenant-b get pods -l app=demo-target -o custom-columns=NS:.metadata.namespace,NAME:.metadata.name
NS NAME
tenant-b target
$ kubectl -n tenant-a get bpfapplicationstate -o json | jq '[.items[].status.programs[].tc.links[]] | length'
1
```
Two matching pods across two namespaces, one link, on the exact agent that produced two links with distinct names. The agent is still listing pods cluster-wide; the dedup by name is folding the foreign pod into the local one behind their shared name. This masking is specific to the netns selectors: the container-selector types (uprobe, uretprobe) attach per container and never call `GetOneContainerPerPod`, so they show the extra pod even with identical names. When you check whether a cluster is affected with a TC, TCX or XDP selector, give the pods distinct names -- identical names will talk you into thinking it is fine when it is not, because all three steps are on the record above: with distinct names the agent attaches two links and the second netns is tenant-b's pod; with identical names the same agent attaches one; and a single link is indistinguishable from a clean, correctly-scoped result.
## Fixing it
The namespace of a namespaced program is always its owning application's namespace; there is nothing for the user to choose. The sturdy fix is to derive it from the owner rather than copy it into a field that can be left unset: hold the `BpfApplication` (or just its namespace) on the reconciler and have `getNamespace()` read from it, so "namespaced reconciler with no namespace" stops being a representable state. Threading `r.currentApp.Namespace` into the four constructions also closes the hole, but it keeps the same shape that lost the value in the first place, so it can be lost again the same way.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.