tor-commits April 2014

tor-commits@lists.torproject.org

22 participants
2020 discussions

[tor/master] Remove nonsensical exec permission from sandbox code.
by nickm＠torproject.org 17 Apr '14

17 Apr '14

commit e051e192a8b199c20ece2a4205c9642a4a0cee22 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 11 02:40:15 2014 -0400 Remove nonsensical exec permission from sandbox code. --- src/or/main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/or/main.c b/src/or/main.c index 3c248bb..e6c1636 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -2836,8 +2836,6 @@ sandbox_init_filter(void) ); } - sandbox_cfg_allow_execve(&cfg, "/usr/local/bin/tor"); - init_addrinfo(); return cfg;

1 0

[tor/master] Drop 'fr' parameter from sandbox code.
by nickm＠torproject.org 17 Apr '14

17 Apr '14

commit 71eaebd971f4d42b26fb6b85780163bbc0111aae Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 11 03:04:16 2014 -0400 Drop 'fr' parameter from sandbox code. Appearently, the majority of the filenames we pass to sandbox_cfg_allow() functions are "freeable right after". So, consider _all_ of them safe-to-steal, and add a tor_strdup() in the few cases that aren't. (Maybe buggy; revise when I can test.) --- src/common/sandbox.c | 55 ++++++++++--------------- src/common/sandbox.h | 53 +++++++++---------------- src/or/main.c | 108 +++++++++++++++++++++++++------------------------- 3 files changed, 92 insertions(+), 124 deletions(-) diff --git a/src/common/sandbox.c b/src/common/sandbox.c index c2f482d..b97b900 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -1056,11 +1056,11 @@ new_element(int syscall, int index, intptr_t value) #endif int -sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file, int fr) +sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file) { sandbox_cfg_t *elem = NULL; - elem = new_element(SCMP_stat, 0, (intptr_t)(void*) tor_strdup(file)); + elem = new_element(SCMP_stat, 0, (intptr_t)(void*) file); if (!elem) { log_err(LD_BUG,"(Sandbox) failed to register parameter!"); return -1; @@ -1069,7 +1069,6 @@ sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file, int fr) elem->next = *cfg; *cfg = elem; - if (fr) tor_free(file); return 0; } @@ -1083,9 +1082,7 @@ sandbox_cfg_allow_stat_filename_array(sandbox_cfg_t **cfg, ...) va_start(ap, cfg); while ((fn = va_arg(ap, char*)) != NULL) { - int fr = va_arg(ap, int); - - rc = sandbox_cfg_allow_stat_filename(cfg, fn, fr); + rc = sandbox_cfg_allow_stat_filename(cfg, fn); if (rc) { log_err(LD_BUG,"(Sandbox) sandbox_cfg_allow_stat_filename_array fail"); goto end; @@ -1098,11 +1095,11 @@ sandbox_cfg_allow_stat_filename_array(sandbox_cfg_t **cfg, ...) } int -sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file, int fr) +sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file) { sandbox_cfg_t *elem = NULL; - elem = new_element(SCMP_SYS(open), 0, (intptr_t)(void *)tor_strdup(file)); + elem = new_element(SCMP_SYS(open), 0, (intptr_t)(void *) file); if (!elem) { log_err(LD_BUG,"(Sandbox) failed to register parameter!"); return -1; @@ -1111,8 +1108,6 @@ sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file, int fr) elem->next = *cfg; *cfg = elem; - if (fr) tor_free(file); - return 0; } @@ -1122,8 +1117,8 @@ 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)); + (intptr_t)(void *) file1, + (intptr_t)(void *) file2); if (!elem) { log_err(LD_BUG,"(Sandbox) failed to register parameter!"); @@ -1142,8 +1137,6 @@ sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2) elem->next = *cfg; *cfg = elem; - tor_free(file1); - tor_free(file2); return 0; } @@ -1157,9 +1150,7 @@ sandbox_cfg_allow_open_filename_array(sandbox_cfg_t **cfg, ...) va_start(ap, cfg); while ((fn = va_arg(ap, char*)) != NULL) { - int fr = va_arg(ap, int); - - rc = sandbox_cfg_allow_open_filename(cfg, fn, fr); + rc = sandbox_cfg_allow_open_filename(cfg, fn); if (rc) { log_err(LD_BUG,"(Sandbox) sandbox_cfg_allow_open_filename_array fail"); goto end; @@ -1172,11 +1163,11 @@ sandbox_cfg_allow_open_filename_array(sandbox_cfg_t **cfg, ...) } int -sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file, int fr) +sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file) { sandbox_cfg_t *elem = NULL; - elem = new_element(SCMP_SYS(openat), 1, (intptr_t)(void *)tor_strdup(file)); + elem = new_element(SCMP_SYS(openat), 1, (intptr_t)(void *) file); if (!elem) { log_err(LD_BUG,"(Sandbox) failed to register parameter!"); return -1; @@ -1185,8 +1176,6 @@ sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file, int fr) elem->next = *cfg; *cfg = elem; - if (fr) tor_free(file); - return 0; } @@ -1200,9 +1189,7 @@ sandbox_cfg_allow_openat_filename_array(sandbox_cfg_t **cfg, ...) va_start(ap, cfg); while ((fn = va_arg(ap, char*)) != NULL) { - int fr = va_arg(ap, int); - - rc = sandbox_cfg_allow_openat_filename(cfg, fn, fr); + rc = sandbox_cfg_allow_openat_filename(cfg, fn); if (rc) { log_err(LD_BUG,"(Sandbox) sandbox_cfg_allow_openat_filename_array fail"); goto end; @@ -1219,7 +1206,7 @@ sandbox_cfg_allow_execve(sandbox_cfg_t **cfg, const char *com) { sandbox_cfg_t *elem = NULL; - elem = new_element(SCMP_SYS(execve), 1, (intptr_t)(void *)tor_strdup(com)); + elem = new_element(SCMP_SYS(execve), 1, (intptr_t)(void *) com); if (!elem) { log_err(LD_BUG,"(Sandbox) failed to register parameter!"); return -1; @@ -1519,7 +1506,8 @@ register_cfg(sandbox_cfg_t* cfg) return 0; } - for (elem = filter_dynamic; elem->next != NULL; elem = elem->next); + for (elem = filter_dynamic; elem->next != NULL; elem = elem->next) + ; elem->next = cfg; @@ -1583,10 +1571,9 @@ sandbox_init(sandbox_cfg_t *cfg) #ifndef USE_LIBSECCOMP int -sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file, - int fr) +sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file) { - (void)cfg; (void)file; (void)fr; + (void)cfg; (void)file; return 0; } @@ -1598,10 +1585,9 @@ sandbox_cfg_allow_open_filename_array(sandbox_cfg_t **cfg, ...) } int -sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file, - int fr) +sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file) { - (void)cfg; (void)file; (void)fr; + (void)cfg; (void)file; return 0; } @@ -1627,10 +1613,9 @@ sandbox_cfg_allow_execve_array(sandbox_cfg_t **cfg, ...) } int -sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file, - int fr) +sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file) { - (void)cfg; (void)file; (void)fr; + (void)cfg; (void)file; return 0; } diff --git a/src/common/sandbox.h b/src/common/sandbox.h index b15f31c..8fcd221 100644 --- a/src/common/sandbox.h +++ b/src/common/sandbox.h @@ -171,12 +171,10 @@ sandbox_cfg_t * sandbox_cfg_new(void); /** * Function used to add a open allowed filename to a supplied configuration. - * The (char*) specifies the path to the allowed file, fr = 1 tells the - * function that the char* needs to be free-ed, 0 means the pointer does not - * need to be free-ed. + * The (char*) specifies the path to the allowed file; we take ownership + * of the pointer. */ -int sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file, - int fr); +int sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file); /**DOCDOC*/ int sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2); @@ -184,66 +182,51 @@ 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. - * @param ... all future parameters are specified as pairs of <(char*), 1 / 0> - * the char* specifies the path to the allowed file, 1 tells the function - * that the char* needs to be free-ed, 0 means the pointer does not need to - * be free-ed; the final parameter needs to be <NULL, 0>. - */ + * @param ... a list of stealable pointers to permitted files. The last + * one must be NULL. +*/ int sandbox_cfg_allow_open_filename_array(sandbox_cfg_t **cfg, ...); /** * Function used to add a openat allowed filename to a supplied configuration. - * The (char*) specifies the path to the allowed file, fr = 1 tells the - * function that the char* needs to be free-ed, 0 means the pointer does not - * need to be free-ed. + * The (char*) specifies the path to the allowed file; we steal the pointer to + * that file. */ -int sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file, - int fr); +int sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file); /** Function used to add a series of openat allowed filenames to a supplied * configuration. * @param cfg sandbox configuration. - * @param ... all future parameters are specified as pairs of <(char*), 1 / 0> - * the char* specifies the path to the allowed file, 1 tells the function - * that the char* needs to be free-ed, 0 means the pointer does not need to - * be free-ed; the final parameter needs to be <NULL, 0>. + * @param ... a list of stealable pointers to permitted files. The last + * one must be NULL. */ int sandbox_cfg_allow_openat_filename_array(sandbox_cfg_t **cfg, ...); /** * Function used to add a execve allowed filename to a supplied configuration. - * The (char*) specifies the path to the allowed file, fr = 1 tells the - * function that the char* needs to be free-ed, 0 means the pointer does not - * need to be free-ed. + * The (char*) specifies the path to the allowed file; that pointer is stolen. */ int sandbox_cfg_allow_execve(sandbox_cfg_t **cfg, const char *com); /** Function used to add a series of execve allowed filenames to a supplied * configuration. * @param cfg sandbox configuration. - * @param ... all future parameters are specified as pairs of <(char*), 1 / 0> - * the char* specifies the path to the allowed file, 1 tells the function - * that the char* needs to be free-ed, 0 means the pointer does not need to - * be free-ed; the final parameter needs to be <NULL, 0>. + * @param ... an array of stealable pointers to permitted files. The last + * one must be NULL. */ int sandbox_cfg_allow_execve_array(sandbox_cfg_t **cfg, ...); /** * Function used to add a stat/stat64 allowed filename to a configuration. - * The (char*) specifies the path to the allowed file, fr = 1 tells the - * function that the char* needs to be free-ed, 0 means the pointer does not - * need to be free-ed. + * The (char*) specifies the path to the allowed file; that pointer is stolen. */ -int sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file, - int fr); +int sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file); /** Function used to add a series of stat64 allowed filenames to a supplied * configuration. * @param cfg sandbox configuration. - * @param ... all future parameters are specified as pairs of <(char*), 1 / 0> - * the char* specifies the path to the allowed file, 1 tells the function - * that the char* needs to be free-ed, 0 means the pointer does not need to - * be free-ed; the final parameter needs to be <NULL, 0>. + * @param ... an array of stealable pointers to permitted files. The last + * one must be NULL. */ int sandbox_cfg_allow_stat_filename_array(sandbox_cfg_t **cfg, ...); diff --git a/src/or/main.c b/src/or/main.c index e6c1636..341f22a 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -2726,41 +2726,41 @@ sandbox_init_filter(void) sandbox_cfg_t *cfg = sandbox_cfg_new(); sandbox_cfg_allow_openat_filename(&cfg, - get_datadir_fname("cached-status"), 1); + get_datadir_fname("cached-status")); sandbox_cfg_allow_open_filename_array(&cfg, - get_datadir_fname("cached-certs"), 1, - get_datadir_fname("cached-certs.tmp"), 1, - get_datadir_fname("cached-consensus"), 1, - get_datadir_fname("cached-consensus.tmp"), 1, - get_datadir_fname("unverified-consensus"), 1, - get_datadir_fname("unverified-consensus.tmp"), 1, - get_datadir_fname("unverified-microdesc-consensus"), 1, - get_datadir_fname("unverified-microdesc-consensus.tmp"), 1, - get_datadir_fname("cached-microdesc-consensus"), 1, - get_datadir_fname("cached-microdesc-consensus.tmp"), 1, - get_datadir_fname("cached-microdescs"), 1, - 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("cached-descriptors"), 1, - get_datadir_fname("cached-descriptors.new"), 1, - get_datadir_fname("cached-descriptors.tmp"), 1, - get_datadir_fname("cached-descriptors.new.tmp"), 1, - get_datadir_fname("cached-descriptors.tmp.tmp"), 1, - get_datadir_fname("cached-extrainfo"), 1, - get_datadir_fname("cached-extrainfo.new"), 1, - get_datadir_fname("cached-extrainfo.tmp"), 1, - get_datadir_fname("cached-extrainfo.new.tmp"), 1, - get_datadir_fname("cached-extrainfo.tmp.tmp"), 1, - get_datadir_fname("state.tmp"), 1, - get_datadir_fname("unparseable-desc.tmp"), 1, - get_datadir_fname("unparseable-desc"), 1, - get_datadir_fname("v3-status-votes"), 1, - get_datadir_fname("v3-status-votes.tmp"), 1, - "/dev/srandom", 0, - "/dev/urandom", 0, - "/dev/random", 0, + get_datadir_fname("cached-certs"), + get_datadir_fname("cached-certs.tmp"), + get_datadir_fname("cached-consensus"), + get_datadir_fname("cached-consensus.tmp"), + get_datadir_fname("unverified-consensus"), + get_datadir_fname("unverified-consensus.tmp"), + get_datadir_fname("unverified-microdesc-consensus"), + get_datadir_fname("unverified-microdesc-consensus.tmp"), + get_datadir_fname("cached-microdesc-consensus"), + get_datadir_fname("cached-microdesc-consensus.tmp"), + get_datadir_fname("cached-microdescs"), + get_datadir_fname("cached-microdescs.tmp"), + get_datadir_fname("cached-microdescs.new"), + get_datadir_fname("cached-microdescs.new.tmp"), + get_datadir_fname("cached-descriptors"), + get_datadir_fname("cached-descriptors.new"), + get_datadir_fname("cached-descriptors.tmp"), + get_datadir_fname("cached-descriptors.new.tmp"), + get_datadir_fname("cached-descriptors.tmp.tmp"), + get_datadir_fname("cached-extrainfo"), + get_datadir_fname("cached-extrainfo.new"), + get_datadir_fname("cached-extrainfo.tmp"), + get_datadir_fname("cached-extrainfo.new.tmp"), + get_datadir_fname("cached-extrainfo.tmp.tmp"), + get_datadir_fname("state.tmp"), + get_datadir_fname("unparseable-desc.tmp"), + get_datadir_fname("unparseable-desc"), + get_datadir_fname("v3-status-votes"), + get_datadir_fname("v3-status-votes.tmp"), + tor_strdup("/dev/srandom"), + tor_strdup("/dev/urandom"), + tor_strdup("/dev/random"), NULL, 0 ); @@ -2793,31 +2793,31 @@ sandbox_init_filter(void) RENAME_SUFFIX("v3-status-votes", ".tmp"); sandbox_cfg_allow_stat_filename_array(&cfg, - get_datadir_fname(NULL), 1, - get_datadir_fname("lock"), 1, - get_datadir_fname("state"), 1, - get_datadir_fname("router-stability"), 1, - get_datadir_fname("cached-extrainfo.new"), 1, + get_datadir_fname(NULL), + get_datadir_fname("lock"), + get_datadir_fname("state"), + get_datadir_fname("router-stability"), + get_datadir_fname("cached-extrainfo.new"), NULL, 0 ); // orport if (server_mode(get_options())) { sandbox_cfg_allow_open_filename_array(&cfg, - get_datadir_fname2("keys", "secret_id_key"), 1, - get_datadir_fname2("keys", "secret_onion_key"), 1, - get_datadir_fname2("keys", "secret_onion_key_ntor"), 1, - get_datadir_fname2("keys", "secret_onion_key_ntor.tmp"), 1, - get_datadir_fname2("keys", "secret_id_key.old"), 1, - get_datadir_fname2("keys", "secret_onion_key.old"), 1, - get_datadir_fname2("keys", "secret_onion_key_ntor.old"), 1, - get_datadir_fname2("keys", "secret_onion_key.tmp"), 1, - get_datadir_fname2("keys", "secret_id_key.tmp"), 1, - get_datadir_fname("fingerprint"), 1, - get_datadir_fname("fingerprint.tmp"), 1, - get_datadir_fname("hashed-fingerprint"), 1, - get_datadir_fname("hashed-fingerprint.tmp"), 1, - "/etc/resolv.conf", 0, + get_datadir_fname2("keys", "secret_id_key"), + get_datadir_fname2("keys", "secret_onion_key"), + get_datadir_fname2("keys", "secret_onion_key_ntor"), + get_datadir_fname2("keys", "secret_onion_key_ntor.tmp"), + get_datadir_fname2("keys", "secret_id_key.old"), + get_datadir_fname2("keys", "secret_onion_key.old"), + get_datadir_fname2("keys", "secret_onion_key_ntor.old"), + get_datadir_fname2("keys", "secret_onion_key.tmp"), + get_datadir_fname2("keys", "secret_id_key.tmp"), + get_datadir_fname("fingerprint"), + get_datadir_fname("fingerprint.tmp"), + get_datadir_fname("hashed-fingerprint"), + get_datadir_fname("hashed-fingerprint.tmp"), + tor_strdup("/etc/resolv.conf"), NULL, 0 ); @@ -2830,8 +2830,8 @@ sandbox_init_filter(void) RENAME_SUFFIX("hashed-fingerprint", ".tmp"); sandbox_cfg_allow_stat_filename_array(&cfg, - get_datadir_fname("keys"), 1, - get_datadir_fname("stats/dirreq-stats"), 1, + get_datadir_fname("keys"), + get_datadir_fname("stats/dirreq-stats"), NULL, 0 ); }

1 0

[tor/master] Only intern one copy of each magic string for the sandbox
by nickm＠torproject.org 17 Apr '14

17 Apr '14

commit 3802e32c7d94c599546069d8246636b0d3a4ad10 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 28 02:10:19 2014 -0400 Only intern one copy of each magic string for the sandbox If we intern two copies of a string, later calls to sandbox_intern_string will give the wrong one sometimes. --- src/common/sandbox.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/common/sandbox.c b/src/common/sandbox.c index 299c6f2..363333a 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -27,6 +27,7 @@ #include <stdlib.h> #include "sandbox.h" +#include "container.h" #include "torlog.h" #include "torint.h" #include "util.h" @@ -859,8 +860,9 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) size_t pr_mem_size = 0, pr_mem_left = 0; char *pr_mem_next = NULL, *pr_mem_base; sandbox_cfg_t *el = NULL; + strmap_t *locations = NULL; - // get total number of bytes required to mmap + // get total number of bytes required to mmap. (Overestimate.) for (el = cfg; el != NULL; el = el->next) { pr_mem_size += strlen((char*) ((smp_param_t*)el->param)->value) + 1; } @@ -878,12 +880,24 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) pr_mem_next = pr_mem_base + MALLOC_MP_LIM; pr_mem_left = pr_mem_size; + locations = strmap_new(); + // change el value pointer to protected for (el = cfg; el != NULL; el = el->next) { char *param_val = (char*)((smp_param_t *)el->param)->value; size_t param_size = strlen(param_val) + 1; - if (pr_mem_left >= param_size) { + void *location = strmap_get(locations, param_val); + if (location) { + // We already interned this string. + { + void *old_val = (void *) ((smp_param_t*)el->param)->value; + tor_free(old_val); + } + ((smp_param_t*)el->param)->value = (intptr_t) location; + ((smp_param_t*)el->param)->prot = 1; + + } else if (pr_mem_left >= param_size) { // copy to protected memcpy(pr_mem_next, param_val, param_size); @@ -895,6 +909,8 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) ((smp_param_t*)el->param)->value = (intptr_t) pr_mem_next; ((smp_param_t*)el->param)->prot = 1; + strmap_set(locations, pr_mem_next, pr_mem_next); + // move next available protected memory pr_mem_next += param_size; pr_mem_left -= param_size; @@ -962,7 +978,8 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) } out: - return ret; + strmap_free(locations, NULL); + return ret; } /**

1 0

[tor/master] Add missing rename function for non-linux platforms
by nickm＠torproject.org 17 Apr '14

17 Apr '14

commit 6807b76a5e004a2d6c9f46f8b4dec0c90007996c Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 11 03:06:05 2014 -0400 Add missing rename function for non-linux platforms --- src/common/sandbox.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/common/sandbox.c b/src/common/sandbox.c index b97b900..01aefe1 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -1625,5 +1625,12 @@ sandbox_cfg_allow_stat_filename_array(sandbox_cfg_t **cfg, ...) (void)cfg; return 0; } + +int +sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2) +{ + (void)cfg; (void)file1; (void)file2; + return 0; +} #endif

1 0

[tor/master] Add 'rename' to the sandboxed syscalls
by nickm＠torproject.org 17 Apr '14

17 Apr '14

commit cbfb8e703ed9c7e31848ebf959ac7a4cf27b4a64 Author: Nick Mathewson <nickm(a)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)); }

1 0

[tor/master] Upgrade warning about missing interned string for sandbox
by nickm＠torproject.org 17 Apr '14

17 Apr '14

commit 739a52592bdb771d7ba4e40b6c9df84ea539f7fd Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Apr 16 12:48:56 2014 -0400 Upgrade warning about missing interned string for sandbox --- src/common/sandbox.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/sandbox.c b/src/common/sandbox.c index 8cbcd04..73966de 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -883,7 +883,8 @@ sandbox_intern_string(const char *str) } } - log_info(LD_GENERAL, "(Sandbox) Parameter %s not found", str); + if (sandbox_active) + log_warn(LD_BUG, "No interned sandbox parameter found for %s", str); return str; }

1 0

[tor/master] Introduce arg-counting macros to wrap seccomp_rule_add()
by nickm＠torproject.org 17 Apr '14

17 Apr '14

commit 8dc6755f6d65d7ff847bd5e8cf681e6de7fabbc5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Apr 16 12:59:33 2014 -0400 Introduce arg-counting macros to wrap seccomp_rule_add() The compiler doesn't warn about this code: rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(openat), 1, SCMP_CMP(0, SCMP_CMP_EQ, AT_FDCWD), SCMP_CMP(1, SCMP_CMP_EQ, param->value), SCMP_CMP(2, SCMP_CMP_EQ, O_RDONLY|...)); but note that the arg_cnt argument above is only 1. This means that only the first filter (argument 0 == AT_FDCWD) is actually checked! This patch also fixes the above error in the openat() filter. Earlier I fixed corresponding errors in filters for rename() and mprotect(). --- src/common/sandbox.c | 124 +++++++++++++++++++++++++++----------------------- 1 file changed, 68 insertions(+), 56 deletions(-) diff --git a/src/common/sandbox.c b/src/common/sandbox.c index d50e074..2a1ddd1 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -160,6 +160,19 @@ static int filter_nopar_gen[] = { SCMP_SYS(unlink) }; +/* These macros help avoid the error where the number of filters we add on a + * single rule don't match the arg_cnt param. */ +#define seccomp_rule_add_0(ctx,act,call) \ + seccomp_rule_add((ctx),(act),(call),0) +#define seccomp_rule_add_1(ctx,act,call,f1) \ + seccomp_rule_add((ctx),(act),(call),1,(f1)) +#define seccomp_rule_add_2(ctx,act,call,f1,f2) \ + seccomp_rule_add((ctx),(act),(call),2,(f1),(f2)) +#define seccomp_rule_add_3(ctx,act,call,f1,f2,f3) \ + seccomp_rule_add((ctx),(act),(call),3,(f1),(f2),(f3)) +#define seccomp_rule_add_4(ctx,act,call,f1,f2,f3,f4) \ + seccomp_rule_add((ctx),(act),(call),4,(f1),(f2),(f3),(f4)) + /** * Function responsible for setting up the rt_sigaction syscall for * the seccomp filter sandbox. @@ -177,7 +190,7 @@ sb_rt_sigaction(scmp_filter_ctx ctx, sandbox_cfg_t *filter) (void) filter; for (i = 0; i < ARRAY_LENGTH(param); i++) { - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(rt_sigaction), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(rt_sigaction), SCMP_CMP(0, SCMP_CMP_EQ, param[i])); if (rc) break; @@ -202,7 +215,7 @@ sb_execve(scmp_filter_ctx ctx, sandbox_cfg_t *filter) if (param != NULL && param->prot == 1 && param->syscall == SCMP_SYS(execve)) { - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(execve), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(execve), SCMP_CMP(0, SCMP_CMP_EQ, param->value)); if (rc != 0) { log_err(LD_BUG,"(Sandbox) failed to add execve syscall, received " @@ -223,7 +236,7 @@ static int sb_time(scmp_filter_ctx ctx, sandbox_cfg_t *filter) { (void) filter; - return seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(time), 1, + return seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(time), SCMP_CMP(0, SCMP_CMP_EQ, 0)); } @@ -238,19 +251,19 @@ sb_accept4(scmp_filter_ctx ctx, sandbox_cfg_t *filter) (void)filter; #ifdef __i386__ - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socketcall), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socketcall), SCMP_CMP(0, SCMP_CMP_EQ, 18)); if (rc) { return rc; } #endif - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(accept4), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(accept4), SCMP_CMP(3, SCMP_CMP_EQ, SOCK_CLOEXEC)); if (rc) { return rc; } - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(accept4), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(accept4), SCMP_CMP(3, SCMP_CMP_EQ, SOCK_CLOEXEC|SOCK_NONBLOCK)); if (rc) { return rc; @@ -270,49 +283,49 @@ sb_mmap2(scmp_filter_ctx ctx, sandbox_cfg_t *filter) int rc = 0; (void)filter; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mmap2), 2, + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mmap2), SCMP_CMP(2, SCMP_CMP_EQ, PROT_READ), SCMP_CMP(3, SCMP_CMP_EQ, MAP_PRIVATE)); if (rc) { return rc; } - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mmap2), 2, + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mmap2), SCMP_CMP(2, SCMP_CMP_EQ, PROT_NONE), SCMP_CMP(3, SCMP_CMP_EQ, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE)); if (rc) { return rc; } - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mmap2), 2, + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mmap2), SCMP_CMP(2, SCMP_CMP_EQ, PROT_READ|PROT_WRITE), SCMP_CMP(3, SCMP_CMP_EQ, MAP_PRIVATE|MAP_ANONYMOUS)); if (rc) { return rc; } - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mmap2), 2, + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mmap2), SCMP_CMP(2, SCMP_CMP_EQ, PROT_READ|PROT_WRITE), SCMP_CMP(3, SCMP_CMP_EQ,MAP_PRIVATE|MAP_ANONYMOUS|MAP_STACK)); if (rc) { return rc; } - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mmap2), 2, + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mmap2), SCMP_CMP(2, SCMP_CMP_EQ, PROT_READ|PROT_WRITE), SCMP_CMP(3, SCMP_CMP_EQ, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE)); if (rc) { return rc; } - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mmap2), 2, + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mmap2), SCMP_CMP(2, SCMP_CMP_EQ, PROT_READ|PROT_WRITE), SCMP_CMP(3, SCMP_CMP_EQ, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS)); if (rc) { return rc; } - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mmap2), 2, + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mmap2), SCMP_CMP(2, SCMP_CMP_EQ, PROT_READ|PROT_EXEC), SCMP_CMP(3, SCMP_CMP_EQ, MAP_PRIVATE|MAP_DENYWRITE)); if (rc) { @@ -339,7 +352,7 @@ sb_open(scmp_filter_ctx ctx, sandbox_cfg_t *filter) if (param != NULL && param->prot == 1 && param->syscall == SCMP_SYS(open)) { - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(open), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(open), SCMP_CMP(0, SCMP_CMP_EQ, param->value)); if (rc != 0) { log_err(LD_BUG,"(Sandbox) failed to add open syscall, received " @@ -349,7 +362,7 @@ sb_open(scmp_filter_ctx ctx, sandbox_cfg_t *filter) } } - rc = seccomp_rule_add(ctx, SCMP_ACT_ERRNO(-1), SCMP_SYS(open), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ERRNO(-1), SCMP_SYS(open), SCMP_CMP(1, SCMP_CMP_EQ, O_RDONLY|O_CLOEXEC)); if (rc != 0) { log_err(LD_BUG,"(Sandbox) failed to add open syscall, received libseccomp " @@ -377,8 +390,7 @@ sb_rename(scmp_filter_ctx ctx, sandbox_cfg_t *filter) if (param != NULL && param->prot == 1 && param->syscall == SCMP_SYS(rename)) { - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, - SCMP_SYS(rename), 2, + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(rename), SCMP_CMP(0, SCMP_CMP_EQ, param->value), SCMP_CMP(1, SCMP_CMP_EQ, param->value2)); if (rc != 0) { @@ -408,7 +420,7 @@ sb_openat(scmp_filter_ctx ctx, sandbox_cfg_t *filter) if (param != NULL && param->prot == 1 && param->syscall == SCMP_SYS(openat)) { - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(openat), 1, + rc = seccomp_rule_add_3(ctx, SCMP_ACT_ALLOW, SCMP_SYS(openat), SCMP_CMP(0, SCMP_CMP_EQ, AT_FDCWD), SCMP_CMP(1, SCMP_CMP_EQ, param->value), SCMP_CMP(2, SCMP_CMP_EQ, O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY| @@ -435,40 +447,40 @@ sb_socket(scmp_filter_ctx ctx, sandbox_cfg_t *filter) (void) filter; #ifdef __i386__ - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socket), 0); + rc = seccomp_rule_add_0(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socket)); if (rc) return rc; #endif - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socket), 3, + rc = seccomp_rule_add_3(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socket), SCMP_CMP(0, SCMP_CMP_EQ, PF_FILE), SCMP_CMP(1, SCMP_CMP_EQ, SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK), SCMP_CMP(2, SCMP_CMP_EQ, IPPROTO_IP)); if (rc) return rc; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socket), 3, + rc = seccomp_rule_add_3(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socket), SCMP_CMP(0, SCMP_CMP_EQ, PF_INET), SCMP_CMP(1, SCMP_CMP_EQ, SOCK_STREAM|SOCK_CLOEXEC), SCMP_CMP(2, SCMP_CMP_EQ, IPPROTO_TCP)); if (rc) return rc; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socket), 3, + rc = seccomp_rule_add_3(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socket), SCMP_CMP(0, SCMP_CMP_EQ, PF_INET), SCMP_CMP(1, SCMP_CMP_EQ, SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK), SCMP_CMP(2, SCMP_CMP_EQ, IPPROTO_TCP)); if (rc) return rc; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socket), 3, + rc = seccomp_rule_add_3(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socket), SCMP_CMP(0, SCMP_CMP_EQ, PF_INET), SCMP_CMP(1, SCMP_CMP_EQ, SOCK_DGRAM|SOCK_CLOEXEC|SOCK_NONBLOCK), SCMP_CMP(2, SCMP_CMP_EQ, IPPROTO_IP)); if (rc) return rc; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socket), 3, + rc = seccomp_rule_add_3(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socket), SCMP_CMP(0, SCMP_CMP_EQ, PF_NETLINK), SCMP_CMP(1, SCMP_CMP_EQ, SOCK_RAW), SCMP_CMP(2, SCMP_CMP_EQ, 0)); @@ -489,12 +501,12 @@ sb_socketpair(scmp_filter_ctx ctx, sandbox_cfg_t *filter) (void) filter; #ifdef __i386__ - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socketpair), 0); + rc = seccomp_rule_add_0(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socketpair)); if (rc) return rc; #endif - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socketpair), 2, + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(socketpair), SCMP_CMP(0, SCMP_CMP_EQ, PF_FILE), SCMP_CMP(1, SCMP_CMP_EQ, SOCK_STREAM|SOCK_CLOEXEC)); if (rc) @@ -514,19 +526,19 @@ sb_setsockopt(scmp_filter_ctx ctx, sandbox_cfg_t *filter) (void) filter; #ifdef __i386__ - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(setsockopt), 0); + rc = seccomp_rule_add_0(ctx, SCMP_ACT_ALLOW, SCMP_SYS(setsockopt)); if (rc) return rc; #endif - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(setsockopt), 2, + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(setsockopt), SCMP_CMP(1, SCMP_CMP_EQ, SOL_SOCKET), SCMP_CMP(2, SCMP_CMP_EQ, SO_REUSEADDR)); if (rc) return rc; #ifdef IP_TRANSPARENT - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(setsockopt), 2, + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(setsockopt), SCMP_CMP(1, SCMP_CMP_EQ, SOL_IP), SCMP_CMP(2, SCMP_CMP_EQ, IP_TRANSPARENT)); if (rc) @@ -547,12 +559,12 @@ sb_getsockopt(scmp_filter_ctx ctx, sandbox_cfg_t *filter) (void) filter; #ifdef __i386__ - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(getsockopt), 0); + rc = seccomp_rule_add_0(ctx, SCMP_ACT_ALLOW, SCMP_SYS(getsockopt)); if (rc) return rc; #endif - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(getsockopt), 2, + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(getsockopt), SCMP_CMP(1, SCMP_CMP_EQ, SOL_SOCKET), SCMP_CMP(2, SCMP_CMP_EQ, SO_ERROR)); if (rc) @@ -572,23 +584,23 @@ sb_fcntl64(scmp_filter_ctx ctx, sandbox_cfg_t *filter) int rc = 0; (void) filter; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(fcntl64), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(fcntl64), SCMP_CMP(1, SCMP_CMP_EQ, F_GETFL)); if (rc) return rc; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(fcntl64), 2, + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(fcntl64), SCMP_CMP(1, SCMP_CMP_EQ, F_SETFL), SCMP_CMP(2, SCMP_CMP_EQ, O_RDWR|O_NONBLOCK)); if (rc) return rc; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(fcntl64), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(fcntl64), SCMP_CMP(1, SCMP_CMP_EQ, F_GETFD)); if (rc) return rc; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(fcntl64), 2, + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(fcntl64), SCMP_CMP(1, SCMP_CMP_EQ, F_SETFD), SCMP_CMP(2, SCMP_CMP_EQ, FD_CLOEXEC)); if (rc) @@ -610,17 +622,17 @@ sb_epoll_ctl(scmp_filter_ctx ctx, sandbox_cfg_t *filter) int rc = 0; (void) filter; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(epoll_ctl), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(epoll_ctl), SCMP_CMP(1, SCMP_CMP_EQ, EPOLL_CTL_ADD)); if (rc) return rc; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(epoll_ctl), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(epoll_ctl), SCMP_CMP(1, SCMP_CMP_EQ, EPOLL_CTL_MOD)); if (rc) return rc; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(epoll_ctl), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(epoll_ctl), SCMP_CMP(1, SCMP_CMP_EQ, EPOLL_CTL_DEL)); if (rc) return rc; @@ -641,7 +653,7 @@ sb_prctl(scmp_filter_ctx ctx, sandbox_cfg_t *filter) int rc = 0; (void) filter; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(prctl), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(prctl), SCMP_CMP(0, SCMP_CMP_EQ, PR_SET_DUMPABLE)); if (rc) return rc; @@ -662,12 +674,12 @@ sb_mprotect(scmp_filter_ctx ctx, sandbox_cfg_t *filter) int rc = 0; (void) filter; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mprotect), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mprotect), SCMP_CMP(2, SCMP_CMP_EQ, PROT_READ)); if (rc) return rc; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mprotect), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mprotect), SCMP_CMP(2, SCMP_CMP_EQ, PROT_NONE)); if (rc) return rc; @@ -685,12 +697,12 @@ sb_rt_sigprocmask(scmp_filter_ctx ctx, sandbox_cfg_t *filter) int rc = 0; (void) filter; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(rt_sigprocmask), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(rt_sigprocmask), SCMP_CMP(0, SCMP_CMP_EQ, SIG_UNBLOCK)); if (rc) return rc; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(rt_sigprocmask), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(rt_sigprocmask), SCMP_CMP(0, SCMP_CMP_EQ, SIG_SETMASK)); if (rc) return rc; @@ -710,12 +722,12 @@ sb_flock(scmp_filter_ctx ctx, sandbox_cfg_t *filter) int rc = 0; (void) filter; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(flock), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(flock), SCMP_CMP(1, SCMP_CMP_EQ, LOCK_EX|LOCK_NB)); if (rc) return rc; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(flock), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(flock), SCMP_CMP(1, SCMP_CMP_EQ, LOCK_UN)); if (rc) return rc; @@ -734,18 +746,18 @@ sb_futex(scmp_filter_ctx ctx, sandbox_cfg_t *filter) (void) filter; // can remove - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(futex), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(futex), SCMP_CMP(1, SCMP_CMP_EQ, FUTEX_WAIT_BITSET_PRIVATE|FUTEX_CLOCK_REALTIME)); if (rc) return rc; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(futex), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(futex), SCMP_CMP(1, SCMP_CMP_EQ, FUTEX_WAKE_PRIVATE)); if (rc) return rc; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(futex), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(futex), SCMP_CMP(1, SCMP_CMP_EQ, FUTEX_WAIT_PRIVATE)); if (rc) return rc; @@ -765,7 +777,7 @@ sb_mremap(scmp_filter_ctx ctx, sandbox_cfg_t *filter) int rc = 0; (void) filter; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mremap), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mremap), SCMP_CMP(3, SCMP_CMP_EQ, MREMAP_MAYMOVE)); if (rc) return rc; @@ -783,7 +795,7 @@ sb_poll(scmp_filter_ctx ctx, sandbox_cfg_t *filter) int rc = 0; (void) filter; - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(poll), 2, + rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(poll), SCMP_CMP(1, SCMP_CMP_EQ, 1), SCMP_CMP(2, SCMP_CMP_EQ, 10)); if (rc) @@ -809,7 +821,7 @@ sb_stat64(scmp_filter_ctx ctx, sandbox_cfg_t *filter) if (param != NULL && param->prot == 1 && (param->syscall == SCMP_SYS(open) || param->syscall == SCMP_SYS(stat64))) { - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(stat64), 1, + rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(stat64), SCMP_CMP(0, SCMP_CMP_EQ, param->value)); if (rc != 0) { log_err(LD_BUG,"(Sandbox) failed to add open syscall, received " @@ -993,7 +1005,7 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) * Setting sandbox restrictions so the string memory cannot be tampered with */ // no mremap of the protected base address - ret = seccomp_rule_add(ctx, SCMP_ACT_KILL, SCMP_SYS(mremap), 1, + ret = seccomp_rule_add_1(ctx, SCMP_ACT_KILL, SCMP_SYS(mremap), SCMP_CMP(0, SCMP_CMP_EQ, (intptr_t) pr_mem_base)); if (ret) { log_err(LD_BUG,"(Sandbox) mremap protected memory filter fail!"); @@ -1001,7 +1013,7 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) } // no munmap of the protected base address - ret = seccomp_rule_add(ctx, SCMP_ACT_KILL, SCMP_SYS(munmap), 1, + ret = seccomp_rule_add_1(ctx, SCMP_ACT_KILL, SCMP_SYS(munmap), SCMP_CMP(0, SCMP_CMP_EQ, (intptr_t) pr_mem_base)); if (ret) { log_err(LD_BUG,"(Sandbox) munmap protected memory filter fail!"); @@ -1018,7 +1030,7 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) * There is a restriction on how much you can mprotect with R|W up to the * size of the canary. */ - ret = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mprotect), 3, + ret = seccomp_rule_add_3(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mprotect), SCMP_CMP(0, SCMP_CMP_LT, (intptr_t) pr_mem_base), SCMP_CMP(1, SCMP_CMP_LE, MALLOC_MP_LIM), SCMP_CMP(2, SCMP_CMP_EQ, PROT_READ|PROT_WRITE)); @@ -1027,7 +1039,7 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) return ret; } - ret = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mprotect), 3, + ret = seccomp_rule_add_3(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mprotect), SCMP_CMP(0, SCMP_CMP_GT, (intptr_t) pr_mem_base + pr_mem_size + MALLOC_MP_LIM), SCMP_CMP(1, SCMP_CMP_LE, MALLOC_MP_LIM), @@ -1356,7 +1368,7 @@ add_noparam_filter(scmp_filter_ctx ctx) // add general filters for (i = 0; i < ARRAY_LENGTH(filter_nopar_gen); i++) { - rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, filter_nopar_gen[i], 0); + rc = seccomp_rule_add_0(ctx, SCMP_ACT_ALLOW, filter_nopar_gen[i]); if (rc != 0) { log_err(LD_BUG,"(Sandbox) failed to add syscall index %d (NR=%d), " "received libseccomp error %d", i, filter_nopar_gen[i], rc);

1 0

[tor/master] Clean up sandbox structures a bit
by nickm＠torproject.org 17 Apr '14

17 Apr '14

commit f268101a612bd637e270914365271d08fc5813db Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Apr 16 12:24:08 2014 -0400 Clean up sandbox structures a bit Drop pindex,pindex2 as unused. Admit a type to avoid using a void* --- src/common/sandbox.c | 42 ++++++++++++++++++++---------------------- src/common/sandbox.h | 6 +----- 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/src/common/sandbox.c b/src/common/sandbox.c index 01aefe1..85a2f23 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -198,7 +198,7 @@ sb_execve(scmp_filter_ctx ctx, sandbox_cfg_t *filter) // for each dynamic parameter filters for (elem = filter; elem != NULL; elem = elem->next) { - smp_param_t *param = (smp_param_t*) elem->param; + smp_param_t *param = elem->param; if (param != NULL && param->prot == 1 && param->syscall == SCMP_SYS(execve)) { @@ -899,7 +899,8 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) // get total number of bytes required to mmap. (Overestimate.) for (el = cfg; el != NULL; el = el->next) { - pr_mem_size += strlen((char*) ((smp_param_t*)el->param)->value) + 1; + pr_mem_size += strlen((char*) el->param->value) + 1; + } // allocate protected memory with MALLOC_MP_LIM canary @@ -919,7 +920,7 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) // change el value pointer to protected for (el = cfg; el != NULL; el = el->next) { - char *param_val = (char*)((smp_param_t *)el->param)->value; + char *param_val = (char*)(el->param)->value; size_t param_size = strlen(param_val) + 1; void *location = strmap_get(locations, param_val); @@ -927,11 +928,11 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) if (location) { // We already interned this string. { - void *old_val = (void *) ((smp_param_t*)el->param)->value; + void *old_val = (void *) el->param->value; tor_free(old_val); } - ((smp_param_t*)el->param)->value = (intptr_t) location; - ((smp_param_t*)el->param)->prot = 1; + el->param->value = (intptr_t) location; + el->param->prot = 1; } else if (pr_mem_left >= param_size) { // copy to protected @@ -939,11 +940,11 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) // re-point el parameter to protected { - void *old_val = (void *) ((smp_param_t*)el->param)->value; + void *old_val = (void *) el->param->value; tor_free(old_val); } - ((smp_param_t*)el->param)->value = (intptr_t) pr_mem_next; - ((smp_param_t*)el->param)->prot = 1; + el->param->value = (intptr_t) pr_mem_next; + el->param->prot = 1; strmap_set(locations, pr_mem_next, pr_mem_next); @@ -1025,17 +1026,14 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) * point. */ static sandbox_cfg_t* -new_element2(int syscall, int index, int index2, intptr_t value, intptr_t value2) +new_element2(int syscall, intptr_t value, intptr_t value2) { smp_param_t *param = NULL; sandbox_cfg_t *elem = tor_malloc_zero(sizeof(sandbox_cfg_t)); - elem->param = tor_malloc_zero(sizeof(smp_param_t)); + param = 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; @@ -1044,9 +1042,9 @@ new_element2(int syscall, int index, int index2, intptr_t value, intptr_t value2 } static sandbox_cfg_t* -new_element(int syscall, int index, intptr_t value) +new_element(int syscall, intptr_t value) { - return new_element2(syscall, index, -1, value, 0); + return new_element2(syscall, value, 0); } #ifdef __NR_stat64 @@ -1060,7 +1058,7 @@ sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file) { sandbox_cfg_t *elem = NULL; - elem = new_element(SCMP_stat, 0, (intptr_t)(void*) file); + elem = new_element(SCMP_stat, (intptr_t)(void*) file); if (!elem) { log_err(LD_BUG,"(Sandbox) failed to register parameter!"); return -1; @@ -1099,7 +1097,7 @@ sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file) { sandbox_cfg_t *elem = NULL; - elem = new_element(SCMP_SYS(open), 0, (intptr_t)(void *) file); + elem = new_element(SCMP_SYS(open), (intptr_t)(void *) file); if (!elem) { log_err(LD_BUG,"(Sandbox) failed to register parameter!"); return -1; @@ -1116,7 +1114,7 @@ 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, + elem = new_element2(SCMP_SYS(rename), (intptr_t)(void *) file1, (intptr_t)(void *) file2); @@ -1129,7 +1127,7 @@ sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2) *cfg = elem; /* For interning */ - elem = new_element(-1, 0, (intptr_t)(void*)tor_strdup(file2)); + elem = new_element(-1, (intptr_t)(void*)tor_strdup(file2)); if (!elem) { log_err(LD_BUG,"(Sandbox) failed to register parameter!"); return -1; @@ -1167,7 +1165,7 @@ sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file) { sandbox_cfg_t *elem = NULL; - elem = new_element(SCMP_SYS(openat), 1, (intptr_t)(void *) file); + elem = new_element(SCMP_SYS(openat), (intptr_t)(void *) file); if (!elem) { log_err(LD_BUG,"(Sandbox) failed to register parameter!"); return -1; @@ -1206,7 +1204,7 @@ sandbox_cfg_allow_execve(sandbox_cfg_t **cfg, const char *com) { sandbox_cfg_t *elem = NULL; - elem = new_element(SCMP_SYS(execve), 1, (intptr_t)(void *) com); + elem = new_element(SCMP_SYS(execve), (intptr_t)(void *) com); if (!elem) { log_err(LD_BUG,"(Sandbox) failed to register parameter!"); return -1; diff --git a/src/common/sandbox.h b/src/common/sandbox.h index 8fcd221..c4144db 100644 --- a/src/common/sandbox.h +++ b/src/common/sandbox.h @@ -65,10 +65,6 @@ typedef struct smp_param { /** syscall associated with parameter. */ int syscall; - /** parameter index. */ - int pindex; - /** parameter index, second one. */ - int pindex2; /** parameter value. */ intptr_t value; /** parameter value, second argument. */ @@ -89,7 +85,7 @@ struct sandbox_cfg_elem { SB_IMPL implem; /** Configuration parameter. */ - void *param; + smp_param_t *param; /** Next element of the configuration*/ struct sandbox_cfg_elem *next;

1 0

[tor/master] Fix sandbox protection for rename
by nickm＠torproject.org 17 Apr '14

17 Apr '14

commit 12028c29e6ee8d0d9c02b32f1a52a35138e148e3 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Apr 16 12:50:24 2014 -0400 Fix sandbox protection for rename (We were only checking the first parameter of each rename call.) --- src/common/sandbox.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/common/sandbox.c b/src/common/sandbox.c index 73966de..d50e074 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -377,13 +377,10 @@ sb_rename(scmp_filter_ctx ctx, sandbox_cfg_t *filter) 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_SYS(rename), 2, SCMP_CMP(0, SCMP_CMP_EQ, param->value), - SCMP_CMP(1, SCMP_CMP_EQ, value2)); + SCMP_CMP(1, SCMP_CMP_EQ, param->value2)); if (rc != 0) { log_err(LD_BUG,"(Sandbox) failed to add rename syscall, received " "libseccomp error %d", rc); @@ -1152,15 +1149,6 @@ sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2) elem->next = *cfg; *cfg = elem; - /* For interning */ - elem = new_element(-1, (intptr_t)(void*)tor_strdup(file2)); - if (!elem) { - log_err(LD_BUG,"(Sandbox) failed to register parameter!"); - return -1; - } - elem->next = *cfg; - *cfg = elem; - return 0; }

1 0

[tor/master] Have sandbox string protection include multi-valued parmeters.
by nickm＠torproject.org 17 Apr '14

17 Apr '14

commit 5aaac938a94fc0b5d7dc63cac92e78d3125f08c8 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Apr 16 12:45:34 2014 -0400 Have sandbox string protection include multi-valued parmeters. --- src/common/sandbox.c | 103 +++++++++++++++++++++++++++++++------------------- 1 file changed, 64 insertions(+), 39 deletions(-) diff --git a/src/common/sandbox.c b/src/common/sandbox.c index 85a2f23..8cbcd04 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -862,7 +862,7 @@ static sandbox_filter_func_t filter_func[] = { sb_socketpair }; -const char* +const char * sandbox_intern_string(const char *str) { sandbox_cfg_t *elem; @@ -873,8 +873,13 @@ sandbox_intern_string(const char *str) for (elem = filter_dynamic; elem != NULL; elem = elem->next) { smp_param_t *param = elem->param; - if (param->prot && !strcmp(str, (char*)(param->value))) { - return (char*)(param->value); + if (param->prot) { + if (!strcmp(str, (char*)(param->value))) { + return (char*)param->value; + } + if (param->value2 && !strcmp(str, (char*)param->value2)) { + return (char*)param->value2; + } } } @@ -882,6 +887,50 @@ sandbox_intern_string(const char *str) return str; } +/** DOCDOC */ +static int +prot_strings_helper(strmap_t *locations, + char **pr_mem_next_p, + size_t *pr_mem_left_p, + intptr_t *value_p) +{ + char *param_val; + size_t param_size; + void *location; + + if (*value_p == 0) + return 0; + + param_val = (char*) *value_p; + param_size = strlen(param_val) + 1; + location = strmap_get(locations, param_val); + + if (location) { + // We already interned this string. + tor_free(param_val); + *value_p = (intptr_t) location; + return 0; + } else if (*pr_mem_left_p >= param_size) { + // copy to protected + location = *pr_mem_next_p; + memcpy(location, param_val, param_size); + + // re-point el parameter to protected + tor_free(param_val); + *value_p = (intptr_t) location; + + strmap_set(locations, location, location); /* good real estate advice */ + + // move next available protected memory + *pr_mem_next_p += param_size; + *pr_mem_left_p -= param_size; + return 0; + } else { + log_err(LD_BUG,"(Sandbox) insufficient protected memory!"); + return -1; + } +} + /** * Protects all the strings in the sandbox's parameter list configuration. It * works by calculating the total amount of memory required by the parameter @@ -900,7 +949,8 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) // get total number of bytes required to mmap. (Overestimate.) for (el = cfg; el != NULL; el = el->next) { pr_mem_size += strlen((char*) el->param->value) + 1; - + if (el->param->value2) + pr_mem_size += strlen((char*) el->param->value2) + 1; } // allocate protected memory with MALLOC_MP_LIM canary @@ -920,42 +970,17 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) // change el value pointer to protected for (el = cfg; el != NULL; el = el->next) { - char *param_val = (char*)(el->param)->value; - size_t param_size = strlen(param_val) + 1; - - void *location = strmap_get(locations, param_val); - - if (location) { - // We already interned this string. - { - void *old_val = (void *) el->param->value; - tor_free(old_val); - } - el->param->value = (intptr_t) location; - el->param->prot = 1; - - } else if (pr_mem_left >= param_size) { - // copy to protected - memcpy(pr_mem_next, param_val, param_size); - - // re-point el parameter to protected - { - void *old_val = (void *) el->param->value; - tor_free(old_val); - } - el->param->value = (intptr_t) pr_mem_next; - el->param->prot = 1; - - strmap_set(locations, pr_mem_next, pr_mem_next); - - // move next available protected memory - pr_mem_next += param_size; - pr_mem_left -= param_size; - } else { - log_err(LD_BUG,"(Sandbox) insufficient protected memory!"); + if (prot_strings_helper(locations, &pr_mem_next, &pr_mem_left, + &el->param->value) < 0) { + ret = -2; + goto out; + } + if (prot_strings_helper(locations, &pr_mem_next, &pr_mem_left, + &el->param->value2) < 0) { ret = -2; goto out; } + el->param->prot = 1; } // protecting from writes @@ -995,7 +1020,7 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) * There is a restriction on how much you can mprotect with R|W up to the * size of the canary. */ - ret = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mprotect), 2, + ret = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mprotect), 3, SCMP_CMP(0, SCMP_CMP_LT, (intptr_t) pr_mem_base), SCMP_CMP(1, SCMP_CMP_LE, MALLOC_MP_LIM), SCMP_CMP(2, SCMP_CMP_EQ, PROT_READ|PROT_WRITE)); @@ -1004,7 +1029,7 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg) return ret; } - ret = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mprotect), 2, + ret = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mprotect), 3, SCMP_CMP(0, SCMP_CMP_GT, (intptr_t) pr_mem_base + pr_mem_size + MALLOC_MP_LIM), SCMP_CMP(1, SCMP_CMP_LE, MALLOC_MP_LIM),

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

tor-commits April 2014