Update apparmor profile to support v4.0.0

[NeilW]

AppArmor v4.0.0 introduced podman, runc and crun profiles for /usr/bin/podman, /usr/sbin/runc and /usr/bin/crun respectively[1]. This change breaks the stopping of containers, because the built-in profile assigned to containers doesn't accept signals from podman, runc and crun peers.

This commit extends the default profile with rules that allow receiving signals from processes that run confined with the podman, runc or crun profile. It is backward compatible because the peer value is a regular expression (AARE) so the referenced profile doesn't have to exist for this profile to successfully compile and load.

The signal set from runc or crun remains unconstrained as the user can issue any signal via the kill sub-command of podman.

Signals from podman itself are restricted to the common interrupt and termination signals.

Closes #1898

[1] https://gitlab.com/apparmor/apparmor/-/commit/2594d936

[packit-as-a-service]

Ephemeral COPR build failed. @containers/packit-build please check.

[rhatdan]

Does this support globs?

signal (receive) peer={/usr/bin/,/usr/sbin/,}crun*,

[NeilW]

It's an "AppArmor Regular Expression" which is a glob with knobs on

Globbing (AARE)

File resources and other parameters accepting an AARE may be specified with a globbing syntax similar to that used by popular shells, such as [csh(1)](https://man.archlinux.org/man/csh.1.en), [bash(1)](https://man.archlinux.org/man/bash.1.en), [zsh(1)](https://man.archlinux.org/man/zsh.1.en).

*
can substitute for any number of characters, excepting '/'
**
can substitute for any number of characters, including '/'
?
can substitute for any single character excepting '/'
[abc]
will substitute for the single character a, b, or c
[a-c]
will substitute for the single character a, b, or c
[^a-c]
will substitute for any single character not matching a, b or c
{ab,cd}
will expand to one rule to match ab, one rule to match cd
Can also include variables.
@{variable}
will expand to all values assigned to the given variable.
When AppArmor looks up a directory the pathname being looked up will end with a slash (e.g., /var/tmp/); otherwise it will not end with a slash. Only rules that match a trailing slash will match directories. Some examples, none matching the /tmp/ directory itself, are:

/tmp/*
Files directly in /tmp.
/tmp/*/
Directories directly in /tmp.
/tmp/**
Files and directories anywhere underneath /tmp.
/tmp/**/
Directories anywhere underneath /tmp.

[rhatdan]

Ok change the crun to allow for crun-wasm, crun-qm and other future OCI Runtimes based off of crun.

[NeilW]

That's done.

[rhatdan]

LGTM, one small question.

[Luap99]

podman uses kill(pid,0) to check if the process is still alive, would that get blocked by this list?
Is there any practical reason to limit podman to certain signals?

[NeilW]

The permission to check a pid is given in the imported abstractions/base along with the permissions to self-signal and receive unconfined processes

  # Allow unconfined processes to send us signals by default
  signal (receive) peer=unconfined,

  # Allow us to signal ourselves
  signal peer=@{profile_name},

  # Checking for PID existence is quite common so add it by default for now
  signal (receive, send) set=("exists"),

The two lines that already appear in the linux_template are really superfluous as they are handled by abstractions/base and have been since apparmor 2.9 which introduced signal filtering a decade or more ago.

The reason for filtering signals is the same reason for denying access to anything via apparmor - defence against error and compromise. SIGSTOP to conmon for example. The conmon code appears to use default dispositions for most signals. Arguably SIGQUIT could be removed from the list to avoid a Core default disposition (does anything allow core dumps any more?).

I couldn't tell from testing (or the podman code) what signals podman expects to send. I just got a DENIED error in the logs. I plumped for a standard set and that seemed to do the trick.

May 14 11:14:41 srv-omzr6 kernel: audit: type=1400 audit(1715685281.392:118): apparmor="DENIED" operation="signal" class="signal" profile="containers-default-0.57.4" pid=7458 comm="conmon" requested_mask="receive" denied_mask="receive" signal=term peer="podman"

If you have the actual signals podman intends to send, then we can limit to that.

[Luap99]

The reason for filtering signals is the same reason for denying access to anything via apparmor - defence against error and compromise. SIGSTOP to conmon for example. The conmon code appears to use default dispositions for most signals. Arguably SIGQUIT could be removed from the list to avoid a Core default disposition (does anything allow core dumps any more?).

Sure but I don't see how anyone can expect a container to be safe in case of a podman compromise. Blocking certain signals for security seems pointless. As a user you can run podman kill to send any signal, sure that is funnelled through the oci runtime (crun,runc) and you have to allow everything there anyway. Because nothing can prevent a compromised from calling the oci runtime preventing podman from sending a certain signal does not add any security IMO.

I don't think we send any other signals today but I cannot be sure about that. But if it changes in the future the profiles will not allow it causing unnecessary bugs. And no maintainer will remember to edit the apparmor profile until a bug will be reported.

[NeilW]

It's the usual debate about whether to be open to all (the standard unix approach) or be locked down (which is what selinux and apparmor add in).

I can't really help with that. It's a philosophical point for the podman project to decide. I don't know in detail what conmon is there to do and whether it requires signal filtering.

All I want is for my database containers to stop failing.

[Luap99]

Yeah fair enough, the current profile doesn't work at all and this fixes the problems we are seeing today so I am ok merging it as is. If we start sending other signals we fail again but it is unlikely enough that I can live with such possibilities.

[Luap99]

LGTM

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, NeilW

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 [Luap99]

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

[mheon]

/lgtm