Bug 1871135

same with nwfilter-define/ nwfilter-undefine

(In reply to yalzhang from comment #0)
> Description of problem:
> polkit setting can not work with split daemon
> 
> Version-Release number of selected component (if applicable):
> # rpm -q libvirt polkit
> libvirt-6.6.0-2.module+el8.3.0+7567+dc41c0a9.x86_64
> polkit-0.115-11.el8.x86_64
> 
> How reproducible:
> 100%
> 
> Steps to Reproduce:
> 1. Enable split daemon:
> # systemctl stop libvirtd.service# systemctl stop
> libvirtd{,-ro,-admin,-tcp,-tls}.socket
> # for drv in qemu interface network nodedev nwfilter secret storage; do
> systemctl unmask virt${drv}d.service; systemctl unmask
> virt${drv}d{,-ro,-admin}.socket; systemctl enable virt${drv}d.service;
> systemctl enable virt${drv}d{,-ro,-admin}.socket; done
> #  for drv in qemu network nodedev nwfilter secret storage; do systemctl
> start virt${drv}d{,-ro,-admin}.socket; done
> 
> 2. Edit the /etc/libvirt/virtqemud.conf  to enable 'polkit' as the Access
> control driver:
> # cat /etc/libvirt/virtqemud.conf
> ...
> access_drivers = [ "polkit" ]
> unix_sock_rw_perms = "0777"
> auth_unix_rw = "none"
> 
> # systemctl restart virtqemud
> 
> 3. add unprivileged user:
> # useradd test1
> # passwd test1
> # su - test1
> 
> 4. connected to qemu:///system as unprivileged user:
> $ virsh -c qemu:///system
> Welcome to virsh, the virtualization interactive terminal.
> 
> Type:  'help' for help with commands
>        'quit' to quit
> 
> virsh # list --all
>  Id   Name   State
> -----------------------
>  -    rel    shut off
> 
> virsh # start rel
> error: Failed to start domain rel
> error: access denied: 'QEMU' denied access   ----> this is expected as no
> polkit rules set
> 
> virsh # net-list --all
>  Name      State    Autostart   Persistent
> --------------------------------------------
>  default   active   yes         yes
>  net-1     active   no          yes
> 
> virsh # net-destroy net-1 Network net-1 destroyed   -------> should fail
> 
> Actual results:
> In step 4, when we execute the net-destroy command, the virtqemud forward
> the command unchanged to virtnetworkd, but it seems virtnetworkd execute the
> command as root.
> 
> Expected results:
> In step 4, the net-destroy command should fail as we connected as
> unprivileged user

> 
> Additional info:
> Once we configure /etc/libvirt/virtnetworkd.conf to enable 'polkit' as the
> Access control driver just as the virtqemud.conf, the result is as expected:

Yes, this is the key.

I think that overall this bug report simply reflects a configuration mistake.

If you're going to enable polkit access control with the modular daemons it
needs to be enabled on all of them at once.

Admittedly this isn't exactly obvious at first glance, but with the way we
proxy API calls between daemons there's no real way around it.

Test on latest rhel 9 libvirt-7.8.0-1.el9.x86_64, all the commands net-destroy/start net-define/undefine can not work as expected. I'm not sure if the configuration need to be updated, please help to have a look, Thank you!
 
1. Current rhel 9 enable modular daemons by default, configure the daemons by below script:
# cd /etc/libvirt
# cat edit_polkit.sh
#!/bin/bash
for drv in qemu interface network nodedev nwfilter secret storage proxy;
do
cp virt${drv}d.conf  virt${drv}d.conf_bak
cat>>virt${drv}d.conf<<EOF
access_drivers = [ "polkit" ] 
unix_sock_rw_perms = "0777"
auth_unix_rw = "none"
EOF
systemctl restart virt${drv}d.service;
done
# sh edit_polkit.sh
 
2. Add user:
# useradd test1
# passwd test1
 
3. Add polkit rules:
# cat /etc/polkit-1/rules.d/100-libvirt.rules 
polkit.addRule(function(action, subject) {
    if (action.id == 'org.libvirt.api.network.stop' && subject.user == 'test1') {
        if (action.lookup('connect_driver') == 'QEMU' && action.lookup('network_name') == 'default') {
            return polkit.Result.YES;
        } else {
            return polkit.Result.NO;
        }
    }
});
 
4. Try to destroy the default network:
# su - test1 -c '/usr/bin/virsh -c network:///system net-destroy default'
error: Failed to destroy network default
error: access denied: 'network' denied access

# su - test1 -c '/usr/bin/virsh -c qemu:///system net-destroy  default'
error: Failed to destroy network default
error: access denied: 'network' denied access
 
If change to monolithic daemon libvirtd, the command executed successfully by:
# su - test1 -c '/usr/bin/virsh -c qemu:///system net-destroy  default'

After discuss with yafu, the configuration in comment 4 is incorrect.

Update the connect_driver to be "network", the command succeed:
# cat /etc/polkit-1/rules.d/100-libvirt.rules 
polkit.addRule(function(action, subject) {
    if (action.id == 'org.libvirt.api.network.stop' && subject.user == 'test1') {
        if (action.lookup('connect_driver') == 'network' && action.lookup('network_name') == 'default') {
            return polkit.Result.YES;
        } else {
            return polkit.Result.NO;
        }
    }
});

Both works:
# su - test1 -c '/usr/bin/virsh -c qemu:///system net-destroy default'
Network default destroyed

(start network default on another terminal)

# su - test1 -c '/usr/bin/virsh -c network:///system net-destroy default'
Network default destroyed

The positive scenario in comment 4 and 5 proves the statement in comment 3 from Daniel.

But the issue on comment 0 still exists on libvirt-7.8.0-1.el9.x86_64, which I think may be a security issue.

1. Enable polkit *only* in /etc/libvirt/virtqemud.conf, but do not add any polkit rules;

2. Try to destroy network with unprivileged user:

# su - test1 -c '/usr/bin/virsh -c network:///system net-destroy default'
Error creating textual authentication agent: Error opening current controlling terminal for the process (`/dev/tty'): No such device or address (polkit-error-quark, 0)
error: failed to connect to the hypervisor
error: authentication unavailable: no polkit agent available to authenticate action 'org.libvirt.unix.manage'

# su - test1 -c '/usr/bin/virsh -c qemu:///system net-destroy default'
Network default destroyed

When we connect to qemu:///system, it succeeds unexpectedly. Same with nwfilter, nodedev, 

# su - test1 -c '/usr/bin/virsh -c qemu:///system nwfilter-undefine  qemu-announce-self'
Network filter qemu-announce-self undefined

# su - test1 -c '/usr/bin/virsh -c qemu:///system nodedev-list --cap net'
net_eno1_34_48_ed_f8_d8_b8
net_eno2_34_48_ed_f8_d8_b9
...

# su - test1 -c '/usr/bin/virsh -c qemu:///system nodedev-detach pci_0000_3b_00_1'
error: Failed to detach device pci_0000_3b_00_1
error: access denied: 'QEMU' denied access

And I have another question about the polkit rule setting. 
"action.lookup('connect_driver') == 'QEMU'" works well with monolithic daemon libvirtd. When we transfer to modular daemon, it can not work and needs to be updated to "action.lookup('connect_driver') == 'network'". Is there any backward compatibility concern here?

(In reply to yalzhang from comment #6)
> The positive scenario in comment 4 and 5 proves the statement in comment 3
> from Daniel.
> 
> But the issue on comment 0 still exists on libvirt-7.8.0-1.el9.x86_64, which
> I think may be a security issue.
> 
> 1. Enable polkit *only* in /etc/libvirt/virtqemud.conf, but do not add any
> polkit rules;

As per my earlier comment, this is considered a configuration mistake. It needs to be enabled all all the modular daemons that you will use.
(In reply to yalzhang from comment #7)
> And I have another question about the polkit rule setting. 
> "action.lookup('connect_driver') == 'QEMU'" works well with monolithic
> daemon libvirtd. When we transfer to modular daemon, it can not work and
> needs to be updated to "action.lookup('connect_driver') == 'network'". Is
> there any backward compatibility concern here?

Existing deployments using libvirtd will continue to use libvirt after in-place upgrades. So this change only affects new deployments, and we consider that an acceptable change in behaviour - arguably it is now a more accurate check.

(In reply to Daniel Berrangé from comment #8)
> (In reply to yalzhang from comment #6)
> > The positive scenario in comment 4 and 5 proves the statement in comment 3
> > from Daniel.
> > 
> > But the issue on comment 0 still exists on libvirt-7.8.0-1.el9.x86_64, which
> > I think may be a security issue.
> > 
> > 1. Enable polkit *only* in /etc/libvirt/virtqemud.conf, but do not add any
> > polkit rules;
> 
> As per my earlier comment, this is considered a configuration mistake. It
> needs to be enabled all all the modular daemons that you will use.

Hi Daniel, Thank you for quick the answer. Yes, this is a configuration mistake. But with this configuration mistake, unprivileged user without polkit rules can destroy and start the network, I think it's not safe and need a fix, what do you think?

After evaluating this issue, there are no plans to address it further or fix it in an upcoming release.  Therefore, it is being closed.  If plans change such that this issue will be fixed in an upcoming release, then the bug can be reopened.

Hi Daniel, do you think we need a fix to this bug? Or we can close it as won't fix?

There are no code changes planned, given that this is just a config mistake (comment #8). I was leaving it open as a documentation task though.

After evaluating this issue, there are no plans to address it further or fix it in an upcoming release.  Therefore, it is being closed.  If plans change such that this issue will be fixed in an upcoming release, then the bug can be reopened.

Hi Jiri, the command does not work as expected.
# find /etc/libvirt/ -name virt*d.conf | sed -i 's/#access_drivers = [ "polkit" ]/access_drivers = [ "polkit" ]'
sed: -e expression #1, char 62: unterminated `s' command

And I have tried this one works well:
# find /etc/libvirt/ -name virt*d.conf -exec sed -i 's/#access_drivers = \[ "polkit" \]/access_drivers = \[ "polkit" \]/g' {} +

After the update, the file updated as expected:
# find /etc/libvirt/ -name virt*d.conf   -exec  grep 'access_drivers'  {} +
/etc/libvirt/virtproxyd.conf:access_drivers = [ "polkit" ]
/etc/libvirt/virtstoraged.conf:access_drivers = [ "polkit" ]
/etc/libvirt/virtnetworkd.conf:access_drivers = [ "polkit" ]
/etc/libvirt/virtnwfilterd.conf:access_drivers = [ "polkit" ]
/etc/libvirt/virtinterfaced.conf:access_drivers = [ "polkit" ]
/etc/libvirt/virtnodedevd.conf:access_drivers = [ "polkit" ]
/etc/libvirt/virtsecretd.conf:access_drivers = [ "polkit" ]
/etc/libvirt/virtqemud.conf:access_drivers = [ "polkit" ]

and for rhel 8, the same issue:
# sed -i 's/#access_drivers = [ "polkit" ]/access_drivers = [ "polkit" ]' /etc/libvirt/libvirtd.conf

use below command instead can update the file as expected.
# sed -i 's/#access_drivers = \[ "polkit" \]/access_drivers = \[ "polkit" \]/' /etc/libvirt/libvirtd.conf

And the file name suffix it better to be ".rules" instead of ".conf"
> Create a new .conf file in the /etc/polkit-1/rules.d/ directory.

The updated docs are now live on the Customer Portal:

RHEL 8: https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/configuring_and_managing_virtualization/securing-virtual-machines-in-rhel_configuring-and-managing-virtualization#proc_limiting-what-actions-are-available-to-virtual-machine-users_securing-virtual-machines-in-rhel

RHEL 9: https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/9/html/configuring_and_managing_virtualization/securing-virtual-machines-in-rhel_configuring-and-managing-virtualization#proc_limiting-what-actions-are-available-to-virtual-machine-users_securing-virtual-machines-in-rhel

Therefore, I will be closing this ticket. However, if there is still anything amiss with the docs, please let me know.

Thanks again to everyone who helped on this!