diff --git a/changes/bug20559 b/changes/bug20559 new file mode 100644 index 0000000000..f117162dde --- /dev/null +++ b/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. diff --git a/src/common/util.c b/src/common/util.c index 3421d117b0..417aa89433 100644 --- a/src/common/util.c +++ b/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); diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 457821cb49..457c2a02a9 100644 --- a/src/or/rendservice.c +++ b/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 s 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) {