This might not be immediately obvious to someone who ended up on the page without passing through acl.html first. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- docs/aclpolkit.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/aclpolkit.rst b/docs/aclpolkit.rst index 07f4735001..a9e2a305a5 100644 --- a/docs/aclpolkit.rst +++ b/docs/aclpolkit.rst @@ -23,6 +23,13 @@ all APIs, or just read-only access. The polkit access control driver in libvirt builds on this capability to allow for fine grained control over the operations a user may perform on an object. +Enabling the polkit driver +-------------------------- + +The polkit driver is disabled by default. The `access control +<acl.html#access-control-drivers>`__ documentation includes +information on how to enable it. + Permission names ---------------- -- 2.42.0

It's always set to false for now. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- src/libvirt_remote.syms | 1 + src/locking/lock_daemon.c | 6 ++++-- src/logging/log_daemon.c | 6 ++++-- src/lxc/lxc_controller.c | 3 ++- src/remote/remote_daemon.c | 6 ++++-- src/rpc/virnetserver.c | 16 ++++++++++++++-- src/rpc/virnetserver.h | 4 +++- tests/virnetdaemontest.c | 3 ++- 8 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index f0f90815cf..16a8adcdcc 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -136,6 +136,7 @@ virNetServerGetMaxUnauthClients; virNetServerGetName; virNetServerGetThreadPoolParameters; virNetServerHasClients; +virNetServerHasGranularPolkit; virNetServerNeedsAuth; virNetServerNew; virNetServerNewPostExecRestart; diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index ba52ce7d77..ac44c80927 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -129,7 +129,8 @@ virLockDaemonNew(virLockDaemonConfig *config, bool privileged) virLockDaemonClientNew, virLockDaemonClientPreExecRestart, virLockDaemonClientFree, - (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) + (void*)(intptr_t)(privileged ? 0x1 : 0x0), + false))) goto error; if (virNetDaemonAddServer(lockd->dmn, srv) < 0) @@ -142,7 +143,8 @@ virLockDaemonNew(virLockDaemonConfig *config, bool privileged) remoteAdmClientNew, remoteAdmClientPreExecRestart, remoteAdmClientFree, - lockd->dmn))) + lockd->dmn, + false))) goto error; if (virNetDaemonAddServer(lockd->dmn, srv) < 0) diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index daf7ef4b2f..752f4bd7b6 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -124,7 +124,8 @@ virLogDaemonNew(virLogDaemonConfig *config, bool privileged) virLogDaemonClientNew, virLogDaemonClientPreExecRestart, virLogDaemonClientFree, - (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) + (void*)(intptr_t)(privileged ? 0x1 : 0x0), + false))) goto error; if (virNetDaemonAddServer(logd->dmn, srv) < 0) @@ -137,7 +138,8 @@ virLogDaemonNew(virLogDaemonConfig *config, bool privileged) remoteAdmClientNew, remoteAdmClientPreExecRestart, remoteAdmClientFree, - logd->dmn))) + logd->dmn, + false))) goto error; if (virNetDaemonAddServer(logd->dmn, srv) < 0) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 48f5c73fce..54409d6961 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -928,7 +928,8 @@ static int virLXCControllerSetupServer(virLXCController *ctrl) virLXCControllerClientPrivateNew, NULL, virLXCControllerClientPrivateFree, - ctrl))) + ctrl, + false))) goto error; if (virSecurityManagerSetSocketLabel(ctrl->securityManager, ctrl->def) < 0) diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 657c053f6f..59170373cb 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -1035,7 +1035,8 @@ int main(int argc, char **argv) { remoteClientNew, NULL, remoteClientFree, - NULL))) { + NULL, + false))) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } @@ -1102,7 +1103,8 @@ int main(int argc, char **argv) { remoteAdmClientNew, NULL, remoteAdmClientFree, - dmn))) { + dmn, + false))) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 770476c1a6..d8c91172d0 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -71,6 +71,8 @@ struct _virNetServer { virNetServerClientPrivPreExecRestart clientPrivPreExecRestart; virFreeCallback clientPrivFree; void *clientPrivOpaque; + + bool granularPolkit; }; @@ -365,7 +367,8 @@ virNetServerNew(const char *name, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, virFreeCallback clientPrivFree, - void *clientPrivOpaque) + void *clientPrivOpaque, + bool granularPolkit) { g_autoptr(virNetServer) srv = NULL; g_autofree char *jobName = g_strdup_printf("rpc-%s", name); @@ -402,6 +405,8 @@ virNetServerNew(const char *name, srv->clientPrivFree = clientPrivFree; srv->clientPrivOpaque = clientPrivOpaque; + srv->granularPolkit = granularPolkit; + return g_steal_pointer(&srv); } @@ -486,7 +491,7 @@ virNetServerNewPostExecRestart(virJSONValue *object, max_anonymous_clients, keepaliveInterval, keepaliveCount, clientPrivNew, clientPrivPreExecRestart, - clientPrivFree, clientPrivOpaque))) + clientPrivFree, clientPrivOpaque, false))) return NULL; if (!(services = virJSONValueObjectGet(object, "services"))) { @@ -988,6 +993,13 @@ virNetServerGetName(virNetServer *srv) } +bool +virNetServerHasGranularPolkit(virNetServer *srv) +{ + return srv->granularPolkit; +} + + int virNetServerGetThreadPoolParameters(virNetServer *srv, size_t *minWorkers, diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 9f0cf3a3fc..efdfab03b8 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -41,7 +41,8 @@ virNetServer *virNetServerNew(const char *name, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, virFreeCallback clientPrivFree, - void *clientPrivOpaque) + void *clientPrivOpaque, + bool granularPolkit) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(10) ATTRIBUTE_NONNULL(12); virNetServer *virNetServerNewPostExecRestart(virJSONValue *object, @@ -100,6 +101,7 @@ void virNetServerSetClientAuthenticated(virNetServer *srv, virNetServerClient *c void virNetServerUpdateServices(virNetServer *srv, bool enabled); const char *virNetServerGetName(virNetServer *srv); +bool virNetServerHasGranularPolkit(virNetServer *srv); int virNetServerGetThreadPoolParameters(virNetServer *srv, size_t *minWorkers, diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index 110ec748f8..a52f427d89 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -104,7 +104,8 @@ testCreateServer(const char *server_name, const char *host, int family) testClientNew, testClientPreExec, testClientFree, - NULL))) + NULL, + false))) goto error; if (!(svc1 = virNetServerServiceNewTCP(host, -- 2.42.0

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(a)redhat.com&gt; --- 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, + }; char *ident = NULL; struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); @@ -4009,7 +4013,7 @@ remoteDispatchAuthPolkit(virNetServer *server, callerPid, timestamp, callerUid, - NULL, + attrs, true); if (rv == -1) goto authfail; -- 2.42.0

Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- docs/aclpolkit.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/aclpolkit.rst b/docs/aclpolkit.rst index 9b0a374c53..fe825c504b 100644 --- a/docs/aclpolkit.rst +++ b/docs/aclpolkit.rst @@ -70,6 +70,15 @@ to be approved by Polkit before any further APIs can be called. Read-only access is granted to all local users by default, but read/write access needs to be explicitly allowed. +:since:`Since 9.10.0`, these requests will come with the ``granular`` +attribute (see below) set to either ``"true"``, if the Polkit access +driver is enabled, or ``"false"`` otherwise. A policy designed to +work with the Polkit access driver should only allow the +``org.libvirt.unix.manage`` action if the ``granular`` attribute is +set to ``"true"``: failing to do so might result in accidentally +granting full administrative access to libvirt to more users than +intended if the Polkit access driver is later disabled. + Object identity attributes -------------------------- -- 2.42.0