[libvirt] [PATCH 2/3] util: Introduce API's for Polkit text authentication

[Daniel P. Berrange]

On Wed, Feb 10, 2016 at 02:46:35PM -0500, John Ferlan wrote:
> Introduce virPolkitAgentCreate, virPolkitAgentCheck, and virPolkitAgentDestroy
> 
> virPolkitAgentCreate will run the polkit pkttyagent image as an asynchronous
> command in order to handle the local agent authentication via stdin/stdout.
> 
> virPolkitAgentCheck will run the polkit pkcheck command against the async
> command process in order to perform the authentication

Err, we already have virPolkitCheckAuth which does this via the DBus
API. Using pkcheck is a security flaw in many versions of Linux because
it didn't accept the full set of args required for race-free auth
checking.

> virPolkitAgentDestroy will close the command effectively reaping our
> child process
> 
> Needed to move around or add the "#include vircommand.h" since,
> virpolkit.h now uses it.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/libvirt_private.syms |  3 ++
>  src/util/virpolkit.c     | 96 +++++++++++++++++++++++++++++++++++++++++++++++-
>  src/util/virpolkit.h     |  7 ++++
>  tests/virpolkittest.c    |  3 +-
>  4 files changed, 107 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 5ae3618..e4d791d 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2028,6 +2028,9 @@ virPidFileWritePath;
>  
>  
>  # util/virpolkit.h
> +virPolkitAgentCheck;
> +virPolkitAgentCreate;
> +virPolkitAgentDestroy;
>  virPolkitCheckAuth;
>  
>  
> diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c
> index 56b1c31..e7c8603 100644
> --- a/src/util/virpolkit.c
> +++ b/src/util/virpolkit.c
> @@ -26,8 +26,8 @@
>  # include <polkit-dbus/polkit-dbus.h>
>  #endif
>  
> -#include "virpolkit.h"
>  #include "vircommand.h"
> +#include "virpolkit.h"
>  #include "virerror.h"
>  #include "virlog.h"
>  #include "virstring.h"
> @@ -136,6 +136,77 @@ int virPolkitCheckAuth(const char *actionid,
>  }
>  
>  
> +/* virPolkitAgentDestroy:
> + * @cmd: Pointer to the virCommandPtr created during virPolkitAgentCreate
> + *
> + * Destroy resources used by Polkit Agent
> + */
> +void
> +virPolkitAgentDestroy(virCommandPtr cmd)
> +{
> +    virCommandFree(cmd);
> +}
> +
> +/* virPolkitAgentCreate:
> + *
> + * Allocate and setup a polkit agent
> + *
> + * Returns a virCommandPtr on success and NULL on failure
> + */
> +virCommandPtr
> +virPolkitAgentCreate(void)
> +{
> +    virCommandPtr cmd = virCommandNewArgList(PKTTYAGENT, "--process", NULL);
> +    int outfd = STDOUT_FILENO;
> +    int errfd = STDERR_FILENO;
> +
> +    virCommandAddArgFormat(cmd, "%lld", (long long int) getpid());
> +    virCommandAddArg(cmd, "--fallback");
> +    virCommandSetInputFD(cmd, STDIN_FILENO);
> +    virCommandSetOutputFD(cmd, &outfd);
> +    virCommandSetErrorFD(cmd, &errfd);
> +    if (virCommandRunAsync(cmd, NULL) < 0)
> +        goto error;
> +    /* Give it a chance to get started */
> +    sleep(1);

Sigh, needing a sleep(1) is a sure sign that we'd be better off
doing this via the Polkit API and not spawning programs. This
sleep is just asking for bug reports about race conditions.

> +
> +    return cmd;
> +
> + error:
> +    virCommandFree(cmd);
> +    return NULL;
> +}

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