diff mbox

remote: Don't reject remote polkit if client lacks support

Message ID 1eada2fac71a7ed9f751ce6b779a46d1e3972955.1460662877.git.crobinso@redhat.com
State Accepted
Commit 84371303d8302e59e535b4ca2d7e2d5f43d7679e
Headers show

Commit Message

Cole Robinson April 14, 2016, 7:41 p.m. UTC
If you compile a client --without-polkit, and connect to a URI that needs
polkit auth, the connection will fail with:

$ ./tools/virsh --connect qemu+ssh://crobinso@machine/system
error: failed to connect to the hypervisor
error: authentication failed: unsupported authentication type 2

This is because the client side portion of the polkit handling is
compiled out. However, nothing polkit specific is actually required
of the client.

Fix that error by unconditionally compiling the basic polkit client
handling.

https://bugzilla.redhat.com/show_bug.cgi?id=635529
---
Granted, if polkit needs to do any interaction at all, and you are
connecting to a remote machine, then things are going to fail anyways
with a 'missing agent' error. But if you're user is in the 'libvirt'
group polkit doesn't need to auth it should all work.


 src/remote/remote_driver.c | 69 +++++++++++++++++++++-------------------------
 1 file changed, 31 insertions(+), 38 deletions(-)

-- 
2.7.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Comments

Cole Robinson April 22, 2016, 1:51 p.m. UTC | #1
ping

On 04/14/2016 03:41 PM, Cole Robinson wrote:
> If you compile a client --without-polkit, and connect to a URI that needs

> polkit auth, the connection will fail with:

> 

> $ ./tools/virsh --connect qemu+ssh://crobinso@machine/system

> error: failed to connect to the hypervisor

> error: authentication failed: unsupported authentication type 2

> 

> This is because the client side portion of the polkit handling is

> compiled out. However, nothing polkit specific is actually required

> of the client.

> 

> Fix that error by unconditionally compiling the basic polkit client

> handling.

> 

> https://bugzilla.redhat.com/show_bug.cgi?id=635529

> ---

> Granted, if polkit needs to do any interaction at all, and you are

> connecting to a remote machine, then things are going to fail anyways

> with a 'missing agent' error. But if you're user is in the 'libvirt'

> group polkit doesn't need to auth it should all work.

> 

> 

>  src/remote/remote_driver.c | 69 +++++++++++++++++++++-------------------------

>  1 file changed, 31 insertions(+), 38 deletions(-)

> 

> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c

> index da94411..6bed2c5 100644

> --- a/src/remote/remote_driver.c

> +++ b/src/remote/remote_driver.c

> @@ -132,11 +132,9 @@ static int remoteAuthenticate(virConnectPtr conn, struct private_data *priv,

>  #if WITH_SASL

>  static int remoteAuthSASL(virConnectPtr conn, struct private_data *priv,

>                            virConnectAuthPtr auth, const char *mech);

> -#endif

> -#if WITH_POLKIT

> +#endif /* WITH_SASL */

>  static int remoteAuthPolkit(virConnectPtr conn, struct private_data *priv,

>                              virConnectAuthPtr auth);

> -#endif /* WITH_POLKIT */

>  

>  static virDomainPtr get_nonnull_domain(virConnectPtr conn, remote_nonnull_domain domain);

>  static virNetworkPtr get_nonnull_network(virConnectPtr conn, remote_nonnull_network network);

> @@ -3326,14 +3324,12 @@ remoteAuthenticate(virConnectPtr conn, struct private_data *priv,

>      }

>  #endif

>  

> -#if WITH_POLKIT

>      case REMOTE_AUTH_POLKIT:

>          if (remoteAuthPolkit(conn, priv, auth) < 0) {

>              VIR_FREE(ret.types.types_val);

>              return -1;

>          }

>          break;

> -#endif

>  

>      case REMOTE_AUTH_NONE:

>          /* Nothing todo, hurrah ! */

> @@ -3904,30 +3900,10 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv,

>  #endif /* WITH_SASL */

>  

>  

> -#if WITH_POLKIT

> -# if WITH_POLKIT1

> -static int

> -remoteAuthPolkit(virConnectPtr conn, struct private_data *priv,

> -                 virConnectAuthPtr auth ATTRIBUTE_UNUSED)

> -{

> -    remote_auth_polkit_ret ret;

> -    VIR_DEBUG("Client initialize PolicyKit-1 authentication");

> -

> -    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 */

> -    }

> -

> -    VIR_DEBUG("PolicyKit-1 authentication complete");

> -    return 0;

> -}

> -# elif WITH_POLKIT0

> -/* Perform the PolicyKit authentication process

> - */

> +#if WITH_POLKIT0

> +/* Perform the PolicyKit0 authentication process */

>  static int

> -remoteAuthPolkit(virConnectPtr conn, struct private_data *priv,

> +remoteAuthPolkit0(virConnectPtr conn, struct private_data *priv,

>                   virConnectAuthPtr auth)

>  {

>      remote_auth_polkit_ret ret;

> @@ -3943,14 +3919,8 @@ remoteAuthPolkit(virConnectPtr conn, struct private_data *priv,

>      };

>      VIR_DEBUG("Client initialize PolicyKit-0 authentication");

>  

> -    /* Check auth first and if it succeeds we are done. */

> -    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)

> -        goto out;

> -

> -    /* Auth failed.  Ask client to obtain it and check again. */

> +    /* 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++) {

> @@ -3986,8 +3956,31 @@ remoteAuthPolkit(virConnectPtr conn, struct private_data *priv,

>      VIR_DEBUG("PolicyKit-0 authentication complete");

>      return 0;

>  }

> -# endif /* WITH_POLKIT0 */

> -#endif /* WITH_POLKIT */

> +#endif /* WITH_POLKIT0 */

> +

> +static int

> +remoteAuthPolkit(virConnectPtr conn, struct private_data *priv,

> +                 virConnectAuthPtr auth ATTRIBUTE_UNUSED)

> +{

> +    remote_auth_polkit_ret ret;

> +    VIR_DEBUG("Client initialize PolicyKit authentication");

> +

> +    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 */

> +    }

> +

> +#if WITH_POLKIT0

> +    if (remoteAuthPolkit0(conn, priv, auth) < 0)

> +        return -1;

> +#endif /* WITH_POLKIT0 */

> +

> +    VIR_DEBUG("PolicyKit authentication complete");

> +    return 0;

> +}

> +

>  /*----------------------------------------------------------------------*/

>  

>  static int

> 


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
diff mbox

Patch

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index da94411..6bed2c5 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -132,11 +132,9 @@  static int remoteAuthenticate(virConnectPtr conn, struct private_data *priv,
 #if WITH_SASL
 static int remoteAuthSASL(virConnectPtr conn, struct private_data *priv,
                           virConnectAuthPtr auth, const char *mech);
-#endif
-#if WITH_POLKIT
+#endif /* WITH_SASL */
 static int remoteAuthPolkit(virConnectPtr conn, struct private_data *priv,
                             virConnectAuthPtr auth);
-#endif /* WITH_POLKIT */
 
 static virDomainPtr get_nonnull_domain(virConnectPtr conn, remote_nonnull_domain domain);
 static virNetworkPtr get_nonnull_network(virConnectPtr conn, remote_nonnull_network network);
@@ -3326,14 +3324,12 @@  remoteAuthenticate(virConnectPtr conn, struct private_data *priv,
     }
 #endif
 
-#if WITH_POLKIT
     case REMOTE_AUTH_POLKIT:
         if (remoteAuthPolkit(conn, priv, auth) < 0) {
             VIR_FREE(ret.types.types_val);
             return -1;
         }
         break;
-#endif
 
     case REMOTE_AUTH_NONE:
         /* Nothing todo, hurrah ! */
@@ -3904,30 +3900,10 @@  remoteAuthSASL(virConnectPtr conn, struct private_data *priv,
 #endif /* WITH_SASL */
 
 
-#if WITH_POLKIT
-# if WITH_POLKIT1
-static int
-remoteAuthPolkit(virConnectPtr conn, struct private_data *priv,
-                 virConnectAuthPtr auth ATTRIBUTE_UNUSED)
-{
-    remote_auth_polkit_ret ret;
-    VIR_DEBUG("Client initialize PolicyKit-1 authentication");
-
-    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 */
-    }
-
-    VIR_DEBUG("PolicyKit-1 authentication complete");
-    return 0;
-}
-# elif WITH_POLKIT0
-/* Perform the PolicyKit authentication process
- */
+#if WITH_POLKIT0
+/* Perform the PolicyKit0 authentication process */
 static int
-remoteAuthPolkit(virConnectPtr conn, struct private_data *priv,
+remoteAuthPolkit0(virConnectPtr conn, struct private_data *priv,
                  virConnectAuthPtr auth)
 {
     remote_auth_polkit_ret ret;
@@ -3943,14 +3919,8 @@  remoteAuthPolkit(virConnectPtr conn, struct private_data *priv,
     };
     VIR_DEBUG("Client initialize PolicyKit-0 authentication");
 
-    /* Check auth first and if it succeeds we are done. */
-    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)
-        goto out;
-
-    /* Auth failed.  Ask client to obtain it and check again. */
+    /* 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++) {
@@ -3986,8 +3956,31 @@  remoteAuthPolkit(virConnectPtr conn, struct private_data *priv,
     VIR_DEBUG("PolicyKit-0 authentication complete");
     return 0;
 }
-# endif /* WITH_POLKIT0 */
-#endif /* WITH_POLKIT */
+#endif /* WITH_POLKIT0 */
+
+static int
+remoteAuthPolkit(virConnectPtr conn, struct private_data *priv,
+                 virConnectAuthPtr auth ATTRIBUTE_UNUSED)
+{
+    remote_auth_polkit_ret ret;
+    VIR_DEBUG("Client initialize PolicyKit authentication");
+
+    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 */
+    }
+
+#if WITH_POLKIT0
+    if (remoteAuthPolkit0(conn, priv, auth) < 0)
+        return -1;
+#endif /* WITH_POLKIT0 */
+
+    VIR_DEBUG("PolicyKit authentication complete");
+    return 0;
+}
+
 /*----------------------------------------------------------------------*/
 
 static int