On 02/25/2016 09:25 AM, Daniel P. Berrange wrote:
On Fri, Feb 12, 2016 at 12:12:32PM -0500, John Ferlan wrote:
Introduce virPolkitAgentCreate and virPolkitAgentDestroy
virPolkitAgentCreate will run the polkit pkttyagent image as an asynchronous command in order to handle the local agent authentication via stdin/stdout. The code makes use of the pkttyagent --notify-fd mechanism to let it know when the agent is successfully registered.
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.
This part of the commit message was removed with Martin's request/change.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 2 ++ src/util/virpolkit.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++- src/util/virpolkit.h | 5 ++++ tests/virpolkittest.c | 1 + 4 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4cfaed5..8f2358f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2029,6 +2029,8 @@ virPidFileWritePath;
# util/virpolkit.h +virPolkitAgentCreate; +virPolkitAgentDestroy; virPolkitCheckAuth;
diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c index df707f1..6941d74 100644 --- a/src/util/virpolkit.c +++ b/src/util/virpolkit.c @@ -20,20 +20,22 @@ */
#include <config.h> +#include <poll.h>
#if WITH_POLKIT0 # include <polkit/polkit.h> # 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" #include "virprocess.h" #include "viralloc.h" #include "virdbus.h" +#include "virfile.h"
#define VIR_FROM_THIS VIR_FROM_POLKIT
@@ -136,6 +138,65 @@ 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 pipe_fd[2] = {-1, -1}; + struct pollfd pollfd; + int outfd = STDOUT_FILENO; + int errfd = STDERR_FILENO; + + if (!isatty(STDIN_FILENO)) + goto error; + + if (pipe2(pipe_fd, 0) < 0) + goto error; + + virCommandAddArgFormat(cmd, "%lld", (long long int) getpid()); + virCommandAddArg(cmd, "--notify-fd"); + virCommandAddArgFormat(cmd, "%d", pipe_fd[1]); + virCommandAddArg(cmd, "--fallback"); + virCommandSetInputFD(cmd, STDIN_FILENO); + virCommandSetOutputFD(cmd, &outfd); + virCommandSetErrorFD(cmd, &errfd); + virCommandPassFD(cmd, pipe_fd[1], VIR_COMMAND_PASS_FD_CLOSE_PARENT); + if (virCommandRunAsync(cmd, NULL) < 0) + goto error; + + pollfd.fd = pipe_fd[0]; + pollfd.events = POLLHUP; + + if (poll(&pollfd, 1, -1) < 0) + goto error; + + return cmd; + + error: + VIR_FORCE_CLOSE(pipe_fd[0]); + VIR_FORCE_CLOSE(pipe_fd[1]); + virCommandFree(cmd); + return NULL; +} + + #elif WITH_POLKIT0 int virPolkitCheckAuth(const char *actionid, pid_t pid, @@ -254,4 +315,18 @@ int virPolkitCheckAuth(const char *actionid ATTRIBUTE_UNUSED, }
+void +virPolkitAgentDestroy(virCommandPtr cmd ATTRIBUTE_UNUSED) +{ + return; /* do nothing */ +} + + +virCommandPtr +virPolkitAgentCreate(void) +{ + virReportError(VIR_ERR_AUTH_FAILED, "%s", + _("polkit text authentication agent unavailable")); + return NULL; +} #endif /* WITH_POLKIT1 */ diff --git a/src/util/virpolkit.h b/src/util/virpolkit.h index 36122d0..f0aea37 100644 --- a/src/util/virpolkit.h +++ b/src/util/virpolkit.h @@ -24,6 +24,8 @@
# include "internal.h"
Based on Martin's comments - I included "vircommand.h" here
+# define PKTTYAGENT "/usr/bin/pkttyagent" + int virPolkitCheckAuth(const char *actionid, pid_t pid, unsigned long long startTime, @@ -31,4 +33,7 @@ int virPolkitCheckAuth(const char *actionid, const char **details, bool allowInteraction);
+void virPolkitAgentDestroy(virCommandPtr cmd); +virCommandPtr virPolkitAgentCreate(void);
Rather than exposing use of virCommand in the API, I'd suggest you create a
typedef struct virPolkitAgent virPolkitAgent; typedef virPolkitAgent *virPolkitAgentPtr;
okidoke... Funny I had done it this way at some point, but when virCommandPtr was the only thing in the structure, I just opted to use virCommandPtr directly. Anyway, the following is now defined: typedef struct _virPolkitAgent virPolkitAgent; typedef virPolkitAgent *virPolkitAgentPtr; void virPolkitAgentDestroy(virPolkitAgentPtr cmd); virPolkitAgentPtr virPolkitAgentCreate(void);
in the header file here
and in the .c do
struct virPolitAgent { virCommandPtr; };
Inside the "#if WITH_POLKIT1" there is now a : struct _virPolkitAgent { virCommandPtr cmd; }; results in mods to virPolkitAgentDestroy: virPolkitAgentDestroy(virPolkitAgentPtr agent) { if (!agent) return; virCommandFree(agent->cmd); VIR_FREE(agent); } and the VIR_ALLOC(agent) in virPolkitAgentCreate as well as calling virPolkitAgentDestroy(agent) instead of virCommandFree in error:
so we hide use of virCommand
diff --git a/tests/virpolkittest.c b/tests/virpolkittest.c index 73f001b..b46e65f 100644 --- a/tests/virpolkittest.c +++ b/tests/virpolkittest.c @@ -27,6 +27,7 @@ # include <stdlib.h> # include <dbus/dbus.h>
+# include "vircommand.h"
Forgot to note in my response to Martin that virpolkittest.c doesn't need a change here since virpolkit.h now includes vircommand.h. Same of course for virsh.h. Should I post a v4 of patches 2 & 3? I'll still wait for the release to push the patches though. Tks - John