Bug 2209191 - VM with passt fails to start if logFile is set to /var/log
Summary: VM with passt fails to start if logFile is set to /var/log
Keywords:
Status: VERIFIED
Alias: None
Product: Red Hat Enterprise Linux 9
Classification: Red Hat
Component: libvirt
Version: 9.2
Hardware: x86_64
OS: Linux
unspecified
medium
Target Milestone: rc
: ---
Assignee: Michal Privoznik
QA Contact: yalzhang@redhat.com
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-05-23 03:43 UTC by Germano Veit Michel
Modified: 2023-08-16 18:47 UTC (History)
10 users (show)

Fixed In Version: libvirt-9.5.0-5.el9
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: Bug
Target Upstream Version: 9.5.0
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Issue Tracker RHELPLAN-157887 0 None None None 2023-05-23 03:43:49 UTC
Red Hat Knowledge Base (Solution) 7017541 0 None None None 2023-06-06 22:24:23 UTC

Description Germano Veit Michel 2023-05-23 03:43:10 UTC
Description of problem:

Configuring the domain XML to set up user networking with passt for a VM fails to start the VM out of the box if the entry contains logFile to /var/log (as shown in upstream libvirt docs too, see https://libvirt.org/formatdomain.html#userspace-slirp-or-passt-connection).

    <interface type='user'>
      <mac address='52:54:00:26:3e:6c'/>
      <ip address='10.0.1.1' family='ipv4' prefix='24'/>
      <model type='virtio'/>
      <backend type='passt' logFile='/var/log/passt.log'/>
      <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/>
    </interface>

# virsh start guest
error: Failed to start domain 'guest'
error: internal error: Could not start 'passt': Couldn't open log file /var/log/passt.log: Permission denied

While setting up logging for other devices and features generally works out of the box. Example:

    <serial type='file'>
      <source path='/var/log/vm-serial.log'/>
      <target type='isa-serial' port='1'>
        <model name='isa-serial'/>
      </target>
    </serial>

This should ideally work out of the box without the need for further configuration, else it can slow down troubleshooting of issues.

Libvirt has enough privileges and should adjust the necessary permissions on its own and allow the VM to start.

Version-Release number of selected component (if applicable):
# rpm -q passt libvirt qemu-kvm kernel
passt-0^20230222.g4ddbcb9-2.el9_2.x86_64
libvirt-9.0.0-10.1.el9_2.x86_64
qemu-kvm-7.2.0-14.el9_2.x86_64
kernel-5.14.0-284.11.1.el9_2.x86_64

How reproducible:
100%

Steps to Reproduce:
* Setup passt backend with logFile to /var/log

Actual results:
* VM fails to start out of the box without further configuration

Expected results:
* VM starts

Comment 1 Germano Veit Michel 2023-05-23 03:49:36 UTC
Workaround:

# mkdir -p /var/log/passt
# chown 107:107 /var/log/passt

# virsh dumpxml guest | grep logFile
      <backend type='passt' logFile='/var/log/passt/guest.log'/>

# virsh start guest
Domain 'guest' started

Comment 2 Michal Privoznik 2023-06-06 11:57:57 UTC
I have a patch for libvirt, but first, we need to extend passt capabilities a bit:

https://archives.passt.top/passt-dev/cover.1686037337.git.mprivozn@redhat.com/T/#t

Comment 3 Michal Privoznik 2023-06-12 08:16:35 UTC
Well, that didn't go as planned. But that's okay, I can work around it in libvirt:

https://listman.redhat.com/archives/libvir-list/2023-June/240279.html

Comment 4 Michal Privoznik 2023-06-26 13:55:24 UTC
Merged upstream as:

ceb4dc8e17 docs: Move passt log file in our example XML
8511b96a31 qemu_passt: Precreate passt logfile

v9.4.0-80-gceb4dc8e17

Comment 5 yalzhang@redhat.com 2023-06-30 03:44:37 UTC
Before this patch/bug, with libvirt libvirt-9.4.0-1.el9.x86_64 or older, passt's SELinux policies limit its writing of log files to /run/user/<UID>/filename.log, where <UID> is the uid used to run the passt process.

So for root user, 
1) prepare the log directory for qemu user with uid as 107:
# mkdir /run/user/107
# chown qemu:qemu /run/user/107

2) configure the interface xml as below:
# virsh dumpxml rhel --xpath //interface 
<interface type="user">
  <mac address="52:54:00:49:84:12"/>
  <source dev="eno1"/>
  <model type="virtio"/>
  <backend type="passt" logFile="/run/user/107/passt.log"/>
</interface>

# virsh start rhel 
Domain 'rhel' started

# ll -Z /run/user/107/passt.log 
-rw-------. 1 qemu qemu system_u:object_r:user_tmp_t:s0 1738 Jun 29 23:26 /run/user/107/passt.log

For unpreviledged user, for example "test":
1) swith to "test" user:
# machinectl shell test@

Check the uid of this unpreviledged user:
$ id
uid=1000(test) gid=1000(test) groups=1000(test) context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023

Check the permission of the XDG_RUNTIME_DIR for this user:
$ ll -d -Z  /run/user/1000
drwx------. 5 test test unconfined_u:object_r:user_tmp_t:s0 140 Jun 29 23:31 /run/user/1000

2) configure vm interface like as below:
$ virsh dumpxml test1 --xpath //interface 
<interface type="user">
  <mac address="52:54:00:26:44:e5"/>
  <source dev="eno1"/>
  <model type="virtio"/>
  <backend type="passt" logFile="/run/user/1000/passt.log"/>
  <address type="pci" domain="0x0000" bus="0x01" slot="0x00" function="0x0"/>
</interface>

$ virsh start test1
Domain 'test1' started

$ ll -Z /run/user/1000/passt.log
-rw-------. 1 test test unconfined_u:object_r:user_tmp_t:s0 1369 Jun 29 23:40 /run/user/1000/passt.log

Comment 6 yalzhang@redhat.com 2023-06-30 04:01:30 UTC
Hi Michal,Laine, Could you please help to check the results with libvirt-9.5.0-0rc1.1.el9.x86_64?

1) With root user and the same settings in comment 5, the vm can not start successfully; 
No impact for unpriviledged user with settings in commet 5. Is this expected?

2) For root user, VM can not start successfully with I set log file to /tmp. Do we need to update passt as well?

3) For unpriviledged user, could it use /tmp as a log directory? 

4) Should the "/run/user/<UID>/filename.log" still be valid configuration both for root and non-root user?

Details:
#  rpm -q libvirt passt passt-selinux
libvirt-9.5.0-0rc1.1.el9.x86_64
passt-0^20230222.g4ddbcb9-4.el9.x86_64
passt-selinux-0^20230222.g4ddbcb9-4.el9.noarch

S1: root user with same settings in comment 5:
# ll -Z -d  /run/user/107/
drwxr-xr-x. 2 qemu qemu unconfined_u:object_r:user_tmp_t:s0 40 Jun 29 23:51 /run/user/107/

# virsh dumpxml rhel --xpath //interface 
<interface type="user">
  <mac address="52:54:00:49:84:12"/>
  <source dev="eno1"/>
  <model type="virtio"/>
  <backend type="passt" logFile="/run/user/107/passt.log"/>
  <address type="pci" domain="0x0000" bus="0x01" slot="0x00" function="0x0"/>
</interface>

# virsh start rhel 
error: Failed to start domain 'rhel'
error: internal error: Child process (/usr/bin/passt --one-off --socket /run/libvirt/qemu/passt/1-rhel-net0.socket --mac-addr 52:54:00:49:84:12 --pid /run/libvirt/qemu/passt/1-rhel-net0-passt.pid --interface eno1 --log-file /run/user/107/passt.log) unexpected exit status 1: Couldn't open log file /run/user/107/passt.log: Permission denied


S2: root user, with log file in /tmp(this bug fix)
# virsh dumpxml rhel --xpath //interface 
<interface type="user">
  <mac address="52:54:00:49:84:12"/>
  <source dev="eno1"/>
  <model type="virtio"/>
  <backend type="passt" logFile="/tmp/passt.log"/>
  <address type="pci" domain="0x0000" bus="0x01" slot="0x00" function="0x0"/>
</interface>

# getenforce 
Enforcing

# virsh start rhel 
error: Failed to start domain 'rhel'
error: internal error: Child process (/usr/bin/passt --one-off --socket /run/libvirt/qemu/passt/1-rhel-net0.socket --mac-addr 52:54:00:49:84:12 --pid /run/libvirt/qemu/passt/1-rhel-net0-passt.pid --interface eno1 --log-file /tmp/passt.log) unexpected exit status 1: Couldn't open log file /tmp/passt.log: Permission denied

# ll -Z -d /tmp
drwxrwxrwt. 10 root root system_u:object_r:tmp_t:s0 4096 Jun 29 23:56 /tmp

Comment 7 yalzhang@redhat.com 2023-06-30 05:18:36 UTC
(In reply to yalzhang from comment #6) 
> 1) With root user and the same settings in comment 5, the vm can not start
> successfully; 
> No impact for unpriviledged user with settings in commet 5. Is this expected?
> 
Correct the 2nd sentence: both for user and non-root user, the vm can not start successfully.

Comment 8 Michal Privoznik 2023-07-12 10:15:53 UTC
Alright, after I reproduced the issue I see the following message in audit.log:

type=AVC msg=audit(1689149172.232:406): avc:  denied  { read append } for  pid=4898 comm="passt.avx2" name="passt.log" dev="tmpfs" ino=1577 scontext=system_u:system_r:passt_t:s0:c158,c647 tcontext=system_u:object_r:svirt_image_t:s0:c158,c647 tclass=file permissive=0

Basically, the SELinux policy denies passt_t from accessing svirt_image_t. Now, the question is how to fix this issue. We have two options:

1) Have libvirt create the file and just set uid:gid and don't touch SELinux label at all (hoping that passt_t is generous enough to allow open+append to the most used locations (like /run/user/...)), or

2) fix passt-selinux policy an allow passt_t to open+append to svirt_image_t.

Stefano, since you are maintainer of passt, which one would you pick?

Comment 9 David Gibson 2023-07-13 01:08:05 UTC
(In reply to Michal Privoznik from comment #8)
> Alright, after I reproduced the issue I see the following message in
> audit.log:
> 
> type=AVC msg=audit(1689149172.232:406): avc:  denied  { read append } for 
> pid=4898 comm="passt.avx2" name="passt.log" dev="tmpfs" ino=1577
> scontext=system_u:system_r:passt_t:s0:c158,c647
> tcontext=system_u:object_r:svirt_image_t:s0:c158,c647 tclass=file
> permissive=0
> 
> Basically, the SELinux policy denies passt_t from accessing svirt_image_t.
> Now, the question is how to fix this issue. We have two options:
> 
> 1) Have libvirt create the file and just set uid:gid and don't touch SELinux
> label at all (hoping that passt_t is generous enough to allow open+append to
> the most used locations (like /run/user/...)), or
> 
> 2) fix passt-selinux policy an allow passt_t to open+append to svirt_image_t.
> 
> Stefano, since you are maintainer of passt, which one would you pick?

Stefano is on holiday this week, I'm handling most passt & pasta stuff.  Unfortunately, I'm pretty ignorant on SELinux.  Here's some questions based on what I can figure out so far.

a) passt's own selinux files define a passt_log_t type, which seems to be what we want.  Is there a reason libvirt can't label the file passt_log_t?

b) What other things have the svirt_image_t label?  Does it include VM images, as the name suggests?  If so passt really shouldn't have permission to that - it has no business accessing VM images.

Comment 10 Michal Privoznik 2023-07-14 10:08:35 UTC
(In reply to David Gibson from comment #9)
> (In reply to Michal Privoznik from comment #8)
> > Alright, after I reproduced the issue I see the following message in
> > audit.log:
> > 
> > type=AVC msg=audit(1689149172.232:406): avc:  denied  { read append } for 
> > pid=4898 comm="passt.avx2" name="passt.log" dev="tmpfs" ino=1577
> > scontext=system_u:system_r:passt_t:s0:c158,c647
> > tcontext=system_u:object_r:svirt_image_t:s0:c158,c647 tclass=file
> > permissive=0
> > 
> > Basically, the SELinux policy denies passt_t from accessing svirt_image_t.
> > Now, the question is how to fix this issue. We have two options:
> > 
> > 1) Have libvirt create the file and just set uid:gid and don't touch SELinux
> > label at all (hoping that passt_t is generous enough to allow open+append to
> > the most used locations (like /run/user/...)), or
> > 
> > 2) fix passt-selinux policy an allow passt_t to open+append to svirt_image_t.
> > 
> > Stefano, since you are maintainer of passt, which one would you pick?
> 
> Stefano is on holiday this week, I'm handling most passt & pasta stuff. 
> Unfortunately, I'm pretty ignorant on SELinux.  Here's some questions based
> on what I can figure out so far.
> 
> a) passt's own selinux files define a passt_log_t type, which seems to be
> what we want.  Is there a reason libvirt can't label the file passt_log_t?

Problem with that is missing SELinux rule that would allow virtd_t to set pass_t label:

type=AVC msg=audit(1689329031.294:1123): avc:  denied  { relabelto } for  pid=148603 comm="rpc-virtqemud" name="passt.log" dev="dm-0" ino=134350295 scontext=system_u:system_r:virtd_t:s0-s0:c0.c1023 tcontext=system_u:system_r:passt_t:s0:c425,c467 tclass=file permissive=0

At any rate, that's something that passt-selinux policy must fix.

> 
> b) What other things have the svirt_image_t label?  Does it include VM
> images, as the name suggests?  If so passt really shouldn't have permission
> to that - it has no business accessing VM images.

Yeah, that might be too wide. Let's discard this then.

Comment 11 David Gibson 2023-07-19 06:25:25 UTC
Looks like the initial changes weren't enough to fix this, re-opening.

Comment 15 Stefano Brivio 2023-07-20 10:21:01 UTC
Sorry for the delay, and sorry for not actually having an answer at the moment, rather some questions:

(In reply to Michal Privoznik from comment #8)
> Alright, after I reproduced the issue I see the following message in
> audit.log:
> 
> type=AVC msg=audit(1689149172.232:406): avc:  denied  { read append } for 
> pid=4898 comm="passt.avx2" name="passt.log" dev="tmpfs" ino=1577
> scontext=system_u:system_r:passt_t:s0:c158,c647
> tcontext=system_u:object_r:svirt_image_t:s0:c158,c647 tclass=file
> permissive=0
> 
> Basically, the SELinux policy denies passt_t from accessing svirt_image_t.
> Now, the question is how to fix this issue. We have two options:
> 
> 1) Have libvirt create the file and just set uid:gid and don't touch SELinux
> label at all (hoping that passt_t is generous enough to allow open+append to
> the most used locations (like /run/user/...)), or

Isn't this what's already happening, but the pre-created file has a svirt_image_t label, and passt_t can't append to it?

(In reply to Michal Privoznik from comment #10)
>
> (In reply to David Gibson from comment #9)
>
> > a) passt's own selinux files define a passt_log_t type, which seems to be
> > what we want.  Is there a reason libvirt can't label the file passt_log_t?
> 
> Problem with that is missing SELinux rule that would allow virtd_t to set
> pass_t label:
> 
> type=AVC msg=audit(1689329031.294:1123): avc:  denied  { relabelto } for 
> pid=148603 comm="rpc-virtqemud" name="passt.log" dev="dm-0" ino=134350295
> scontext=system_u:system_r:virtd_t:s0-s0:c0.c1023
> tcontext=system_u:system_r:passt_t:s0:c425,c467 tclass=file permissive=0

Here we would need to relabel the file to passt_log_t (not passt_t), but I guess that doesn't change things.

> At any rate, that's something that passt-selinux policy must fix.

...is there any other label that libvirt could use when it pre-creates the file, so that we can update the passt-selinux policy to include appending and reading for that label?

Or do you have something else in mind as to how the passt-selinux policy could fix this?

At the moment the only direction I can think of is something on the lines of:

    https://github.com/fedora-selinux/selinux-policy/pull/1631/commits/2f450e6425a44a90594a6183ee40164e5cf60c68

but applied to libvirt's own (kind of) log files.

Comment 16 Michal Privoznik 2023-07-20 13:05:41 UTC
(In reply to Stefano Brivio from comment #15)
> Sorry for the delay, and sorry for not actually having an answer at the
> moment, rather some questions:
> 
> (In reply to Michal Privoznik from comment #8)
> > Alright, after I reproduced the issue I see the following message in
> > audit.log:
> > 
> > type=AVC msg=audit(1689149172.232:406): avc:  denied  { read append } for 
> > pid=4898 comm="passt.avx2" name="passt.log" dev="tmpfs" ino=1577
> > scontext=system_u:system_r:passt_t:s0:c158,c647
> > tcontext=system_u:object_r:svirt_image_t:s0:c158,c647 tclass=file
> > permissive=0
> > 
> > Basically, the SELinux policy denies passt_t from accessing svirt_image_t.
> > Now, the question is how to fix this issue. We have two options:
> > 
> > 1) Have libvirt create the file and just set uid:gid and don't touch SELinux
> > label at all (hoping that passt_t is generous enough to allow open+append to
> > the most used locations (like /run/user/...)), or
> 
> Isn't this what's already happening, but the pre-created file has a
> svirt_image_t label, and passt_t can't append to it?

More or less yes.

> 
> (In reply to Michal Privoznik from comment #10)
> >
> > (In reply to David Gibson from comment #9)
> >
> > > a) passt's own selinux files define a passt_log_t type, which seems to be
> > > what we want.  Is there a reason libvirt can't label the file passt_log_t?
> > 
> > Problem with that is missing SELinux rule that would allow virtd_t to set
> > pass_t label:
> > 
> > type=AVC msg=audit(1689329031.294:1123): avc:  denied  { relabelto } for 
> > pid=148603 comm="rpc-virtqemud" name="passt.log" dev="dm-0" ino=134350295
> > scontext=system_u:system_r:virtd_t:s0-s0:c0.c1023
> > tcontext=system_u:system_r:passt_t:s0:c425,c467 tclass=file permissive=0
> 
> Here we would need to relabel the file to passt_log_t (not passt_t), but I
> guess that doesn't change things.

Question is, how to get passt_log_t from SELinux policy. Libvirt uses combination of getfilecon("/usr/bin/passt", .. ) + security_compute_create() to compute the seclabel under which passt is going to run. I'm not exactly sure how to get to passt_log_t from there. Here's the upstream code:

https://gitlab.com/libvirt/libvirt/-/blob/master/src/security/security_selinux.c#L565

I have some patches on my local branch that reuse this code to compute SELinux label for the log file, but apparently, they end up with passt_t.

> 
> > At any rate, that's something that passt-selinux policy must fix.
> 
> ...is there any other label that libvirt could use when it pre-creates the
> file, so that we can update the passt-selinux policy to include appending
> and reading for that label?

No, I don't think there's a label we could use for this. I mean, libvirt uses basically two labels for files:

system_u:object_r:svirt_image_t:s0 for allowing RW access to a file, and
system_u:object_r:virt_content_t:s0 for allowing RO access to a file.

(label are then augmented to be unique across the host by adding range)

> 
> Or do you have something else in mind as to how the passt-selinux policy
> could fix this?
> 
> At the moment the only direction I can think of is something on the lines of:
> 
>    
> https://github.com/fedora-selinux/selinux-policy/pull/1631/commits/
> 2f450e6425a44a90594a6183ee40164e5cf60c68
> 
> but applied to libvirt's own (kind of) log files.

If libvirt could somehow guess the passt_log_t then we would just need to allow the relabeling.
But at this point I start to wonder, whether this has gone too far. I mean, log file is certainly helpful but what gain there is in allowing it to exist just anywhere? Especially since we want passt to run restricted.

Comment 17 Michal Privoznik 2023-08-01 14:43:06 UTC
Since this is far from getting fixed properly, let's do the best effort approach: stop libvirt creating the file, and let passt create it once again. This has higher chance of succeeding as passt SELinux policy already handles that.
Then again, this is so specific corner case that I think is not worth investing our time into. Having domain specific log file in such generic location as /var/log/ is really unfavorable and log files in general are good for libvirt/passt developers.

Therefore I've proposed revert of one of the patches from comment 4 (turning this basically into a documentation bug):

https://listman.redhat.com/archives/libvir-list/2023-August/241060.html

Comment 26 yalzhang@redhat.com 2023-08-10 13:48:14 UTC
Test with libvirt-9.5.0-5.el9.x86_64, the result is as the same with comment 5.
The log path need to be as /run/user/<UID>/

Comment 27 Michal Privoznik 2023-08-16 07:00:10 UTC
(In reply to yalzhang from comment #26)
> Test with libvirt-9.5.0-5.el9.x86_64, the result is as the same with comment
> 5.
> The log path need to be as /run/user/<UID>/

What UID value should be passed here? I mean, it certainly can't be the UID of the logged in user, because QEMU runs under different user (by default). But that user is not logged in, thus it doesn't have /run/user/$UID. @sbrivio I believe this is controlled by passt's selinux policy. What is the recommended log location?

Comment 28 Stefano Brivio 2023-08-16 18:47:08 UTC
First off, thanks for taking care of this -- I wanted to try out some other workarounds, but a number of (blocking) issues with SELinux interactions showed up on Fedora Rawhide meanwhile, and I just finished sorting them.

For the record, the most promising thing I found was letting libvirt not just pre-create the file, but also label it as passt_log_t. It's remarkably ugly and just works as root. So I much prefer the way you went.

An alternative I would find acceptable, I guess I mentioned it in the past, would be to take a relative path there, and use a safe, pre-defined location to write log files depending on the user. But this is inconsistent with other services handled by libvirt I guess. Excluding this, that would probably be my favourite option in terms of usability.

(In reply to Michal Privoznik from comment #27)
> (In reply to yalzhang from comment #26)
> > Test with libvirt-9.5.0-5.el9.x86_64, the result is as the same with comment
> > 5.
> > The log path need to be as /run/user/<UID>/
> 
> What UID value should be passed here? I mean, it certainly can't be the UID
> of the logged in user, because QEMU runs under different user (by default).

If you run libvirt as unprivileged user, QEMU and passt will run under that user, so it can actually be that user (that's how I actually... use it).

> But that user is not logged in, thus it doesn't have /run/user/$UID.

If you run it as root, then yes, it's qemu (for both, passt and QEMU). But... why do you say it doesn't exist? Comment #5 actually shows it does. I think it's also a natural choice -- passt runs under that user.

> @sbrivio I believe this is controlled by passt's selinux policy.

Kind of -- passt-selinux exports a 'passt_logfile_use' interface, but it's not currently used by libvirt's policy. However:

> What is the recommended log location?

passt can write and append to files under /run/user/$UID (which I would recommend for unprivileged users), and under /tmp (which can be used if started as root, if you find /run/user/107 confusing -- I don't really have a strong preference between these two).


Note You need to log in before you can comment on or make changes to this bug.