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.

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

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.

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
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
t
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


Jim Fehlig (3):
  Apparmor: Add profile for virtqemud
  Apparmor: Add profile for virtlxcd
  Apparmor: Add profile for virtxend

 src/security/apparmor/libvirt-lxc           |   4 +-
 src/security/apparmor/libvirt-qemu          |   6 +
 src/security/apparmor/meson.build           |   3 +
 src/security/apparmor/usr.sbin.virtlxcd.in  |  89 +++++++++++++
 src/security/apparmor/usr.sbin.virtqemud.in | 135 ++++++++++++++++++++
 src/security/apparmor/usr.sbin.virtxend.in  |  78 +++++++++++
 6 files changed, 314 insertions(+), 1 deletion(-)
 create mode 100644 src/security/apparmor/usr.sbin.virtlxcd.in
 create mode 100644 src/security/apparmor/usr.sbin.virtqemud.in
 create mode 100644 src/security/apparmor/usr.sbin.virtxend.in

-- 
2.31.1

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.

> 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.)

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.

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 ;-) )

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.


Regards,

Christian Boltz
-- 
<cboltz> I wonder if I should add "sponsored by Aspirin" ;-)
<jjohansen> you could have a nice little side business if Asprin
            was sponsoring all the bugs you find
[from #apparmor]

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