[libvirt] [PATCH] Fix build when using polkit0
Here's a hacked attempt at fixing the build on older distros using polkit0. It works and user is authorized or denied depending on settings in PolicyKit.conf. I'm not too happy with it but haven't yet digested all the changes in rpc and daemon code. In the meantime, hopefully someone can suggest improvements. Regards, Jim
On Thu, Jul 07, 2011 at 03:18:45PM -0600, Jim Fehlig wrote:
Here's a hacked attempt at fixing the build on older distros using polkit0. It works and user is authorized or denied depending on settings in PolicyKit.conf.
Sorry for breaking this. It completely slipped my mind to test this codepath
I'm not too happy with it but haven't yet digested all the changes in rpc and daemon code. In the meantime, hopefully someone can suggest improvements.
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 5e1719b..6e9eb2c 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -39,6 +39,9 @@ #if HAVE_AVAHI # include "virnetservermdns.h" #endif +#if HAVE_POLKIT0 +# include <dbus/dbus.h> +#endif
#define VIR_FROM_THIS VIR_FROM_RPC #define virNetError(code, ...) \ @@ -84,6 +87,10 @@ struct _virNetServer { virNetServerMDNSGroupPtr mdnsGroup; #endif
+#if HAVE_POLKIT0 + DBusConnection *sysbus; +#endif + size_t nservices; virNetServerServicePtr *services;
@@ -270,6 +277,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, size_t max_workers, size_t max_clients, const char *mdnsGroupName, + bool usePolkit, virNetServerClientInitHook clientInitHook) { virNetServerPtr srv; @@ -306,6 +314,25 @@ virNetServerPtr virNetServerNew(size_t min_workers, } #endif
+#if HAVE_POLKIT0 + if (usePolkit) { + DBusError derr; + + dbus_connection_set_change_sigpipe(FALSE); + dbus_threads_init_default(); + + dbus_error_init(&derr); + srv->sysbus = dbus_bus_get(DBUS_BUS_SYSTEM, &derr); + if (!(srv->sysbus)) { + VIR_ERROR(_("Failed to connect to system bus for PolicyKit auth: %s"), + derr.message); + dbus_error_free(&derr); + goto error; + } + dbus_connection_set_exit_on_disconnect(srv->sysbus, FALSE); + } +#endif + if (virMutexInit(&srv->lock) < 0) { virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize mutex")); @@ -363,6 +390,14 @@ bool virNetServerIsPrivileged(virNetServerPtr srv) }
+#if HAVE_POLKIT0 +DBusConnection* virNetServerGetDBusConn(virNetServerPtr srv) +{ + return srv->sysbus; +} +#endif + + void virNetServerAutoShutdown(virNetServerPtr srv, unsigned int timeout, virNetServerAutoShutdownFunc func, @@ -747,6 +782,11 @@ void virNetServerFree(virNetServerPtr srv)
VIR_FREE(srv->mdnsGroupName);
+#if HAVE_POLKIT0 + if (srv->sysbus) + dbus_connection_unref(srv->sysbus); +#endif + virNetServerUnlock(srv); virMutexDestroy(&srv->lock); VIR_FREE(srv); diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 6e7a21b..d96280e 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -25,6 +25,9 @@ # define __VIR_NET_SERVER_H__
# include <signal.h> +# if HAVE_POLKIT0 +# include <dbus/dbus.h> +# endif
# include "virnettlscontext.h" # include "virnetserverprogram.h" @@ -38,6 +41,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, size_t max_workers, size_t max_clients, const char *mdnsGroupName, + bool usePolkit, virNetServerClientInitHook clientInitHook);
typedef int (*virNetServerAutoShutdownFunc)(virNetServerPtr srv, void *opaque); @@ -46,6 +50,10 @@ void virNetServerRef(virNetServerPtr srv);
bool virNetServerIsPrivileged(virNetServerPtr srv);
+# if HAVE_POLKIT0 +DBusConnection* virNetServerGetDBusConn(virNetServerPtr srv); +# endif +
I'd like the virNetServer stuff to not have any mention of policy kit in it. So rather than saying 'bool usePolkit', have a 'bool connectDBus', and just remove those HAVE_POLKIT0 conditionals so that the DBus API is always available in virNetServer. The changes you made under daemon/ are all fine Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Daniel P. Berrange wrote:
I'd like the virNetServer stuff to not have any mention of policy kit in it. So rather than saying 'bool usePolkit', have a 'bool connectDBus', and just remove those HAVE_POLKIT0 conditionals so that the DBus API is always available in virNetServer. The changes you made under daemon/ are all fine
The attached patch introduces a HAVE_DBUS conditional, which AFAICT is only used by polkit0 so is only set when polkit0 is detected during configure. Is this what you had in mind? Regards, Jim
On Mon, Jul 11, 2011 at 04:16:55PM -0600, Jim Fehlig wrote:
Daniel P. Berrange wrote:
I'd like the virNetServer stuff to not have any mention of policy kit in it. So rather than saying 'bool usePolkit', have a 'bool connectDBus', and just remove those HAVE_POLKIT0 conditionals so that the DBus API is always available in virNetServer. The changes you made under daemon/ are all fine
The attached patch introduces a HAVE_DBUS conditional, which AFAICT is only used by polkit0 so is only set when polkit0 is detected during configure. Is this what you had in mind?
Regards, Jim
From c21f206a8196cef7657e30b9ee830bcc4bbfc3ff Mon Sep 17 00:00:00 2001 From: Jim Fehlig <jfehlig@novell.com> Date: Thu, 7 Jul 2011 15:12:26 -0600 Subject: [V2] Fix build when using polkit0
V2: Remove policy kit references from virNetServer and use DBus APIs directly, if available. --- configure.ac | 5 +++++ daemon/libvirtd.c | 24 ++++-------------------- daemon/remote.c | 21 ++++++++++----------- src/Makefile.am | 4 +++- src/rpc/virnetserver.c | 41 ++++++++++++++++++++++++++++++++++++++++- src/rpc/virnetserver.h | 8 ++++++++ 6 files changed, 70 insertions(+), 33 deletions(-)
diff --git a/configure.ac b/configure.ac index ae747fb..e9d5be4 100644 --- a/configure.ac +++ b/configure.ac @@ -1010,6 +1010,7 @@ AC_ARG_WITH([polkit], [with_polkit=check])
with_polkit0=no +with_dbus=no with_polkit1=no if test "x$with_polkit" = "xyes" || test "x$with_polkit" = "xcheck"; then dnl Check for new polkit first - just a binary @@ -1038,6 +1039,8 @@ if test "x$with_polkit" = "xyes" || test "x$with_polkit" = "xcheck"; then [use PolicyKit for UNIX socket access checks]) AC_DEFINE_UNQUOTED([HAVE_POLKIT0], 1, [use PolicyKit for UNIX socket access checks]) + AC_DEFINE_UNQUOTED([HAVE_DBUS], 1, + [use DBus for PolicyKit])
old_CFLAGS=$CFLAGS old_LIBS=$LIBS @@ -1052,11 +1055,13 @@ if test "x$with_polkit" = "xyes" || test "x$with_polkit" = "xcheck"; then AC_DEFINE_UNQUOTED([POLKIT_AUTH],["$POLKIT_AUTH"],[Location of polkit-auth program]) fi with_polkit0="yes" + with_dbus="yes" fi fi fi AM_CONDITIONAL([HAVE_POLKIT], [test "x$with_polkit" = "xyes"]) AM_CONDITIONAL([HAVE_POLKIT0], [test "x$with_polkit0" = "xyes"]) +AM_CONDITIONAL([HAVE_DBUS], [test "x$with_dbus" = "xyes"]) AM_CONDITIONAL([HAVE_POLKIT1], [test "x$with_polkit1" = "xyes"]) AC_SUBST([POLKIT_CFLAGS]) AC_SUBST([POLKIT_LIBS]) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index a4198d9..c7ee605 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -576,26 +576,6 @@ static int daemonSetupNetworking(virNetServerPtr srv, } #endif
-#if HAVE_POLKIT0 - if (auth_unix_rw == REMOTE_AUTH_POLKIT || - auth_unix_ro == REMOTE_AUTH_POLKIT) { - DBusError derr; - - dbus_connection_set_change_sigpipe(FALSE); - dbus_threads_init_default(); - - dbus_error_init(&derr); - server->sysbus = dbus_bus_get(DBUS_BUS_SYSTEM, &derr); - if (!(server->sysbus)) { - VIR_ERROR(_("Failed to connect to system bus for PolicyKit auth: %s"), - derr.message); - dbus_error_free(&derr); - goto error; - } - dbus_connection_set_exit_on_disconnect(server->sysbus, FALSE); - } -#endif - return 0;
error: @@ -1285,6 +1265,7 @@ int main(int argc, char **argv) { struct daemonConfig *config; bool privileged = geteuid() == 0 ? true : false; bool implicit_conf = false; + bool use_polkit_dbus;
struct option opts[] = { { "verbose", no_argument, &verbose, 1}, @@ -1445,10 +1426,13 @@ int main(int argc, char **argv) { umask(old_umask); }
+ use_polkit_dbus = config->auth_unix_rw == REMOTE_AUTH_POLKIT || + config->auth_unix_ro == REMOTE_AUTH_POLKIT; if (!(srv = virNetServerNew(config->min_workers, config->max_workers, config->max_clients, config->mdns_adv ? config->mdns_name : NULL, + use_polkit_dbus, remoteClientInitHook))) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; diff --git a/daemon/remote.c b/daemon/remote.c index a2e79ef..0172626 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -43,6 +43,7 @@ #include "command.h" #include "intprops.h" #include "virnetserverservice.h" +#include "virnetserver.h"
#include "remote_protocol.h" #include "qemu_protocol.h" @@ -2115,7 +2116,7 @@ authdeny: } #elif HAVE_POLKIT0 static int -remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthPolkit(virNetServerPtr server, virNetServerClientPtr client, virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -2137,21 +2138,19 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
memset(ident, 0, sizeof ident);
- virMutexLock(&server->lock); - virMutexLock(&client->lock); - virMutexUnlock(&server->lock); + virMutexLock(&priv->lock);
- action = client->readonly ? + action = virNetServerClientGetReadonly(client) ? "org.libvirt.unix.monitor" : "org.libvirt.unix.manage";
VIR_DEBUG("Start PolicyKit auth %d", virNetServerClientGetFD(client)); - if (client->auth != REMOTE_AUTH_POLKIT) { + if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_POLKIT) { VIR_ERROR(_("client tried invalid PolicyKit init request")); goto authfail; }
- if (qemudGetSocketIdentity(virNetServerClientGetFD(client), &callerUid, &callerPid) < 0) { + if (virNetServerClientGetLocalIdentity(client, &callerUid, &callerPid) < 0) { VIR_ERROR(_("cannot get peer socket identity")); goto authfail; } @@ -2164,7 +2163,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
VIR_INFO("Checking PID %d running as %d", callerPid, callerUid); dbus_error_init(&err); - if (!(pkcaller = polkit_caller_new_from_pid(server->sysbus, + if (!(pkcaller = polkit_caller_new_from_pid(virNetServerGetDBusConn(server), callerPid, &err))) { VIR_ERROR(_("Failed to lookup policy kit caller: %s"), err.message); dbus_error_free(&err); @@ -2226,9 +2225,9 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, action, callerPid, callerUid, polkit_result_to_string_representation(pkresult)); ret->complete = 1; - client->auth = REMOTE_AUTH_NONE; + virNetServerClientSetIdentity(client, ident);
- virMutexUnlock(&client->lock); + virMutexUnlock(&priv->lock); return 0;
error: @@ -2236,7 +2235,7 @@ error: virNetError(VIR_ERR_AUTH_FAILED, "%s", _("authentication failed")); virNetMessageSaveError(rerr); - virMutexUnlock(&client->lock); + virMutexUnlock(&priv->lock); return -1;
authfail: diff --git a/src/Makefile.am b/src/Makefile.am index cd8a7e9..a3ee8ba 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1268,10 +1268,12 @@ EXTRA_DIST += \ endif libvirt_net_rpc_server_la_CFLAGS = \ $(AVAHI_CFLAGS) \ - $(AM_CFLAGS) + $(AM_CFLAGS) \ + $(POLKIT_CFLAGS) libvirt_net_rpc_server_la_LDFLAGS = \ $(AM_LDFLAGS) \ $(AVAHI_LIBS) \ + $(POLKIT_LIBS) \ $(CYGWIN_EXTRA_LDFLAGS) \ $(MINGW_EXTRA_LDFLAGS) libvirt_net_rpc_server_la_LIBADD = \ diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 5e1719b..94d46f6 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -39,6 +39,9 @@ #if HAVE_AVAHI # include "virnetservermdns.h" #endif +#if HAVE_DBUS +# include <dbus/dbus.h> +#endif
#define VIR_FROM_THIS VIR_FROM_RPC #define virNetError(code, ...) \ @@ -84,6 +87,10 @@ struct _virNetServer { virNetServerMDNSGroupPtr mdnsGroup; #endif
+#if HAVE_DBUS + DBusConnection *sysbus; +#endif + size_t nservices; virNetServerServicePtr *services;
@@ -270,6 +277,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, size_t max_workers, size_t max_clients, const char *mdnsGroupName, + bool connectDBus, virNetServerClientInitHook clientInitHook) { virNetServerPtr srv; @@ -306,6 +314,25 @@ virNetServerPtr virNetServerNew(size_t min_workers, } #endif
+#if HAVE_DBUS + if (connectDBus) { + DBusError derr; + + dbus_connection_set_change_sigpipe(FALSE); + dbus_threads_init_default(); + + dbus_error_init(&derr); + srv->sysbus = dbus_bus_get(DBUS_BUS_SYSTEM, &derr); + if (!(srv->sysbus)) { + VIR_ERROR(_("Failed to connect to system bus for PolicyKit auth: %s"), + derr.message); + dbus_error_free(&derr); + goto error; + } + dbus_connection_set_exit_on_disconnect(srv->sysbus, FALSE); + } +#endif + if (virMutexInit(&srv->lock) < 0) { virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize mutex")); @@ -363,6 +390,14 @@ bool virNetServerIsPrivileged(virNetServerPtr srv) }
+#if HAVE_DBUS +DBusConnection* virNetServerGetDBusConn(virNetServerPtr srv) +{ + return srv->sysbus; +} +#endif + + void virNetServerAutoShutdown(virNetServerPtr srv, unsigned int timeout, virNetServerAutoShutdownFunc func, @@ -377,7 +412,6 @@ void virNetServerAutoShutdown(virNetServerPtr srv, virNetServerUnlock(srv); }
- static sig_atomic_t sigErrors = 0; static int sigLastErrno = 0; static int sigWrite = -1; @@ -747,6 +781,11 @@ void virNetServerFree(virNetServerPtr srv)
VIR_FREE(srv->mdnsGroupName);
+#if HAVE_DBUS + if (srv->sysbus) + dbus_connection_unref(srv->sysbus); +#endif + virNetServerUnlock(srv); virMutexDestroy(&srv->lock); VIR_FREE(srv); diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 6e7a21b..810d8a3 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -25,6 +25,9 @@ # define __VIR_NET_SERVER_H__
# include <signal.h> +# if HAVE_DBUS +# include <dbus/dbus.h> +# endif
# include "virnettlscontext.h" # include "virnetserverprogram.h" @@ -38,6 +41,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, size_t max_workers, size_t max_clients, const char *mdnsGroupName, + bool connectDBus, virNetServerClientInitHook clientInitHook);
typedef int (*virNetServerAutoShutdownFunc)(virNetServerPtr srv, void *opaque); @@ -46,6 +50,10 @@ void virNetServerRef(virNetServerPtr srv);
bool virNetServerIsPrivileged(virNetServerPtr srv);
+# if HAVE_DBUS +DBusConnection* virNetServerGetDBusConn(virNetServerPtr srv); +# endif + void virNetServerAutoShutdown(virNetServerPtr srv, unsigned int timeout, virNetServerAutoShutdownFunc func,
ACK, this looks fine now Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Daniel P. Berrange wrote:
On Mon, Jul 11, 2011 at 04:16:55PM -0600, Jim Fehlig wrote:
Daniel P. Berrange wrote:
I'd like the virNetServer stuff to not have any mention of policy kit in it. So rather than saying 'bool usePolkit', have a 'bool connectDBus', and just remove those HAVE_POLKIT0 conditionals so that the DBus API is always available in virNetServer. The changes you made under daemon/ are all fine
The attached patch introduces a HAVE_DBUS conditional, which AFAICT is only used by polkit0 so is only set when polkit0 is detected during configure. Is this what you had in mind?
[...]
ACK, this looks fine now
Thanks, pushed. Regards, Jim
Jim Fehlig wrote:
Here's a hacked attempt at fixing the build on older distros using polkit0. It works and user is authorized or denied depending on settings in PolicyKit.conf.
[...]
diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index 8e1843c..18a10ef 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -91,30 +91,4 @@ extern virNetSASLContextPtr saslCtxt; extern virNetServerProgramPtr remoteProgram; extern virNetServerProgramPtr qemuProgram;
-/* Main server state */ -struct qemud_server { - int privileged; - - int sigread; - int sigwrite; - char *logDir; - pthread_t eventThread; - unsigned int hasEventThread :1; - unsigned int quitEventThread :1; -# ifdef HAVE_AVAHI - struct libvirtd_mdns *mdns; -# endif -# if HAVE_SASL - char **saslUsernameWhitelist; -# endif -# if HAVE_POLKIT0 - DBusConnection *sysbus; -# endif -}; - - -# if HAVE_POLKIT -int qemudGetSocketIdentity(int fd, uid_t *uid, pid_t *pid); -# endif - #endif
This hunk, and another in libvirtd.h, have nothing to do with polkit build fix and are just removing dead code. I'll submit those as a separate patch. Regards, Jim
participants (2)
-
Daniel P. Berrange -
Jim Fehlig