[libvirt] [PATCH v3 0/4] lxc improvements

Hi all, Here is a series grouping several small patches I sent independently to the mailing list. Main change since v2: * <inituser> and <initgroup> have been changed to hold either a uid or name as text child, rather than in an attribute. * Moved the uid/gid setting to after the pivot_root to allow getting the uid/gid from name. Cédric Bosdonnat (4): lxc: allow defining environment variables util: share code between virExec and virCommandExec lxc: allow user to specify command working directory lxc: add possibility to define init uid/gid docs/formatdomain.html.in | 17 +++++++++ docs/schemas/domaincommon.rng | 29 +++++++++++++++ src/conf/domain_conf.c | 52 ++++++++++++++++++++++++++ src/conf/domain_conf.h | 11 ++++++ src/lxc/lxc_container.c | 59 ++++++++++++++++++++++++++++++ src/util/vircommand.c | 69 ++++++++++++++++++++--------------- tests/lxcxml2xmldata/lxc-initdir.xml | 30 +++++++++++++++ tests/lxcxml2xmldata/lxc-initenv.xml | 30 +++++++++++++++ tests/lxcxml2xmldata/lxc-inituser.xml | 31 ++++++++++++++++ tests/lxcxml2xmltest.c | 3 ++ 10 files changed, 302 insertions(+), 29 deletions(-) create mode 100644 tests/lxcxml2xmldata/lxc-initdir.xml create mode 100644 tests/lxcxml2xmldata/lxc-initenv.xml create mode 100644 tests/lxcxml2xmldata/lxc-inituser.xml -- 2.12.2

When running an application container, setting environment variables could be important. The newly introduced <initenv> tag in domain configuration will allow setting environment variables to the init program. --- docs/formatdomain.html.in | 5 +++++ docs/schemas/domaincommon.rng | 10 ++++++++++ src/conf/domain_conf.c | 38 ++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 8 ++++++++ src/lxc/lxc_container.c | 5 +++++ tests/lxcxml2xmldata/lxc-initenv.xml | 30 ++++++++++++++++++++++++++++ tests/lxcxml2xmltest.c | 1 + 7 files changed, 97 insertions(+) create mode 100644 tests/lxcxml2xmldata/lxc-initenv.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a55a9e139..d2db5a4f9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -326,6 +326,10 @@ element, if set will be used to provide an equivalent to <code>/proc/cmdline</code> but will not affect init argv. </p> + <p> + To set environment variables, use the <code>initenv</code> element, one + for each variable. + </p> <pre> <os> @@ -333,6 +337,7 @@ <init>/bin/systemd</init> <initarg>--unit</initarg> <initarg>emergency.service</initarg> + <initenv name='MYENV'>some value</initenv> </os> </pre> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index e259e3ee2..1e9fccc9e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -385,6 +385,16 @@ <text/> </element> </zeroOrMore> + <zeroOrMore> + <element name="initenv"> + <attribute name="name"> + <data type='string'> + <param name='pattern'>[a-zA-Z_]+[a-zA-Z0-9_]*</param> + </data> + </attribute> + <text/> + </element> + </zeroOrMore> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fdf85d5dd..868aa522e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2868,6 +2868,9 @@ void virDomainDefFree(virDomainDefPtr def) for (i = 0; def->os.initargv && def->os.initargv[i]; i++) VIR_FREE(def->os.initargv[i]); VIR_FREE(def->os.initargv); + for (i = 0; def->os.initenv && def->os.initenv[i]; i++) + VIR_FREE(def->os.initenv[i]); + VIR_FREE(def->os.initenv); VIR_FREE(def->os.kernel); VIR_FREE(def->os.initrd); VIR_FREE(def->os.cmdline); @@ -17001,6 +17004,7 @@ virDomainDefParseBootOptions(virDomainDefPtr def, xmlNodePtr *nodes = NULL; xmlNodePtr oldnode; char *tmp = NULL; + char *name = NULL; int ret = -1; size_t i; int n; @@ -17036,6 +17040,37 @@ virDomainDefParseBootOptions(virDomainDefPtr def, } def->os.initargv[n] = NULL; VIR_FREE(nodes); + + if ((n = virXPathNodeSet("./os/initenv", ctxt, &nodes)) < 0) + goto error; + + if (VIR_ALLOC_N(def->os.initenv, n+1) < 0) + goto error; + for (i = 0; i < n; i++) { + if (!(name = virXMLPropString(nodes[i], "name"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("No name supplied for <initenv> element")); + goto error; + } + + if (!nodes[i]->children || + !nodes[i]->children->content) { + virReportError(VIR_ERR_XML_ERROR, + _("No value supplied for <initenv name='%s'> element"), + name); + goto error; + } + + if (VIR_ALLOC(def->os.initenv[i]) < 0) + goto error; + + def->os.initenv[i]->name = name; + if (VIR_STRDUP(def->os.initenv[i]->value, + (const char*) nodes[i]->children->content) < 0) + goto error; + } + def->os.initenv[n] = NULL; + VIR_FREE(nodes); } if (def->os.type == VIR_DOMAIN_OSTYPE_XEN || @@ -24864,6 +24899,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, for (i = 0; def->os.initargv && def->os.initargv[i]; i++) virBufferEscapeString(buf, "<initarg>%s</initarg>\n", def->os.initargv[i]); + for (i = 0; def->os.initenv && def->os.initenv[i]; i++) + virBufferAsprintf(buf, "<initenv name='%s'>%s</initenv>\n", + def->os.initenv[i]->name, def->os.initenv[i]->value); if (def->os.loader) virDomainLoaderDefFormat(buf, def->os.loader); virBufferEscapeString(buf, "<kernel>%s</kernel>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6d9ee9787..5e47e2e97 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1831,6 +1831,13 @@ typedef enum { VIR_ENUM_DECL(virDomainIOAPIC); /* Operating system configuration data & machine / arch */ +typedef struct _virDomainOSEnv virDomainOSEnv; +typedef virDomainOSEnv *virDomainOSEnvPtr; +struct _virDomainOSEnv { + char *name; + char *value; +}; + typedef struct _virDomainOSDef virDomainOSDef; typedef virDomainOSDef *virDomainOSDefPtr; struct _virDomainOSDef { @@ -1844,6 +1851,7 @@ struct _virDomainOSDef { bool bm_timeout_set; char *init; char **initargv; + virDomainOSEnvPtr *initenv; char *kernel; char *initrd; char *cmdline; diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index af02b5460..ffafc39d7 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -246,6 +246,11 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef, if (vmDef->os.cmdline) virCommandAddEnvPair(cmd, "LIBVIRT_LXC_CMDLINE", vmDef->os.cmdline); + for (i = 0; vmDef->os.initenv[i]; i++) { + virCommandAddEnvPair(cmd, vmDef->os.initenv[i]->name, + vmDef->os.initenv[i]->value); + } + virBufferFreeAndReset(&buf); return cmd; } diff --git a/tests/lxcxml2xmldata/lxc-initenv.xml b/tests/lxcxml2xmldata/lxc-initenv.xml new file mode 100644 index 000000000..933d836a2 --- /dev/null +++ b/tests/lxcxml2xmldata/lxc-initenv.xml @@ -0,0 +1,30 @@ +<domain type='lxc'> + <name>jessie</name> + <uuid>e21987a5-e98e-9c99-0e35-803e4d9ad1fe</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <resource> + <partition>/machine</partition> + </resource> + <os> + <type arch='x86_64'>exe</type> + <init>/sbin/sh</init> + <initenv name='FOO'>bar</initenv> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/libexec/libvirt_lxc</emulator> + <filesystem type='mount' accessmode='passthrough'> + <source dir='/mach/jessie'/> + <target dir='/'/> + </filesystem> + <console type='pty'> + <target type='lxc' port='0'/> + </console> + </devices> + <seclabel type='none'/> +</domain> diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c index 226a73d27..2a24b60b3 100644 --- a/tests/lxcxml2xmltest.c +++ b/tests/lxcxml2xmltest.c @@ -98,6 +98,7 @@ mymain(void) DO_TEST("ethernet-hostip"); DO_TEST_FULL("filesystem-root", 0, false, VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS); + DO_TEST("initenv"); virObjectUnref(caps); virObjectUnref(xmlopt); -- 2.12.2

On Mon, Jun 26, 2017 at 11:40:57AM +0200, Cédric Bosdonnat wrote:
When running an application container, setting environment variables could be important.
The newly introduced <initenv> tag in domain configuration will allow setting environment variables to the init program. --- docs/formatdomain.html.in | 5 +++++ docs/schemas/domaincommon.rng | 10 ++++++++++ src/conf/domain_conf.c | 38 ++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 8 ++++++++ src/lxc/lxc_container.c | 5 +++++ tests/lxcxml2xmldata/lxc-initenv.xml | 30 ++++++++++++++++++++++++++++ tests/lxcxml2xmltest.c | 1 + 7 files changed, 97 insertions(+) create mode 100644 tests/lxcxml2xmldata/lxc-initenv.xml
Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Mon, 2017-06-26 at 16:29 +0100, Daniel P. Berrange wrote:
On Mon, Jun 26, 2017 at 11:40:57AM +0200, Cédric Bosdonnat wrote:
When running an application container, setting environment variables could be important.
The newly introduced <initenv> tag in domain configuration will allow setting environment variables to the init program. --- docs/formatdomain.html.in | 5 +++++ docs/schemas/domaincommon.rng | 10 ++++++++++ src/conf/domain_conf.c | 38 ++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 8 ++++++++ src/lxc/lxc_container.c | 5 +++++ tests/lxcxml2xmldata/lxc-initenv.xml | 30 ++++++++++++++++++++++++++++ tests/lxcxml2xmltest.c | 1 + 7 files changed, 97 insertions(+) create mode 100644 tests/lxcxml2xmldata/lxc-initenv.xml
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Thanks. I'll push it post freeze. Did you have some time to review the other ones of the series? Regards, -- Cedric

virCommand is a version of virExec that doesn't fork, however it is just calling execve and doesn't honors setting uid/gid and pwd. This commit extrac those pieces from virExec() to a virExecCommon() function that is called from both virExec() and virCommandExec(). --- src/util/vircommand.c | 69 +++++++++++++++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 29 deletions(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index e1bbc0526..60c1121da 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -464,6 +464,41 @@ virCommandHandshakeChild(virCommandPtr cmd) return 0; } +static int +virExecCommon(virCommandPtr cmd) +{ + gid_t *groups = NULL; + int ngroups; + int ret = -1; + + if ((ngroups = virGetGroupList(cmd->uid, cmd->gid, &groups)) < 0) + goto cleanup; + + if (cmd->uid != (uid_t)-1 || cmd->gid != (gid_t)-1 || + cmd->capabilities || (cmd->flags & VIR_EXEC_CLEAR_CAPS)) { + VIR_DEBUG("Setting child uid:gid to %d:%d with caps %llx", + (int)cmd->uid, (int)cmd->gid, cmd->capabilities); + if (virSetUIDGIDWithCaps(cmd->uid, cmd->gid, groups, ngroups, + cmd->capabilities, + !!(cmd->flags & VIR_EXEC_CLEAR_CAPS)) < 0) + goto cleanup; + } + + if (cmd->pwd) { + VIR_DEBUG("Running child in %s", cmd->pwd); + if (chdir(cmd->pwd) < 0) { + virReportSystemError(errno, + _("Unable to change to %s"), cmd->pwd); + goto cleanup; + } + } + ret = 0; + + cleanup: + VIR_FREE(groups); + return ret; +} + /* * virExec: * @cmd virCommandPtr containing all information about the program to @@ -484,8 +519,6 @@ virExec(virCommandPtr cmd) const char *binary = NULL; int ret; struct sigaction waxon, waxoff; - gid_t *groups = NULL; - int ngroups; if (cmd->args[0][0] != '/') { if (!(binary = binarystr = virFindFileInPath(cmd->args[0]))) { @@ -556,9 +589,6 @@ virExec(virCommandPtr cmd) childerr = null; } - if ((ngroups = virGetGroupList(cmd->uid, cmd->gid, &groups)) < 0) - goto cleanup; - pid = virFork(); if (pid < 0) @@ -578,7 +608,6 @@ virExec(virCommandPtr cmd) cmd->pid = pid; VIR_FREE(binarystr); - VIR_FREE(groups); return 0; } @@ -727,28 +756,8 @@ virExec(virCommandPtr cmd) } # endif - /* The steps above may need to do something privileged, so we delay - * setuid and clearing capabilities until the last minute. - */ - if (cmd->uid != (uid_t)-1 || cmd->gid != (gid_t)-1 || - cmd->capabilities || (cmd->flags & VIR_EXEC_CLEAR_CAPS)) { - VIR_DEBUG("Setting child uid:gid to %d:%d with caps %llx", - (int)cmd->uid, (int)cmd->gid, cmd->capabilities); - if (virSetUIDGIDWithCaps(cmd->uid, cmd->gid, groups, ngroups, - cmd->capabilities, - !!(cmd->flags & VIR_EXEC_CLEAR_CAPS)) < 0) { - goto fork_error; - } - } - - if (cmd->pwd) { - VIR_DEBUG("Running child in %s", cmd->pwd); - if (chdir(cmd->pwd) < 0) { - virReportSystemError(errno, - _("Unable to change to %s"), cmd->pwd); - goto fork_error; - } - } + if (virExecCommon(cmd) < 0) + goto fork_error; if (virCommandHandshakeChild(cmd) < 0) goto fork_error; @@ -789,7 +798,6 @@ virExec(virCommandPtr cmd) /* This is cleanup of parent process only - child should never jump here on error */ - VIR_FREE(groups); VIR_FREE(binarystr); /* NB we don't virReportError() on any failures here @@ -2166,6 +2174,9 @@ int virCommandExec(virCommandPtr cmd) return -1; } + if (virExecCommon(cmd) < 0) + return -1; + execve(cmd->args[0], cmd->args, cmd->env); virReportSystemError(errno, -- 2.12.2

Some containers may want the application to run in a special directory. Add <initdir> element in the domain configuration to handle this case and use it in the lxc driver. --- docs/formatdomain.html.in | 5 +++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 5 +++++ src/conf/domain_conf.h | 1 + src/lxc/lxc_container.c | 2 ++ tests/lxcxml2xmldata/lxc-initdir.xml | 30 ++++++++++++++++++++++++++++++ tests/lxcxml2xmltest.c | 1 + 7 files changed, 49 insertions(+) create mode 100644 tests/lxcxml2xmldata/lxc-initdir.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d2db5a4f9..e79a9d5be 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -330,6 +330,10 @@ To set environment variables, use the <code>initenv</code> element, one for each variable. </p> + <p> + To set a custom work directory for the init, use the <code>initdir</code> + element. + </p> <pre> <os> @@ -338,6 +342,7 @@ <initarg>--unit</initarg> <initarg>emergency.service</initarg> <initenv name='MYENV'>some value</initenv> + <initdir>/my/custom/cwd</initdir> </os> </pre> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 1e9fccc9e..06fe62305 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -395,6 +395,11 @@ <text/> </element> </zeroOrMore> + <optional> + <element name="initdir"> + <ref name="absFilePath"/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 868aa522e..7835852f1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2870,6 +2870,7 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def->os.initargv); for (i = 0; def->os.initenv && def->os.initenv[i]; i++) VIR_FREE(def->os.initenv[i]); + VIR_FREE(def->os.initdir); VIR_FREE(def->os.initenv); VIR_FREE(def->os.kernel); VIR_FREE(def->os.initrd); @@ -17021,6 +17022,7 @@ virDomainDefParseBootOptions(virDomainDefPtr def, if (def->os.type == VIR_DOMAIN_OSTYPE_EXE) { def->os.init = virXPathString("string(./os/init[1])", ctxt); def->os.cmdline = virXPathString("string(./os/cmdline[1])", ctxt); + def->os.initdir = virXPathString("string(./os/initdir[1])", ctxt); if ((n = virXPathNodeSet("./os/initarg", ctxt, &nodes)) < 0) goto error; @@ -24902,6 +24904,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, for (i = 0; def->os.initenv && def->os.initenv[i]; i++) virBufferAsprintf(buf, "<initenv name='%s'>%s</initenv>\n", def->os.initenv[i]->name, def->os.initenv[i]->value); + if (def->os.initdir) + virBufferEscapeString(buf, "<initdir>%s</initdir>\n", + def->os.initdir); if (def->os.loader) virDomainLoaderDefFormat(buf, def->os.loader); virBufferEscapeString(buf, "<kernel>%s</kernel>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5e47e2e97..4d41de2a4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1852,6 +1852,7 @@ struct _virDomainOSDef { char *init; char **initargv; virDomainOSEnvPtr *initenv; + char *initdir; char *kernel; char *initrd; char *cmdline; diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index ffafc39d7..8d8e1a735 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -245,6 +245,8 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef, virCommandAddEnvPair(cmd, "LIBVIRT_LXC_NAME", vmDef->name); if (vmDef->os.cmdline) virCommandAddEnvPair(cmd, "LIBVIRT_LXC_CMDLINE", vmDef->os.cmdline); + if (vmDef->os.initdir) + virCommandSetWorkingDirectory(cmd, vmDef->os.initdir); for (i = 0; vmDef->os.initenv[i]; i++) { virCommandAddEnvPair(cmd, vmDef->os.initenv[i]->name, diff --git a/tests/lxcxml2xmldata/lxc-initdir.xml b/tests/lxcxml2xmldata/lxc-initdir.xml new file mode 100644 index 000000000..2940bda91 --- /dev/null +++ b/tests/lxcxml2xmldata/lxc-initdir.xml @@ -0,0 +1,30 @@ +<domain type='lxc'> + <name>jessie</name> + <uuid>e21987a5-e98e-9c99-0e35-803e4d9ad1fe</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <resource> + <partition>/machine</partition> + </resource> + <os> + <type arch='x86_64'>exe</type> + <init>/sbin/sh</init> + <initdir>/path/to/pwd</initdir> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/libexec/libvirt_lxc</emulator> + <filesystem type='mount' accessmode='passthrough'> + <source dir='/mach/jessie'/> + <target dir='/'/> + </filesystem> + <console type='pty'> + <target type='lxc' port='0'/> + </console> + </devices> + <seclabel type='none'/> +</domain> diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c index 2a24b60b3..c81b0eace 100644 --- a/tests/lxcxml2xmltest.c +++ b/tests/lxcxml2xmltest.c @@ -99,6 +99,7 @@ mymain(void) DO_TEST_FULL("filesystem-root", 0, false, VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS); DO_TEST("initenv"); + DO_TEST("initdir"); virObjectUnref(caps); virObjectUnref(xmlopt); -- 2.12.2

On Mon, Jun 26, 2017 at 11:40:59AM +0200, Cédric Bosdonnat wrote:
Some containers may want the application to run in a special directory. Add <initdir> element in the domain configuration to handle this case and use it in the lxc driver. --- docs/formatdomain.html.in | 5 +++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 5 +++++ src/conf/domain_conf.h | 1 + src/lxc/lxc_container.c | 2 ++ tests/lxcxml2xmldata/lxc-initdir.xml | 30 ++++++++++++++++++++++++++++++ tests/lxcxml2xmltest.c | 1 + 7 files changed, 49 insertions(+) create mode 100644 tests/lxcxml2xmldata/lxc-initdir.xml
Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Users may want to run the init command of a container as a special user / group. This is achieved by adding <inituser> and <initgroup> elements. Note that the user can either provide a name or an ID to specify the user / group to be used. This commit also fixes a side effect of being able to run the command as a non-root user: the user needs rights on the tty to allow shell job control. --- docs/formatdomain.html.in | 7 +++++ docs/schemas/domaincommon.rng | 14 ++++++++++ src/conf/domain_conf.c | 9 ++++++ src/conf/domain_conf.h | 2 ++ src/lxc/lxc_container.c | 52 +++++++++++++++++++++++++++++++++++ tests/lxcxml2xmldata/lxc-inituser.xml | 31 +++++++++++++++++++++ tests/lxcxml2xmltest.c | 1 + 7 files changed, 116 insertions(+) create mode 100644 tests/lxcxml2xmldata/lxc-inituser.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e79a9d5be..f9a5177e0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -334,6 +334,11 @@ To set a custom work directory for the init, use the <code>initdir</code> element. </p> + <p> + To run the init command as a given user or group, use the <code>inituser</code> + or <code>initgroup</code> elements respectively. Both elements can be provided + either a user (resp. group) id or a name. + </p> <pre> <os> @@ -343,6 +348,8 @@ <initarg>emergency.service</initarg> <initenv name='MYENV'>some value</initenv> <initdir>/my/custom/cwd</initdir> + <inituser>tester</inituser> + <initgroup>1000</initgroup> </os> </pre> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 06fe62305..0b8294a9d 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -400,6 +400,20 @@ <ref name="absFilePath"/> </element> </optional> + <optional> + <element name="inituser"> + <choice> + <ref name="unsignedInt"/> + <ref name="genericName"/> + </choice> + </element> + <element name="initgroup"> + <choice> + <ref name="unsignedInt"/> + <ref name="genericName"/> + </choice> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7835852f1..82c413e98 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2871,6 +2871,8 @@ void virDomainDefFree(virDomainDefPtr def) for (i = 0; def->os.initenv && def->os.initenv[i]; i++) VIR_FREE(def->os.initenv[i]); VIR_FREE(def->os.initdir); + VIR_FREE(def->os.inituser); + VIR_FREE(def->os.initgroup); VIR_FREE(def->os.initenv); VIR_FREE(def->os.kernel); VIR_FREE(def->os.initrd); @@ -17023,6 +17025,8 @@ virDomainDefParseBootOptions(virDomainDefPtr def, def->os.init = virXPathString("string(./os/init[1])", ctxt); def->os.cmdline = virXPathString("string(./os/cmdline[1])", ctxt); def->os.initdir = virXPathString("string(./os/initdir[1])", ctxt); + def->os.inituser = virXPathString("string(./os/inituser[1])", ctxt); + def->os.initgroup = virXPathString("string(./os/initgroup[1])", ctxt); if ((n = virXPathNodeSet("./os/initarg", ctxt, &nodes)) < 0) goto error; @@ -24907,6 +24911,11 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->os.initdir) virBufferEscapeString(buf, "<initdir>%s</initdir>\n", def->os.initdir); + if (def->os.inituser) + virBufferAsprintf(buf, "<inituser>%s</inituser>\n", def->os.inituser); + if (def->os.initgroup) + virBufferAsprintf(buf, "<initgroup>%s</initgroup>\n", def->os.initgroup); + if (def->os.loader) virDomainLoaderDefFormat(buf, def->os.loader); virBufferEscapeString(buf, "<kernel>%s</kernel>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4d41de2a4..bbffcda61 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1853,6 +1853,8 @@ struct _virDomainOSDef { char **initargv; virDomainOSEnvPtr *initenv; char *initdir; + char *inituser; + char *initgroup; char *kernel; char *initrd; char *cmdline; diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 8d8e1a735..6309abe4b 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2110,6 +2110,55 @@ static int lxcAttachNS(int *ns_fd) return 0; } +/** + * lxcContainerSetUserGroup: + * @cmd: command to update + * @vmDef: domain definition for the container + * @ttyPath: guest path to the tty + * + * Set the command UID and GID. As this function attempts at + * converting the user/group name into uid/gid, it needs to + * be called after the pivot root is done. + * + * The owner of the tty is also changed to the given user. + */ +static int lxcContainerSetUserGroup(virCommandPtr cmd, + virDomainDefPtr vmDef, + const char *ttyPath) +{ + uid_t uid; + gid_t gid; + + if (vmDef->os.inituser) { + if (virGetUserID(vmDef->os.inituser, &uid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, _("User %s doesn't exist"), + vmDef->os.inituser); + return -1; + } + virCommandSetUID(cmd, uid); + + /* Change the newly created tty owner to the inituid for + * shells to have job control. */ + if (chown(ttyPath, uid, -1) < 0) { + virReportSystemError(errno, + _("Failed to change ownership of tty %s"), + ttyPath); + return -1; + } + } + + if (vmDef->os.initgroup) { + if (virGetGroupID(vmDef->os.initgroup, &gid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, _("Group %s doesn't exist"), + vmDef->os.initgroup); + return -1; + } + virCommandSetGID(cmd, gid); + } + + return 0; +} + /** * lxcContainerChild: @@ -2208,6 +2257,9 @@ static int lxcContainerChild(void *data) goto cleanup; } + if (lxcContainerSetUserGroup(cmd, vmDef, argv->ttyPaths[0]) < 0) + goto cleanup; + /* rename and enable interfaces */ if (lxcContainerRenameAndEnableInterfaces(vmDef, argv->nveths, diff --git a/tests/lxcxml2xmldata/lxc-inituser.xml b/tests/lxcxml2xmldata/lxc-inituser.xml new file mode 100644 index 000000000..08338a2b7 --- /dev/null +++ b/tests/lxcxml2xmldata/lxc-inituser.xml @@ -0,0 +1,31 @@ +<domain type='lxc'> + <name>jessie</name> + <uuid>e21987a5-e98e-9c99-0e35-803e4d9ad1fe</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <resource> + <partition>/machine</partition> + </resource> + <os> + <type arch='x86_64'>exe</type> + <init>/sbin/sh</init> + <inituser>tester</inituser> + <initgroup>1234</initgroup> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/libexec/libvirt_lxc</emulator> + <filesystem type='mount' accessmode='passthrough'> + <source dir='/mach/jessie'/> + <target dir='/'/> + </filesystem> + <console type='pty'> + <target type='lxc' port='0'/> + </console> + </devices> + <seclabel type='none'/> +</domain> diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c index c81b0eace..9b9314cf8 100644 --- a/tests/lxcxml2xmltest.c +++ b/tests/lxcxml2xmltest.c @@ -100,6 +100,7 @@ mymain(void) VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS); DO_TEST("initenv"); DO_TEST("initdir"); + DO_TEST("inituser"); virObjectUnref(caps); virObjectUnref(xmlopt); -- 2.12.2

On Mon, Jun 26, 2017 at 11:41:00AM +0200, Cédric Bosdonnat wrote:
Users may want to run the init command of a container as a special user / group. This is achieved by adding <inituser> and <initgroup> elements. Note that the user can either provide a name or an ID to specify the user / group to be used.
This commit also fixes a side effect of being able to run the command as a non-root user: the user needs rights on the tty to allow shell job control. --- docs/formatdomain.html.in | 7 +++++ docs/schemas/domaincommon.rng | 14 ++++++++++ src/conf/domain_conf.c | 9 ++++++ src/conf/domain_conf.h | 2 ++ src/lxc/lxc_container.c | 52 +++++++++++++++++++++++++++++++++++ tests/lxcxml2xmldata/lxc-inituser.xml | 31 +++++++++++++++++++++ tests/lxcxml2xmltest.c | 1 + 7 files changed, 116 insertions(+) create mode 100644 tests/lxcxml2xmldata/lxc-inituser.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e79a9d5be..f9a5177e0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -334,6 +334,11 @@ To set a custom work directory for the init, use the <code>initdir</code> element. </p> + <p> + To run the init command as a given user or group, use the <code>inituser</code> + or <code>initgroup</code> elements respectively. Both elements can be provided + either a user (resp. group) id or a name. + </p>
Should mention that you can prefix the user/group with a '+' to force it to be treated as a numeric UID/GID. Without a '+' the numeric value will first be tried as username. If that is noted, then Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (3)
-
Cedric Bosdonnat
-
Cédric Bosdonnat
-
Daniel P. Berrange