[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [tor/maint-0.2.9] Update unit tests for 20484, 20529
commit 91abd60cad2fa3ca9f85fe20956f5f6a336c9c67
Author: teor <teor2345@xxxxxxxxx>
Date: Fri Nov 18 14:32:13 2016 +1100
Update unit tests for 20484, 20529
Add extra logging and extra validity checks for hidden services.
---
src/or/rendservice.c | 73 ++++++++++++++++++++++++++++++++++++++++------------
src/or/rendservice.h | 4 +++
src/test/test_hs.c | 28 ++++++++++++++++----
3 files changed, 83 insertions(+), 22 deletions(-)
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index d4d2405..91844e8 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -216,16 +216,30 @@ rend_service_free_all(void)
rend_service_list = NULL;
}
-/** Validate <b>service</b> and add it to rend_service_list if possible.
+/** Validate <b>service</b> and add it to <b>service_list</b>, or to
+ * the global rend_service_list if <b>service_list</b> is NULL.
* Return 0 on success. On failure, free <b>service</b> and return -1.
* Takes ownership of <b>service</b>.
*/
static int
-rend_add_service(rend_service_t *service)
+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;
+ }
+
service->intro_nodes = smartlist_new();
service->expiring_nodes = smartlist_new();
@@ -247,7 +261,8 @@ rend_add_service(rend_service_t *service)
}
if (service->auth_type != REND_NO_AUTH &&
- smartlist_len(service->clients) == 0) {
+ (!service->clients ||
+ smartlist_len(service->clients) == 0)) {
log_warn(LD_CONFIG, "Hidden service (%s) with client authorization but no "
"clients; ignoring.",
rend_service_escaped_dir(service));
@@ -255,7 +270,7 @@ rend_add_service(rend_service_t *service)
return -1;
}
- if (!smartlist_len(service->ports)) {
+ if (!service->ports || !smartlist_len(service->ports)) {
log_warn(LD_CONFIG, "Hidden service (%s) with no ports configured; "
"ignoring.",
rend_service_escaped_dir(service));
@@ -278,8 +293,9 @@ rend_add_service(rend_service_t *service)
* lock file. But this is enough to detect a simple mistake that
* at least one person has actually made.
*/
- if (service->directory != NULL) { /* Skip dupe for ephemeral services. */
- SMARTLIST_FOREACH(rend_service_list, rend_service_t*, ptr,
+ if (service->directory != NULL) {
+ /* Skip dupe for ephemeral services. */
+ SMARTLIST_FOREACH(s_list, rend_service_t*, ptr,
dupe = dupe ||
!strcmp(ptr->directory, service->directory));
if (dupe) {
@@ -290,7 +306,7 @@ rend_add_service(rend_service_t *service)
return -1;
}
}
- smartlist_add(rend_service_list, service);
+ smartlist_add(s_list, service);
log_debug(LD_REND,"Configuring service with directory \"%s\"",
service->directory);
for (i = 0; i < smartlist_len(service->ports); ++i) {
@@ -445,16 +461,18 @@ rend_service_port_config_free(rend_service_port_config_t *p)
tor_free(p);
}
-/* Check the directory for <b>service</b>, and add the service to the global
- * list if <b>validate_only</b> is false.
+/* Check the directory for <b>service</b>, and add the service to
+ * <b>service_list</b>, or to the global list if <b>service_list</b> is NULL.
+ * Only add the service to the list if <b>validate_only</b> is false.
* If <b>validate_only</b> is true, free the service.
* If <b>service</b> is NULL, ignore it, and return 0.
* Returns 0 on success, and -1 on failure.
* Takes ownership of <b>service</b>, either freeing it, or adding it to the
* global service list.
*/
-static int
-rend_service_check_dir_and_add(const or_options_t *options,
+STATIC int
+rend_service_check_dir_and_add(smartlist_t *service_list,
+ const or_options_t *options,
rend_service_t *service,
int validate_only)
{
@@ -463,6 +481,22 @@ rend_service_check_dir_and_add(const or_options_t *options,
return 0;
}
+ smartlist_t *s_list = NULL;
+ /* If no special service list is provided, then just use the global one. */
+ if (!service_list) {
+ if (!rend_service_list) {
+ /* No global HS list, which is a failure if we plan on adding to it */
+ if (BUG(!validate_only)) {
+ return -1;
+ }
+ /* Otherwise, we validate, */
+ }
+
+ s_list = rend_service_list;
+ } else {
+ s_list = service_list;
+ }
+
if (rend_service_check_private_dir(options, service, !validate_only)
< 0) {
rend_service_free(service);
@@ -474,8 +508,12 @@ rend_service_check_dir_and_add(const or_options_t *options,
return 0;
} else {
/* rend_add_service takes ownership, either adding or freeing the service
+ * 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
*/
- rend_add_service(service);
+ tor_assert(s_list);
+ /* Ignore service failures until 030 */
+ rend_add_service(s_list, service);
return 0;
}
}
@@ -504,8 +542,8 @@ rend_config_services(const or_options_t *options, int validate_only)
/* register the service we just finished parsing
* this code registers every service except the last one parsed,
* which is registered below the loop */
- if (rend_service_check_dir_and_add(options, service, !validate_only)
- < 0) {
+ if (rend_service_check_dir_and_add(NULL, options, service,
+ validate_only) < 0) {
return -1;
}
service = tor_malloc_zero(sizeof(rend_service_t));
@@ -715,8 +753,8 @@ rend_config_services(const or_options_t *options, int validate_only)
/* register the final service after we have finished parsing all services
* this code only registers the last service, other services are registered
* within the loop */
- if (rend_service_check_dir_and_add(options, service, !validate_only)
- < 0) {
+ if (rend_service_check_dir_and_add(NULL, options, service,
+ validate_only) < 0) {
return -1;
}
@@ -874,7 +912,7 @@ rend_service_add_ephemeral(crypto_pk_t *pk,
}
/* Initialize the service. */
- if (rend_add_service(s)) {
+ if (rend_add_service(NULL, s)) {
return RSAE_INTERNAL;
}
*service_id_out = tor_strdup(s->service_id);
@@ -1310,6 +1348,7 @@ rend_service_check_private_dir(const or_options_t *options,
}
/* Check/create directory */
if (check_private_dir(s->directory, check_opts, options->User) < 0) {
+ log_warn(LD_REND, "Checking service directory %s failed.", s->directory);
return -1;
}
return 0;
diff --git a/src/or/rendservice.h b/src/or/rendservice.h
index 630191e..bd3fb1f 100644
--- a/src/or/rendservice.h
+++ b/src/or/rendservice.h
@@ -119,6 +119,10 @@ typedef struct rend_service_t {
STATIC void rend_service_free(rend_service_t *service);
STATIC char *rend_service_sos_poison_path(const rend_service_t *service);
+STATIC int rend_service_check_dir_and_add(smartlist_t *service_list,
+ const or_options_t *options,
+ rend_service_t *service,
+ int validate_only);
#endif
diff --git a/src/test/test_hs.c b/src/test/test_hs.c
index b42bc59..e1f39b1 100644
--- a/src/test/test_hs.c
+++ b/src/test/test_hs.c
@@ -571,8 +571,28 @@ test_single_onion_poisoning(void *arg)
service_1->directory = dir1;
service_2->directory = dir2;
+ /* The services own the directory pointers now */
dir1 = dir2 = NULL;
- smartlist_add(services, service_1);
+ /* Add port to service 1 */
+ service_1->ports = smartlist_new();
+ service_2->ports = smartlist_new();
+ char *err_msg = NULL;
+ rend_service_port_config_t *port1 = rend_service_parse_port_config("80", " ",
+ &err_msg);
+ tt_assert(port1);
+ tt_assert(!err_msg);
+ smartlist_add(service_1->ports, port1);
+
+ rend_service_port_config_t *port2 = rend_service_parse_port_config("90", " ",
+ &err_msg);
+ /* Add port to service 2 */
+ tt_assert(port2);
+ tt_assert(!err_msg);
+ smartlist_add(service_2->ports, port2);
+
+ /* Add the first service */
+ ret = rend_service_check_dir_and_add(services, mock_options, service_1, 0);
+ tt_assert(ret == 0);
/* But don't add the second service yet. */
/* Service directories, but no previous keys, no problem! */
@@ -636,7 +656,7 @@ test_single_onion_poisoning(void *arg)
tt_assert(ret == 0);
/* Now add the second service: it has no key and no poison file */
- smartlist_add(services, service_2);
+ ret = rend_service_check_dir_and_add(services, mock_options, service_2, 0);
/* A new service, and an existing poisoned service. Not ok. */
mock_options->HiddenServiceSingleHopMode = 0;
@@ -698,13 +718,11 @@ test_single_onion_poisoning(void *arg)
done:
/* The test harness deletes the directories at exit */
+ smartlist_free(services);
rend_service_free(service_1);
rend_service_free(service_2);
- smartlist_free(services);
UNMOCK(get_options);
tor_free(mock_options->DataDirectory);
- tor_free(dir1);
- tor_free(dir2);
}
struct testcase_t hs_tests[] = {
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits