common icon indicating copy to clipboard operation
common copied to clipboard

AppArmor profile: recently added signal peers allow too much

Open cboltz opened this issue 1 year ago • 5 comments

In https://github.com/containers/common/commit/1aedc12e356cfd29a5bb54d94e9b2e09da3649ca you added the following signal rules to the AppArmor profile:

  # Allow certain signals from OCI runtimes (podman, runc and crun)
  signal (receive) peer={/usr/bin/,/usr/sbin/,}runc,
  signal (receive) peer={/usr/bin/,/usr/sbin/,}crun*,
  signal (receive) set=(int, quit, kill, term) peer={/usr/bin/,/usr/sbin/,}podman,

This is not completely wrong, but it allows more than really needed.

a) The profiles added in https://gitlab.com/apparmor/apparmor/-/commit/2594d936 are all "named" profiles:

profile runc /usr/sbin/runc flags=(unconfined) {
profile crun /usr/bin/crun flags=(unconfined) {
profile podman /usr/bin/podman flags=(unconfined) {

This means you can reference them by their name (runc, crun and podman). Including the path in peer= is superfluous, peer=runc is enough.

b) Wildcard for crun*

I don't know why you allow crun* instead of just crun, but that means that profiles matching that name (for example "cruncher") will be allowed to send signals. If this isn't intentional, I'd recommend to remove the *.

.

To sum it up: I propose to change the lines added in https://github.com/containers/common/commit/1aedc12e356cfd29a5bb54d94e9b2e09da3649ca to

  # Allow certain signals from OCI runtimes (podman, runc and crun)
  signal (receive) peer=runc,
  signal (receive) peer=crun,
  signal (receive) set=(int, quit, kill, term) peer=podman,

cboltz avatar May 28 '24 21:05 cboltz