Merge branch 'bug20599_030_v4'

changes/bug20559

@@ -0,0 +1,4 @@
+  o Minor bugfixes (hidden services):
+    - Stop ignoring misconfigured hidden services. Instead, refuse to start
+      tor until the misconfigurations have been corrected.
+      Fixes bug 20559; bugfix on multiple commits in 0.2.7.1-alpha and earlier.

src/common/util.c

@@ -2270,10 +2270,14 @@ check_private_dir,(const char *dirname, cpd_check_t check,
        * permissions on the directory will be checked again below.*/
       fd = open(sandbox_intern_string(dirname), O_NOFOLLOW);
 
-      if (fd == -1)
+      if (fd == -1) {
+        log_warn(LD_FS, "Could not reopen recently created directory %s: %s",
+                 dirname,
+                 strerror(errno));
         return -1;
-      else
+      } else {
         close(fd);
+      }
 
     } else if (!(check & CPD_CHECK)) {
       log_warn(LD_FS, "Directory %s does not exist.", dirname);

src/or/rendservice.c

@@ -79,6 +79,10 @@ static int rend_service_check_private_dir(const or_options_t *options,
 static int rend_service_check_private_dir_impl(const or_options_t *options,
                                                const rend_service_t *s,
                                                int create);
+static const smartlist_t* rend_get_service_list(
+                                  const smartlist_t* substitute_service_list);
+static smartlist_t* rend_get_service_list_mutable(
+                                  smartlist_t* substitute_service_list);
 
 /** Represents the mapping from a virtual port of a rendezvous service to
  * a real port on some IP.
@@ -124,8 +128,44 @@ static const char *hostname_fname = "hostname";
 static const char *client_keys_fname = "client_keys";
 static const char *sos_poison_fname = "onion_service_non_anonymous";
 
+/** A list of rend_service_t's for services run on this OP.
+ */
+static smartlist_t *rend_service_list = NULL;
+
+/* Like rend_get_service_list_mutable, but returns a read-only list. */
+static const smartlist_t*
+rend_get_service_list(const smartlist_t* substitute_service_list)
+{
+  /* It is safe to cast away the const here, because
+   * rend_get_service_list_mutable does not actually modify the list */
+  return rend_get_service_list_mutable((smartlist_t*)substitute_service_list);
+}
+
+/* Return a mutable list of hidden services.
+ * If substitute_service_list is not NULL, return it.
+ * Otherwise, check if the global rend_service_list is non-NULL, and if so,
+ * return it.
+ * Otherwise, return NULL.
+ * */
+static smartlist_t*
+rend_get_service_list_mutable(smartlist_t* substitute_service_list)
+{
+  if (substitute_service_list) {
+    return substitute_service_list;
+  }
+
+  /* If no special service list is provided, then just use the global one. */
+
+  if (BUG(!rend_service_list)) {
+    /* No global HS list, which is a programmer error. */
+    return NULL;
+  }
+
+  return rend_service_list;
+}
+
 /** Tells if onion service <b>s</b> is ephemeral.
-*/
+ */
 static unsigned int
 rend_service_is_ephemeral(const struct rend_service_t *s)
 {
@@ -140,10 +180,6 @@ rend_service_escaped_dir(const struct rend_service_t *s)
   return rend_service_is_ephemeral(s) ? "[EPHEMERAL]" : escaped(s->directory);
 }
 
-/** A list of rend_service_t's for services run on this OP.
- */
-static smartlist_t *rend_service_list = NULL;
-
 /** Return the number of rendezvous services we have configured. */
 int
 num_rend_services(void)
@@ -239,17 +275,10 @@ rend_add_service(smartlist_t *service_list, rend_service_t *service)
   int i;
   rend_service_port_config_t *p;
 
-  smartlist_t *s_list;
-  /* If no special service list is provided, then just use the global one. */
-  if (!service_list) {
-    if (BUG(!rend_service_list)) {
-      /* No global HS list, which is a failure. */
-      return -1;
-    }
-
-    s_list = rend_service_list;
-  } else {
-    s_list = service_list;
+  /* Use service_list for unit tests */
+  smartlist_t *s_list = rend_get_service_list_mutable(service_list);
+  if (BUG(!s_list)) {
+    return -1;
   }
 
   service->intro_nodes = smartlist_new();
@@ -257,7 +286,7 @@ rend_add_service(smartlist_t *service_list, rend_service_t *service)
 
   if (service->max_streams_per_circuit < 0) {
     log_warn(LD_CONFIG, "Hidden service (%s) configured with negative max "
-                        "streams per circuit; ignoring.",
+                        "streams per circuit.",
              rend_service_escaped_dir(service));
     rend_service_free(service);
     return -1;
@@ -266,7 +295,7 @@ rend_add_service(smartlist_t *service_list, rend_service_t *service)
   if (service->max_streams_close_circuit < 0 ||
       service->max_streams_close_circuit > 1) {
     log_warn(LD_CONFIG, "Hidden service (%s) configured with invalid "
-                        "max streams handling; ignoring.",
+                        "max streams handling.",
              rend_service_escaped_dir(service));
     rend_service_free(service);
     return -1;
@@ -276,15 +305,14 @@ rend_add_service(smartlist_t *service_list, rend_service_t *service)
       (!service->clients ||
        smartlist_len(service->clients) == 0)) {
     log_warn(LD_CONFIG, "Hidden service (%s) with client authorization but no "
-                        "clients; ignoring.",
+                        "clients.",
              rend_service_escaped_dir(service));
     rend_service_free(service);
     return -1;
   }
 
   if (!service->ports || !smartlist_len(service->ports)) {
-    log_warn(LD_CONFIG, "Hidden service (%s) with no ports configured; "
-             "ignoring.",
+    log_warn(LD_CONFIG, "Hidden service (%s) with no ports configured.",
              rend_service_escaped_dir(service));
     rend_service_free(service);
     return -1;
@@ -312,13 +340,12 @@ rend_add_service(smartlist_t *service_list, rend_service_t *service)
                                !strcmp(ptr->directory, service->directory));
       if (dupe) {
         log_warn(LD_REND, "Another hidden service is already configured for "
-                 "directory %s, ignoring.",
+                 "directory %s.",
                  rend_service_escaped_dir(service));
         rend_service_free(service);
         return -1;
       }
     }
-    smartlist_add(s_list, service);
     log_debug(LD_REND,"Configuring service with directory %s",
               rend_service_escaped_dir(service));
     for (i = 0; i < smartlist_len(service->ports); ++i) {
@@ -334,14 +361,16 @@ rend_add_service(smartlist_t *service_list, rend_service_t *service)
                   "Service maps port %d to socket at \"%s\"",
                   p->virtual_port, p->unix_addr);
 #else
-        log_debug(LD_REND,
-                  "Service maps port %d to an AF_UNIX socket, but we "
-                  "have no AF_UNIX support on this platform.  This is "
-                  "probably a bug.",
-                  p->virtual_port);
+        log_warn(LD_BUG,
+                 "Service maps port %d to an AF_UNIX socket, but we "
+                 "have no AF_UNIX support on this platform.  This is "
+                 "probably a bug.",
+                 p->virtual_port);
+        return -1;
 #endif /* defined(HAVE_SYS_UN_H) */
       }
     }
+    smartlist_add(s_list, service);
     return 0;
   }
   /* NOTREACHED */
@@ -502,27 +531,14 @@ rend_service_check_dir_and_add(smartlist_t *service_list,
     return 0;
   } else {
     /* Use service_list for unit tests */
-    smartlist_t *s_list = NULL;
-    /* If no special service list is provided, then just use the global one. */
-    if (!service_list) {
-      if (BUG(!rend_service_list)) {
-        /* No global HS list, which is a failure, because we plan on adding to
-         * it */
-        return -1;
-      }
-      s_list = rend_service_list;
-    } else {
-      s_list = service_list;
-    }
+    smartlist_t *s_list = rend_get_service_list_mutable(service_list);
     /* s_list can not be NULL here - if both service_list and rend_service_list
      * are NULL, and validate_only is false, we exit earlier in the function
      */
     if (BUG(!s_list)) {
       return -1;
     }
-    /* Ignore service failures until 030 */
-    rend_add_service(s_list, service);
-    return 0;
+    return rend_add_service(s_list, service);
   }
 }
 
@@ -1261,15 +1277,10 @@ rend_service_poison_new_single_onion_dir(const rend_service_t *s,
 int
 rend_service_load_all_keys(const smartlist_t *service_list)
 {
-  const smartlist_t *s_list = NULL;
-  /* If no special service list is provided, then just use the global one. */
-  if (!service_list) {
-    if (BUG(!rend_service_list)) {
-      return -1;
-    }
-    s_list = rend_service_list;
-  } else {
-    s_list = service_list;
+  /* Use service_list for unit tests */
+  const smartlist_t *s_list = rend_get_service_list(service_list);
+  if (BUG(!s_list)) {
+    return -1;
   }
 
   SMARTLIST_FOREACH_BEGIN(s_list, rend_service_t *, s) {