commit cbfb8e703ed9c7e31848ebf959ac7a4cf27b4a64 Author: Nick Mathewson nickm@torproject.org Date: Fri Mar 28 03:51:50 2014 -0400
Add 'rename' to the sandboxed syscalls
(If we don't restrict rename, there's not much point in restricting open, since an attacker could always use rename to make us open whatever they want.) --- src/common/compat.c | 12 ++++++-- src/common/compat.h | 1 + src/common/sandbox.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++--- src/common/sandbox.h | 7 +++++ src/or/config.c | 2 +- src/or/main.c | 39 ++++++++++++++++++++++-- src/or/statefile.c | 3 +- 7 files changed, 136 insertions(+), 11 deletions(-)
diff --git a/src/common/compat.c b/src/common/compat.c index 04c9d59..5c18535 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -175,6 +175,14 @@ tor_fopen_cloexec(const char *path, const char *mode) return result; }
+/** As rename(), but work correctly with the sandbox. */ +int +tor_rename(const char *path_old, const char *path_new) +{ + return rename(sandbox_intern_string(path_old), + sandbox_intern_string(path_new)); +} + #if defined(HAVE_SYS_MMAN_H) || defined(RUNNING_DOXYGEN) /** Try to create a memory mapping for <b>filename</b> and return it. On * failure, return NULL. Sets errno properly, using ERANGE to mean @@ -799,7 +807,7 @@ int replace_file(const char *from, const char *to) { #ifndef _WIN32 - return rename(from,to); + return tor_rename(from, to); #else switch (file_status(to)) { @@ -814,7 +822,7 @@ replace_file(const char *from, const char *to) errno = EISDIR; return -1; } - return rename(from,to); + return tor_rename(from,to); #endif }
diff --git a/src/common/compat.h b/src/common/compat.h index bb88818..9a381fb 100644 --- a/src/common/compat.h +++ b/src/common/compat.h @@ -410,6 +410,7 @@ struct tm *tor_gmtime_r(const time_t *timep, struct tm *result); /* ===== File compatibility */ int tor_open_cloexec(const char *path, int flags, unsigned mode); FILE *tor_fopen_cloexec(const char *path, const char *mode); +int tor_rename(const char *path_old, const char *path_new);
int replace_file(const char *from, const char *to); int touch_file(const char *fname); diff --git a/src/common/sandbox.c b/src/common/sandbox.c index 363333a..c2f482d 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -122,7 +122,6 @@ static int filter_nopar_gen[] = { SCMP_SYS(mmap), SCMP_SYS(munmap), SCMP_SYS(read), - SCMP_SYS(rename), SCMP_SYS(rt_sigreturn), SCMP_SYS(set_robust_list), SCMP_SYS(_sysctl), @@ -362,6 +361,41 @@ sb_open(scmp_filter_ctx ctx, sandbox_cfg_t *filter) }
/** + * Function responsible for setting up the rename syscall for + * the seccomp filter sandbox. + */ +static int +sb_rename(scmp_filter_ctx ctx, sandbox_cfg_t *filter) +{ + int rc; + sandbox_cfg_t *elem = NULL; + + // for each dynamic parameter filters + for (elem = filter; elem != NULL; elem = elem->next) { + smp_param_t *param = elem->param; + + if (param != NULL && param->prot == 1 && + param->syscall == SCMP_SYS(rename)) { + + intptr_t value2 = (intptr_t)(void*)sandbox_intern_string( + (char*)param->value2); + + rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, + SCMP_SYS(rename), 1, + SCMP_CMP(0, SCMP_CMP_EQ, param->value), + SCMP_CMP(1, SCMP_CMP_EQ, value2)); + if (rc != 0) { + log_err(LD_BUG,"(Sandbox) failed to add rename syscall, received " + "libseccomp error %d", rc); + return rc; + } + } + } + + return 0; +} + +/** * Function responsible for setting up the openat syscall for * the seccomp filter sandbox. */ @@ -807,6 +841,7 @@ static sandbox_filter_func_t filter_func[] = { #endif sb_open, sb_openat, + sb_rename, #ifdef __NR_fcntl64 sb_fcntl64, #endif @@ -888,6 +923,7 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) size_t param_size = strlen(param_val) + 1;
void *location = strmap_get(locations, param_val); + if (location) { // We already interned this string. { @@ -989,22 +1025,30 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) * point. */ static sandbox_cfg_t* -new_element(int syscall, int index, intptr_t value) +new_element2(int syscall, int index, int index2, intptr_t value, intptr_t value2) { smp_param_t *param = NULL;
- sandbox_cfg_t *elem = tor_malloc(sizeof(sandbox_cfg_t)); - elem->param = tor_malloc(sizeof(smp_param_t)); + sandbox_cfg_t *elem = tor_malloc_zero(sizeof(sandbox_cfg_t)); + elem->param = tor_malloc_zero(sizeof(smp_param_t));
param = elem->param; param->syscall = syscall; param->pindex = index; + param->pindex2 = index2; param->value = value; + param->value2 = value2; param->prot = 0;
return elem; }
+static sandbox_cfg_t* +new_element(int syscall, int index, intptr_t value) +{ + return new_element2(syscall, index, -1, value, 0); +} + #ifdef __NR_stat64 #define SCMP_stat SCMP_SYS(stat64) #else @@ -1073,6 +1117,37 @@ sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file, int fr) }
int +sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2) +{ + sandbox_cfg_t *elem = NULL; + + elem = new_element2(SCMP_SYS(rename), 0, 1, + (intptr_t)(void *)tor_strdup(file1), + (intptr_t)(void *)tor_strdup(file2)); + + if (!elem) { + log_err(LD_BUG,"(Sandbox) failed to register parameter!"); + return -1; + } + + elem->next = *cfg; + *cfg = elem; + + /* For interning */ + elem = new_element(-1, 0, (intptr_t)(void*)tor_strdup(file2)); + if (!elem) { + log_err(LD_BUG,"(Sandbox) failed to register parameter!"); + return -1; + } + elem->next = *cfg; + *cfg = elem; + + tor_free(file1); + tor_free(file2); + return 0; +} + +int sandbox_cfg_allow_open_filename_array(sandbox_cfg_t **cfg, ...) { int rc = 0; diff --git a/src/common/sandbox.h b/src/common/sandbox.h index d64d427..b15f31c 100644 --- a/src/common/sandbox.h +++ b/src/common/sandbox.h @@ -67,8 +67,12 @@ typedef struct smp_param {
/** parameter index. */ int pindex; + /** parameter index, second one. */ + int pindex2; /** parameter value. */ intptr_t value; + /** parameter value, second argument. */ + intptr_t value2;
/** parameter flag (0 = not protected, 1 = protected). */ int prot; @@ -174,6 +178,9 @@ sandbox_cfg_t * sandbox_cfg_new(void); int sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file, int fr);
+/**DOCDOC*/ +int sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2); + /** Function used to add a series of open allowed filenames to a supplied * configuration. * @param cfg sandbox configuration. diff --git a/src/or/config.c b/src/or/config.c index ca99d01..89aedcc 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -6276,7 +6276,7 @@ write_configuration_file(const char *fname, const or_options_t *options) ++i; } log_notice(LD_CONFIG, "Renaming old configuration file to "%s"", fn_tmp); - if (rename(fname, fn_tmp) < 0) { + if (tor_rename(fname, fn_tmp) < 0) {//XXXX sandbox doesn't allow log_warn(LD_FS, "Couldn't rename configuration file "%s" to "%s": %s", fname, fn_tmp, strerror(errno)); diff --git a/src/or/main.c b/src/or/main.c index 1614954..3c248bb 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -2743,7 +2743,6 @@ sandbox_init_filter(void) get_datadir_fname("cached-microdescs.tmp"), 1, get_datadir_fname("cached-microdescs.new"), 1, get_datadir_fname("cached-microdescs.new.tmp"), 1, - get_datadir_fname("unverified-microdesc-consensus"), 1, get_datadir_fname("cached-descriptors"), 1, get_datadir_fname("cached-descriptors.new"), 1, get_datadir_fname("cached-descriptors.tmp"), 1, @@ -2765,6 +2764,34 @@ sandbox_init_filter(void) NULL, 0 );
+#define RENAME_SUFFIX(name, suffix) \ + sandbox_cfg_allow_rename(&cfg, \ + get_datadir_fname(name suffix), \ + get_datadir_fname(name)) + +#define RENAME_SUFFIX2(prefix, name, suffix) \ + sandbox_cfg_allow_rename(&cfg, \ + get_datadir_fname2(prefix, name suffix), \ + get_datadir_fname2(prefix, name)) + + RENAME_SUFFIX("cached-certs", ".tmp"); + RENAME_SUFFIX("cached-consensus", ".tmp"); + RENAME_SUFFIX("unverified-consensus", ".tmp"); + RENAME_SUFFIX("unverified-microdesc-consensus", ".tmp"); + RENAME_SUFFIX("cached-microdesc-consensus", ".tmp"); + RENAME_SUFFIX("cached-microdescs", ".tmp"); + RENAME_SUFFIX("cached-microdescs", ".new"); + RENAME_SUFFIX("cached-microdescs.new", ".tmp"); + RENAME_SUFFIX("cached-descriptors", ".tmp"); + RENAME_SUFFIX("cached-descriptors", ".new"); + RENAME_SUFFIX("cached-descriptors.new", ".tmp"); + RENAME_SUFFIX("cached-extrainfo", ".tmp"); + RENAME_SUFFIX("cached-extrainfo", ".new"); + RENAME_SUFFIX("cached-extrainfo.new", ".tmp"); + RENAME_SUFFIX("state", ".tmp"); + RENAME_SUFFIX("unparseable-desc", ".tmp"); + RENAME_SUFFIX("v3-status-votes", ".tmp"); + sandbox_cfg_allow_stat_filename_array(&cfg, get_datadir_fname(NULL), 1, get_datadir_fname("lock"), 1, @@ -2790,12 +2817,18 @@ sandbox_init_filter(void) get_datadir_fname("fingerprint.tmp"), 1, get_datadir_fname("hashed-fingerprint"), 1, get_datadir_fname("hashed-fingerprint.tmp"), 1, - get_datadir_fname("cached-consensus"), 1, - get_datadir_fname("cached-consensus.tmp"), 1, "/etc/resolv.conf", 0, NULL, 0 );
+ RENAME_SUFFIX("fingerprint", ".tmp"); + RENAME_SUFFIX2("keys", "secret_onion_key_ntor", ".tmp"); + RENAME_SUFFIX2("keys", "secret_id_key", ".tmp"); + RENAME_SUFFIX2("keys", "secret_id_key.old", ".tmp"); + RENAME_SUFFIX2("keys", "secret_onion_key", ".tmp"); + RENAME_SUFFIX2("keys", "secret_onion_key.old", ".tmp"); + RENAME_SUFFIX("hashed-fingerprint", ".tmp"); + sandbox_cfg_allow_stat_filename_array(&cfg, get_datadir_fname("keys"), 1, get_datadir_fname("stats/dirreq-stats"), 1, diff --git a/src/or/statefile.c b/src/or/statefile.c index 2251f25..da31341 100644 --- a/src/or/statefile.c +++ b/src/or/statefile.c @@ -13,6 +13,7 @@ #include "hibernate.h" #include "rephist.h" #include "router.h" +#include "sandbox.h" #include "statefile.h"
/** A list of state-file "abbreviations," for compatibility. */ @@ -285,7 +286,7 @@ or_state_save_broken(char *fname) log_warn(LD_BUG, "Unable to parse state in "%s". Moving it aside " "to "%s". This could be a bug in Tor; please tell " "the developers.", fname, fname2); - if (rename(fname, fname2) < 0) { + if (tor_rename(fname, fname2) < 0) {//XXXX sandbox prohibits log_warn(LD_BUG, "Weirdly, I couldn't even move the state aside. The " "OS gave an error of %s", strerror(errno)); }