Integrate the eBPF based AppArmor recorder into the API

[ccojocar]

What type of PR is this?

/kind feature

What this PR does / why we need it:

It integrates the eBPF based AppArmor profile recorder into the API. This will allow to record AppArmor profiles for workloads directly into the cluster in the same fashion like seccomp profiles.

Which issue(s) this PR fixes:

Does this PR have test?

Yes

Special notes for your reviewer:

I will follow up with some integration tests in a separate pull request.

Does this PR introduce a user-facing change?

Add the eBPF based AppArmor profile recorder into the API.

cc @mhils @@milkmix_

[mhils]

🚀 🚀🚀

[ccojocar]

@mhils please could you review the changes and let me know if you find any issues? Thanks a lot!

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 44.44444% with 285 lines in your changes missing coverage. Please review.

Project coverage is 42.02%. Comparing base (11d77f4) to head (0f49aa7).
Report is 270 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2296      +/-   ##
==========================================
- Coverage   45.50%   42.02%   -3.48%     
==========================================
  Files          79      110      +31     
  Lines        7782    15913    +8131     
==========================================
+ Hits         3541     6688    +3147     
- Misses       4099     8732    +4633     
- Partials      142      493     +351     

[ccojocar]

@saschagrunert @mhils This is ready for final review. All tests finally passed. Thanks

[saschagrunert]

Great work! Just a documentation nit, otherwise LGTM.

[mhils]

🚀

[mhils]

Why the added sleep here?

[ccojocar]

It was not enough when running the tests. Sometimes needed to run a bit longer.

[mhils]

Is this because we now have a race with ProcessIDByName? I tried hard to make things work for short-lived processes, so if there is a different reason I'd be happy to dig into things.

[ccojocar]

Yes, that's the reason. It takes a bit of time to find the pid into the list. It is less than 30 seconds, but this is large enough to keep the tests consistently running.

[mhils]

For seccomp, it currently works as follows if --no-proc is passed:

1. We record syscalls for all mount namespaces by binaries with the target program name.
2. We merge them below in processSeccomp.

Is there a reason why we don't want to do the same for AppArmor, i.e. merge all recorded profiles?

[ccojocar]

My understanding of this flag is that the process is not started by the spoc CLI but externally and spoc will start recording the profile for it as soon as the PID comes live.

I think in the previous implementation, it was recording everything for all process running into the system until it got interrupted.

In this implementation, it will look up first the PID by command name, and then the namespace where that PID is running. This is required to retrieve the recorded information from the maps by namespace.

Now we create an apparmor profile for an entire namespace where the process is running. This is also the case for seccomp. This is not ideal, if other processes run in parallel in that namespace when the CLI is recording. The profiles will contain more stuff than expected. I think for CLI this is acceptable, since people will most likely use this feature to record profiles for a single process running into a wider system, and that process will have its own namesapce unless they will start it in an already existing namesapce.

I would argue that this feature is probably not the best option, if you want to record a profile for an entire container (e.g. form outside). In that case, I would use the in-cluster option.

[mhils]

I think in the previous implementation, it was recording everything for all process running into the system until it got interrupted.

Not quite. The ebpf code itself checks the program name and only records events for matching processes. So yes, you would get all events for "curl", no matter from which namespace, but you would not get events from the rest of the system. This is why the seccomp implementation takes all namespaces into account (mntns is 0) with --no-proc. I think we could do the same for AppArmor and avoid the PID race.

[ccojocar]

@saschagrunert @mhils if you are happy please could you approve? I'll follow up with e2e tests and other small fixes if needed since this got already quite large. Thanks a lot!

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ccojocar, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [ccojocar,saschagrunert]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment