commit e425fc78045f99725d256956acc7360ed71bfaa5 Author: Nick Mathewson nickm@torproject.org Date: Thu May 22 17:39:36 2014 -0400
sandbox: revamp sandbox_getaddrinfo cacheing
The old cache had problems: * It needed to be manually preloaded. (It didn't remember any address you didn't tell it to remember) * It was AF_INET only. * It looked at its cache even if the sandbox wasn't turned on. * It couldn't remember errors. * It had some memory management problems. (You can't use memcpy to copy an addrinfo safely; it has pointers in.)
This patch fixes those issues, and moves to a hash table.
Fixes bug 11970; bugfix on 0.2.5.1-alpha. --- changes/bug11970 | 7 +++ src/common/address.c | 2 +- src/common/sandbox.c | 158 +++++++++++++++++++++++++++++++++++++------------- src/common/sandbox.h | 20 ++----- src/or/main.c | 1 + 5 files changed, 133 insertions(+), 55 deletions(-)
diff --git a/changes/bug11970 b/changes/bug11970 new file mode 100644 index 0000000..896f0cf --- /dev/null +++ b/changes/bug11970 @@ -0,0 +1,7 @@ + o Minor bugfixes (linux seccomp sandbox): + - Refactor the getaddrinfo workaround that the seccomp sandbox + uses to avoid calling getaddrinfo() after installing the sandbox + filters. Previously, it preloaded a cache with the IPv4 address + for our hostname, and nothing else. Now, it loads the cache with + every address that it used to initialize the Tor process. Fixes + bug 11970; bugfix on 0.2.5.1-alpha. diff --git a/src/common/address.c b/src/common/address.c index 2825b12..29d4c04 100644 --- a/src/common/address.c +++ b/src/common/address.c @@ -264,7 +264,7 @@ tor_addr_lookup(const char *name, uint16_t family, tor_addr_t *addr) &((struct sockaddr_in6*)best->ai_addr)->sin6_addr); result = 0; } - freeaddrinfo(res); + sandbox_freeaddrinfo(res); return result; } return (err == EAI_AGAIN) ? 1 : -1; diff --git a/src/common/sandbox.c b/src/common/sandbox.c index bb2b3ed..eba766b 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -33,6 +33,8 @@ #include "util.h" #include "tor_queue.h"
+#include "ht.h" + #define DEBUGGING_CLOSE
#if defined(USE_LIBSECCOMP) @@ -71,8 +73,6 @@ static int sandbox_active = 0; /** Holds the parameter list configuration for the sandbox.*/ static sandbox_cfg_t *filter_dynamic = NULL; -/** Holds a list of pre-recorded results from getaddrinfo().*/ -static sb_addr_info_t *sb_addr_info = NULL;
#undef SCMP_CMP #define SCMP_CMP(a,b,c) ((struct scmp_arg_cmp){(a),(b),(c),0}) @@ -1288,73 +1288,153 @@ sandbox_cfg_allow_execve_array(sandbox_cfg_t **cfg, ...) } #endif
+/** Cache entry for getaddrinfo results; used when sandboxing is implemented + * so that we can consult the cache when the sandbox prevents us from doing + * getaddrinfo. + * + * We support only a limited range of getaddrinfo calls, where servname is null + * and hints contains only socktype=SOCK_STREAM, family in INET,INET6,UNSPEC. + */ +typedef struct cached_getaddrinfo_item_t { + HT_ENTRY(cached_getaddrinfo_item_t) node; + char *name; + int family; + /** set if no error; otherwise NULL */ + struct addrinfo *res; + /** 0 for no error; otherwise an EAI_* value */ + int err; +} cached_getaddrinfo_item_t; + +static unsigned +cached_getaddrinfo_item_hash(const cached_getaddrinfo_item_t *item) +{ + return siphash24g(item->name, strlen(item->name)) + item->family; +} + +static unsigned +cached_getaddrinfo_items_eq(const cached_getaddrinfo_item_t *a, + const cached_getaddrinfo_item_t *b) +{ + return (a->family == b->family) && 0 == strcmp(a->name, b->name); +} + +static void +cached_getaddrinfo_item_free(cached_getaddrinfo_item_t *item) +{ + if (item == NULL) + return; + + tor_free(item->name); + if (item->res) + freeaddrinfo(item->res); + tor_free(item); +} + +static HT_HEAD(getaddrinfo_cache, cached_getaddrinfo_item_t) + getaddrinfo_cache = HT_INITIALIZER(); + +HT_PROTOTYPE(getaddrinfo_cache, cached_getaddrinfo_item_t, node, + cached_getaddrinfo_item_hash, + cached_getaddrinfo_items_eq); +HT_GENERATE(getaddrinfo_cache, cached_getaddrinfo_item_t, node, + cached_getaddrinfo_item_hash, + cached_getaddrinfo_items_eq, + 0.6, tor_malloc_, tor_realloc_, tor_free_); + int sandbox_getaddrinfo(const char *name, const char *servname, const struct addrinfo *hints, struct addrinfo **res) { - sb_addr_info_t *el; + int err; + struct cached_getaddrinfo_item_t search, *item;
- if (servname != NULL) - return -1; + if (servname != NULL) { + log_warn(LD_BUG, "called with non-NULL servname"); + return EAI_NONAME; + } + if (name == NULL) { + log_warn(LD_BUG, "called with NULL name"); + return EAI_NONAME; + }
*res = NULL;
- for (el = sb_addr_info; el; el = el->next) { - if (!strcmp(el->name, name)) { - *res = tor_malloc(sizeof(struct addrinfo)); + memset(&search, 0, sizeof(search)); + search.name = (char *) name; + search.family = hints ? hints->ai_family : AF_UNSPEC; + item = HT_FIND(getaddrinfo_cache, &getaddrinfo_cache, &search);
- memcpy(*res, el->info, sizeof(struct addrinfo)); - /* XXXX What if there are multiple items in the list? */ - return 0; + if (! sandbox_is_active()) { + /* If the sandbox is not turned on yet, then getaddrinfo and store the + result. */ + + err = getaddrinfo(name, NULL, hints, res); + log_info(LD_NET,"(Sandbox) getaddrinfo %s.", err ? "failed" : "succeeded"); + + if (! item) { + item = tor_malloc_zero(sizeof(*item)); + item->name = tor_strdup(name); + item->family = hints ? hints->ai_family : AF_UNSPEC; + HT_INSERT(getaddrinfo_cache, &getaddrinfo_cache, item); } - }
- if (!sandbox_active) { - if (getaddrinfo(name, NULL, hints, res)) { - log_err(LD_BUG,"(Sandbox) getaddrinfo failed!"); - return -1; + if (item->res) { + freeaddrinfo(item->res); + item->res = NULL; } + item->res = *res; + item->err = err; + return err; + }
- return 0; + /* Otherwise, the sanbox is on. If we have an item, yield its cached + result. */ + if (item) { + *res = item->res; + return item->err; }
- // getting here means something went wrong + /* getting here means something went wrong */ log_err(LD_BUG,"(Sandbox) failed to get address %s!", name); - if (*res) { - tor_free(*res); - res = NULL; - } return -1; }
int -sandbox_add_addrinfo(const char* name) +sandbox_add_addrinfo(const char *name) { - int ret; + struct addrinfo *res; struct addrinfo hints; - sb_addr_info_t *el = NULL; - - el = tor_malloc(sizeof(sb_addr_info_t)); + int i; + static const int families[] = { AF_INET, AF_INET6, AF_UNSPEC };
memset(&hints, 0, sizeof(hints)); - hints.ai_family = AF_INET; hints.ai_socktype = SOCK_STREAM; + for (i = 0; i < 3; ++i) { + hints.ai_family = families[i];
- ret = getaddrinfo(name, NULL, &hints, &(el->info)); - if (ret) { - log_err(LD_BUG,"(Sandbox) failed to getaddrinfo"); - ret = -2; - tor_free(el); - goto out; + res = NULL; + (void) sandbox_getaddrinfo(name, NULL, &hints, &res); + if (res) + sandbox_freeaddrinfo(res); }
- el->name = tor_strdup(name); - el->next = sb_addr_info; - sb_addr_info = el; + return 0; +}
- out: - return ret; +void +sandbox_free_getaddrinfo_cache(void) +{ + cached_getaddrinfo_item_t **next, **item; + + for (item = HT_START(getaddrinfo_cache, &getaddrinfo_cache); + item; + item = next) { + next = HT_NEXT_RMV(getaddrinfo_cache, &getaddrinfo_cache, item); + cached_getaddrinfo_item_free(*item); + } + + HT_CLEAR(getaddrinfo_cache, &getaddrinfo_cache); }
/** diff --git a/src/common/sandbox.h b/src/common/sandbox.h index b572152..7763570 100644 --- a/src/common/sandbox.h +++ b/src/common/sandbox.h @@ -91,21 +91,6 @@ struct sandbox_cfg_elem { struct sandbox_cfg_elem *next; };
-/** - * Structure used for keeping a linked list of getaddrinfo pre-recorded - * results. - */ -struct sb_addr_info_el { - /** Name of the address info result. */ - char *name; - /** Pre-recorded getaddrinfo result. */ - struct addrinfo *info; - /** Next element in the list. */ - struct sb_addr_info_el *next; -}; -/** Typedef to structure used to manage an addrinfo list. */ -typedef struct sb_addr_info_el sb_addr_info_t; - /** Function pointer defining the prototype of a filter function.*/ typedef int (*sandbox_filter_func_t)(scmp_filter_ctx ctx, sandbox_cfg_t *filter); @@ -146,11 +131,16 @@ struct addrinfo; int sandbox_getaddrinfo(const char *name, const char *servname, const struct addrinfo *hints, struct addrinfo **res); +#define sandbox_freeaddrinfo(addrinfo) ((void)0) +void sandbox_free_getaddrinfo_cache(void); #else #define sandbox_getaddrinfo(name, servname, hints, res) \ getaddrinfo((name),(servname), (hints),(res)) #define sandbox_add_addrinfo(name) \ ((void)(name)) +#define sandbox_freeaddrinfo(addrinfo) \ + freeaddrinfo((addrinfo)) +#define sandbox_free_getaddrinfo_cache() #endif
#ifdef USE_LIBSECCOMP diff --git a/src/or/main.c b/src/or/main.c index 1168f43..f665f87 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -2543,6 +2543,7 @@ tor_free_all(int postfork) microdesc_free_all(); ext_orport_free_all(); control_free_all(); + sandbox_free_getaddrinfo_cache(); if (!postfork) { config_free_all(); or_state_free_all();