Policy-Kit has been replaced by polkit (referred to as POLKIT0 and POLKIT1 in our Makefiles). The last build fix with old Policy-Kit was in May 2013: commit <442eb2ba> and build with -Wunused-label was broken since April 2016: commit <8437130> This includes a partial revert of commit <e1019e9>, which added an extra step to generating the policy file. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- .gitignore | 1 - m4/virt-polkit.m4 | 44 +-------- src/libvirt.c | 27 ----- src/remote/Makefile.inc.am | 24 +---- src/remote/{libvirtd.policy.in => libvirtd.policy} | 6 +- src/remote/remote_driver.c | 63 ------------ src/util/Makefile.inc.am | 2 - src/util/virpolkit.c | 109 +-------------------- 8 files changed, 8 insertions(+), 268 deletions(-) rename src/remote/{libvirtd.policy.in => libvirtd.policy} (92%) diff --git a/.gitignore b/.gitignore index 2ca7d9776..dd00fc5cc 100644 --- a/.gitignore +++ b/.gitignore @@ -135,7 +135,6 @@ /src/libvirt_lxc /src/libvirtd /src/libvirtd*.logrotate -/src/libvirtd.policy /src/locking/libxl-lockd.conf /src/locking/libxl-sanlock.conf /src/locking/lock_daemon_dispatch_stubs.h diff --git a/m4/virt-polkit.m4 b/m4/virt-polkit.m4 index 7bdbf804d..9426c7d5d 100644 --- a/m4/virt-polkit.m4 +++ b/m4/virt-polkit.m4 @@ -25,12 +25,8 @@ AC_DEFUN([LIBVIRT_ARG_POLKIT], [ AC_DEFUN([LIBVIRT_CHECK_POLKIT], [ AC_REQUIRE([LIBVIRT_CHECK_DBUS]) - POLKIT_REQUIRED="0.6" - POLKIT_CFLAGS= - POLKIT_LIBS= PKCHECK_PATH= - with_polkit0=no with_polkit1=no if test "x$with_polkit" = "xyes" || test "x$with_polkit" = "xcheck"; then @@ -56,52 +52,14 @@ AC_DEFUN([LIBVIRT_CHECK_POLKIT], [ [You must install dbus to compile libvirt with polkit-1]) fi fi - else - dnl Check for old polkit second - library + binary - PKG_CHECK_MODULES(POLKIT, polkit-dbus >= $POLKIT_REQUIRED, - [with_polkit=yes], [ - if test "x$with_polkit" = "xcheck" ; then - with_polkit=no - else - AC_MSG_ERROR( - [You must install PolicyKit >= $POLKIT_REQUIRED to compile libvirt]) - fi - ]) - if test "x$with_polkit" = "xyes" ; then - AC_DEFINE_UNQUOTED([WITH_POLKIT], 1, - [use PolicyKit for UNIX socket access checks]) - AC_DEFINE_UNQUOTED([WITH_POLKIT0], 1, - [use PolicyKit for UNIX socket access checks]) - - old_CFLAGS=$CFLAGS - old_LIBS=$LIBS - CFLAGS="$CFLAGS $POLKIT_CFLAGS" - LIBS="$LIBS $POLKIT_LIBS" - AC_CHECK_FUNCS([polkit_context_is_caller_authorized]) - CFLAGS="$old_CFLAGS" - LIBS="$old_LIBS" - - AC_PATH_PROG([POLKIT_AUTH], [polkit-auth]) - if test "x$POLKIT_AUTH" != "x"; then - AC_DEFINE_UNQUOTED([POLKIT_AUTH],["$POLKIT_AUTH"],[Location of polkit-auth program]) - fi - with_polkit0="yes" - fi fi fi AM_CONDITIONAL([WITH_POLKIT], [test "x$with_polkit" = "xyes"]) - AM_CONDITIONAL([WITH_POLKIT0], [test "x$with_polkit0" = "xyes"]) AM_CONDITIONAL([WITH_POLKIT1], [test "x$with_polkit1" = "xyes"]) - AC_SUBST([POLKIT_CFLAGS]) - AC_SUBST([POLKIT_LIBS]) ]) AC_DEFUN([LIBVIRT_RESULT_POLKIT], [ - if test "$with_polkit0" = "yes" ; then - msg="$POLKIT_CFLAGS $POLKIT_LIBS (version 0)" - else - msg="$PKCHECK_PATH (version 1)" - fi + msg="$PKCHECK_PATH (version 1)" LIBVIRT_RESULT([polkit], [$with_polkit], [$msg]) ]) diff --git a/src/libvirt.c b/src/libvirt.c index 536d56f0a..b7bcf8022 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -121,28 +121,6 @@ static virSecretDriverPtr virSharedSecretDriver; static virNWFilterDriverPtr virSharedNWFilterDriver; -#if defined(POLKIT_AUTH) -static int -virConnectAuthGainPolkit(const char *privilege) -{ - virCommandPtr cmd; - int ret = -1; - - if (geteuid() == 0) - return 0; - - cmd = virCommandNewArgList(POLKIT_AUTH, "--obtain", privilege, NULL); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; -} -#endif - - static int virConnectAuthCallbackDefault(virConnectCredentialPtr cred, unsigned int ncred, @@ -160,16 +138,11 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr cred, if (STRNEQ(cred[i].challenge, "PolicyKit")) return -1; -#if defined(POLKIT_AUTH) - if (virConnectAuthGainPolkit(cred[i].prompt) < 0) - return -1; -#else /* * Ignore & carry on. Although we can't auth * directly, the user may have authenticated * themselves already outside context of libvirt */ -#endif break; } diff --git a/src/remote/Makefile.inc.am b/src/remote/Makefile.inc.am index a6e8ecabf..12600b8bb 100644 --- a/src/remote/Makefile.inc.am +++ b/src/remote/Makefile.inc.am @@ -75,7 +75,7 @@ EXTRA_DIST += \ remote/test_libvirtd.aug.in \ remote/libvirtd.aug \ remote/libvirtd.conf \ - remote/libvirtd.policy.in \ + remote/libvirtd.policy \ remote/libvirtd.rules \ remote/libvirtd.sasl \ remote/libvirtd.sysctl \ @@ -120,18 +120,9 @@ conf_DATA += remote/libvirtd.conf CLEANFILES += test_libvirtd.aug if WITH_POLKIT -if WITH_POLKIT0 -policydir = $(datadir)/PolicyKit/policy -policyauth = auth_admin_keep_session -else ! WITH_POLKIT0 policydir = $(datadir)/polkit-1/actions -policyauth = auth_admin_keep -endif ! WITH_POLKIT0 endif WITH_POLKIT -BUILT_SOURCES += libvirtd.policy -CLEANFILES += libvirtd.policy - man8_MANS += libvirtd.8 libvirtd_SOURCES = $(LIBVIRTD_SOURCES) @@ -218,20 +209,17 @@ endif ! WITH_SYSCTL if WITH_POLKIT install-polkit:: $(MKDIR_P) $(DESTDIR)$(policydir) - $(INSTALL_DATA) libvirtd.policy $(DESTDIR)$(policydir)/org.libvirt.unix.policy -if ! WITH_POLKIT0 + $(INSTALL_DATA) $(srcdir)/remote/libvirtd.policy \ + $(DESTDIR)$(policydir)/org.libvirt.unix.policy $(MKDIR_P) $(DESTDIR)$(datadir)/polkit-1/rules.d $(INSTALL_DATA) $(srcdir)/remote/libvirtd.rules \ $(DESTDIR)$(datadir)/polkit-1/rules.d/50-libvirt.rules -endif ! WITH_POLKIT0 uninstall-polkit:: rm -f $(DESTDIR)$(policydir)/org.libvirt.unix.policy rmdir $(DESTDIR)$(policydir) || : -if ! WITH_POLKIT0 rm -f $(DESTDIR)$(datadir)/polkit-1/rules.d/50-libvirt.rules rmdir $(DESTDIR)$(datadir)/polkit-1/rules.d || : -endif ! WITH_POLKIT0 else ! WITH_POLKIT install-polkit:: @@ -267,12 +255,6 @@ install-sasl: uninstall-sasl: endif ! WITH_SASL -libvirtd.policy: remote/libvirtd.policy.in $(top_builddir)/config.status - $(AM_V_GEN) sed \ - -e 's|[@]authaction[@]|$(policyauth)|g' \ - < $< > $@-t && \ - mv $@-t $@ - libvirtd.init: remote/libvirtd.init.in $(top_builddir)/config.status $(AM_V_GEN)sed \ -e 's|[@]localstatedir[@]|$(localstatedir)|g' \ diff --git a/src/remote/libvirtd.policy.in b/src/remote/libvirtd.policy similarity index 92% rename from src/remote/libvirtd.policy.in rename to src/remote/libvirtd.policy index de1aba459..e834d2432 100644 --- a/src/remote/libvirtd.policy.in +++ b/src/remote/libvirtd.policy @@ -43,9 +43,9 @@ License along with this library. If not, see <defaults> <!-- Any program can use libvirt in read/write mode if they provide the root password --> - <allow_any>@authaction@</allow_any> - <allow_inactive>@authaction@</allow_inactive> - <allow_active>@authaction@</allow_active> + <allow_any>auth_admin_keep</allow_any> + <allow_inactive>auth_admin_keep</allow_inactive> + <allow_active>auth_admin_keep</allow_active> </defaults> </action> </policyconfig> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 9ea726dc4..bf00e3210 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4289,64 +4289,6 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv, #endif /* WITH_SASL */ -#if WITH_POLKIT0 -/* Perform the PolicyKit0 authentication process */ -static int -remoteAuthPolkit0(virConnectPtr conn, struct private_data *priv, - virConnectAuthPtr auth) -{ - remote_auth_polkit_ret ret; - size_t i; - int allowcb = 0; - virConnectCredential cred = { - VIR_CRED_EXTERNAL, - conn->flags & VIR_CONNECT_RO ? "org.libvirt.unix.monitor" : "org.libvirt.unix.manage", - "PolicyKit", - NULL, - NULL, - 0, - }; - VIR_DEBUG("Client initialize PolicyKit-0 authentication"); - - /* We only make it here if auth already failed - * Ask client to obtain it and check again. */ - if (auth && auth->cb) { - /* Check if the necessary credential type for PolicyKit is supported */ - for (i = 0; i < auth->ncredtype; i++) { - if (auth->credtype[i] == VIR_CRED_EXTERNAL) - allowcb = 1; - } - - if (allowcb) { - VIR_DEBUG("Client run callback for PolicyKit authentication"); - /* Run the authentication callback */ - if ((*(auth->cb))(&cred, 1, auth->cbdata) < 0) { - virReportError(VIR_ERR_AUTH_FAILED, "%s", - _("Failed to collect auth credentials")); - return -1; - } - } else { - VIR_DEBUG("Client auth callback does not support PolicyKit"); - return -1; - } - } else { - VIR_DEBUG("No auth callback provided"); - return -1; - } - - memset(&ret, 0, sizeof(ret)); - if (call(conn, priv, 0, REMOTE_PROC_AUTH_POLKIT, - (xdrproc_t) xdr_void, (char *)NULL, - (xdrproc_t) xdr_remote_auth_polkit_ret, (char *) &ret) != 0) { - return -1; /* virError already set by call */ - } - - out: - VIR_DEBUG("PolicyKit-0 authentication complete"); - return 0; -} -#endif /* WITH_POLKIT0 */ - static int remoteAuthPolkit(virConnectPtr conn, struct private_data *priv, virConnectAuthPtr auth ATTRIBUTE_UNUSED) @@ -4361,11 +4303,6 @@ remoteAuthPolkit(virConnectPtr conn, struct private_data *priv, return -1; /* virError already set by call */ } -#if WITH_POLKIT0 - if (remoteAuthPolkit0(conn, priv, auth) < 0) - return -1; -#endif /* WITH_POLKIT0 */ - VIR_DEBUG("PolicyKit authentication complete"); return 0; } diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am index a91b30dca..3f9d1164b 100644 --- a/src/util/Makefile.inc.am +++ b/src/util/Makefile.inc.am @@ -251,7 +251,6 @@ libvirt_util_la_CFLAGS = \ $(DBUS_CFLAGS) \ $(LDEXP_LIBM) \ $(NUMACTL_CFLAGS) \ - $(POLKIT_CFLAGS) \ $(GNUTLS_CFLAGS) \ $(ACL_CFLAGS) \ $(NULL) @@ -269,7 +268,6 @@ libvirt_util_la_LIBADD = \ $(SECDRIVER_LIBS) \ $(NUMACTL_LIBS) \ $(ACL_LIBS) \ - $(POLKIT_LIBS) \ $(GNUTLS_LIBS) \ $(NULL) diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c index 4559431ba..2e8660188 100644 --- a/src/util/virpolkit.c +++ b/src/util/virpolkit.c @@ -22,11 +22,6 @@ #include <config.h> #include <poll.h> -#if WITH_POLKIT0 -# include <polkit/polkit.h> -# include <polkit-dbus/polkit-dbus.h> -#endif - #include "virpolkit.h" #include "virerror.h" #include "virlog.h" @@ -211,109 +206,7 @@ virPolkitAgentCreate(void) } -#elif WITH_POLKIT0 -int virPolkitCheckAuth(const char *actionid, - pid_t pid, - unsigned long long startTime ATTRIBUTE_UNUSED, - uid_t uid, - const char **details, - bool allowInteraction ATTRIBUTE_UNUSED) -{ - PolKitCaller *pkcaller = NULL; - PolKitAction *pkaction = NULL; - PolKitContext *pkcontext = NULL; - PolKitError *pkerr = NULL; - PolKitResult pkresult; - DBusError err; - DBusConnection *sysbus; - int ret = -1; - - if (details) { - virReportError(VIR_ERR_AUTH_FAILED, "%s", - _("Details not supported with polkit v0")); - return -1; - } - - if (!(sysbus = virDBusGetSystemBus())) - goto cleanup; - - VIR_INFO("Checking PID %lld running as %d", - (long long) pid, uid); - dbus_error_init(&err); - if (!(pkcaller = polkit_caller_new_from_pid(sysbus, - pid, &err))) { - VIR_DEBUG("Failed to lookup policy kit caller: %s", err.message); - dbus_error_free(&err); - goto cleanup; - } - - if (!(pkaction = polkit_action_new())) { - char ebuf[1024]; - VIR_DEBUG("Failed to create polkit action %s", - virStrerror(errno, ebuf, sizeof(ebuf))); - goto cleanup; - } - polkit_action_set_action_id(pkaction, actionid); - - if (!(pkcontext = polkit_context_new()) || - !polkit_context_init(pkcontext, &pkerr)) { - char ebuf[1024]; - VIR_DEBUG("Failed to create polkit context %s", - (pkerr ? polkit_error_get_error_message(pkerr) - : virStrerror(errno, ebuf, sizeof(ebuf)))); - if (pkerr) - polkit_error_free(pkerr); - dbus_error_free(&err); - goto cleanup; - } - -# if HAVE_POLKIT_CONTEXT_IS_CALLER_AUTHORIZED - pkresult = polkit_context_is_caller_authorized(pkcontext, - pkaction, - pkcaller, - 0, - &pkerr); - if (pkerr && polkit_error_is_set(pkerr)) { - VIR_DEBUG("Policy kit failed to check authorization %d %s", - polkit_error_get_error_code(pkerr), - polkit_error_get_error_message(pkerr)); - goto cleanup; - } -# else - pkresult = polkit_context_can_caller_do_action(pkcontext, - pkaction, - pkcaller); -# endif - if (pkresult != POLKIT_RESULT_YES) { - VIR_DEBUG("Policy kit denied action %s from pid %lld, uid %d, result: %s", - actionid, (long long) pid, uid, - polkit_result_to_string_representation(pkresult)); - ret = -2; - goto cleanup; - } - - VIR_DEBUG("Policy allowed action %s from pid %lld, uid %d", - actionid, (long long)pid, (int)uid); - - ret = 0; - - cleanup: - if (ret < 0) { - virResetLastError(); - virReportError(VIR_ERR_AUTH_FAILED, "%s", - _("authentication failed")); - } - if (pkcontext) - polkit_context_unref(pkcontext); - if (pkcaller) - polkit_caller_unref(pkcaller); - if (pkaction) - polkit_action_unref(pkaction); - return ret; -} - - -#else /* ! WITH_POLKIT1 && ! WITH_POLKIT0 */ +#else /* ! WITH_POLKIT1 */ int virPolkitCheckAuth(const char *actionid ATTRIBUTE_UNUSED, pid_t pid ATTRIBUTE_UNUSED, -- 2.16.1

There is just one polkit now. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- m4/virt-polkit.m4 | 6 ------ src/access/Makefile.inc.am | 6 +++--- src/access/viraccessmanager.c | 4 ++-- src/util/virpolkit.c | 6 +++--- tests/Makefile.am | 4 ++-- 5 files changed, 10 insertions(+), 16 deletions(-) diff --git a/m4/virt-polkit.m4 b/m4/virt-polkit.m4 index 9426c7d5d..5c2a3c1e3 100644 --- a/m4/virt-polkit.m4 +++ b/m4/virt-polkit.m4 @@ -27,8 +27,6 @@ AC_DEFUN([LIBVIRT_CHECK_POLKIT], [ PKCHECK_PATH= - with_polkit1=no - if test "x$with_polkit" = "xyes" || test "x$with_polkit" = "xcheck"; then dnl Check for new polkit first. We directly talk over DBus dnl but we use existence of pkcheck binary as a sign that @@ -40,10 +38,7 @@ AC_DEFUN([LIBVIRT_CHECK_POLKIT], [ if test "x$with_dbus" = "xyes" ; then AC_DEFINE_UNQUOTED([WITH_POLKIT], 1, [use PolicyKit for UNIX socket access checks]) - AC_DEFINE_UNQUOTED([WITH_POLKIT1], 1, - [use PolicyKit for UNIX socket access checks]) with_polkit="yes" - with_polkit1="yes" else if test "x$with_polkit" = "xcheck" ; then with_polkit=no @@ -56,7 +51,6 @@ AC_DEFUN([LIBVIRT_CHECK_POLKIT], [ fi AM_CONDITIONAL([WITH_POLKIT], [test "x$with_polkit" = "xyes"]) - AM_CONDITIONAL([WITH_POLKIT1], [test "x$with_polkit1" = "xyes"]) ]) AC_DEFUN([LIBVIRT_RESULT_POLKIT], [ diff --git a/src/access/Makefile.inc.am b/src/access/Makefile.inc.am index c68ba5f04..6d57ca1a1 100644 --- a/src/access/Makefile.inc.am +++ b/src/access/Makefile.inc.am @@ -64,7 +64,7 @@ $(ACCESS_DRIVER_POLKIT_POLICY): $(srcdir)/access/viraccessperm.h \ $(srcdir)/access/genpolkit.pl Makefile.am $(AM_V_GEN)$(PERL) $(srcdir)/access/genpolkit.pl < $< > $@ || rm -f $@ -if WITH_POLKIT1 +if WITH_POLKIT libvirt_driver_access_la_SOURCES += $(ACCESS_DRIVER_POLKIT_SOURCES) polkitactiondir = $(datadir)/polkit-1/actions @@ -74,9 +74,9 @@ endif WITH_LIBVIRTD CLEANFILES += $(ACCESS_DRIVER_POLKIT_POLICY) BUILT_SOURCES += $(ACCESS_DRIVER_POLKIT_POLICY) -else ! WITH_POLKIT1 +else ! WITH_POLKIT EXTRA_DIST += $(ACCESS_DRIVER_POLKIT_SOURCES) -endif ! WITH_POLKIT1 +endif ! WITH_POLKIT BUILT_SOURCES += \ diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c index cbfefb9d4..c268ec57f 100644 --- a/src/access/viraccessmanager.c +++ b/src/access/viraccessmanager.c @@ -23,7 +23,7 @@ #include "viraccessmanager.h" #include "viraccessdrivernop.h" #include "viraccessdriverstack.h" -#if WITH_POLKIT1 +#if WITH_POLKIT # include "viraccessdriverpolkit.h" #endif #include "viralloc.h" @@ -112,7 +112,7 @@ static virAccessManagerPtr virAccessManagerNewDriver(virAccessDriverPtr drv) static virAccessDriverPtr accessDrivers[] = { &accessDriverNop, -#if WITH_POLKIT1 +#if WITH_POLKIT &accessDriverPolkit, #endif }; diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c index 2e8660188..198439cea 100644 --- a/src/util/virpolkit.c +++ b/src/util/virpolkit.c @@ -35,7 +35,7 @@ VIR_LOG_INIT("util.polkit"); -#if WITH_POLKIT1 +#if WITH_POLKIT struct _virPolkitAgent { virCommandPtr cmd; @@ -206,7 +206,7 @@ virPolkitAgentCreate(void) } -#else /* ! WITH_POLKIT1 */ +#else /* ! WITH_POLKIT */ int virPolkitCheckAuth(const char *actionid ATTRIBUTE_UNUSED, pid_t pid ATTRIBUTE_UNUSED, @@ -236,4 +236,4 @@ virPolkitAgentCreate(void) _("polkit text authentication agent unavailable")); return NULL; } -#endif /* WITH_POLKIT1 */ +#endif /* WITH_POLKIT */ diff --git a/tests/Makefile.am b/tests/Makefile.am index d794df3e5..6ca34092c 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -250,9 +250,9 @@ test_programs += virdbustest \ virsystemdtest \ $(NULL) test_libraries += virdbusmock.la -if WITH_POLKIT1 +if WITH_POLKIT test_programs += virpolkittest -endif WITH_POLKIT1 +endif WITH_POLKIT endif WITH_DBUS if WITH_SECDRIVER_SELINUX -- 2.16.1

All we need is DBus. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- m4/virt-polkit.m4 | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/m4/virt-polkit.m4 b/m4/virt-polkit.m4 index 5c2a3c1e3..1016df4b3 100644 --- a/m4/virt-polkit.m4 +++ b/m4/virt-polkit.m4 @@ -25,27 +25,19 @@ AC_DEFUN([LIBVIRT_ARG_POLKIT], [ AC_DEFUN([LIBVIRT_CHECK_POLKIT], [ AC_REQUIRE([LIBVIRT_CHECK_DBUS]) - PKCHECK_PATH= - if test "x$with_polkit" = "xyes" || test "x$with_polkit" = "xcheck"; then - dnl Check for new polkit first. We directly talk over DBus - dnl but we use existence of pkcheck binary as a sign that - dnl we should prefer polkit-1 over polkit-0, so we check - dnl for it even though we don't ultimately use it - AC_PATH_PROG([PKCHECK_PATH], [pkcheck], [], [$LIBVIRT_SBIN_PATH]) - if test "x$PKCHECK_PATH" != "x" ; then - dnl Found pkcheck, so ensure dbus-devel is present - if test "x$with_dbus" = "xyes" ; then - AC_DEFINE_UNQUOTED([WITH_POLKIT], 1, - [use PolicyKit for UNIX socket access checks]) - with_polkit="yes" + dnl All we need to talk to polkit is DBus, no need to check + dnl for anything else. + if test "x$with_dbus" = "xyes" ; then + AC_DEFINE_UNQUOTED([WITH_POLKIT], 1, + [use PolicyKit for UNIX socket access checks]) + with_polkit="yes" + else + if test "x$with_polkit" = "xcheck" ; then + with_polkit=no else - if test "x$with_polkit" = "xcheck" ; then - with_polkit=no - else - AC_MSG_ERROR( - [You must install dbus to compile libvirt with polkit-1]) - fi + AC_MSG_ERROR( + [You must install dbus to compile libvirt with polkit-1]) fi fi fi -- 2.16.1