Re: [libvirt] [PATCH sandbox 1/4] push changing of user ID down into child process
On Fri, 2015-09-04 at 12:40 +0100, Daniel P. Berrange wrote:
> When running interactive sandboxes, don't drop privileges in the
> long running libvirt-sandbox-init-common process. This needs to
> be privileged in order to sync, unmount and shutdown the guest
> when the user command is finished. Push changing of user ID into
> the child process, between fork & exec.
>
> Signed-off-by: Daniel P. Berrange
> ---
> libvirt-sandbox/libvirt-sandbox-init-common.c | 60
> +++
> 1 file changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c
> b/libvirt-sandbox/libvirt-sandbox-init-common.c
> index d35f760..760509f 100644
> --- a/libvirt-sandbox/libvirt-sandbox-init-common.c
> +++ b/libvirt-sandbox/libvirt-sandbox-init-common.c
> @@ -390,30 +390,43 @@ static int change_user(const gchar *user,
> return 0;
> }
>
> -static gboolean run_command(gboolean interactive, gchar **argv,
> +static gboolean run_command(GVirSandboxConfig *config,
> pid_t *child,
> int *appin, int *appout, int *apperr)
> {
> +GVirSandboxConfigInteractive *iconfig =
> GVIR_SANDBOX_CONFIG_INTERACTIVE(config);
> int pid;
> int master = -1;
> int slave = -1;
> int pipein[2] = { -1, -1};
> int pipeout[2] = { -1, -1};
> int pipeerr[2] = { -1, -1};
> +gchar **appargv = gvir_sandbox_config_get_command(config);
> +gboolean wanttty = gvir_sandbox_config_interactive_get_tty(iconfig);
>
> if (debug)
> fprintf(stderr, "libvirt-sandbox-init-common: running command %s
> %d\n",
> -argv[0], interactive);
> +appargv[0], wanttty);
>
> *appin = *appout = -1;
>
> -if (interactive) {
> +if (wanttty) {
> if (openpty(&master, &slave, NULL, NULL, NULL) < 0) {
> fprintf(stderr, "Cannot setup slave for child: %s\n",
> strerror(errno));
> goto error;
> }
>
> +if (!g_str_equal(gvir_sandbox_config_get_username(config), "root")) {
> +if (fchown(slave,
> + gvir_sandbox_config_get_userid(config),
> + gvir_sandbox_config_get_groupid(config)) < 0) {
> +fprintf(stderr, "Cannot make PTY available to user: %s\n",
> +strerror(errno));
> +goto error;
> +}
> +}
> +
> *appin = master;
> *appout = master;
> *apperr = master;
> @@ -436,7 +449,13 @@ static gboolean run_command(gboolean interactive, gchar
> **argv,
> }
>
> if (pid == 0) {
> -if (interactive) {
> +if (change_user(gvir_sandbox_config_get_username(config),
> +gvir_sandbox_config_get_userid(config),
> +gvir_sandbox_config_get_groupid(config),
> +gvir_sandbox_config_get_homedir(config)) < 0)
> +exit(EXIT_FAILURE);
> +
> +if (wanttty) {
> close(master);
> close(STDIN_FILENO);
> close(STDOUT_FILENO);
> @@ -469,24 +488,25 @@ static gboolean run_command(gboolean interactive, gchar
> **argv,
> abort();
> }
>
> -execv(argv[0], argv);
> -fprintf(stderr, "Cannot execute '%s': %s\n", argv[0],
> strerror(errno));
> +execv(appargv[0], appargv);
> +fprintf(stderr, "Cannot execute '%s': %s\n", appargv[0],
> strerror(errno));
> exit(EXIT_FAILURE);
> }
>
> -if (interactive)
> +if (wanttty) {
> close(slave);
> -else {
> +} else {
> close(pipein[0]);
> close(pipeout[1]);
> close(pipeerr[1]);
> }
>
> *child = pid;
> +g_strfreev(appargv);
> return TRUE;
>
> error:
> -if (interactive) {
> +if (wanttty) {
> if (master != -1)
> close(master);
> if (slave != -1)
> @@ -506,6 +526,7 @@ static gboolean run_command(gboolean interactive, gchar
> **argv,
> close(pipeerr[1]);
> }
> *appin = *appout = *apperr = -1;
> +g_strfreev(appargv);
> return FALSE;
> }
>
> @@ -639,8 +660,7 @@ typedef enum {
> GVIR_SANDBOX_CONSOLE_STATE_RUNNING,
> } GVirSandboxConsoleState;
>
> -static gboolean eventloop(gboolean interactive,
> - gchar **appargv,
> +static gboolean eventloop(GVirSandboxConfig *config,
>int sigread,
>int host)
> {
> @@ -825,8 +845,7 @@ static gboolean eventloop(gboolean interactive,
> if (rx->buffer[0] ==
> GVIR_SANDBOX_PROTOCOL_HANDSHAKE_SYNC) {
> if (debug)
> fprintf(stderr, "Running
> command\n");
> -if (!run_command(interactive,
> -
[libvirt] [PATCH sandbox 1/4] push changing of user ID down into child process
When running interactive sandboxes, don't drop privileges in the
long running libvirt-sandbox-init-common process. This needs to
be privileged in order to sync, unmount and shutdown the guest
when the user command is finished. Push changing of user ID into
the child process, between fork & exec.
Signed-off-by: Daniel P. Berrange
---
libvirt-sandbox/libvirt-sandbox-init-common.c | 60 +++
1 file changed, 33 insertions(+), 27 deletions(-)
diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c
b/libvirt-sandbox/libvirt-sandbox-init-common.c
index d35f760..760509f 100644
--- a/libvirt-sandbox/libvirt-sandbox-init-common.c
+++ b/libvirt-sandbox/libvirt-sandbox-init-common.c
@@ -390,30 +390,43 @@ static int change_user(const gchar *user,
return 0;
}
-static gboolean run_command(gboolean interactive, gchar **argv,
+static gboolean run_command(GVirSandboxConfig *config,
pid_t *child,
int *appin, int *appout, int *apperr)
{
+GVirSandboxConfigInteractive *iconfig =
GVIR_SANDBOX_CONFIG_INTERACTIVE(config);
int pid;
int master = -1;
int slave = -1;
int pipein[2] = { -1, -1};
int pipeout[2] = { -1, -1};
int pipeerr[2] = { -1, -1};
+gchar **appargv = gvir_sandbox_config_get_command(config);
+gboolean wanttty = gvir_sandbox_config_interactive_get_tty(iconfig);
if (debug)
fprintf(stderr, "libvirt-sandbox-init-common: running command %s %d\n",
-argv[0], interactive);
+appargv[0], wanttty);
*appin = *appout = -1;
-if (interactive) {
+if (wanttty) {
if (openpty(&master, &slave, NULL, NULL, NULL) < 0) {
fprintf(stderr, "Cannot setup slave for child: %s\n",
strerror(errno));
goto error;
}
+if (!g_str_equal(gvir_sandbox_config_get_username(config), "root")) {
+if (fchown(slave,
+ gvir_sandbox_config_get_userid(config),
+ gvir_sandbox_config_get_groupid(config)) < 0) {
+fprintf(stderr, "Cannot make PTY available to user: %s\n",
+strerror(errno));
+goto error;
+}
+}
+
*appin = master;
*appout = master;
*apperr = master;
@@ -436,7 +449,13 @@ static gboolean run_command(gboolean interactive, gchar
**argv,
}
if (pid == 0) {
-if (interactive) {
+if (change_user(gvir_sandbox_config_get_username(config),
+gvir_sandbox_config_get_userid(config),
+gvir_sandbox_config_get_groupid(config),
+gvir_sandbox_config_get_homedir(config)) < 0)
+exit(EXIT_FAILURE);
+
+if (wanttty) {
close(master);
close(STDIN_FILENO);
close(STDOUT_FILENO);
@@ -469,24 +488,25 @@ static gboolean run_command(gboolean interactive, gchar
**argv,
abort();
}
-execv(argv[0], argv);
-fprintf(stderr, "Cannot execute '%s': %s\n", argv[0], strerror(errno));
+execv(appargv[0], appargv);
+fprintf(stderr, "Cannot execute '%s': %s\n", appargv[0],
strerror(errno));
exit(EXIT_FAILURE);
}
-if (interactive)
+if (wanttty) {
close(slave);
-else {
+} else {
close(pipein[0]);
close(pipeout[1]);
close(pipeerr[1]);
}
*child = pid;
+g_strfreev(appargv);
return TRUE;
error:
-if (interactive) {
+if (wanttty) {
if (master != -1)
close(master);
if (slave != -1)
@@ -506,6 +526,7 @@ static gboolean run_command(gboolean interactive, gchar
**argv,
close(pipeerr[1]);
}
*appin = *appout = *apperr = -1;
+g_strfreev(appargv);
return FALSE;
}
@@ -639,8 +660,7 @@ typedef enum {
GVIR_SANDBOX_CONSOLE_STATE_RUNNING,
} GVirSandboxConsoleState;
-static gboolean eventloop(gboolean interactive,
- gchar **appargv,
+static gboolean eventloop(GVirSandboxConfig *config,
int sigread,
int host)
{
@@ -825,8 +845,7 @@ static gboolean eventloop(gboolean interactive,
if (rx->buffer[0] ==
GVIR_SANDBOX_PROTOCOL_HANDSHAKE_SYNC) {
if (debug)
fprintf(stderr, "Running
command\n");
-if (!run_command(interactive,
- appargv,
+if (!run_command(config,
&child,
&appin,
&appout,
@@ -1097,13 +1116,11 @@ sta
