[PATCH 0/3] Apparmor: Add profiles for hypervisor daemons

[Jim Fehlig]

On 6/16/21 11:21 AM, Christian Boltz wrote:
> Hello,
> 
> [I'm not subscribed to the libvirt list, please CC me in replies]
> 
> Am Mittwoch, 16. Juni 2021, 05:41:01 CEST schrieb Jim Fehlig:
>> This series is a first attempt at creating apparmor profiles for the
>> modular daemons. It introduces profiles for virt{lxc,qemu,xen}d, which
>> AFAIK are the only hypervisors supported by apparmor. The profiles
>> are copies of the libvirtd profile, with all the non
>> hypervisor-specific rules removed. E.g. qemu related rules removed
>> from the virtxend profile and vice versa. Likely more rules could be
>> trimmed from the xen and lxc profiles. I'll need to investigate how
>> the apparmor tools can help identify such rules.
> 
> There are two ways to do this:
> - prefix the rules with "audit" (for example "audit capability
>    sys_admin,"), reload and use the profile, and check your audit.log for
>    AUDIT events mentioning it. (Note: the aa-* tools won't help you with
>    AUDIT events.)
> - remove the rules in question and optionally set the profile to
>    complain mode, then reload and use the profile. Afterwards, check the
>    audit.log or use aa-logprof.
>    Note: aa-logprof doesn't support adding unix, mount and pivot_root
>    rules yet, so you'll have to add those manually.

Thanks for the tips!

>> So far things look okay with apparmor and modular daemons. One issue I
>> have yet to resolve is interaction between dnsmasq and
>> libvirt_leaseshelper. Trying to start e.g. the default network results
>> in the following apparmor denial
>>
>> type=AVC msg=audit(1623791662.885:655): apparmor="DENIED"
>> operation="exec" profile="/usr/sbin/dnsmasq"
>> name="/usr/lib/libvirt_leaseshelper" pid=8154 comm="sh"
>> requested_mask="x" denied_mask="x" fsuid=0 ouid=0
> 
> The dnsmasq profile already has
> 
>    # libvirt lease helper
>    /usr/lib{,64}/libvirt/libvirt_leaseshelper Cx -> libvirt_leaseshelper,
>    /usr/libexec/libvirt_leaseshelper Cx -> libvirt_leaseshelper,
> 
> /usr/lib/libvirt_leaseshelper looks like yet another path.
> Did libvirt_leaseshelp move? (I still have it as
> /usr/lib64/libvirt/libvirt_leaseshelper on openSUSE Tumbleweed.)

Facepalm!! I didn't notice the "incorrect" path! I got distracted thinking it 
was an issue with virtnetworkd permissions. And no, the path has not changed. 
The path in my snippet is from a test system where I was experimenting with the 
openSUSE libexecdir change and mistakenly left the meson configuration with 
--libexecdir=/usr/lib :-(.

Do you think it still makes sense to add a /usr/lib{,64}/libvit_leaseshelper 
rule? The only complaint thus far is from a developer with a misconfigured 
system :-).

> Technically, the dnsmasq profile will need two additions for the new
> path:
> - a Cx rule in the main profile
> - a m rule inside the libvirt_leaseshelper child profile
> 
>> Perhaps some apparmor experts can make better sense of that error than
>> me :-). It would be nice to avoid adjusting the dnsmasq profile,
>> which is not in the libvirt project, if possible.
> 
> This will be a change to the dnsmasq profile, but that's not a real
> problem.
> 
>> I noticed a few more denial messages that I _think_ are unrelated to
>> modular daemons, which also need further investigation
>>
>> type=AVC msg=audit(1623797296.856:593): apparmor="DENIED"
>> operation="open" profile="virt-aa-helper" name="/etc/ssl/openssl.cnf"
>> pid=6511 comm="virt-aa-helper" requested_mask="r" denied_mask="r"
>> fsuid=0 ouid=0
> 
> include <abstractions/openssl>
> 
>> type=AVC msg=audit(1623797296.856:594):
>> apparmor="DENIED" operation="open" profile="virt-aa-helper"
>> name="/etc/libnl/classid" pid=6511 comm="virt-aa-helper"
>> requested_mask="r" denied_mask="r" fsuid=0 ouid=0
>> type=AVC msg=audit(1623797297.732:623): apparmor="DENIED"
>> operation="open"
>> profile="libvirt-481c2d22-76d5-404b-a4b0-dc2069c7e19e"
>> name="/etc/libnl/classid" pid=6539 comm="qemu-system-x86"
>> requested_mask="r" denied_mask="r" fsuid=107 ouid=0
> 
> I don't know what libnl is/does, but allowing read permissions to this
> file doesn't look too critical.

According to the package description

The libnl suite is a collection of libraries providing APIs to
netlink protocol based Linux kernel interfaces.

Regardless, it is safe to say these are unrelated to modular daemons.

> BTW: The dnsmasq libvirt_leaseshelper child profile and
> abstractions/nameservice have
>    /etc/libnl-3/classid r,
> 
> Note the slightly different path, git blame says it's a Debian path
> added to the profile in 2016.
> (I don't remember any denial for /etc/libnl/classid on openSUSE,
> therefore I'm not sure if we should add that path to the upstream
> dnsmasq profile and/or abstractions/nameservice. Feedback welcome ;-) )

I only see the denial on SLE, and only from virt-aa-helper and per-VM profiles. 
No need to add it to the dnsmasq profile IMO.

> Also note that abstractions/nameservice allows a lot, so even if the
> path would match, please don't add it just because you need read
> permissions for a single file.

Thanks again for the review and all the information!

Regards,
Jim