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)
