On Mon, Nov 20, 2023 at 03:49:56PM +0100, Andrea Bolognani wrote:
This makes it possible to write Polkit rules that won't accidentally grant undesired privileges to users.
To understand why this is necessary, suppose we wanted to grant user "fred" full access to the QEMU domain "demo".
A JavaScript rule along the lines of
polkit.addRule(function(action, subject) {
// user "fred" if (subject.user == "fred") {
// can authenticate in read/write mode if (action.id == "org.libvirt.unix.manage") { return polkit.Result.YES; }
// and manage the QEMU domain "demo" if (action.id.indexOf("org.libvirt.api.domain.") == 0 && action.lookup("connect_driver") == "QEMU" && action.lookup("domain_name") == "demo") { return polkit.Result.YES; } } });
would do the trick.
However, suppose that at some point after creating this rule we disabled the Polkit access control driver and forgot to delete the file.
All of a sudden, allowing "org.libvirt.unix.manage" is no longer a trivial matter: since the Polkit access driver doesn't broker access to subsequent API calls anymore, user "fred" now has full administrative access to all drivers.
Rewriting the check seen above as
if (action.id == "org.libvirt.unix.manage" && action.lookup("granular") == "true") { return polkit.Result.YES; }
ensures that this undesired scenario will not happen, by only allowing "org.libvirt.unix.manage" when the Polkit access driver is enabled.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/remote/remote_daemon_dispatch.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 7daf503b51..2a9ee19cc3 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -3975,6 +3975,10 @@ remoteDispatchAuthPolkit(virNetServer *server, uid_t callerUid = -1; unsigned long long timestamp; const char *action; + const char *attrs[] = { + "granular", virNetServerHasGranularPolkit(server) ? "true" : "false", + NULL,
We potentially have a list of access drivers available, and I think we should expose each of their names as an attr 'access_driver.$NAME' IOW, if access_drivers = [], then don't expose any attr, but if access_drivers = ["polkit", "selinux"] we would set "access_driver.polkit=true" "access_driver.selinux=true" With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|