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@xxxxxxxxxx>
> ---
> 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 :|
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
