haytok
commented
1 year ago Hi, Team I have completed an investigation that will cause this issue, so I will summarize it tomorrow.
Open haytok opened 1 year ago
haytok
commented
1 year ago Hi, Team I have completed an investigation that will cause this issue, so I will summarize it tomorrow.
haytok
commented
1 year ago Hi, @nyrahul
I have completed an investigation so far. Could you check it if you have time ?
When kubectl delete -f <manifestfile> is executed to initiate Pod deletion, KubeArmor will receive a MODIFIED event from the k8s watch API followed by a DELETED event.
When KubeArmor receives the MODIFIED event, if the container in the Pod is being deleted or has been deleted, the ContainerStatuses in the event received by KubeArmor from the k8s watch API will show the container's status as "terminated" instead of "running". The details of the event are shown below.
The below current implementation in KubeArmor does not check the status of containers in this Pod, but rather implements the container that has already been removed from dm.Containers by the container handler (ex. dockerHandoler.go) and re-added to dm.Containers in a layer of kubeUpdate.go. Containers which has no containerID are re-added in the kubeUpdate.go layer, which may be the reason for this issue.
for _, container := range event.Object.Status.ContainerStatuses {
if len(container.ContainerID) > 0 {
if strings.HasPrefix(container.ContainerID, "docker://") {
containerID := strings.TrimPrefix(container.ContainerID, "docker://")
pod.Containers[containerID] = container.Name
pod.ContainerImages[containerID] = container.Image + kl.GetSHA256ofImage(container.ImageID)
} else if strings.HasPrefix(container.ContainerID, "containerd://") {
containerID := strings.TrimPrefix(container.ContainerID, "containerd://")
pod.Containers[containerID] = container.Name
pod.ContainerImages[containerID] = container.Image + kl.GetSHA256ofImage(container.ImageID)
} else if strings.HasPrefix(container.ContainerID, "cri-o://") {
containerID := strings.TrimPrefix(container.ContainerID, "cri-o://")
pod.Containers[containerID] = container.Name
pod.ContainerImages[containerID] = container.Image + kl.GetSHA256ofImage(container.ImageID)
}
}
}When kubectl delete -f https://raw.githubusercontent.com/kubearmor/KubeArmor/main/tests/syscalls/manifests/ubuntu-deployment.yaml is executed, events receved from the k8s watch API are shown below.
This events are obtained from curl localhost:8001/api/v1/pods/?watch=true. This behavior is considered to be a k8s specification.
When KubeArmor received MODIFIED, containers in the Pod may have already been deleted from dm.Containers in the container handler layer by the following implementation.
delete(dm.Containers, containerID)However, in the implementation below, pod.Containers is created based on the containerStatuses of the event received from the k8s watch API, without determining from dm.Containers whether the containers in the Pod have already been deleted.
for _, container := range event.Object.Status.ContainerStatuses {
if len(container.ContainerID) > 0 {
if strings.HasPrefix(container.ContainerID, "docker://") {
containerID := strings.TrimPrefix(container.ContainerID, "docker://")
pod.Containers[containerID] = container.Name
pod.ContainerImages[containerID] = container.Image + kl.GetSHA256ofImage(container.ImageID)
} else if strings.HasPrefix(container.ContainerID, "containerd://") {
containerID := strings.TrimPrefix(container.ContainerID, "containerd://")
pod.Containers[containerID] = container.Name
pod.ContainerImages[containerID] = container.Image + kl.GetSHA256ofImage(container.ImageID)
} else if strings.HasPrefix(container.ContainerID, "cri-o://") {
containerID := strings.TrimPrefix(container.ContainerID, "cri-o://")
pod.Containers[containerID] = container.Name
pod.ContainerImages[containerID] = container.Image + kl.GetSHA256ofImage(container.ImageID)
}
}
}pod.Containers is used, for example, by the following implementation, where dm.Containers is updated using pod.Containers with information about containers that have already been deleted.
// update containers
for k := range pod.Containers {
newEndPoint.Containers = append(newEndPoint.Containers, k)
}
containersAppArmorProfiles := map[string]string{}
// update containers and apparmors
dm.ContainersLock.Lock()
for _, containerID := range newEndPoint.Containers {
container := dm.Containers[containerID]
...
dm.Containers[containerID] = containerThis may have caused the following issue described when the issue was created
Reproduction steps and Question
When I apply the manifest file below, and after deployment is complete, delete the manifest file to remove the Pod, the number of dm.Containers may differ in number from the number of containers I can see from docker ps.
Is this expected behavior? If maintainers know about below behavior, please let me know.
Note that in the current implementation, there is another location using pod.Containers (ex. KubeArmor/KubeArmor/core/kubeUpdate.go), which may have issue in other places as well.
UpdateEndPointWithPod() is called from WatchK8sPods() when a Pod is changed or modified.
Within UpdateEndPointWithPod(), processing is performed based on the pod.Containers object created within WatchK8sPods().
Therefore, I believe that to fundamentally resolve this event (the issue of dm.Containers storing information on containers that have already been deleted), it is necessary to process containers that have already been deleted so that they are not added to pod.Containers.
Below is the code to achieve this.
ubuntu ~/KubeArmor
> git diff KubeArmor/core/kubeUpdate.go
diff --git a/KubeArmor/core/kubeUpdate.go b/KubeArmor/core/kubeUpdate.go
index d5fc141d..d66e6b1e 100644
--- a/KubeArmor/core/kubeUpdate.go
+++ b/KubeArmor/core/kubeUpdate.go
@@ -613,16 +637,18 @@ func (dm *KubeArmorDaemon) WatchK8sPods() {
pod.ContainerImages = map[string]string{}
for _, container := range event.Object.Status.ContainerStatuses {
if len(container.ContainerID) > 0 {
+ containerID := ""
+
if strings.HasPrefix(container.ContainerID, "docker://") {
- containerID := strings.TrimPrefix(container.ContainerID, "docker://")
- pod.Containers[containerID] = container.Name
- pod.ContainerImages[containerID] = container.Image + kl.GetSHA256ofImage(container.ImageID)
+ containerID = strings.TrimPrefix(container.ContainerID, "docker://")
} else if strings.HasPrefix(container.ContainerID, "containerd://") {
- containerID := strings.TrimPrefix(container.ContainerID, "containerd://")
- pod.Containers[containerID] = container.Name
- pod.ContainerImages[containerID] = container.Image + kl.GetSHA256ofImage(container.ImageID)
+ containerID = strings.TrimPrefix(container.ContainerID, "containerd://")
} else if strings.HasPrefix(container.ContainerID, "cri-o://") {
- containerID := strings.TrimPrefix(container.ContainerID, "cri-o://")
+ containerID = strings.TrimPrefix(container.ContainerID, "cri-o://")
+ }
+
+ _, ok := dm.Containers[containerID]
+ if containerID != "" && ok {
pod.Containers[containerID] = container.Name
pod.ContainerImages[containerID] = container.Image + kl.GetSHA256ofImage(container.ImageID)
}Could you check it if you have time ? I would appreciate it if the maintainers could review what I have investigated. Please let me know if there are any points that are not clear. I will provide additional explanations.
daemon1024
commented
1 year ago Related #1188
daemon1024
commented
1 year ago @haytok We appreciate a lot you doing a complete and deep investigation on this!
I am looking into this actively. I will get back to you on this early next week.
haytok
commented
1 year ago Thanks for comments !!! OK :)
Bug Report
General Information
My development environment is as follows. (Note that, more details can be found at this link)
To Reproduce
Reproduction steps and Question
When I apply the manifest file below, and after deployment is complete, delete the manifest file to remove the Pod, the number of dm.Containers may differ in number from the number of containers I can see from
docker ps.https://raw.githubusercontent.com/kubearmor/KubeArmor/main/tests/syscalls/manifests/ubuntu-deployment.yaml
Is this expected behavior? If maintainers know about below behavior, please let me know.
Note that @nyrahul advised me to raise a new issue on Slack.
What I have investigated
The logs from running
make runare shown below. Debugging print is added. Note that the container name in the deployed Pod isubuntu-1-containeras shown in the manifest file above.Log of make run after kubectl delete -f https://raw.githubusercontent.com/kubearmor/KubeArmor/main/tests/syscalls/manifests/ubuntu-deployment.yaml
```bash # At this point, kubectl delete -f https://raw.githubusercontent.com/kubearmor/KubeArmor/main/tests/syscalls/manifests/ubuntu-deployment.yaml 2023-08-07 12:55:10.810485 INFO Updated visibility map with key={PidNS:4026532689 MntNS:4026532686} for cid 0aaca1d8f13922ca9d75ebcbf05df55907888be42237512ceb79c2cfb14d56f9 2023-08-07 12:55:10.810525 INFO Updated visibility map with key={PidNS:4026532764 MntNS:4026532762} for cid 645bfdebfdd058076bb76e99874c15cce2cb50440478847bdcf0831a6135fdff 2023-08-07 12:55:10.810540 INFO Namespace syscalls visibiliy configured {File:true Process:true Network:true Capabilities:false} 2023-08-07 12:55:10.845483 INFO Detected a Pod (modified/syscalls/ubuntu-1-deployment-9c9dbdb8-gc4xk) Before dm.UpdateDockerContainer(): 13 After dm.UpdateDockerContainer(): 13 container: types.Container{ContainerID:"645bfdebfdd058076bb76e99874c15cce2cb50440478847bdcf0831a6135fdff", ContainerName:"ubuntu-1-container", ContainerImage:"kubearmor/ubuntu-w-utils:0.1@sha256:b4693b003ed1fbf7f5ef2c8b9b3f96fd853c30e1b39549cf98bd772fbd99e260", NamespaceName:"syscalls", Owner:types.PodOwner{Ref:"", Name:"", Namespace:""}, EndPointName:"ubuntu-1-deployment-9c9dbdb8-gc4xk", Labels:"container=ubuntu-1", AppArmorProfile:"kubearmor-syscalls-ubuntu-1-deployment-ubuntu-1-container", PidNS:0xf000039c, MntNS:0xf000039a, MergedDir:"/var/lib/docker/overlay2/bc2ee370784ecdc6fde4c1df8f3e6c5f143883194d1b36bf89ace3fecc9bcd4e/merged", PolicyEnabled:1, ProcessVisibilityEnabled:true, FileVisibilityEnabled:true, NetworkVisibilityEnabled:true, CapabilitiesVisibilityEnabled:false} 2023-08-07 12:55:16.009577 INFO Updated visibility map with key={PidNS:4026532689 MntNS:4026532686} for cid 0aaca1d8f13922ca9d75ebcbf05df55907888be42237512ceb79c2cfb14d56f9 2023-08-07 12:55:16.009629 INFO Updated visibility map with key={PidNS:4026532764 MntNS:4026532762} for cid 645bfdebfdd058076bb76e99874c15cce2cb50440478847bdcf0831a6135fdff 2023-08-07 12:55:16.009644 INFO Namespace syscalls visibiliy configured {File:true Process:true Network:true Capabilities:false} Before dm.UpdateDockerContainer(): 13 After dm.UpdateDockerContainer(): 13 container: types.Container{ContainerID:"645bfdebfdd058076bb76e99874c15cce2cb50440478847bdcf0831a6135fdff", ContainerName:"ubuntu-1-container", ContainerImage:"kubearmor/ubuntu-w-utils:0.1@sha256:b4693b003ed1fbf7f5ef2c8b9b3f96fd853c30e1b39549cf98bd772fbd99e260", NamespaceName:"syscalls", Owner:types.PodOwner{Ref:"", Name:"", Namespace:""}, EndPointName:"ubuntu-1-deployment-9c9dbdb8-gc4xk", Labels:"container=ubuntu-1", AppArmorProfile:"kubearmor-syscalls-ubuntu-1-deployment-ubuntu-1-container", PidNS:0xf000039c, MntNS:0xf000039a, MergedDir:"/var/lib/docker/overlay2/bc2ee370784ecdc6fde4c1df8f3e6c5f143883194d1b36bf89ace3fecc9bcd4e/merged", PolicyEnabled:1, ProcessVisibilityEnabled:true, FileVisibilityEnabled:true, NetworkVisibilityEnabled:true, CapabilitiesVisibilityEnabled:false} Before dm.UpdateDockerContainer(): 13 After dm.UpdateDockerContainer(): 13 container: types.Container{ContainerID:"645bfdebfdd058076bb76e99874c15cce2cb50440478847bdcf0831a6135fdff", ContainerName:"ubuntu-1-container", ContainerImage:"kubearmor/ubuntu-w-utils:0.1@sha256:b4693b003ed1fbf7f5ef2c8b9b3f96fd853c30e1b39549cf98bd772fbd99e260", NamespaceName:"syscalls", Owner:types.PodOwner{Ref:"", Name:"", Namespace:""}, EndPointName:"ubuntu-1-deployment-9c9dbdb8-gc4xk", Labels:"container=ubuntu-1", AppArmorProfile:"kubearmor-syscalls-ubuntu-1-deployment-ubuntu-1-container", PidNS:0xf000039c, MntNS:0xf000039a, MergedDir:"/var/lib/docker/overlay2/bc2ee370784ecdc6fde4c1df8f3e6c5f143883194d1b36bf89ace3fecc9bcd4e/merged", PolicyEnabled:1, ProcessVisibilityEnabled:true, FileVisibilityEnabled:true, NetworkVisibilityEnabled:true, CapabilitiesVisibilityEnabled:false} Before dm.UpdateDockerContainer(): 13 2023-08-07 12:55:41.029125 INFO Successfully deleted visibility map with key={PidNS:4026532764 MntNS:4026532762} from the kernel 2023-08-07 12:55:41.029164 INFO Detected a container (removed/645bfdebfdd0) After dm.UpdateDockerContainer(): 12 Before dm.UpdateDockerContainer(): 12 After dm.UpdateDockerContainer(): 12 Before dm.UpdateDockerContainer(): 12 After dm.UpdateDockerContainer(): 12 Before dm.UpdateDockerContainer(): 12 2023-08-07 12:55:41.229097 INFO Successfully deleted visibility map with key={PidNS:4026532689 MntNS:4026532686} from the kernel 2023-08-07 12:55:41.229138 INFO Detected a container (removed/0aaca1d8f139) After dm.UpdateDockerContainer(): 11 2023-08-07 12:55:41.385315 INFO Detected a Pod (modified/syscalls/ubuntu-1-deployment-9c9dbdb8-gc4xk) Before dm.UpdateDockerContainer(): 12 2023-08-07 12:55:41.388907 INFO Detected a container (removed/645bfdebfdd0) After dm.UpdateDockerContainer(): 11 2023-08-07 12:55:41.670425 INFO Detected a Pod (modified/syscalls/ubuntu-1-deployment-9c9dbdb8-gc4xk) 2023-08-07 12:55:41.676224 INFO Removed ubuntu-1-deployment-9c9dbdb8-gc4xk from the pod list of the AppArmor profile (kubearmor-syscalls-ubuntu-1-deployment-ubuntu-1-container, 0) 2023-08-07 12:55:41.676283 INFO Detected a Pod (deleted/syscalls/ubuntu-1-deployment-9c9dbdb8-gc4xk) 2023-08-07 12:55:43.236736 INFO Namespace syscalls visibiliy configured {File:true Process:true Network:true Capabilities:false} # This is where I'm questioning the operation. Before dm.UpdateDockerContainer(): 12 After dm.UpdateDockerContainer(): 12 container: types.Container{ContainerID:"", ContainerName:"ubuntu-1-container", ContainerImage:"kubearmor/ubuntu-w-utils:0.1@sha256:b4693b003ed1fbf7f5ef2c8b9b3f96fd853c30e1b39549cf98bd772fbd99e260", NamespaceName:"syscalls", Owner:types.PodOwner{Ref:"", Name:"", Namespace:""}, EndPointName:"ubuntu-1-deployment-9c9dbdb8-gc4xk", Labels:"container=ubuntu-1", AppArmorProfile:"", PidNS:0x0, MntNS:0x0, MergedDir:"", PolicyEnabled:1, ProcessVisibilityEnabled:true, FileVisibilityEnabled:true, NetworkVisibilityEnabled:true, CapabilitiesVisibilityEnabled:false} ```Note that log
Detected a Pod (modified/<namespace>/<Pod name)is output by below logic.Also, The debug messages are added to dockerHandler.go with the following print debugg code.
print debug codes
```bash ubuntu ~/KubeArmor > git diff KubeArmor/core/dockerHandler.go diff --git a/KubeArmor/core/dockerHandler.go b/KubeArmor/core/dockerHandler.go index 3c569aba..e025735f 100644 --- a/KubeArmor/core/dockerHandler.go +++ b/KubeArmor/core/dockerHandler.go @@ -441,7 +441,14 @@ func (dm *KubeArmorDaemon) MonitorDockerEvents() { // if message type is container if msg.Type == "container" { + fmt.Printf("Before dm.UpdateDockerContainer(): %d\n", len(dm.Containers)) dm.UpdateDockerContainer(msg.ID, msg.Action) + fmt.Printf("After dm.UpdateDockerContainer(): %d\n", len(dm.Containers)) + for _, container := range dm.Containers { + if container.ContainerName == "ubuntu-1-container" { + fmt.Printf("container: %#v\n", container) + } + } } } } ```As you can see from
docker ps, the number of containers running is11. Containers is expected to be11, but it is12.Results of docker ps
```bash ubuntu ~ > docker ps CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES f66dbbe6470c accuknox/knoxautopolicy "/knoxAutoPolicy" 5 days ago Up 5 days k8s_discovery-engine_discovery-engine-6d57586d46-9lxg7_accuknox-agents_011a94e4-f805-4cd6-9d87-bccd17d972c4_7 0790abc6b991 accuknox/dsp-controller "/manager --leader-e…" 5 days ago Up 5 days k8s_manager_discovery-engine-6d57586d46-9lxg7_accuknox-agents_011a94e4-f805-4cd6-9d87-bccd17d972c4_6 c581362cd49b kubearmor/kubearmor-relay-server "/KubeArmor/kubearmo…" 5 days ago Up 5 days k8s_kubearmor-relay-server_kubearmor-relay-5656cc5bf7-8qkl5_kube-system_9c4e1ee2-144a-4a8f-a89e-f80e169c758f_1 388ee914ce6f rancher/mirrored-pause:3.6 "/pause" 5 days ago Up 5 days k8s_POD_discovery-engine-6d57586d46-9lxg7_accuknox-agents_011a94e4-f805-4cd6-9d87-bccd17d972c4_1 857878e6a686 fb9b574e03c3 "local-path-provisio…" 5 days ago Up 5 days k8s_local-path-provisioner_local-path-provisioner-6c79684f77-ndq4v_kube-system_c053c3f7-ac66-46a9-b917-854ac3e36677_4 b5fc90ea9bb9 99376d8f35e0 "/coredns -conf /etc…" 5 days ago Up 5 days k8s_coredns_coredns-d76bd69b-j9l92_kube-system_c901dff1-3e7e-43d1-b9c6-350e9ce00e9c_4 6e67fb7528af rancher/mirrored-pause:3.6 "/pause" 5 days ago Up 5 days k8s_POD_local-path-provisioner-6c79684f77-ndq4v_kube-system_c053c3f7-ac66-46a9-b917-854ac3e36677_4 455014a0d399 f73640fb5061 "/metrics-server --c…" 5 days ago Up 5 days k8s_metrics-server_metrics-server-7cd5fcb6b7-6vzc6_kube-system_b96543ec-99ed-423c-910b-ded7f4d34179_4 0a708100413a rancher/mirrored-pause:3.6 "/pause" 5 days ago Up 5 days k8s_POD_coredns-d76bd69b-j9l92_kube-system_c901dff1-3e7e-43d1-b9c6-350e9ce00e9c_4 c1e0f735a20e rancher/mirrored-pause:3.6 "/pause" 5 days ago Up 5 days k8s_POD_kubearmor-relay-5656cc5bf7-8qkl5_kube-system_9c4e1ee2-144a-4a8f-a89e-f80e169c758f_1 c2291c0ac09b rancher/mirrored-pause:3.6 "/pause" 5 days ago Up 5 days k8s_POD_metrics-server-7cd5fcb6b7-6vzc6_kube-system_b96543ec-99ed-423c-910b-ded7f4d34179_4 ```Based on this information, it is possible that there is a issue with containers in the deleted Pod being added to dm.Containers.
Root Cause
The application container (
645bfdebfdd0) in the deployed Pod is deleted, followed by the corresponding pause container (0aaca1d8f139), and the Pod deletion is complete at the point before the following log.However, it appears from the log that when all containers in the Pod were deleted, a Pod modification event was detected to have occurred, which added deleted container information to dm.Containers.
The point is that an object without conainerID has been added to dm.containers, as can also be seen from the print debug.
Now, I have not been able to investigate why the
Detected a Pod (modified/syscalls/ubuntu-1-deployment-9c9dbdb8-gc4xk)event occurs when the pause container is deleted and the Pod seems to have been completely removed.Next Actions
From the above logs of
make run, when the pause container is removed, KubeArmor detects that the Pod has been modified and theDetected a Pod (modified/syscalls/ubuntu-1-deployment-9c9dbdb8-gc4xk)event is generated.The process to be performed at that time is as follows.
It is confirmed from the operation that the deleted container is added to dm.Containers by
else if action == "MODIFIED"after the event of modified occurs.Therefore, it is thought that the issue will not occur if the processing in the following section is modified as shown in the following diff.
Code to fix this issue
```bash ubuntu ~/KubeArmor > git diff KubeArmor/core/kubeUpdate.go diff --git a/KubeArmor/core/kubeUpdate.go b/KubeArmor/core/kubeUpdate.go index d5fc141d..712bfa3d 100644 --- a/KubeArmor/core/kubeUpdate.go +++ b/KubeArmor/core/kubeUpdate.go @@ -397,6 +397,10 @@ func (dm *KubeArmorDaemon) UpdateEndPointWithPod(action string, pod tp.K8sPod) { // update containers and apparmors dm.ContainersLock.Lock() for _, containerID := range newEndPoint.Containers { + if _, ok := dm.Containers[containerID]; !ok { + continue + } + container := dm.Containers[containerID] container.NamespaceName = newEndPoint.NamespaceName ```However, in any case, when creating the following cotainer object, it is performed under the condition of
action == “MODIFID", so if there is no containerID already in the key of the dm.containers, I think it's better to check the key of the map object.Please let me know if the above correction is correct. I would like to modify the code and create a pull request.
Expected behavior
It is expected that dm.Containers contains objects with the number of containers that can be confirmed from
docker ps.Screenshots
N/A