[libvirt] [PATCH] apparmor: support finer-grained ptrace checks

Fri Sep 22 15:39:01 UTC 2017

On 22.09.2017 14:52, Guido Günther wrote:
> Hi Jim,
> On Wed, Sep 20, 2017 at 11:17:06AM -0600, Jim Fehlig wrote:
>> On 09/20/2017 08:57 AM, Jim Fehlig wrote:
>>> On 09/20/2017 12:51 AM, Guido Günther wrote:
>>>> Hi Jim,
>>>> On Mon, Sep 18, 2017 at 02:06:13PM -0600, Jim Fehlig wrote:
>>>>> Kernel 4.13 introduced finer-grained ptrace checks
>>>>>
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?h=v4.13.2&id=290f458a4f16f9cf6cb6562b249e69fe1c3c3a07
>>>>>
>>>>>
>>>>> When Apparmor is enabled and libvirtd is confined, attempting to start
>>>>> a domain fails
>>>>>
>>>>> virsh start test
>>>>> error: Failed to start domain test
>>>>> error: internal error: child reported: Kernel does not provide mount
>>>>>         namespace: Permission denied
>>>>>
>>>>> The audit log contains
>>>>>
>>>>> type=AVC msg=audit(1505466699.828:534): apparmor="DENIED"
>>>>> operation="ptrace" profile="/usr/sbin/libvirtd" pid=6621
>>>>> comm="libvirtd" requested_mask="trace" denied_mask="trace"
>>>>> peer="/usr/sbin/libvirtd"
>>>>
>>>> It seems access to /proc/<pid>/tasks already requires trace permissions.
>>>>
>>>>>
>>>>> It was also noticed that simply connecting to libvirtd (e.g. virsh list)
>>>>> resulted in the following entries in the audit log
>>>>>
>>>>> type=AVC msg=audit(1505755799.975:65): apparmor="DENIED"
>>>>> operation="ptrace" profile="/usr/sbin/libvirtd" pid=1418
>>>>> comm="libvirtd" requested_mask="trace" denied_mask="trace"
>>>>> peer="unconfined"
>>>>> type=AVC msg=audit(1505755799.976:66): apparmor="DENIED"
>>>>> operation="ptrace" profile="/usr/sbin/libvirtd" pid=1418
>>>>> comm="libvirtd" requested_mask="trace" denied_mask="trace"
>>>>> peer="unconfined"
>>>>>
>>>>> Both Apparmor denials can be fixed by adding ptrace rules to the
>>>>> libvirtd profile. The new rules only grant trace permission.
>>>>
>>>> I'm seeing the same denials with 4.13 (4.13.1-1~exp1 (2017-09-11) in
>>>> Debian) but the proposed profile change does not fix the vm start issue
>>>> for me. I can't tell why atm, will have to look into this in more detail
>>>> at the WE.
>>>
>>> I have other problems when running with 'security_default_confined = 1'
>>> in qemu.conf, but the changes allow starting unconfined domains.
>>>
>>> Cedric remembered this old thread
>>>
>>> https://www.redhat.com/archives/libvir-list/2014-October/msg00011.html
>>>
>>> Some of those changes have been merged, but the ptrace, dbus, signal,
>>> etc. have not. I used Stefan's changes to the libvirtd profile but still
>>> see the same issue with confined domains
>>
>> I dug a bit further in that thread to find Stefan's most recent version of
>> the patches
>>
>> https://www.redhat.com/archives/libvir-list/2014-October/msg00556.html
>>
>> I took the ptrace, dbus, signal, etc. changes out of patch 2 and used the
>> attached patch to successfully start confined domains.
>>
>> Since a few years have passed, I'm not sure if patch 1 is still relevant.
>> IIUC, it allows to conditionalize profile content based on apparmor version,
>> which patch 2 uses to add some stuff if version >= 2.9. 2.9 has been out for
>> a while...
> 
> Great, this helps a lot!

Sorry I missed the updates (and also sorry/warning that I have not read the
thread very closely... brain entangled in various mental corners). Christian and
I had started to finally get rid of things we carry by properly upstreaming
things. Sadly about maybe halfway through and then other things popped up.
But for the special parsing, yes we both also thought that nowadays there is no
reason anymore to add the whole parsing stuff as everybody should be on versions
off apparmor supporting those things.

I cannot promise anything but maybe there will be a little bit of time next week
to give some better answers.

-Stefan
> 
>> From e3bb609812776b30acfc0349b25b2e4d539c45c2 Mon Sep 17 00:00:00 2001
>> From: Jim Fehlig <jfehlig at suse.com>
>> Date: Mon, 18 Sep 2017 13:41:26 -0600
>> Subject: [PATCH] apparmor: support ptrace checks
>>
>> Kernel 4.13 introduced finer-grained ptrace checks
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?h=v4.13.2&id=290f458a4f16f9cf6cb6562b249e69fe1c3c3a07
>>
>> When Apparmor is enabled and libvirtd is confined, attempting to start
>> a domain fails
>>
>> virsh start test
>> error: Failed to start domain test
>> error: internal error: child reported: Kernel does not provide mount
>>        namespace: Permission denied
>>
>> The audit log contains
>>
>> type=AVC msg=audit(1505466699.828:534): apparmor="DENIED"
>> operation="ptrace" profile="/usr/sbin/libvirtd" pid=6621
>> comm="libvirtd" requested_mask="trace" denied_mask="trace"
>> peer="/usr/sbin/libvirtd"
>>
>> It was also noticed that simply connecting to libvirtd (e.g. virsh list)
>> resulted in the following entries in the audit log
>>
>> type=AVC msg=audit(1505755799.975:65): apparmor="DENIED"
>> operation="ptrace" profile="/usr/sbin/libvirtd" pid=1418
>> comm="libvirtd" requested_mask="trace" denied_mask="trace"
>> peer="unconfined"
>> type=AVC msg=audit(1505755799.976:66): apparmor="DENIED"
>> operation="ptrace" profile="/usr/sbin/libvirtd" pid=1418
>> comm="libvirtd" requested_mask="trace" denied_mask="trace"
>> peer="unconfined"
>>
>> Both Apparmor denials can be fixed by supporting ptrace in the
>> libvirtd, qemu, and lxc profiles. While at it, also add support
>> for dbus, signal, and unix.
>>
>> Resolves: https://bugzilla.suse.com/show_bug.cgi?id=1058847
>> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
>> ---
>>  examples/apparmor/libvirt-lxc       | 3 +++
>>  examples/apparmor/libvirt-qemu      | 3 +++
>>  examples/apparmor/usr.sbin.libvirtd | 6 ++++++
>>  3 files changed, 12 insertions(+)
>>
>> diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc
>> index 4bfb503aa..0db137de0 100644
>> --- a/examples/apparmor/libvirt-lxc
>> +++ b/examples/apparmor/libvirt-lxc
>> @@ -3,6 +3,9 @@
>>    #include <abstractions/base>
>>  
>>    umount,
>> +  dbus,
>> +  signal,
>> +  ptrace,
> 
> Can't we get a long with a
> 
>     ptrace (tracedby) peer=/usr/sbin/libvirtd,
> 
> here too?
> 
>>  
>>    # ignore DENIED message on / remount
>>    deny mount options=(ro, remount) -> /,
>> diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
>> index dcfb1a598..6a4a2335a 100644
>> --- a/examples/apparmor/libvirt-qemu
>> +++ b/examples/apparmor/libvirt-qemu
>> @@ -170,6 +170,9 @@
>>    @{PROC}/device-tree/** r,
>>    /sys/firmware/devicetree/** r,
>>  
>> +  signal (receive) peer=/usr/sbin/libvirtd,
>> +  ptrace (tracedby) peer=/usr/sbin/libvirtd,
> 
> I would have expected to need the ptrace part but don't (see below).
> 
>> +
>>    # for gathering information about available host resources
>>    /sys/devices/system/cpu/ r,
>>    /sys/devices/system/node/ r,
>> diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd
>> index acb59e071..9aadba411 100644
>> --- a/examples/apparmor/usr.sbin.libvirtd
>> +++ b/examples/apparmor/usr.sbin.libvirtd
>> @@ -36,6 +36,12 @@
>>    network inet6 dgram,
>>    network packet dgram,
>>    network packet raw,
>> +  network netlink,
>> +
>> +  dbus bus=system,
>> +  signal,
>> +  ptrace,
> 
> ^^^^^^^
> 
> This single line is enough to make things work for me on 4.13. AFAIK
> dbus mediation is not upstream yet and I think unix socket and signal
> support is neither. Should we drop these for now (the syntax and
> behaviour might change while things are being upsreamed)?
> 
> Regarding the ptrace changes in libvirt-{lxc,qemu}. They're not needed
> here but if you need them to run domains I'm fine (although it would be
> great to know which denials you were seeing).
> 
> Cheers,
>  -- Guido
> 
>> +  unix,
>>  
>>    # Very lenient profile for libvirtd since we want to first focus on confining
>>    # the guests. Guests will have a very restricted profile.
>> -- 
>> 2.14.1
>>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170922/0107571c/attachment-0001.sig>