[gentoo-commits] proj/sandbox:master commit in: libsandbox/wrapper-funcs/, tests/, libsandbox/

["Mike Frysinger" <vapier@gentoo.org>]

commit:     87f753cf677137f8d6c06c56ee6cc4db11ec71b0
Author:     Mike Frysinger <vapier <AT> gentoo <DOT> org>
AuthorDate: Sat Feb 23 05:28:42 2013 +0000
Commit:     Mike Frysinger <vapier <AT> gentoo <DOT> org>
CommitDate: Sat Feb 23 05:28:42 2013 +0000
URL:        https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=87f753cf

libsandbox: preserve more SANDBOX env vars

While we took pains to preserve the LD_PRELOAD setting, this doesn't
help us too much in practice.  If a process is going out of its way
to blow away LD_PRELOAD, chances are good it's blowing away all vars
it doesn't know about.  That means all of our SANDBOX_XXX settings.

Since a preloaded libsandbox.so is useless w/out its SANDBOX_XXX
env vars, make sure we preserve those as well.

These changes also imply some behavioral differences from older
versions.  Previously, you could `unset` a sandbox var in order
to disable it.  That no longer works.  If you wish to disable
things, you have to explicitly set it to "".

Signed-off-by: Mike Frysinger <vapier <AT> gentoo.org>

 libsandbox/libsandbox.c                   | 284 ++++++++++++++++++++++++------
 libsandbox/libsandbox.h                   |   2 +
 libsandbox/wrapper-funcs/__wrapper_exec.c |  84 +--------
 tests/Makefile.am                         |   2 +-
 tests/mkdir-3.sh                          |   2 +-
 tests/script-10.sh                        |  21 +++
 tests/script.at                           |   1 +
 7 files changed, 262 insertions(+), 134 deletions(-)

diff --git a/libsandbox/libsandbox.c b/libsandbox/libsandbox.c
index 5d9a796..4f4589f 100644
--- a/libsandbox/libsandbox.c
+++ b/libsandbox/libsandbox.c
@@ -28,7 +28,8 @@
 char sandbox_lib[SB_PATH_MAX];
 
 typedef struct {
-	bool show_access_violation;
+	bool show_access_violation, on, active, testing, verbose, debug;
+	char *ld_library_path;
 	char **prefixes[5];
 	int num_prefixes[5];
 #define             deny_prefixes     prefixes[0]
@@ -43,6 +44,7 @@ typedef struct {
 #define num_write_denied_prefixes num_prefixes[4]
 #define MAX_DYN_PREFIXES 4 /* the first 4 are dynamic */
 } sbcontext_t;
+static sbcontext_t sbcontext;
 
 static char *cached_env_vars[MAX_DYN_PREFIXES];
 static char log_path[SB_PATH_MAX];
@@ -57,8 +59,7 @@ FILE *(*sbio_popen)(const char *, const char *) = sb_unwrapped_popen;
 static char *resolve_path(const char *, int);
 static int check_prefixes(char **, int, const char *);
 static void clean_env_entries(char ***, int *);
-static void init_context(sbcontext_t *);
-static void init_env_entries(char ***, int *, const char *, const char *, int);
+static void sb_process_env_settings(void);
 
 const char *sbio_message_path;
 const char sbio_fallback_path[] = "/dev/tty";
@@ -84,6 +85,20 @@ void libsb_init(void)
 	get_sandbox_debug_log(debug_log_path, NULL);
 	get_sandbox_message_path(message_path);
 	sbio_message_path = message_path;
+
+	memset(&sbcontext, 0x00, sizeof(sbcontext));
+	sbcontext.show_access_violation = true;
+
+	sb_process_env_settings();
+	is_sandbox_on();
+	sbcontext.verbose = is_env_on(ENV_SANDBOX_VERBOSE);
+	sbcontext.debug = is_env_on(ENV_SANDBOX_DEBUG);
+	sbcontext.testing = is_env_on(ENV_SANDBOX_TESTING);
+	if (sbcontext.testing) {
+		const char *ldpath = getenv("LD_LIBRARY_PATH");
+		if (ldpath)
+			sbcontext.ld_library_path = xstrdup(ldpath);
+	}
 }
 
 /* resolve_dirfd_path - get the path relative to a dirfd
@@ -445,12 +460,6 @@ static bool write_logfile(const char *logfile, const char *func, const char *pat
 	return ret;
 }
 
-static void init_context(sbcontext_t *context)
-{
-	memset(context, 0x00, sizeof(*context));
-	context->show_access_violation = true;
-}
-
 static void clean_env_entries(char ***prefixes_array, int *prefixes_num)
 {
 	if (*prefixes_array == NULL)
@@ -555,6 +564,43 @@ done:
 	return;
 }
 
+static void sb_process_env_settings(void)
+{
+	static const char * const sb_env_names[4] = {
+		ENV_SANDBOX_DENY,
+		ENV_SANDBOX_READ,
+		ENV_SANDBOX_WRITE,
+		ENV_SANDBOX_PREDICT,
+	};
+
+	size_t i;
+	for (i = 0; i < ARRAY_SIZE(sb_env_names); ++i) {
+		char *sb_env = getenv(sb_env_names[i]);
+
+		/* Allow the vars to change values, but not be unset.
+		 * See sb_check_envp() for more details. */
+		if (!sb_env)
+			continue;
+
+		if (/*(!sb_env && cached_env_vars[i]) || -- see above */
+		    !cached_env_vars[i] ||
+		    strcmp(cached_env_vars[i], sb_env) != 0)
+		{
+			clean_env_entries(&sbcontext.prefixes[i], &sbcontext.num_prefixes[i]);
+
+			if (cached_env_vars[i])
+				free(cached_env_vars[i]);
+
+			if (sb_env) {
+				init_env_entries(&sbcontext.prefixes[i], &sbcontext.num_prefixes[i],
+					sb_env_names[i], sb_env, 1);
+				cached_env_vars[i] = xstrdup(sb_env);
+			} else
+				cached_env_vars[i] = NULL;
+		}
+	}
+}
+
 static int check_prefixes(char **prefixes, int num_prefixes, const char *path)
 {
 	if (!prefixes)
@@ -835,15 +881,19 @@ static int check_syscall(sbcontext_t *sbcontext, int sb_nr, const char *func,
 	char *resolved_path = NULL;
 	int old_errno = errno;
 	int result;
-	bool access, debug, verbose;
+	bool access, debug, verbose, set;
 
 	absolute_path = resolve_path(file, 0);
 	resolved_path = resolve_path(file, 1);
 	if (!absolute_path || !resolved_path)
 		goto error;
 
-	verbose = is_env_on(ENV_SANDBOX_VERBOSE);
-	debug = is_env_on(ENV_SANDBOX_DEBUG);
+	verbose = is_env_set_on(ENV_SANDBOX_VERBOSE, &set);
+	if (set)
+		sbcontext->verbose = verbose;
+	debug = is_env_set_on(ENV_SANDBOX_DEBUG, &set);
+	if (set)
+		sbcontext->debug = debug;
 
 	result = check_access(sbcontext, sb_nr, func, flags, absolute_path, resolved_path);
 
@@ -905,7 +955,7 @@ static int check_syscall(sbcontext_t *sbcontext, int sb_nr, const char *func,
 
 bool is_sandbox_on(void)
 {
-	bool result;
+	bool result = false;
 	save_errno();
 
 	/* $SANDBOX_ACTIVE is an env variable that should ONLY
@@ -914,14 +964,24 @@ bool is_sandbox_on(void)
 	 * in some cases when run in parallel with another sandbox,
 	 * but not even in the sandbox shell.
 	 */
-	char *sb_env_active = getenv(ENV_SANDBOX_ACTIVE);
-	if (sandbox_on &&
-	    sb_env_active &&
-	    is_env_on(ENV_SANDBOX_ON) &&
-	    (0 == strcmp(sb_env_active, SANDBOX_ACTIVE)))
-		result = true;
-	else
-		result = false;
+	if (sandbox_on) {
+		if (!sbcontext.active) {
+			/* Once you go active, you never go back */
+			char *sb_env_active = getenv(ENV_SANDBOX_ACTIVE);
+			sbcontext.active = (sb_env_active && !strcmp(sb_env_active, SANDBOX_ACTIVE));
+		}
+
+		if (sbcontext.active) {
+			bool on, set;
+			on = is_env_set_on(ENV_SANDBOX_ON, &set);
+			if (set)
+				sbcontext.on = on;
+
+			if (sbcontext.on)
+				result = true;
+		}
+	}
+
 	restore_errno();
 	return result;
 }
@@ -929,7 +989,6 @@ bool is_sandbox_on(void)
 bool before_syscall(int dirfd, int sb_nr, const char *func, const char *file, int flags)
 {
 	int result;
-	static sbcontext_t sbcontext;
 	char at_file_buf[SB_PATH_MAX];
 
 	/* Some funcs operate on a fd directly and so filename is NULL, but
@@ -962,39 +1021,10 @@ bool before_syscall(int dirfd, int sb_nr, const char *func, const char *file, in
 
 	if (!sb_init) {
 		libsb_init();
-		init_context(&sbcontext);
 		sb_init = true;
 	}
 
-	char *sb_env_names[4] = {
-		ENV_SANDBOX_DENY,
-		ENV_SANDBOX_READ,
-		ENV_SANDBOX_WRITE,
-		ENV_SANDBOX_PREDICT,
-	};
-
-	size_t i;
-	for (i = 0; i < ARRAY_SIZE(sb_env_names); ++i) {
-		char *sb_env = getenv(sb_env_names[i]);
-
-		if ((!sb_env && cached_env_vars[i]) ||
-		    !cached_env_vars[i] ||
-		    strcmp(cached_env_vars[i], sb_env) != 0)
-		{
-			clean_env_entries(&(sbcontext.prefixes[i]),
-				&(sbcontext.num_prefixes[i]));
-
-			if (cached_env_vars[i])
-				free(cached_env_vars[i]);
-
-			if (sb_env) {
-				init_env_entries(&(sbcontext.prefixes[i]),
-					&(sbcontext.num_prefixes[i]), sb_env_names[i], sb_env, 1);
-				cached_env_vars[i] = xstrdup(sb_env);
-			} else
-				cached_env_vars[i] = NULL;
-		}
-	}
+	sb_process_env_settings();
 
 	/* Might have been reset in check_access() */
 	sbcontext.show_access_violation = true;
@@ -1048,3 +1078,153 @@ bool before_syscall_open_char(int dirfd, int sb_nr, const char *func, const char
 		sb_nr = SB_NR_OPEN_WR, ext_func = "open_wr";
 	return before_syscall(dirfd, sb_nr, ext_func, file, 0);
 }
+
+typedef struct {
+	const char *name;
+	size_t len;
+	char *value;
+} env_pair;
+#define ENV_PAIR(x, n, v) [x] = { .name = n, .len = sizeof(n) - 1, .value = v, }
+
+#define str_list_add_item_env(_string_list, _var, _item, _error) \
+	do { \
+		char *str = xmalloc(strlen(_var) + strlen(_item) + 2); \
+		sprintf(str, "%s=%s", _var, _item); \
+		str_list_add_item(_string_list, str, _error); \
+	} while (0)
+/* We need to make sure we pass along sandbox env vars.  If we don't, programs
+ * (like scons) will inadvertently disable us.  While we allow modification
+ * (e.g. export SANDBOX_WRITE=""), we disallow clearing (e.g. unset SANDBOX_WRITE).
+ * The former is clear in the user's intention, but the latter is indicative
+ * of a bad program.
+ *
+ * XXX: Might be much nicer if we could serialize these vars behind the back of
+ *      the program.  Might be hard to handle LD_PRELOAD though ...
+ */
+char **sb_check_envp(char **envp, size_t *mod_cnt)
+{
+	char **my_env;
+	char *entry;
+	size_t count, i;
+	env_pair vars[] = {
+		/* Indices matter -- see init below */
+		ENV_PAIR( 0, ENV_LD_PRELOAD, sandbox_lib),
+		ENV_PAIR( 1, ENV_SANDBOX_LOG, log_path),
+		ENV_PAIR( 2, ENV_SANDBOX_DEBUG_LOG, debug_log_path),
+		ENV_PAIR( 3, ENV_SANDBOX_MESSAGE_PATH, message_path),
+		ENV_PAIR( 4, ENV_SANDBOX_DENY, cached_env_vars[0]),
+		ENV_PAIR( 5, ENV_SANDBOX_READ, cached_env_vars[1]),
+		ENV_PAIR( 6, ENV_SANDBOX_WRITE, cached_env_vars[2]),
+		ENV_PAIR( 7, ENV_SANDBOX_PREDICT, cached_env_vars[3]),
+		ENV_PAIR( 8, ENV_SANDBOX_ON, NULL),
+		ENV_PAIR( 9, ENV_SANDBOX_ACTIVE, NULL),
+		ENV_PAIR(10, ENV_SANDBOX_VERBOSE, NULL),
+		ENV_PAIR(11, ENV_SANDBOX_DEBUG, NULL),
+		ENV_PAIR(12, "LD_LIBRARY_PATH", NULL),
+		ENV_PAIR(13, ENV_SANDBOX_TESTING, NULL),
+	};
+	size_t num_vars = ARRAY_SIZE(vars);
+	char *found_vars[num_vars];
+	size_t found_var_cnt;
+
+	/* First figure out how many vars are already in the env */
+	found_var_cnt = 0;
+	memset(found_vars, 0, sizeof(found_vars));
+
+	str_list_for_each_item(envp, entry, count) {
+		for (i = 0; i < num_vars; ++i) {
+			if (found_vars[i])
+				continue;
+			if (unlikely(!is_env_var(entry, vars[i].name, vars[i].len)))
+				continue;
+			found_vars[i] = entry;
+			++found_var_cnt;
+		}
+	}
+
+	/* Now specially handle merging of LD_PRELOAD */
+	char *ld_preload;
+	bool merge_ld_preload = found_vars[0] && !strstr(found_vars[0], sandbox_lib);
+	if (unlikely(merge_ld_preload)) {
+		/* Ok, there's an existing LD_PRELOAD value that we need to merge
+		 * with.  Handle this specially. */
+		size_t ld_preload_len = strlen(ENV_LD_PRELOAD);
+		count = ld_preload_len + 1 + strlen(sandbox_lib) + 1 +
+			strlen(found_vars[0] + ld_preload_len + 1);
+		ld_preload = xmalloc(count * sizeof(char));
+		sprintf(ld_preload, "%s=%s %s", ENV_LD_PRELOAD, sandbox_lib,
+			found_vars[0] + ld_preload_len + 1);
+		goto mod_env;
+	}
+
+	/* If we found everything, there's nothing to do! */
+	if (num_vars == found_var_cnt)
+		/* Use the user's envp */
+		return envp;
+
+	/* Ok, we need to create our own envp, as we need to restore stuff
+	 * and we should not touch the user's envp.  First we add our vars,
+	 * and just all the rest. */
+ mod_env:
+	/* Indices matter -- see vars[] setup above */
+	if (sbcontext.on)
+		vars[8].value = "1";
+	if (sbcontext.active)
+		vars[9].value = SANDBOX_ACTIVE;
+	if (sbcontext.verbose)
+		vars[10].value = "1";
+	if (sbcontext.debug)
+		vars[11].value = "1";
+	if (sbcontext.testing) {
+		vars[12].value = sbcontext.ld_library_path;
+		vars[13].value = "1";
+	}
+
+	my_env = NULL;
+	if (mod_cnt) {
+		/* Count directly due to variability with LD_PRELOAD and the value
+		 * logic below.  Getting out of sync can mean memory corruption. */
+		*mod_cnt = 0;
+		if (unlikely(merge_ld_preload)) {
+			str_list_add_item(my_env, ld_preload, error);
+			(*mod_cnt)++;
+		}
+		for (i = 0; i < num_vars; ++i) {
+			if (found_vars[i] || !vars[i].value)
+				continue;
+			str_list_add_item_env(my_env, vars[i].name, vars[i].value, error);
+			(*mod_cnt)++;
+		}
+
+		str_list_for_each_item(envp, entry, count) {
+			if (unlikely(merge_ld_preload && is_env_var(entry, vars[0].name, vars[0].len)))
+				continue;
+			str_list_add_item(my_env, entry, error);
+		}
+	} else {
+		if (unlikely(merge_ld_preload))
+			putenv(ld_preload);
+		for (i = 0; i < num_vars; ++i) {
+			if (found_vars[i] || !vars[i].value)
+				continue;
+			setenv(vars[i].name, vars[i].value, 1);
+		}
+	}
+
+ error:
+	return my_env;
+}
+
+void sb_cleanup_envp(char **envp, size_t mod_cnt)
+{
+	/* We assume all the stuffed vars are at the start */
+	size_t i;
+	for (i = 0; i < mod_cnt; ++i)
+		free(envp[i]);
+
+	/* We do not use str_list_free(), as we did not allocate the
+	 * entries except for LD_PRELOAD.  All the other entries are
+	 * pointers to existing envp memory.
+	 */
+	free(envp);
+}

diff --git a/libsandbox/libsandbox.h b/libsandbox/libsandbox.h
index 76dd8c8..596084d 100644
--- a/libsandbox/libsandbox.h
+++ b/libsandbox/libsandbox.h
@@ -56,6 +56,8 @@ void *get_dlsym(const char *symname, const char *symver);
 
 extern char sandbox_lib[SB_PATH_MAX];
 extern bool sandbox_on;
+char **sb_check_envp(char **envp, size_t *mod_cnt);
+void sb_cleanup_envp(char **envp, size_t mod_cnt);
 extern pid_t trace_pid;
 
 extern void sb_lock(void);

diff --git a/libsandbox/wrapper-funcs/__wrapper_exec.c b/libsandbox/wrapper-funcs/__wrapper_exec.c
index 3ac936c..7107c21 100644
--- a/libsandbox/wrapper-funcs/__wrapper_exec.c
+++ b/libsandbox/wrapper-funcs/__wrapper_exec.c
@@ -92,79 +92,6 @@ static void sb_check_exec(const char *filename, char *const argv[])
 		trace_main(filename, argv);
 }
 
-static char **_sb_check_envp(char **envp, bool is_environ)
-{
-	char **my_env = NULL;
-	char *entry;
-	char *ld_preload = NULL;
-	char *old_ld_preload = NULL;
-	size_t count, ld_preload_len;
-
-	ld_preload_len = strlen(ENV_LD_PRELOAD);
-	str_list_for_each_item(envp, entry, count) {
-		if (!is_env_var(entry, ENV_LD_PRELOAD, ld_preload_len))
-			continue;
-
-		/* Check if we do not have to do anything */
-		if (NULL != strstr(entry, sandbox_lib)) {
-			/* Use the user's envp */
-			return envp;
-		} else {
-			/* No need to continue (assuming the env is sane and does not
-			 * include multiple entries for same var); we have to modify
-			 * LD_PRELOAD to include our sandbox overrides
-			 */
-			old_ld_preload = entry;
-			break;
-		}
-	}
-
-	/* Ok, we need to create our own envp, as we need to add LD_PRELOAD,
-	 * and we should not touch the user's envp.  First we add LD_PRELOAD,
-	 * and just all the rest. */
-	count = ld_preload_len + 1 + strlen(sandbox_lib) + 1 +
-		(old_ld_preload ? strlen(old_ld_preload) - ld_preload_len : 0);
-	ld_preload = xmalloc(count * sizeof(char));
-	snprintf(ld_preload, count, "%s=%s%s%s", ENV_LD_PRELOAD, sandbox_lib,
-		 (old_ld_preload) ? " " : "",
-		 (old_ld_preload) ? old_ld_preload + ld_preload_len + 1 : "");
-
-	if (!is_environ) {
-		str_list_add_item(my_env, ld_preload, error);
-
-		str_list_for_each_item(envp, entry, count) {
-			if (!is_env_var(entry, ENV_LD_PRELOAD, ld_preload_len)) {
-				str_list_add_item(my_env, entry, error);
-				continue;
-			}
-		}
-	} else
-		putenv(ld_preload);
-
- error:
-	return my_env;
-}
-static char **sb_check_envp(char **envp)
-{
-	return _sb_check_envp(envp, false);
-}
-static void sb_check_environ(void)
-{
-	_sb_check_envp(environ, true);
-}
-
-static void sb_cleanup_envp(char **envp)
-{
-	/* We assume the LD_PRELOAD is the first entry */
-	free(envp[0]);
-
-	/* We do not use str_list_free(), as we did not allocate the
-	 * entries except for LD_PRELOAD.  All the other entries are
-	 * pointers to existing envp memory.
-	 */
-	free(envp);
-}
-
 #endif
 
 attribute_hidden
@@ -181,10 +108,6 @@ WRAPPER_RET_TYPE WRAPPER_NAME(WRAPPER_ARGS_PROTO)
 {
 	WRAPPER_RET_TYPE result = WRAPPER_RET_DEFAULT;
 
-#ifdef EXEC_MY_ENV
-	char **my_env = (char **)envp;
-#endif
-
 	/* The C library may implement some exec funcs by calling other
 	 * exec funcs.  So we might get a little sandbox recursion going
 	 * on.  But this shouldn't cause a problem now should it ?
@@ -233,9 +156,10 @@ WRAPPER_RET_TYPE WRAPPER_NAME(WRAPPER_ARGS_PROTO)
 #endif
 
 #ifdef EXEC_MY_ENV
-	my_env = sb_check_envp(my_env);
+	size_t mod_cnt;
+	char **my_env = sb_check_envp((char **)envp, &mod_cnt);
 #else
-	sb_check_environ();
+	sb_check_envp(environ, NULL);
 #endif
 
 	restore_errno();
@@ -246,7 +170,7 @@ WRAPPER_RET_TYPE WRAPPER_NAME(WRAPPER_ARGS_PROTO)
 
 #ifdef EXEC_MY_ENV
 	if (my_env != envp)
-		sb_cleanup_envp(my_env);
+		sb_cleanup_envp(my_env, mod_cnt);
 #endif
 
 #ifdef EXEC_RECUR_CHECK

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1d32e2e..0f0c249 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -79,7 +79,7 @@ check_PROGRAMS = \
 	sigsuspend-zsh_static_tst
 
 dist_check_SCRIPTS = \
-	$(wildcard $(srcdir)/*-?.sh) \
+	$(wildcard $(srcdir)/*-[0-9]*.sh) \
 	script-0 \
 	trace-0
 

diff --git a/tests/mkdir-3.sh b/tests/mkdir-3.sh
index 10e8723..e5b7899 100755
--- a/tests/mkdir-3.sh
+++ b/tests/mkdir-3.sh
@@ -18,7 +18,7 @@ chmod a-rx ..
 ln -s / root
 
 # this should trigger a sb violation
-unset SANDBOX_PREDICT
+SANDBOX_PREDICT=""
 (mkdir-0 -1 root/aksdfjasdfjaskdfjasdfla 0777)
 
 chmod a+rx ..

diff --git a/tests/script-10.sh b/tests/script-10.sh
new file mode 100755
index 0000000..801730e
--- /dev/null
+++ b/tests/script-10.sh
@@ -0,0 +1,21 @@
+#!/bin/sh
+# make sure all the SANDBOX env vars make it back in.
+[ "${at_xfail}" = "yes" ] && exit 77 # see script-0
+
+ret=0
+
+out=$(env -i env)
+for var in LOG DEBUG_LOG MESSAGE_PATH DENY READ WRITE PREDICT ON ACTIVE ; do
+	var="SANDBOX_${var}"
+	oval=$(env | grep "^${var}=" | sed 's:^[^=]*=::')
+
+	nval=$(echo "${out}" | sed -n "/^${var}=/s:[^=]*=::p")
+
+	[ "${nval}" != "${oval}" ] && echo "!!! MISMATCH !!!" && ret=1
+	echo "env  [${var}]='${oval}'"
+	echo "env-i[${var}]='${nval}'"
+	[ "${nval}" != "${oval}" ] && echo "!!! MISMATCH !!!"
+	echo
+done
+
+exit ${ret}

diff --git a/tests/script.at b/tests/script.at
index b095ce1..93e370a 100644
--- a/tests/script.at
+++ b/tests/script.at
@@ -7,3 +7,4 @@ SB_CHECK(6,,,8)
 SB_CHECK(7)
 SB_CHECK(8)
 SB_CHECK(9, [wait errpipe... done OK!])
+SB_CHECK(10)