[libvirt] [PATCH v2 4/4] security: don't relabel chardev source if virtlogd is used as stdio handler

[Pavel Hrdina]

On Tue, Jun 13, 2017 at 08:00:41PM -0400, John Ferlan wrote:
> 
> 
> On 05/29/2017 10:31 AM, Pavel Hrdina wrote:
> > In the case that virtlogd is used as stdio handler we pass to QEMU
> > only FD to a PIPE connected to virtlogd instead of the file itself.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1430988
> > 
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> > 
> > Notes:
> >     new in v2
> > 
> >  src/lxc/lxc_process.c            |  6 ++---
> >  src/qemu/qemu_security.c         |  9 +++++--
> >  src/security/security_apparmor.c |  7 ++++--
> >  src/security/security_dac.c      | 54 +++++++++++++++++++++++++++++++---------
> >  src/security/security_driver.h   |  6 +++--
> >  src/security/security_manager.c  | 12 ++++++---
> >  src/security/security_manager.h  |  6 +++--
> >  src/security/security_nop.c      |  6 +++--
> >  src/security/security_selinux.c  | 53 ++++++++++++++++++++++++++++++---------
> >  src/security/security_stack.c    | 12 ++++++---
> >  tests/securityselinuxlabeltest.c |  2 +-
> >  11 files changed, 127 insertions(+), 46 deletions(-)
> > 
> 
> Why is it a (!chr_seclabel && chardevStdioLogd)?  More to the point why
> is (!chr_seclabel) even matter?

If you configure the label we shouldn't ignore it in some cases, that's
just wrong.  If the label is set for the char device we will configure
it every time even if it will fail to start the guest, it's a
responsibility of the user to configure proper label if it is provided.

> IIUC, whether or not someone set the label for the chardev, for this
> particular issue/config where the chardev has a <log file=$path>, we
> don't want to set (or restore) the label. I feel like I'm missing
> something subtle. Maybe a bit more explanation of the adjustment would
> help me...

This is not for the <log file=$path/> but for the <source path=$path/>.
We don't relabel $path for <log file=$path/> at all.

> Wouldn't these changes end up selecting "any" chardev if
> chardevStdioLogd ended up being set regardless of whether they were
> actually using the log file?

I don't know what you mean by this sentence?

> As an aside, I think there's an "oddity" when it comes to the Restore,
> but I'm not sure how to put it into words exactly. If a guest is running
> code prior to this set of changes, would it have successfully run a Set?
> If so, then after applying this change and restarting, the label
> wouldn't be reset, right?  What happens at guest shutdown - does the
> label not get unset now?  Of course this is all "interaction" with
> virtlogd restart that really throws a monkey wrench into things.

No, that's not correct.  The @chardevStdioLogd is stored in the status
XML (the one stored in /var/run/libvirt/qemu/$domain_name.xml).  So when
the libvirtd is stopped and started with this patch applied the status
XML doesn't have the @chardevStdioLogd stored in it so it will be false
and we will reset the label.  The @chardevStdioLogd is updated only when
the domain is started and we will store the value in the status XML
only with new libvirtd and only in that case we will not set/restore
the label.

> Also, why is the Smartcard chardev handling not using this

The smartcard doesn't ever use virtlogd as stdio handler.

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170615/bacf87bf/attachment-0001.sig>