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

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(a)redhat.com&gt; > --- > 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