commit a4585405d6e103b9a48a1401321477ea08fa2ad1 Author: David Goulet dgoulet@ev0ke.net Date: Mon Mar 23 13:07:51 2015 -0400

Multiple fixes for the HSFETCH command

Ref: https://trac.torproject.org/projects/tor/ticket/14847?replyto=31#comment:31

Signed-off-by: David Goulet dgoulet@ev0ke.net --- src/or/control.c | 50 ++++++++++++++++++++++++++------------------------ src/or/directory.c | 3 ++- src/or/rendclient.c | 18 +++++++----------- 3 files changed, 35 insertions(+), 36 deletions(-)

diff --git a/src/or/control.c b/src/or/control.c index 10475bb..b4d671b 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -3263,28 +3263,27 @@ static int handle_control_hsfetch(control_connection_t *conn, uint32_t len, const char *body) { + int i; char digest[DIGEST_LEN], *hsaddress = NULL, *arg1 = NULL, *desc_id = NULL; smartlist_t *args = NULL, *hsdirs = NULL; (void) len; /* body is nul-terminated; it's safe to ignore the length */ + static const char *hsfetch_command = "HSFETCH"; static const char *v2_str = "v2-"; const size_t v2_str_len = strlen(v2_str); rend_data_t *rend_query = NULL;

/* Make sure we have at least one argument, the HSAddress. */ - args = getargs_helper("HSFETCH", conn, body, 1, -1); + args = getargs_helper(hsfetch_command, conn, body, 1, -1); if (!args) { - goto done; + goto exit; }

/* Extract the first argument (either HSAddress or DescID). */ arg1 = smartlist_get(args, 0); - /* Remove it from the argument list so we can safely iterate on all the - * rest to find optional argument(s). */ - smartlist_del(args, 0); /* Test if it's an HS address without the .onion part. */ if (rend_valid_service_id(arg1)) { hsaddress = arg1; - } else if (strstr(arg1, v2_str) && + } else if (strcmpstart(arg1, v2_str) == 0 && strlen(arg1 + v2_str_len) == REND_DESC_ID_V2_LEN_BASE32 && base32_decode(digest, sizeof(digest), arg1 + v2_str_len, REND_DESC_ID_V2_LEN_BASE32) == 0) { @@ -3292,15 +3291,17 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len, * of the id. */ desc_id = digest; } else { - connection_printf_to_buf(conn, "552 Unrecognized "%s"\r\n", + connection_printf_to_buf(conn, "513 Unrecognized "%s"\r\n", arg1); goto done; }

- /* Skip first argument because it's the HSAddress. */ - SMARTLIST_FOREACH_BEGIN(args, char *, arg) { + static const char *opt_server = "SERVER="; + + /* Skip first argument because it's the HSAddress or DescID. */ + for (i = 1; i < smartlist_len(args); ++i) { + const char *arg = smartlist_get(args, i); const node_t *node; - static const char *opt_server = "SERVER=";

if (!strcasecmpstart(arg, opt_server)) { const char *server; @@ -3319,11 +3320,11 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len, /* Valid server, add it to our local list. */ smartlist_add(hsdirs, node->rs); } else { - connection_printf_to_buf(conn, "552 Unexpected argument "%s"\r\n", + connection_printf_to_buf(conn, "513 Unexpected argument "%s"\r\n", arg); goto done; } - } SMARTLIST_FOREACH_END(arg); + }

rend_query = tor_malloc_zero(sizeof(*rend_query));

@@ -3334,7 +3335,8 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len, /* Using a descriptor ID, we force the user to provide at least one * hsdir server using the SERVER= option. */ if (!hsdirs || !smartlist_len(hsdirs)) { - connection_printf_to_buf(conn, "552 SERVER= option is required\r\n"); + connection_printf_to_buf(conn, "512 %s option is required\r\n", + opt_server); goto done; } memcpy(rend_query->descriptor_id, desc_id, @@ -3354,15 +3356,12 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len, rend_client_fetch_v2_desc(rend_query, hsdirs);

done: - if (hsdirs) { - smartlist_free(hsdirs); - } - if (args) { - SMARTLIST_FOREACH(args, char *, cp, tor_free(cp)); - smartlist_free(args); - } + SMARTLIST_FOREACH(args, char *, cp, tor_free(cp)); + smartlist_free(args); + /* Contains data pointer that we don't own thus no cleanup. */ + smartlist_free(hsdirs); tor_free(rend_query); - tor_free(arg1); +exit: return 0; }

@@ -5470,13 +5469,16 @@ control_event_hs_descriptor_content(const char *onion_address, static const char *event_name = "HS_DESC_CONTENT"; char *esc_content = NULL;

- if (!onion_address || !desc_id || !hsdir_id_digest || !content) { + if (!onion_address || !desc_id || !hsdir_id_digest) { log_warn(LD_BUG, "Called with onion_address==%p, desc_id==%p, " - "hsdir_id_digest==%p, content==%p", onion_address, desc_id, - hsdir_id_digest, content); + "hsdir_id_digest==%p", onion_address, desc_id, hsdir_id_digest); return; }

+ if (content == NULL) { + /* Point it to empty content so it can still be escaped. */ + content = ""; + } write_escaped_data(content, strlen(content), &esc_content);

send_control_event(EVENT_HS_DESC_CONTENT, ALL_FORMATS, diff --git a/src/or/directory.c b/src/or/directory.c index 0e7b954..37a46fc 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -2107,7 +2107,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn) control_event_hs_descriptor_content(conn->rend_data->onion_address, \ conn->requested_resource, \ conn->identity_digest, \ - "") ) + NULL) ) tor_assert(conn->rend_data); log_info(LD_REND,"Received rendezvous descriptor (size %d, status %d " "(%s))", @@ -2149,6 +2149,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn) body); conn->base_.purpose = DIR_PURPOSE_HAS_FETCHED_RENDDESC_V2; rend_client_desc_trynow(service_id); + memwipe(service_id, 0, sizeof(service_id)); break; } } diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 7941ba7..d649075 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -496,11 +496,10 @@ get_last_hid_serv_requests(void) REND_SERVICE_ID_LEN_BASE32)

/** Look up the last request time to hidden service directory <b>hs_dir</b> - * for descriptor ID <b>desc_id_base32</b> for the service specified in - * <b>rend_query</b>. If <b>set</b> is non-zero, - * assign the current time <b>now</b> and return that. Otherwise, return - * the most recent request time, or 0 if no such request has been sent - * before. */ + * for descriptor ID <b>desc_id_base32</b>. If <b>set</b> is non-zero, + * assign the current time <b>now</b> and return that. Otherwise, return the + * most recent request time, or 0 if no such request has been sent before. + */ static time_t lookup_last_hid_serv_request(routerstatus_t *hs_dir, const char *desc_id_base32, @@ -612,20 +611,17 @@ rend_client_purge_last_hid_serv_requests(void) * * Return NULL on error else the hsdir node pointer. */ static routerstatus_t * -pick_hsdir(const char *desc_id) +pick_hsdir(const char *desc_id, const char *desc_id_base32) { smartlist_t *responsible_dirs = smartlist_new(); smartlist_t *usable_responsible_dirs = smartlist_new(); const or_options_t *options = get_options(); routerstatus_t *hs_dir; - char desc_id_base32[REND_DESC_ID_V2_LEN_BASE32 + 1]; time_t now = time(NULL); int excluded_some;

tor_assert(desc_id); - - base32_encode(desc_id_base32, sizeof(desc_id_base32), - desc_id, DIGEST_LEN); + tor_assert(desc_id_base32);

/* Determine responsible dirs. Even if we can't get all we want, work with * the ones we have. If it's empty, we'll notice below. */ @@ -705,7 +701,7 @@ directory_get_from_hs_dir(const char *desc_id, const rend_data_t *rend_query,

/* Automatically pick an hs dir if none given. */ if (!rs_hsdir) { - hs_dir = pick_hsdir(desc_id); + hs_dir = pick_hsdir(desc_id, desc_id_base32); if (!hs_dir) { /* No suitable hs dir can be found, stop right now. */ return 0;