14208 zoneadmd should not inherit zoneadm's environment

Reviewed by: Robert Mustacchi <rm+illumos@fingolfin.org>
Reviewed by: Gergő Mihály Doma <domag02@gmail.com>
Reviewed by: Toomas Soome <tsoome@me.com>
Reviewed by: Jason King <jason.brian.king@gmail.com>
Approved by: Dan McDonald <danmcd@joyent.com>

3 files changed, 143 insertions, 20 deletions

diff --git a/usr/src/lib/libzonecfg/Makefile.com b/usr/src/lib/libzonecfg/Makefile.com
index aa7dea9cc9..d73f6cf6d6 100644
--- a/usr/src/lib/libzonecfg/Makefile.com
+++ b/usr/src/lib/libzonecfg/Makefile.com
@@ -20,11 +20,12 @@
 #
 #
 # Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
+# Copyright 2021 OmniOS Community Edition (OmniOSce) Association.
 #
 
 LIBRARY=	libzonecfg.a
 VERS=		.1
-OBJECTS=	libzonecfg.o getzoneent.o scratchops.o
+OBJECTS=	libzonecfg.o getzoneent.o scratchops.o definit.o
 
 include ../../Makefile.lib
 
@@ -37,6 +38,7 @@ NATIVE_LIBS +=	libxml2.so
 
 SRCDIR =	../common
 CPPFLAGS +=	-I$(ADJUNCT_PROTO)/usr/include/libxml2 -I$(SRCDIR) -D_REENTRANT
+CPPFLAGS +=	-I$(SRC)/common/definit
 CERRWARN +=	$(CNOWARN_UNINIT)
 CERRWARN +=	-_gcc=-Wno-parentheses
 
@@ -44,4 +46,8 @@ CERRWARN +=	-_gcc=-Wno-parentheses
 
 all:	$(LIBS)
 
+pics/%.o:	$(SRC)/common/definit/%.c
+		$(COMPILE.c) -o $@ $<
+		$(POST_PROCESS_O)
+
 include ../../Makefile.targ
diff --git a/usr/src/lib/libzonecfg/common/libzonecfg.c b/usr/src/lib/libzonecfg/common/libzonecfg.c
index b4eade5826..2149c546a1 100644
--- a/usr/src/lib/libzonecfg/common/libzonecfg.c
+++ b/usr/src/lib/libzonecfg/common/libzonecfg.c
@@ -24,6 +24,7 @@
  * Copyright (c) 2003, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2018, Joyent, Inc.
  * Copyright 2015 Nexenta Systems, Inc. All rights reserved.
+ * Copyright 2021 OmniOS Community Edition (OmniOSce) Association.
  */
 
 #include <libsysevent.h>
@@ -60,6 +61,7 @@
 #include <secdb.h>
 #include <user_attr.h>
 #include <prof_attr.h>
+#include <definit.h>
 
 #include <arpa/inet.h>
 #include <netdb.h>
@@ -1173,26 +1175,26 @@ zonecfg_refresh_index_file(zone_dochandle_t handle)
  * Strategy:
  *
  * New zone 'foo' configuration:
- * 	Create tmpfile (zonecfg.xxxxxx)
- * 	Write XML to tmpfile
- * 	Rename tmpfile to xmlfile (zonecfg.xxxxxx -> foo.xml)
- * 	Add entry to index file
- * 	If it fails, delete foo.xml, leaving nothing behind.
+ *	Create tmpfile (zonecfg.xxxxxx)
+ *	Write XML to tmpfile
+ *	Rename tmpfile to xmlfile (zonecfg.xxxxxx -> foo.xml)
+ *	Add entry to index file
+ *	If it fails, delete foo.xml, leaving nothing behind.
  *
  * Save existing zone 'foo':
- * 	Make backup of foo.xml -> .backup
- * 	Create tmpfile (zonecfg.xxxxxx)
- * 	Write XML to tmpfile
- * 	Rename tmpfile to xmlfile (zonecfg.xxxxxx -> foo.xml)
- * 	Modify index file as needed
- * 	If it fails, recover from .backup -> foo.xml
+ *	Make backup of foo.xml -> .backup
+ *	Create tmpfile (zonecfg.xxxxxx)
+ *	Write XML to tmpfile
+ *	Rename tmpfile to xmlfile (zonecfg.xxxxxx -> foo.xml)
+ *	Modify index file as needed
+ *	If it fails, recover from .backup -> foo.xml
  *
  * Rename 'foo' to 'bar':
- * 	Create tmpfile (zonecfg.xxxxxx)
- * 	Write XML to tmpfile
- * 	Rename tmpfile to xmlfile (zonecfg.xxxxxx -> bar.xml)
- * 	Add entry for 'bar' to index file, Remove entry for 'foo' (refresh)
- * 	If it fails, delete bar.xml; foo.xml is left behind.
+ *	Create tmpfile (zonecfg.xxxxxx)
+ *	Write XML to tmpfile
+ *	Rename tmpfile to xmlfile (zonecfg.xxxxxx -> bar.xml)
+ *	Add entry for 'bar' to index file, Remove entry for 'foo' (refresh)
+ *	If it fails, delete bar.xml; foo.xml is left behind.
  */
 static int
 zonecfg_save_impl(zone_dochandle_t handle, char *filename)
@@ -7521,6 +7523,110 @@ prepare_audit_context(const char *zone_name)
 	(void) adt_end_session(ah);
 }
 
+static const char **
+get_zoneadmd_envp(void)
+{
+	const char **envp = NULL;
+	size_t envlen;
+	size_t envslot = 0;
+	const char *tok;
+	void *dstate = NULL;
+
+	/*
+	 * This initial array size is enough to hold the two variables
+	 * set below, up to five additional entries from /etc/default/init,
+	 * and the NULL terminator. /etc/default/init commonly includes only
+	 * TZ, LANG and LC_ALL. The array will be grown if necessary by
+	 * doubling the size whenever it is full.
+	 */
+	envlen = 8;
+
+	if ((envp = recallocarray(NULL, 0, envlen, sizeof (char *))) == NULL)
+		return (NULL);
+
+	/*
+	 * See the comment above zonecfg_init_lock_file() for details on the
+	 * implementation of the locking mechanism.
+	 * zoneadmd is started with an inherited lock, indicated by the
+	 * environment variable being set. Once the starting zoneadm exits,
+	 * zoneadmd may persist and will continue to believe that it has the
+	 * lock. This is okay as long as the only things that connect to the
+	 * zoneadmd door and cause it to do work that requires a lock have
+	 * grabbed the lock in advance, which zoneadm does in all cases today.
+	 */
+	envp[envslot++] = zoneadm_lock_held;
+
+	if (asprintf((char **)&tok, "PATH=%s", ZONEADMD_PATH) == -1) {
+		free(envp);
+		return (NULL);
+	}
+	envp[envslot++] = tok;
+
+	if (definit_open(DEFINIT_DEFAULT_FILE, &dstate) != 0) {
+		if (errno == ENOENT) {
+			/*
+			 * If the configuration file does not exist, return the
+			 * environment populated so far (with PATH and the
+			 * zoneadm lock).
+			 */
+			envp[envslot] = NULL;
+			return (envp);
+		}
+		goto err;
+	}
+
+	while ((tok = definit_token(dstate)) != NULL) {
+
+		if (strncmp(tok, "CMASK=", 6) == 0) {
+			long t;
+
+			t = strtol(tok + 6, NULL, 8);
+
+			if (t >= DEFINIT_MIN_UMASK && t <= DEFINIT_MAX_UMASK)
+				(void) umask((int)t);
+			continue;
+		}
+
+		/*
+		 * Always ensure there is space for a terminating
+		 * NULL in addition to the new entry being added.
+		 */
+		if (envslot + 2 >= envlen) {
+			const char **newenvp;
+
+			newenvp = recallocarray(envp, envlen, envlen * 2,
+			    sizeof (char *));
+			if (newenvp == NULL)
+				goto err;
+			envp = newenvp;
+			envlen *= 2;
+		}
+
+		envp[envslot] = strdup(tok);
+		if (envp[envslot] == NULL)
+			goto err;
+		envslot++;
+	}
+
+	definit_close(dstate);
+	envp[envslot] = NULL;
+	return (envp);
+
+err:
+	if (dstate != NULL)
+		definit_close(dstate);
+
+	/*
+	 * The first slot in envp is 'zoneadm_lock_held' and should not be
+	 * freed.
+	 */
+	while (--envslot > 0)
+		free((void *)envp[envslot]);
+	free(envp);
+
+	return (NULL);
+}
+
 static int
 start_zoneadmd(const char *zone_name, boolean_t lock)
 {
@@ -7564,6 +7670,7 @@ start_zoneadmd(const char *zone_name, boolean_t lock)
 
 	if (child_pid == 0) {
 		const char *argv[6], **ap;
+		const char **envp;
 
 		/* child process */
 		prepare_audit_context(zone_name);
@@ -7578,7 +7685,15 @@ start_zoneadmd(const char *zone_name, boolean_t lock)
 		}
 		*ap = NULL;
 
-		(void) execv("/usr/lib/zones/zoneadmd", (char * const *)argv);
+		envp = get_zoneadmd_envp();
+		if (envp == NULL) {
+			zperror(gettext(
+			    "could not build environment for zoneadmd"));
+			_exit(1);
+		}
+
+		(void) execve("/usr/lib/zones/zoneadmd",
+		    (char * const *)argv, (char * const *)envp);
 		/*
 		 * TRANSLATION_NOTE
 		 * zoneadmd is a literal that should not be translated.
diff --git a/usr/src/lib/libzonecfg/common/zonecfg_impl.h b/usr/src/lib/libzonecfg/common/zonecfg_impl.h
index d6be1387e4..c117a13526 100644
--- a/usr/src/lib/libzonecfg/common/zonecfg_impl.h
+++ b/usr/src/lib/libzonecfg/common/zonecfg_impl.h
@@ -21,13 +21,13 @@
 /*
  * Copyright 2006 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
+ *
+ * Copyright 2021 OmniOS Community Edition (OmniOSce) Association.
  */
 
 #ifndef _ZONECFG_IMPL_H
 #define	_ZONECFG_IMPL_H
 
-#pragma ident	"%Z%%M%	%I%	%E% SMI"
-
 #ifdef	__cplusplus
 extern "C" {
 #endif
@@ -39,6 +39,8 @@ extern "C" {
 #define	TEXT_DOMAIN	"SYS_TEST"	/* Use this only if it wasn't */
 #endif
 
+#define	ZONEADMD_PATH	"/usr/sbin:/usr/bin"
+
 typedef enum {
 	PZE_MODIFY = -1,
 	PZE_REMOVE = 0,