Merge remote-tracking branch 'public/bug11970'

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.

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;

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)
@@ -93,8 +95,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})
@@ -1332,73 +1332,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;
+  return EAI_NONAME;
 }
 
 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);
 }
 
 /**

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);
@@ -130,11 +115,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

src/or/main.c

@@ -2545,6 +2545,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();