From b95dd03e5f6505ce2e78fe34a20bf5e5c970e6eb Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 15 May 2011 21:58:46 -0400 Subject: [PATCH 01/12] Log descriptions of nodes, not just nicknames. This patch introduces a few new functions in router.c to produce a more helpful description of a node than its nickame, and then tweaks nearly all log messages taking a nickname as an argument to call these functions instead. There are a few cases where I left the old log messages alone: in these cases, the nickname was that of an authority (whose nicknames are useful and unique), or the message already included an identity and/or an address. I might have missed a couple more too. This is a fix for bug 3045. --- src/or/circuitbuild.c | 43 +++++++------ src/or/circuitlist.c | 6 +- src/or/circuituse.c | 4 +- src/or/command.c | 4 +- src/or/connection_edge.c | 7 ++- src/or/directory.c | 9 +-- src/or/dirserv.c | 45 ++++++++------ src/or/relay.c | 5 +- src/or/rendclient.c | 19 +++--- src/or/rendservice.c | 31 ++++++---- src/or/rephist.c | 6 +- src/or/router.c | 127 +++++++++++++++++++++++++++++++++++++++ src/or/router.h | 15 +++++ src/or/routerlist.c | 36 +++++------ src/or/routerparse.c | 5 +- 15 files changed, 269 insertions(+), 93 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 860cd27567..469e180072 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1753,10 +1753,9 @@ circuit_handle_first_hop(origin_circuit_t *circ) if (!n_conn) { /* not currently connected in a useful way. */ - const char *name = strlen(firsthop->extend_info->nickname) ? - firsthop->extend_info->nickname : fmt_addr(&firsthop->extend_info->addr); log_info(LD_CIRC, "Next router is %s: %s ", - safe_str_client(name), msg?msg:"???"); + safe_str_client(extend_info_describe(firsthop->extend_info)), + msg?msg:"???"); circ->_base.n_hop = extend_info_dup(firsthop->extend_info); if (should_launch) { @@ -2039,7 +2038,7 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) circuit_set_state(TO_CIRCUIT(circ), CIRCUIT_STATE_BUILDING); log_info(LD_CIRC,"First hop: finished sending %s cell to '%s'", fast ? "CREATE_FAST" : "CREATE", - router ? router->nickname : ""); + router ? router_describe(router) : ""); } else { tor_assert(circ->cpath->state == CPATH_STATE_OPEN); tor_assert(circ->_base.state == CIRCUIT_STATE_BUILDING); @@ -2829,7 +2828,7 @@ choose_good_exit_server_general(routerlist_t *dir, int need_uptime, tor_free(n_supported); if (router) { - log_info(LD_CIRC, "Chose exit server '%s'", router->nickname); + log_info(LD_CIRC, "Chose exit server '%s'", router_describe(router)); return router; } if (options->ExitNodes) { @@ -2936,7 +2935,7 @@ warn_if_last_router_excluded(origin_circuit_t *circ, const extend_info_t *exit) log_warn(LD_BUG, "Using %s '%s' which is listed in ExcludeNodes%s, " "even though StrictNodes is set. Please report. " "(Circuit purpose: %s)", - description, exit->nickname, + description, extend_info_describe(exit), rs==options->ExcludeNodes?"":" or ExcludeExitNodes", circuit_purpose_to_string(purpose)); } else { @@ -2945,7 +2944,7 @@ warn_if_last_router_excluded(origin_circuit_t *circ, const extend_info_t *exit) "prevent this (and possibly break your Tor functionality), " "set the StrictNodes configuration option. " "(Circuit purpose: %s)", - description, exit->nickname, + description, extend_info_describe(exit), rs==options->ExcludeNodes?"":" or ExcludeExitNodes", circuit_purpose_to_string(purpose)); } @@ -2976,7 +2975,8 @@ onion_pick_cpath_exit(origin_circuit_t *circ, extend_info_t *exit) if (exit) { /* the circuit-builder pre-requested one */ warn_if_last_router_excluded(circ, exit); - log_info(LD_CIRC,"Using requested exit node '%s'", exit->nickname); + log_info(LD_CIRC,"Using requested exit node '%s'", + extend_info_describe(exit)); exit = extend_info_dup(exit); } else { /* we have to decide one */ routerinfo_t *router = @@ -3025,8 +3025,8 @@ circuit_extend_to_new_exit(origin_circuit_t *circ, extend_info_t *exit) circuit_append_new_exit(circ, exit); circuit_set_state(TO_CIRCUIT(circ), CIRCUIT_STATE_BUILDING); if ((err_reason = circuit_send_next_onion_skin(circ))<0) { - log_warn(LD_CIRC, "Couldn't extend circuit to new point '%s'.", - exit->nickname); + log_warn(LD_CIRC, "Couldn't extend circuit to new point %s.", + extend_info_describe(exit)); circuit_mark_for_close(TO_CIRCUIT(circ), -err_reason); return -1; } @@ -3250,7 +3250,8 @@ onion_extend_cpath(origin_circuit_t *circ) } log_debug(LD_CIRC,"Chose router %s for hop %d (exit is %s)", - info->nickname, cur_len+1, build_state_get_exit_nickname(state)); + extend_info_describe(info), + cur_len+1, build_state_get_exit_nickname(state)); onion_append_hop(&circ->cpath, info); extend_info_free(info); @@ -3526,20 +3527,24 @@ log_entry_guards(int severity) smartlist_t *elements = smartlist_create(); char *s; - SMARTLIST_FOREACH(entry_guards, entry_guard_t *, e, + SMARTLIST_FOREACH_BEGIN(entry_guards, entry_guard_t *, e) { const char *msg = NULL; char *cp; if (entry_is_live(e, 0, 1, 0, &msg)) - tor_asprintf(&cp, "%s (up %s)", + tor_asprintf(&cp, "%s [%s] (up %s)", e->nickname, + hex_str(e->identity, DIGEST_LEN), e->made_contact ? "made-contact" : "never-contacted"); else - tor_asprintf(&cp, "%s (%s, %s)", - e->nickname, msg, + tor_asprintf(&cp, "%s [%s] (%s, %s)", + e->nickname, + hex_str(e->identity, DIGEST_LEN), + msg, e->made_contact ? "made-contact" : "never-contacted"); smartlist_add(elements, cp); - }); + } + SMARTLIST_FOREACH_END(e); s = smartlist_join_strings(elements, ",", 0, NULL); SMARTLIST_FOREACH(elements, char*, cp, tor_free(cp)); @@ -3607,7 +3612,8 @@ add_an_entry_guard(routerinfo_t *chosen, int reset_status) return NULL; } entry = tor_malloc_zero(sizeof(entry_guard_t)); - log_info(LD_CIRC, "Chose '%s' as new entry guard.", router->nickname); + log_info(LD_CIRC, "Chose '%s' as new entry guard.", + router_describe(router)); strlcpy(entry->nickname, router->nickname, sizeof(entry->nickname)); memcpy(entry->identity, router->cache_info.identity_digest, DIGEST_LEN); /* Choose expiry time smudged over the past month. The goal here @@ -3800,8 +3806,9 @@ entry_guards_compute_status(or_options_t *options, time_t now) const char *reason = digestmap_get(reasons, entry->identity); const char *live_msg = ""; routerinfo_t *r = entry_is_live(entry, 0, 1, 0, &live_msg); - log_info(LD_CIRC, "Summary: Entry '%s' is %s, %s%s%s, and %s%s.", + log_info(LD_CIRC, "Summary: Entry %s [%s] is %s, %s%s%s, and %s%s.", entry->nickname, + hex_str(entry->identity, DIGEST_LEN), entry->unreachable_since ? "unreachable" : "reachable", entry->bad_since ? "unusable" : "usable", reason ? ", ": "", diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 8ec46186d9..4ad244dfda 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -274,8 +274,10 @@ circuit_count_pending_on_or_conn(or_connection_t *or_conn) circuit_get_all_pending_on_or_conn(sl, or_conn); cnt = smartlist_len(sl); smartlist_free(sl); - log_debug(LD_CIRC,"or_conn to %s, %d pending circs", - or_conn->nickname ? or_conn->nickname : "NULL", cnt); + log_debug(LD_CIRC,"or_conn to %s at %s, %d pending circs", + or_conn->nickname ? or_conn->nickname : "NULL", + or_conn->_base.address, + cnt); return cnt; } diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 7289aa5c11..4c29bf8359 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1335,8 +1335,8 @@ circuit_get_open_circ_or_launch(edge_connection_t *conn, conn->_base.state = AP_CONN_STATE_RENDDESC_WAIT; return 0; } - log_info(LD_REND,"Chose '%s' as intro point for '%s'.", - extend_info->nickname, + log_info(LD_REND,"Chose %s as intro point for '%s'.", + extend_info_describe(extend_info), safe_str_client(conn->rend_data->onion_address)); } diff --git a/src/or/command.c b/src/or/command.c index 00d9af33fa..d25918b841 100644 --- a/src/or/command.c +++ b/src/or/command.c @@ -274,8 +274,8 @@ command_process_create_cell(cell_t *cell, or_connection_t *conn) cell->circ_id, (int)(time(NULL) - conn->_base.timestamp_created)); if (router) log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "Details: nickname \"%s\", platform %s.", - router->nickname, escaped(router->platform)); + "Details: router %s, platform %s.", + router_describe(router), escaped(router->platform)); return; } diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 7828f16386..78b1b92de9 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -503,12 +503,12 @@ connection_ap_expire_beginning(void) } tor_assert(circ->purpose == CIRCUIT_PURPOSE_C_GENERAL); log_fn(cutoff < 15 ? LOG_INFO : severity, LD_APP, - "We tried for %d seconds to connect to '%s' using exit '%s'." + "We tried for %d seconds to connect to '%s' using exit %s." " Retrying on a new circuit.", seconds_idle, safe_str_client(conn->socks_request->address), conn->cpath_layer ? - conn->cpath_layer->extend_info->nickname : "*unnamed*"); + extend_info_describe(conn->cpath_layer->extend_info): "*unnamed*"); /* send an end down the circuit */ connection_edge_end(conn, END_STREAM_REASON_TIMEOUT); /* un-mark it as ending, since we're going to reuse it */ @@ -1830,7 +1830,8 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn, if (r) { log_info(LD_APP, "Redirecting address %s to exit at enclave router %s", - safe_str_client(socks->address), r->nickname); + safe_str_client(socks->address), + router_describe(r)); /* use the hex digest, not nickname, in case there are two routers with this nickname */ conn->chosen_exit_name = diff --git a/src/or/directory.c b/src/or/directory.c index 347ed42cb8..b238be2abc 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -525,8 +525,9 @@ directory_initiate_command_routerstatus_rend(routerstatus_t *status, router = router_get_by_digest(status->identity_digest); if (!router && anonymized_connection) { - log_info(LD_DIR, "Not sending anonymized request to directory '%s'; we " - "don't have its router descriptor.", status->nickname); + log_info(LD_DIR, "Not sending anonymized request to directory %s; we " + "don't have its router descriptor.", + routerstatus_describe(status)); return; } else if (router) { address = router->address; @@ -539,10 +540,10 @@ directory_initiate_command_routerstatus_rend(routerstatus_t *status, if (options->ExcludeNodes && options->StrictNodes && routerset_contains_routerstatus(options->ExcludeNodes, status)) { - log_warn(LD_DIR, "Wanted to contact directory mirror '%s' for %s, but " + log_warn(LD_DIR, "Wanted to contact directory mirror %s for %s, but " "it's in our ExcludedNodes list and StrictNodes is set. " "Skipping. This choice might make your Tor not work.", - status->nickname, + routerstatus_describe(status), dir_conn_purpose_to_string(dir_purpose)); return; } diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 79b68cdac8..e9355fedb4 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -518,14 +518,15 @@ dirserv_router_has_valid_address(routerinfo_t *ri) if (get_options()->DirAllowPrivateAddresses) return 0; /* whatever it is, we're fine with it */ if (!tor_inet_aton(ri->address, &iaddr)) { - log_info(LD_DIRSERV,"Router '%s' published non-IP address '%s'. Refusing.", - ri->nickname, ri->address); + log_info(LD_DIRSERV,"Router %s published non-IP address '%s'. Refusing.", + router_describe(ri), + ri->address); return -1; } if (is_internal_IP(ntohl(iaddr.s_addr), 0)) { log_info(LD_DIRSERV, - "Router '%s' published internal IP address '%s'. Refusing.", - ri->nickname, ri->address); + "Router %s published internal IP address '%s'. Refusing.", + router_describe(ri), ri->address); return -1; /* it's a private IP, we should reject it */ } return 0; @@ -554,10 +555,11 @@ authdir_wants_to_reject_router(routerinfo_t *ri, const char **msg, /* Is there too much clock skew? */ now = time(NULL); if (ri->cache_info.published_on > now+ROUTER_ALLOW_SKEW) { - log_fn(severity, LD_DIRSERV, "Publication time for nickname '%s' is too " + log_fn(severity, LD_DIRSERV, "Publication time for %s is too " "far (%d minutes) in the future; possible clock skew. Not adding " "(%s)", - ri->nickname, (int)((ri->cache_info.published_on-now)/60), + router_describe(ri), + (int)((ri->cache_info.published_on-now)/60), esc_router_info(ri)); *msg = "Rejected: Your clock is set too far in the future, or your " "timezone is not correct."; @@ -565,9 +567,10 @@ authdir_wants_to_reject_router(routerinfo_t *ri, const char **msg, } if (ri->cache_info.published_on < now-ROUTER_MAX_AGE_TO_PUBLISH) { log_fn(severity, LD_DIRSERV, - "Publication time for router with nickname '%s' is too far " + "Publication time for %s is too far " "(%d minutes) in the past. Not adding (%s)", - ri->nickname, (int)((now-ri->cache_info.published_on)/60), + router_describe(ri), + (int)((now-ri->cache_info.published_on)/60), esc_router_info(ri)); *msg = "Rejected: Server is expired, or your clock is too far in the past," " or your timezone is not correct."; @@ -575,9 +578,10 @@ authdir_wants_to_reject_router(routerinfo_t *ri, const char **msg, } if (dirserv_router_has_valid_address(ri) < 0) { log_fn(severity, LD_DIRSERV, - "Router with nickname '%s' has invalid address '%s'. " + "Router %s has invalid address '%s'. " "Not adding (%s).", - ri->nickname, ri->address, + router_describe(ri), + ri->address, esc_router_info(ri)); *msg = "Rejected: Address is not an IP, or IP is a private address."; return -1; @@ -720,9 +724,9 @@ dirserv_add_descriptor(routerinfo_t *ri, const char **msg, const char *source) && router_differences_are_cosmetic(ri_old, ri) && !router_is_me(ri)) { log_info(LD_DIRSERV, - "Not replacing descriptor from '%s' (source: %s); " + "Not replacing descriptor from %s (source: %s); " "differences are cosmetic.", - ri->nickname, source); + router_describe(ri), source); *msg = "Not replacing router descriptor; no information has changed since " "the last one with this identity."; control_event_or_authdir_new_descriptor("DROPPED", @@ -824,10 +828,12 @@ directory_remove_invalid(void) for (i = 0; i < smartlist_len(rl->routers); ++i) { const char *msg; routerinfo_t *ent = smartlist_get(rl->routers, i); + char description[NODE_DESC_BUF_LEN]; uint32_t r = dirserv_router_get_status(ent, &msg); + router_get_description(description, ent); if (r & FP_REJECT) { - log_info(LD_DIRSERV, "Router '%s' is now rejected: %s", - ent->nickname, msg?msg:""); + log_info(LD_DIRSERV, "Router %s is now rejected: %s", + description, msg?msg:""); routerlist_remove(rl, ent, 0, time(NULL)); i--; changed = 1; @@ -835,25 +841,25 @@ directory_remove_invalid(void) } if (bool_neq((r & FP_NAMED), ent->is_named)) { log_info(LD_DIRSERV, - "Router '%s' is now %snamed.", ent->nickname, + "Router %s is now %snamed.", description, (r&FP_NAMED)?"":"un"); ent->is_named = (r&FP_NAMED)?1:0; changed = 1; } if (bool_neq((r & FP_INVALID), !ent->is_valid)) { - log_info(LD_DIRSERV, "Router '%s' is now %svalid.", ent->nickname, + log_info(LD_DIRSERV, "Router '%s' is now %svalid.", description, (r&FP_INVALID) ? "in" : ""); ent->is_valid = (r&FP_INVALID)?0:1; changed = 1; } if (bool_neq((r & FP_BADDIR), ent->is_bad_directory)) { - log_info(LD_DIRSERV, "Router '%s' is now a %s directory", ent->nickname, + log_info(LD_DIRSERV, "Router '%s' is now a %s directory", description, (r & FP_BADDIR) ? "bad" : "good"); ent->is_bad_directory = (r&FP_BADDIR) ? 1: 0; changed = 1; } if (bool_neq((r & FP_BADEXIT), ent->is_bad_exit)) { - log_info(LD_DIRSERV, "Router '%s' is now a %s exit", ent->nickname, + log_info(LD_DIRSERV, "Router '%s' is now a %s exit", description, (r & FP_BADEXIT) ? "bad" : "good"); ent->is_bad_exit = (r&FP_BADEXIT) ? 1: 0; changed = 1; @@ -3163,7 +3169,8 @@ dirserv_orconn_tls_done(const char *address, if (!bridge_auth || ri->purpose == ROUTER_PURPOSE_BRIDGE) { tor_addr_t addr, *addrp=NULL; log_info(LD_DIRSERV, "Found router %s to be reachable at %s:%d. Yay.", - ri->nickname, address, ri->or_port ); + router_describe(ri), + address, ri->or_port); if (tor_addr_from_str(&addr, ri->address) != -1) addrp = &addr; else diff --git a/src/or/relay.c b/src/or/relay.c index 9effae3036..46e852217d 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -28,6 +28,7 @@ #include "reasons.h" #include "relay.h" #include "rendcommon.h" +#include "router.h" #include "routerlist.h" #include "routerparse.h" @@ -749,9 +750,9 @@ connection_ap_process_end_not_open( (tor_inet_aton(conn->socks_request->address, &in) && !conn->chosen_exit_name))) { log_info(LD_APP, - "Exitrouter '%s' seems to be more restrictive than its exit " + "Exitrouter %s seems to be more restrictive than its exit " "policy. Not using this router as exit for now.", - exitrouter->nickname); + router_describe(exitrouter)); policies_set_router_exitpolicy_to_reject_all(exitrouter); } /* rewrite it to an IP if we learned one. */ diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 77e11c2a07..af6f43aa28 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -20,6 +20,7 @@ #include "rendclient.h" #include "rendcommon.h" #include "rephist.h" +#include "router.h" #include "routerlist.h" static extend_info_t *rend_client_get_random_intro_impl( @@ -90,12 +91,13 @@ rend_client_reextend_intro_circuit(origin_circuit_t *circ) if (circ->remaining_relay_early_cells) { log_info(LD_REND, "Re-extending circ %d, this time to %s.", - circ->_base.n_circ_id, extend_info->nickname); + circ->_base.n_circ_id, + safe_str_client(extend_info_describe(extend_info))); result = circuit_extend_to_new_exit(circ, extend_info); } else { log_info(LD_REND, "Building a new introduction circuit, this time to %s.", - extend_info->nickname); + safe_str_client(extend_info_describe(extend_info))); circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_FINISHED); if (!circuit_launch_by_extend_info(CIRCUIT_PURPOSE_C_INTRODUCING, extend_info, @@ -168,7 +170,8 @@ rend_client_send_introduction(origin_circuit_t *introcirc, "have a v2 rend desc with %d intro points. " "Trying a different intro point...", safe_str_client(introcirc->rend_data->onion_address), - introcirc->build_state->chosen_exit->nickname, + safe_str_client(extend_info_describe( + introcirc->build_state->chosen_exit)), smartlist_len(entry->parsed->intro_nodes)); if (rend_client_reextend_intro_circuit(introcirc)) { @@ -343,8 +346,8 @@ rend_client_introduction_acked(origin_circuit_t *circ, * If none remain, refetch the service descriptor. */ log_info(LD_REND, "Got nack for %s from %s...", - safe_str_client(circ->rend_data->onion_address), - circ->build_state->chosen_exit->nickname); + safe_str_client(circ->rend_data->onion_address), + safe_str_client(extend_info_describe(circ->build_state->chosen_exit))); if (rend_client_remove_intro_point(circ->build_state->chosen_exit, circ->rend_data) > 0) { /* There are introduction points left. Re-extend the circuit to @@ -497,12 +500,12 @@ directory_get_from_hs_dir(const char *desc_id, const rend_data_t *rend_query) log_info(LD_REND, "Sending fetch request for v2 descriptor for " "service '%s' with descriptor ID '%s', auth type %d, " "and descriptor cookie '%s' to hidden service " - "directory '%s' on port %d.", + "directory %s", rend_query->onion_address, desc_id_base32, rend_query->auth_type, (rend_query->auth_type == REND_NO_AUTH ? "[none]" : - escaped_safe_str_client(descriptor_cookie_base64)), - hs_dir->nickname, hs_dir->dir_port); + escaped_safe_str_client(descriptor_cookie_base64)), + routerstatus_describe(hs_dir)); return 1; } diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 35e8b9057a..edcf59d870 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -474,7 +474,8 @@ rend_config_services(or_options_t *options, int validate_only) if (keep_it) continue; log_info(LD_REND, "Closing intro point %s for service %s.", - safe_str_client(oc->build_state->chosen_exit->nickname), + safe_str_client(extend_info_describe( + oc->build_state->chosen_exit)), oc->rend_data->onion_address); circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED); /* XXXX Is there another reason we should use here? */ @@ -1146,7 +1147,7 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, if (!launched) { /* give up */ log_warn(LD_REND, "Giving up launching first hop of circuit to rendezvous " "point %s for service %s.", - escaped_safe_str_client(extend_info->nickname), + safe_str_client(extend_info_describe(extend_info)), serviceid); reason = END_CIRC_REASON_CONNECTFAILED; goto err; @@ -1154,7 +1155,7 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, log_info(LD_REND, "Accepted intro; launching circuit to %s " "(cookie %s) for service %s.", - escaped_safe_str_client(extend_info->nickname), + safe_str_client(extend_info_describe(extend_info)), hexcookie, serviceid); tor_assert(launched->build_state); /* Fill in the circuit's state. */ @@ -1206,7 +1207,8 @@ rend_service_relaunch_rendezvous(origin_circuit_t *oldcirc) "Attempt to build circuit to %s for rendezvous has failed " "too many times or expired; giving up.", oldcirc->build_state ? - oldcirc->build_state->chosen_exit->nickname : "*unknown*"); + safe_str(extend_info_describe(oldcirc->build_state->chosen_exit)) + : "*unknown*"); return; } @@ -1220,7 +1222,7 @@ rend_service_relaunch_rendezvous(origin_circuit_t *oldcirc) } log_info(LD_REND,"Reattempting rendezvous circuit to '%s'", - oldstate->chosen_exit->nickname); + safe_str(extend_info_describe(oldstate->chosen_exit))); newcirc = circuit_launch_by_extend_info(CIRCUIT_PURPOSE_S_CONNECT_REND, oldstate->chosen_exit, @@ -1228,7 +1230,7 @@ rend_service_relaunch_rendezvous(origin_circuit_t *oldcirc) if (!newcirc) { log_warn(LD_REND,"Couldn't relaunch rendezvous circuit to '%s'.", - oldstate->chosen_exit->nickname); + safe_str(extend_info_describe(oldstate->chosen_exit))); return; } newstate = newcirc->build_state; @@ -1252,7 +1254,7 @@ rend_service_launch_establish_intro(rend_service_t *service, log_info(LD_REND, "Launching circuit to introduction point %s for service %s", - escaped_safe_str_client(intro->extend_info->nickname), + safe_str_client(extend_info_describe(intro->extend_info)), service->service_id); rep_hist_note_used_internal(time(NULL), 1, 0); @@ -1265,7 +1267,7 @@ rend_service_launch_establish_intro(rend_service_t *service, if (!launched) { log_info(LD_REND, "Can't launch circuit to establish introduction at %s.", - escaped_safe_str_client(intro->extend_info->nickname)); + safe_str_client(extend_info_describe(intro->extend_info))); return -1; } @@ -1608,9 +1610,9 @@ directory_post_to_hs_dir(rend_service_descriptor_t *renddesc, continue; if (!router_get_by_digest(hs_dir->identity_digest)) { log_info(LD_REND, "Not sending publish request for v2 descriptor to " - "hidden service directory '%s'; we don't have its " + "hidden service directory %s; we don't have its " "router descriptor. Queuing for later upload.", - hs_dir->nickname); + safe_str_client(routerstatus_describe(hs_dir))); failed_upload = -1; continue; } @@ -1818,7 +1820,8 @@ rend_services_introduce(void) router = router_get_by_digest(intro->extend_info->identity_digest); if (!router || !find_intro_circuit(intro, service->pk_digest)) { log_info(LD_REND,"Giving up on %s as intro point for %s.", - intro->extend_info->nickname, service->service_id); + safe_str_client(extend_info_describe(intro->extend_info)), + safe_str_client(service->service_id)); if (service->desc) { SMARTLIST_FOREACH(service->desc->intro_nodes, rend_intro_point_t *, dintro, { @@ -1884,7 +1887,8 @@ rend_services_introduce(void) tor_assert(!crypto_pk_generate_key(intro->intro_key)); smartlist_add(service->intro_nodes, intro); log_info(LD_REND, "Picked router %s as an intro point for %s.", - router->nickname, service->service_id); + safe_str_client(router_describe(router)), + safe_str_client(service->service_id)); } /* If there's no need to launch new circuits, stop here. */ @@ -1897,7 +1901,8 @@ rend_services_introduce(void) r = rend_service_launch_establish_intro(service, intro); if (r<0) { log_warn(LD_REND, "Error launching circuit to node %s for service %s.", - intro->extend_info->nickname, service->service_id); + safe_str_client(extend_info_describe(intro->extend_info)), + safe_str_client(service->service_id)); } } } diff --git a/src/or/rephist.c b/src/or/rephist.c index 8cddd2b5eb..6be8484cc5 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -634,6 +634,7 @@ rep_hist_dump_stats(time_t now, int severity) digestmap_iter_t *orhist_it; const char *name1, *name2, *digest1, *digest2; char hexdigest1[HEX_DIGEST_LEN+1]; + char hexdigest2[HEX_DIGEST_LEN+1]; or_history_t *or_history; link_history_t *link_history; void *or_history_p, *link_history_p; @@ -694,7 +695,10 @@ rep_hist_dump_stats(time_t now, int severity) link_history = (link_history_t*) link_history_p; - ret = tor_snprintf(buffer+len, 2048-len, "%s(%ld/%ld); ", name2, + base16_encode(hexdigest2, sizeof(hexdigest2), digest2, DIGEST_LEN); + ret = tor_snprintf(buffer+len, 2048-len, "%s [%s](%ld/%ld); ", + name2, + hexdigest2, link_history->n_extend_ok, link_history->n_extend_ok+link_history->n_extend_fail); if (ret<0) diff --git a/src/or/router.c b/src/or/router.c index 616a290d8a..a11d3a5923 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2217,6 +2217,133 @@ is_legal_hexdigest(const char *s) strspn(s,HEX_CHARACTERS)==HEX_DIGEST_LEN); } +/** Use buf (which must be at least NODE_DESC_BUF_LEN bytes long) to + * hold a human-readable description of a node with identity digest + * id_digest, named-status is_named, nickname nickname, + * and address addr or addr32h. + * + * The nickname and addr fields are optional and may be set to + * NULL. The addr32h field is optional and may be set to + * addr32h. + * + * Return a pointer to the front of buf. + */ +const char * +format_node_description(char *buf, + const char *id_digest, + int is_named, + const char *nickname, + const tor_addr_t *addr, + uint32_t addr32h) +{ + char *cp; + buf[0] = '$'; + base16_encode(buf+1, HEX_DIGEST_LEN+1, id_digest, DIGEST_LEN); + cp = buf+1+HEX_DIGEST_LEN; + if (nickname) { + buf[1+HEX_DIGEST_LEN] = is_named ? '=' : '~'; + strlcpy(buf+1+HEX_DIGEST_LEN+1, nickname, MAX_NICKNAME_LEN+1); + cp += strlen(cp); + } + if (addr32h || addr) { + memcpy(cp, " at ", 4); + cp += 4; + if (addr) { + tor_addr_to_str(cp, addr, TOR_ADDR_BUF_LEN, 0); + } else { + struct in_addr in; + in.s_addr = htonl(addr32h); + tor_inet_ntoa(&in, cp, INET_NTOA_BUF_LEN); + } + } + return buf; +} + +/** Use buf (which must be at least NODE_DESC_BUF_LEN bytes long) to + * hold a human-readable description of ri. + * + * + * Return a pointer to the front of buf. + */ +const char * +router_get_description(char *buf, const routerinfo_t *ri) +{ + return format_node_description(buf, + ri->cache_info.identity_digest, + ri->is_named, + ri->nickname, + NULL, + ri->addr); +} + +/** Use buf (which must be at least NODE_DESC_BUF_LEN bytes long) to + * hold a human-readable description of rs. + * + * Return a pointer to the front of buf. + */ +const char * +routerstatus_get_description(char *buf, const routerstatus_t *rs) +{ + return format_node_description(buf, + rs->identity_digest, + rs->is_named, + rs->nickname, + NULL, + rs->addr); +} + +/** Use buf (which must be at least NODE_DESC_BUF_LEN bytes long) to + * hold a human-readable description of ei. + * + * Return a pointer to the front of buf. + */ +const char * +extend_info_get_description(char *buf, const extend_info_t *ei) +{ + return format_node_description(buf, + ei->identity_digest, + 0, + ei->nickname, + &ei->addr, + 0); +} + +/** Return a human-readable description of the routerinfo_t ri. + * + * This function is not thread-safe. Each call to this function invalidates + * previous values returned by this function. + */ +const char * +router_describe(const routerinfo_t *ri) +{ + static char buf[NODE_DESC_BUF_LEN]; + return router_get_description(buf, ri); +} + +/** Return a human-readable description of the routerstatus_t rs. + * + * This function is not thread-safe. Each call to this function invalidates + * previous values returned by this function. + */ +const char * +routerstatus_describe(const routerstatus_t *rs) +{ + static char buf[NODE_DESC_BUF_LEN]; + return routerstatus_get_description(buf, rs); +} + +/** Return a human-readable description of the extend_info_t ri. + * + * This function is not thread-safe. Each call to this function invalidates + * previous values returned by this function. + */ +const char * +extend_info_describe(const extend_info_t *ei) +{ + static char buf[NODE_DESC_BUF_LEN]; + return extend_info_get_description(buf, ei); +} + /** Set buf (which must have MAX_VERBOSE_NICKNAME_LEN+1 bytes) to the * verbose representation of the identity of router. The format is: * A dollar sign. diff --git a/src/or/router.h b/src/or/router.h index 1bf1a51ae1..6cabfd56e8 100644 --- a/src/or/router.h +++ b/src/or/router.h @@ -85,6 +85,21 @@ int extrainfo_dump_to_string(char **s, extrainfo_t *extrainfo, int is_legal_nickname(const char *s); int is_legal_nickname_or_hexdigest(const char *s); int is_legal_hexdigest(const char *s); + +#define NODE_DESC_BUF_LEN (MAX_VERBOSE_NICKNAME_LEN+4+TOR_ADDR_BUF_LEN) +const char *format_node_description(char *buf, + const char *id_digest, + int is_named, + const char *nickname, + const tor_addr_t *addr, + uint32_t addr32h); +const char *router_get_description(char *buf, const routerinfo_t *ri); +const char *routerstatus_get_description(char *buf, const routerstatus_t *rs); +const char *extend_info_get_description(char *buf, const extend_info_t *ei); +const char *router_describe(const routerinfo_t *ri); +const char *routerstatus_describe(const routerstatus_t *ri); +const char *extend_info_describe(const extend_info_t *ei); + void router_get_verbose_nickname(char *buf, const routerinfo_t *router); void routerstatus_get_verbose_nickname(char *buf, const routerstatus_t *router); diff --git a/src/or/routerlist.c b/src/or/routerlist.c index a3d9b78ee7..47a57a872d 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -3210,8 +3210,8 @@ router_set_status(const char *digest, int up) router = router_get_by_digest(digest); if (router) { - log_debug(LD_DIR,"Marking router '%s/%s' as %s.", - router->nickname, router->address, up ? "up" : "down"); + log_debug(LD_DIR,"Marking router %s as %s.", + router_describe(router), up ? "up" : "down"); if (!up && router_is_me(router) && !we_are_hibernating()) log_warn(LD_NET, "We just marked ourself as down. Are your external " "addresses reachable?"); @@ -3283,11 +3283,12 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg, router->purpose == ROUTER_PURPOSE_BRIDGE && !was_bridge) { log_info(LD_DIR, "Replacing non-bridge descriptor with bridge " - "descriptor for router '%s'", router->nickname); + "descriptor for router %s", + router_describe(router)); } else { log_info(LD_DIR, - "Dropping descriptor that we already have for router '%s'", - router->nickname); + "Dropping descriptor that we already have for router %s", + router_describe(router)); *msg = "Router descriptor was not new."; routerinfo_free(router); return ROUTER_WAS_NOT_NEW; @@ -3311,8 +3312,8 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg, /* We asked for it, so some networkstatus must have listed it when we * did. Save it if we're a cache in case somebody else asks for it. */ log_info(LD_DIR, - "Received a no-longer-recognized descriptor for router '%s'", - router->nickname); + "Received a no-longer-recognized descriptor for router %s", + router_describe(router)); *msg = "Router descriptor is not referenced by any network-status."; /* Only journal this desc if we'll be serving it. */ @@ -3363,8 +3364,9 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg, if (router->purpose == ROUTER_PURPOSE_BRIDGE && from_cache && !authdir_mode_bridge(options) && !routerinfo_is_a_configured_bridge(router)) { - log_info(LD_DIR, "Dropping bridge descriptor for '%s' because we have " - "no bridge configured at that address.", router->nickname); + log_info(LD_DIR, "Dropping bridge descriptor for %s because we have " + "no bridge configured at that address.", + safe_str_client(router_describe(router))); *msg = "Router descriptor was not a configured bridge."; routerinfo_free(router); return ROUTER_WAS_NOT_WANTED; @@ -3375,8 +3377,8 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg, if (!in_consensus && (router->cache_info.published_on <= old_router->cache_info.published_on)) { /* Same key, but old. This one is not listed in the consensus. */ - log_debug(LD_DIR, "Not-new descriptor for router '%s'", - router->nickname); + log_debug(LD_DIR, "Not-new descriptor for router %s", + router_describe(router)); /* Only journal this desc if we'll be serving it. */ if (!from_cache && should_cache_old_descriptors()) signed_desc_append_to_journal(&router->cache_info, @@ -3386,9 +3388,8 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg, return ROUTER_WAS_NOT_NEW; } else { /* Same key, and either new, or listed in the consensus. */ - log_debug(LD_DIR, "Replacing entry for router '%s/%s' [%s]", - router->nickname, old_router->nickname, - hex_str(id_digest,DIGEST_LEN)); + log_debug(LD_DIR, "Replacing entry for router %s", + router_describe(router)); if (routers_have_same_or_addr(router, old_router)) { /* these carry over when the address and orport are unchanged. */ router->last_reachable = old_router->last_reachable; @@ -3664,8 +3665,8 @@ routerlist_remove_old_routers(void) /* Too old: remove it. (If we're a cache, just move it into * old_routers.) */ log_info(LD_DIR, - "Forgetting obsolete (too old) routerinfo for router '%s'", - router->nickname); + "Forgetting obsolete (too old) routerinfo for router %s", + router_describe(router)); routerlist_remove(routerlist, router, 1, now); i--; } @@ -4585,7 +4586,8 @@ update_consensus_router_descriptor_downloads(time_t now, int is_vote, if (oldrouter) format_iso_time(time_bufold, oldrouter->cache_info.published_on); log_info(LD_DIR, "Learned about %s (%s vs %s) from %s's vote (%s)", - rs->nickname, time_bufnew, + routerstatus_describe(rs), + time_bufnew, oldrouter ? time_bufold : "none", source->nickname, oldrouter ? "known" : "unknown"); } diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 389f22fd99..a877cdff84 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -1210,7 +1210,8 @@ router_parse_list_from_string(const char **s, const char *eos, prepend_annotations); if (router) { log_debug(LD_DIR, "Read router '%s', purpose '%s'", - router->nickname, router_purpose_to_string(router->purpose)); + router_describe(router), + router_purpose_to_string(router->purpose)); signed_desc = &router->cache_info; elt = router; } @@ -2488,7 +2489,7 @@ networkstatus_verify_bw_weights(networkstatus_t *ns) } } else { log_warn(LD_BUG, "Missing consensus bandwidth for router %s", - rs->nickname); + routerstatus_describe(rs)); } } SMARTLIST_FOREACH_END(rs); From b6eee716a88cba0d0ae4be4e542f61b2d300ecec Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 15 May 2011 22:02:24 -0400 Subject: [PATCH 02/12] changes file for bug3045 --- changes/bug3045 | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changes/bug3045 diff --git a/changes/bug3045 b/changes/bug3045 new file mode 100644 index 0000000000..1cbcabaff6 --- /dev/null +++ b/changes/bug3045 @@ -0,0 +1,6 @@ + o Minor features: + - Revise most log messages that refer to nodes by nickname to + instead use the "$key=nickname at address" format. This should be + more useful, especially since nicknames are less and less likely + to be unique. Fixes bug 3045. + From cfeafe5e77c9dd5587b1ec553eb1065f0bf841fd Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 23 May 2011 00:17:48 -0400 Subject: [PATCH 03/12] Use a 64-bit type to hold sockets on win64. On win64, sockets are of type UINT_PTR; on win32 they're u_int; elsewhere they're int. The correct windows way to check a socket for being set is to compare it with INVALID_SOCKET; elsewhere you see if it is negative. On Libevent 2, all callbacks take sockets as evutil_socket_t; we've been passing them int. This patch should fix compilation and correctness when built for 64-bit windows. Fixes bug 3270. --- changes/bug3270 | 4 +++ src/common/compat.c | 32 ++++++++++--------- src/common/compat.h | 21 ++++++++---- src/common/compat_libevent.h | 4 +++ src/common/util.c | 8 ++--- src/common/util.h | 4 +-- src/or/buffers.c | 8 ++--- src/or/buffers.h | 4 +-- src/or/connection.c | 50 +++++++++++++++-------------- src/or/control.c | 2 +- src/or/cpuworker.c | 10 +++--- src/or/dnsserv.c | 2 +- src/or/eventdns.c | 2 +- src/or/eventdns.h | 2 +- src/or/main.c | 62 +++++++++++++++++++----------------- src/or/or.h | 2 +- 16 files changed, 120 insertions(+), 97 deletions(-) create mode 100644 changes/bug3270 diff --git a/changes/bug3270 b/changes/bug3270 new file mode 100644 index 0000000000..b37bb983cc --- /dev/null +++ b/changes/bug3270 @@ -0,0 +1,4 @@ + o Minor bugfixes + - Use a wide type to hold sockets when built for 64-bit Windows builds. + Fixes bug 3270. + diff --git a/src/common/compat.c b/src/common/compat.c index fc066da681..9377959eb4 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -841,7 +841,7 @@ socket_accounting_unlock(void) * Windows, where close()ing a socket doesn't work. Returns 0 on success, -1 * on failure. */ int -tor_close_socket(int s) +tor_close_socket(tor_socket_t s) { int r = 0; @@ -894,8 +894,10 @@ tor_close_socket(int s) /** Helper: if DEBUG_SOCKET_COUNTING is enabled, remember that s is * now an open socket. */ static INLINE void -mark_socket_open(int s) +mark_socket_open(tor_socket_t s) { + /* XXXX This bitarray business will NOT work on windows: sockets aren't + small ints there. */ if (s > max_socket) { if (max_socket == -1) { open_sockets = bitarray_init_zero(s+128); @@ -917,11 +919,11 @@ mark_socket_open(int s) /** @} */ /** As socket(), but counts the number of open sockets. */ -int +tor_socket_t tor_open_socket(int domain, int type, int protocol) { - int s = socket(domain, type, protocol); - if (s >= 0) { + tor_socket_t s = socket(domain, type, protocol); + if (SOCKET_OK(s)) { socket_accounting_lock(); ++n_sockets_open; mark_socket_open(s); @@ -931,11 +933,11 @@ tor_open_socket(int domain, int type, int protocol) } /** As socket(), but counts the number of open sockets. */ -int +tor_socket_t tor_accept_socket(int sockfd, struct sockaddr *addr, socklen_t *len) { - int s = accept(sockfd, addr, len); - if (s >= 0) { + tor_socket_t s = accept(sockfd, addr, len); + if (SOCKET_OK(s)) { socket_accounting_lock(); ++n_sockets_open; mark_socket_open(s); @@ -958,7 +960,7 @@ get_n_open_sockets(void) /** Turn socket into a nonblocking socket. */ void -set_socket_nonblocking(int socket) +set_socket_nonblocking(tor_socket_t socket) { #if defined(MS_WINDOWS) unsigned long nonblocking = 1; @@ -986,7 +988,7 @@ set_socket_nonblocking(int socket) **/ /* It would be nicer just to set errno, but that won't work for windows. */ int -tor_socketpair(int family, int type, int protocol, int fd[2]) +tor_socketpair(int family, int type, int protocol, tor_socket_t fd[2]) { //don't use win32 socketpairs (they are always bad) #if defined(HAVE_SOCKETPAIR) && !defined(MS_WINDOWS) @@ -1011,9 +1013,9 @@ tor_socketpair(int family, int type, int protocol, int fd[2]) * for now, and really, when localhost is down sometimes, we * have other problems too. */ - int listener = -1; - int connector = -1; - int acceptor = -1; + tor_socket_t listener = -1; + tor_socket_t connector = -1; + tor_socket_t acceptor = -1; struct sockaddr_in listen_addr; struct sockaddr_in connect_addr; int size; @@ -2577,11 +2579,11 @@ in_main_thread(void) */ #if defined(MS_WINDOWS) int -tor_socket_errno(int sock) +tor_socket_errno(tor_socket_t sock) { int optval, optvallen=sizeof(optval); int err = WSAGetLastError(); - if (err == WSAEWOULDBLOCK && sock >= 0) { + if (err == WSAEWOULDBLOCK && SOCKET_OK(sock)) { if (getsockopt(sock, SOL_SOCKET, SO_ERROR, (void*)&optval, &optvallen)) return err; if (optval) diff --git a/src/common/compat.h b/src/common/compat.h index eff51ab30c..8f558e0ce5 100644 --- a/src/common/compat.h +++ b/src/common/compat.h @@ -390,9 +390,18 @@ int tor_fd_seekend(int fd); typedef int socklen_t; #endif -int tor_close_socket(int s); -int tor_open_socket(int domain, int type, int protocol); -int tor_accept_socket(int sockfd, struct sockaddr *addr, socklen_t *len); +#ifdef MS_WINDOWS +#define tor_socket_t intptr_t +#define SOCKET_OK(s) ((s) != INVALID_SOCKET) +#else +#define tor_socket_t int +#define SOCKET_OK(s) ((s) >= 0) +#endif + +int tor_close_socket(tor_socket_t s); +tor_socket_t tor_open_socket(int domain, int type, int protocol); +tor_socket_t tor_accept_socket(int sockfd, struct sockaddr *addr, + socklen_t *len); int get_n_open_sockets(void); #define tor_socket_send(s, buf, len, flags) send(s, buf, len, flags) @@ -464,8 +473,8 @@ int tor_inet_aton(const char *cp, struct in_addr *addr) ATTR_NONNULL((1,2)); const char *tor_inet_ntop(int af, const void *src, char *dst, size_t len); int tor_inet_pton(int af, const char *src, void *dst); int tor_lookup_hostname(const char *name, uint32_t *addr) ATTR_NONNULL((1,2)); -void set_socket_nonblocking(int socket); -int tor_socketpair(int family, int type, int protocol, int fd[2]); +void set_socket_nonblocking(tor_socket_t socket); +int tor_socketpair(int family, int type, int protocol, tor_socket_t fd[2]); int network_init(void); /* For stupid historical reasons, windows sockets have an independent @@ -492,7 +501,7 @@ int network_init(void); ((e) == WSAEMFILE || (e) == WSAENOBUFS) /** Return true if e is EADDRINUSE or the local equivalent. */ #define ERRNO_IS_EADDRINUSE(e) ((e) == WSAEADDRINUSE) -int tor_socket_errno(int sock); +int tor_socket_errno(tor_socket_t sock); const char *tor_socket_strerror(int e); #else #define ERRNO_IS_EAGAIN(e) ((e) == EAGAIN) diff --git a/src/common/compat_libevent.h b/src/common/compat_libevent.h index fdf5e0a18f..1decc8d5f9 100644 --- a/src/common/compat_libevent.h +++ b/src/common/compat_libevent.h @@ -9,11 +9,15 @@ struct event; struct event_base; + #ifdef HAVE_EVENT2_EVENT_H #include #else +#ifndef EVUTIL_SOCKET_DEFINED +#define EVUTIL_SOCKET_DEFINED #define evutil_socket_t int #endif +#endif void configure_libevent_logging(void); void suppress_libevent_log_msg(const char *msg); diff --git a/src/common/util.c b/src/common/util.c index 7ffe0a98cb..bf7bb73da8 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1568,7 +1568,7 @@ rate_limit_log(ratelim_t *lim, time_t now) * was returned by open(). Return the number of bytes written, or -1 * on error. Only use if fd is a blocking fd. */ ssize_t -write_all(int fd, const char *buf, size_t count, int isSocket) +write_all(tor_socket_t fd, const char *buf, size_t count, int isSocket) { size_t written = 0; ssize_t result; @@ -1578,7 +1578,7 @@ write_all(int fd, const char *buf, size_t count, int isSocket) if (isSocket) result = tor_socket_send(fd, buf+written, count-written, 0); else - result = write(fd, buf+written, count-written); + result = write((int)fd, buf+written, count-written); if (result<0) return -1; written += result; @@ -1592,7 +1592,7 @@ write_all(int fd, const char *buf, size_t count, int isSocket) * open(). Return the number of bytes read, or -1 on error. Only use * if fd is a blocking fd. */ ssize_t -read_all(int fd, char *buf, size_t count, int isSocket) +read_all(tor_socket_t fd, char *buf, size_t count, int isSocket) { size_t numread = 0; ssize_t result; @@ -1604,7 +1604,7 @@ read_all(int fd, char *buf, size_t count, int isSocket) if (isSocket) result = tor_socket_recv(fd, buf+numread, count-numread, 0); else - result = read(fd, buf+numread, count-numread); + result = read((int)fd, buf+numread, count-numread); if (result<0) return -1; else if (result == 0) diff --git a/src/common/util.h b/src/common/util.h index f32709accd..ebbcf78bdc 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -276,8 +276,8 @@ typedef struct ratelim_t { char *rate_limit_log(ratelim_t *lim, time_t now); /* File helpers */ -ssize_t write_all(int fd, const char *buf, size_t count, int isSocket); -ssize_t read_all(int fd, char *buf, size_t count, int isSocket); +ssize_t write_all(tor_socket_t fd, const char *buf, size_t count, int isSocket); +ssize_t read_all(tor_socket_t fd, char *buf, size_t count, int isSocket); /** Return values from file_status(); see that function's documentation * for details. */ diff --git a/src/or/buffers.c b/src/or/buffers.c index db926955b4..05163637f2 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -587,7 +587,7 @@ buf_add_chunk_with_capacity(buf_t *buf, size_t capacity, int capped) * *reached_eof to 1. Return -1 on error, 0 on eof or blocking, * and the number of bytes read otherwise. */ static INLINE int -read_to_chunk(buf_t *buf, chunk_t *chunk, int fd, size_t at_most, +read_to_chunk(buf_t *buf, chunk_t *chunk, tor_socket_t fd, size_t at_most, int *reached_eof, int *socket_error) { ssize_t read_result; @@ -668,7 +668,7 @@ read_to_chunk_tls(buf_t *buf, chunk_t *chunk, tor_tls_t *tls, */ /* XXXX023 indicate "read blocked" somehow? */ int -read_to_buf(int s, size_t at_most, buf_t *buf, int *reached_eof, +read_to_buf(tor_socket_t s, size_t at_most, buf_t *buf, int *reached_eof, int *socket_error) { /* XXXX023 It's stupid to overload the return values for these functions: @@ -767,7 +767,7 @@ read_to_buf_tls(tor_tls_t *tls, size_t at_most, buf_t *buf) * written on success, 0 on blocking, -1 on failure. */ static INLINE int -flush_chunk(int s, buf_t *buf, chunk_t *chunk, size_t sz, +flush_chunk(tor_socket_t s, buf_t *buf, chunk_t *chunk, size_t sz, size_t *buf_flushlen) { ssize_t write_result; @@ -854,7 +854,7 @@ flush_chunk_tls(tor_tls_t *tls, buf_t *buf, chunk_t *chunk, * -1 on failure. Return 0 if write() would block. */ int -flush_buf(int s, buf_t *buf, size_t sz, size_t *buf_flushlen) +flush_buf(tor_socket_t s, buf_t *buf, size_t sz, size_t *buf_flushlen) { /* XXXX023 It's stupid to overload the return values for these functions: * "error status" and "number of bytes flushed" are not mutually exclusive. diff --git a/src/or/buffers.h b/src/or/buffers.h index e50b9ff6fb..63fab4957a 100644 --- a/src/or/buffers.h +++ b/src/or/buffers.h @@ -24,11 +24,11 @@ size_t buf_datalen(const buf_t *buf); size_t buf_allocation(const buf_t *buf); size_t buf_slack(const buf_t *buf); -int read_to_buf(int s, size_t at_most, buf_t *buf, int *reached_eof, +int read_to_buf(tor_socket_t s, size_t at_most, buf_t *buf, int *reached_eof, int *socket_error); int read_to_buf_tls(tor_tls_t *tls, size_t at_most, buf_t *buf); -int flush_buf(int s, buf_t *buf, size_t sz, size_t *buf_flushlen); +int flush_buf(tor_socket_t s, buf_t *buf, size_t sz, size_t *buf_flushlen); int flush_buf_tls(tor_tls_t *tls, buf_t *buf, size_t sz, size_t *buf_flushlen); int write_to_buf(const char *string, size_t string_len, buf_t *buf); diff --git a/src/or/connection.c b/src/or/connection.c index 72414589c8..e58aba3d98 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -54,8 +54,8 @@ static int connection_reached_eof(connection_t *conn); static int connection_read_to_buf(connection_t *conn, ssize_t *max_to_read, int *socket_error); static int connection_process_inbuf(connection_t *conn, int package_partial); -static void client_check_address_changed(int sock); -static void set_constrained_socket_buffers(int sock, int size); +static void client_check_address_changed(tor_socket_t sock); +static void set_constrained_socket_buffers(tor_socket_t sock, int size); static const char *connection_proxy_state_to_string(int state); static int connection_read_https_proxy_response(connection_t *conn); @@ -439,8 +439,8 @@ _connection_free(connection_t *conn) rend_data_free(dir_conn->rend_data); } - if (conn->s >= 0) { - log_debug(LD_NET,"closing fd %d.",conn->s); + if (SOCKET_OK(conn->s)) { + log_debug(LD_NET,"closing fd %d.",(int)conn->s); tor_close_socket(conn->s); conn->s = -1; } @@ -663,14 +663,14 @@ connection_close_immediate(connection_t *conn) } if (conn->outbuf_flushlen) { log_info(LD_NET,"fd %d, type %s, state %s, %d bytes on outbuf.", - conn->s, conn_type_to_string(conn->type), + (int)conn->s, conn_type_to_string(conn->type), conn_state_to_string(conn->type, conn->state), (int)conn->outbuf_flushlen); } connection_unregister_events(conn); - if (conn->s >= 0) + if (SOCKET_OK(conn->s)) tor_close_socket(conn->s); conn->s = -1; if (conn->linked) @@ -740,7 +740,7 @@ connection_expire_held_open(void) log_fn(severity, LD_NET, "Giving up on marked_for_close conn that's been flushing " "for 15s (fd %d, type %s, state %s).", - conn->s, conn_type_to_string(conn->type), + (int)conn->s, conn_type_to_string(conn->type), conn_state_to_string(conn->type, conn->state)); conn->hold_open_until_flushed = 0; } @@ -893,7 +893,7 @@ check_location_for_unix_socket(or_options_t *options, const char *path) /** Tell the TCP stack that it shouldn't wait for a long time after * sock has closed before reusing its port. */ static void -make_socket_reuseable(int sock) +make_socket_reuseable(tor_socket_t sock) { #ifdef MS_WINDOWS (void) sock; @@ -921,7 +921,7 @@ connection_create_listener(const struct sockaddr *listensockaddr, int type, char* address) { connection_t *conn; - int s; /* the socket we're going to make */ + tor_socket_t s; /* the socket we're going to make */ uint16_t usePort = 0, gotPort = 0; int start_reading = 0; @@ -944,7 +944,7 @@ connection_create_listener(const struct sockaddr *listensockaddr, s = tor_open_socket(PF_INET, is_tcp ? SOCK_STREAM : SOCK_DGRAM, is_tcp ? IPPROTO_TCP: IPPROTO_UDP); - if (s < 0) { + if (!SOCKET_OK(s)) { log_warn(LD_NET,"Socket creation failed."); goto err; } @@ -1137,7 +1137,7 @@ check_sockaddr_family_match(sa_family_t got, connection_t *listener) static int connection_handle_listener_read(connection_t *conn, int new_type) { - int news; /* the new socket */ + tor_socket_t news; /* the new socket */ connection_t *newconn; /* information about the remote peer when connecting to other routers */ char addrbuf[256]; @@ -1150,7 +1150,7 @@ connection_handle_listener_read(connection_t *conn, int new_type) memset(addrbuf, 0, sizeof(addrbuf)); news = tor_accept_socket(conn->s,remote,&remotelen); - if (news < 0) { /* accept() error */ + if (!SOCKET_OK(news)) { /* accept() error */ int e = tor_socket_errno(conn->s); if (ERRNO_IS_ACCEPT_EAGAIN(e)) { return 0; /* he hung up before we could accept(). that's fine. */ @@ -1166,7 +1166,7 @@ connection_handle_listener_read(connection_t *conn, int new_type) } log_debug(LD_NET, "Connection accepted on socket %d (child of fd %d).", - news,conn->s); + (int)news,(int)conn->s); make_socket_reuseable(news); set_socket_nonblocking(news); @@ -1319,7 +1319,8 @@ int connection_connect(connection_t *conn, const char *address, const tor_addr_t *addr, uint16_t port, int *socket_error) { - int s, inprogress = 0; + tor_socket_t s; + int inprogress = 0; char addrbuf[256]; struct sockaddr *dest_addr; socklen_t dest_addr_len; @@ -2393,7 +2394,7 @@ connection_bucket_refill(int seconds_elapsed, time_t now) TO_OR_CONN(conn)->read_bucket > 0)) { /* and either a non-cell conn or a cell conn with non-empty bucket */ LOG_FN_CONN(conn, (LOG_DEBUG,LD_NET, - "waking up conn (fd %d) for read", conn->s)); + "waking up conn (fd %d) for read", (int)conn->s)); conn->read_blocked_on_bw = 0; connection_start_reading(conn); } @@ -2406,7 +2407,7 @@ connection_bucket_refill(int seconds_elapsed, time_t now) conn->state != OR_CONN_STATE_OPEN || TO_OR_CONN(conn)->write_bucket > 0)) { LOG_FN_CONN(conn, (LOG_DEBUG,LD_NET, - "waking up conn (fd %d) for write", conn->s)); + "waking up conn (fd %d) for write", (int)conn->s)); conn->write_blocked_on_bw = 0; connection_start_writing(conn); } @@ -2598,7 +2599,7 @@ connection_read_to_buf(connection_t *conn, ssize_t *max_to_read, log_debug(LD_NET, "%d: starting, inbuf_datalen %ld (%d pending in tls object)." " at_most %ld.", - conn->s,(long)buf_datalen(conn->inbuf), + (int)conn->s,(long)buf_datalen(conn->inbuf), tor_tls_get_pending_bytes(or_conn->tls), (long)at_most); initial_size = buf_datalen(conn->inbuf); @@ -2769,7 +2770,7 @@ connection_handle_write_impl(connection_t *conn, int force) tor_assert(!connection_is_listener(conn)); - if (conn->marked_for_close || conn->s < 0) + if (conn->marked_for_close || !SOCKET_OK(conn->s)) return 0; /* do nothing */ if (conn->in_flushed_some) { @@ -2985,12 +2986,13 @@ _connection_write_to_buf_impl(const char *string, size_t len, /* if it failed, it means we have our package/delivery windows set wrong compared to our max outbuf size. close the whole circuit. */ log_warn(LD_NET, - "write_to_buf failed. Closing circuit (fd %d).", conn->s); + "write_to_buf failed. Closing circuit (fd %d).", (int)conn->s); circuit_mark_for_close(circuit_get_by_edge_conn(TO_EDGE_CONN(conn)), END_CIRC_REASON_INTERNAL); } else { log_warn(LD_NET, - "write_to_buf failed. Closing connection (fd %d).", conn->s); + "write_to_buf failed. Closing connection (fd %d).", + (int)conn->s); connection_mark_for_close(conn); } return; @@ -3030,7 +3032,7 @@ _connection_write_to_buf_impl(const char *string, size_t len, /* this connection is broken. remove it. */ log_warn(LD_BUG, "unhandled error on write for " "conn (type %d, fd %d); removing", - conn->type, conn->s); + conn->type, (int)conn->s); tor_fragile_assert(); /* do a close-immediate here, so we don't try to flush */ connection_close_immediate(conn); @@ -3253,7 +3255,7 @@ alloc_http_authenticator(const char *authenticator) * call init_keys(). */ static void -client_check_address_changed(int sock) +client_check_address_changed(tor_socket_t sock) { uint32_t iface_ip, ip_out; /* host order */ struct sockaddr_in out_addr; @@ -3309,7 +3311,7 @@ client_check_address_changed(int sock) * to the desired size to stay below system TCP buffer limits. */ static void -set_constrained_socket_buffers(int sock, int size) +set_constrained_socket_buffers(tor_socket_t sock, int size) { void *sz = (void*)&size; socklen_t sz_sz = (socklen_t) sizeof(size); @@ -3541,7 +3543,7 @@ assert_connection_ok(connection_t *conn, time_t now) tor_assert(conn->linked); } if (conn->linked) - tor_assert(conn->s < 0); + tor_assert(!SOCKET_OK(conn->s)); if (conn->outbuf_flushlen > 0) { tor_assert(connection_is_writing(conn) || conn->write_blocked_on_bw || diff --git a/src/or/control.c b/src/or/control.c index 384e579f93..a595b2a638 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -1490,7 +1490,7 @@ getinfo_helper_listeners(control_connection_t *control_conn, struct sockaddr_storage ss; socklen_t ss_len = sizeof(ss); - if (conn->type != type || conn->marked_for_close || conn->s < 0) + if (conn->type != type || conn->marked_for_close || !SOCKET_OK(conn->s)) continue; if (getsockname(conn->s, (struct sockaddr *)&ss, &ss_len) < 0) { diff --git a/src/or/cpuworker.c b/src/or/cpuworker.c index 7cbc191333..c5e4863f7f 100644 --- a/src/or/cpuworker.c +++ b/src/or/cpuworker.c @@ -226,8 +226,8 @@ cpuworker_main(void *data) { char question[ONIONSKIN_CHALLENGE_LEN]; uint8_t question_type; - int *fdarray = data; - int fd; + tor_socket_t *fdarray = data; + tor_socket_t fd; /* variables for onion processing */ char keys[CPATH_KEY_MATERIAL_LEN]; @@ -317,12 +317,12 @@ cpuworker_main(void *data) static int spawn_cpuworker(void) { - int *fdarray; - int fd; + tor_socket_t *fdarray; + tor_socket_t fd; connection_t *conn; int err; - fdarray = tor_malloc(sizeof(int)*2); + fdarray = tor_malloc(sizeof(tor_socket_t)*2); if ((err = tor_socketpair(AF_UNIX, SOCK_STREAM, 0, fdarray)) < 0) { log_warn(LD_NET, "Couldn't construct socketpair for cpuworker: %s", tor_socket_strerror(-err)); diff --git a/src/or/dnsserv.c b/src/or/dnsserv.c index 243b730cbf..009ab5f344 100644 --- a/src/or/dnsserv.c +++ b/src/or/dnsserv.c @@ -306,7 +306,7 @@ void dnsserv_configure_listener(connection_t *conn) { tor_assert(conn); - tor_assert(conn->s >= 0); + tor_assert(SOCKET_OK(conn->s)); tor_assert(conn->type == CONN_TYPE_AP_DNS_LISTENER); conn->dns_server_port = diff --git a/src/or/eventdns.c b/src/or/eventdns.c index fc005df2d7..d06fb2cf21 100644 --- a/src/or/eventdns.c +++ b/src/or/eventdns.c @@ -1560,7 +1560,7 @@ evdns_request_data_build(const char *const name, const size_t name_len, /* exported function */ struct evdns_server_port * -evdns_add_server_port(int socket, int is_tcp, evdns_request_callback_fn_type cb, void *user_data) +evdns_add_server_port(tor_socket_t socket, int is_tcp, evdns_request_callback_fn_type cb, void *user_data) { struct evdns_server_port *port; if (!(port = mm_malloc(sizeof(struct evdns_server_port)))) diff --git a/src/or/eventdns.h b/src/or/eventdns.h index 2fe4ac9371..3ff8bba4b6 100644 --- a/src/or/eventdns.h +++ b/src/or/eventdns.h @@ -319,7 +319,7 @@ typedef void (*evdns_request_callback_fn_type)(struct evdns_server_request *, vo #define EVDNS_CLASS_INET 1 -struct evdns_server_port *evdns_add_server_port(int socket, int is_tcp, evdns_request_callback_fn_type callback, void *user_data); +struct evdns_server_port *evdns_add_server_port(tor_socket_t socket, int is_tcp, evdns_request_callback_fn_type callback, void *user_data); void evdns_close_server_port(struct evdns_server_port *port); int evdns_server_request_add_reply(struct evdns_server_request *req, int section, const char *name, int type, int class, int ttl, int datalen, int is_name, const char *data); diff --git a/src/or/main.c b/src/or/main.c index d1ceeec347..a8146e7993 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -62,8 +62,8 @@ void evdns_shutdown(int); static void dumpmemusage(int severity); static void dumpstats(int severity); /* log stats */ -static void conn_read_callback(int fd, short event, void *_conn); -static void conn_write_callback(int fd, short event, void *_conn); +static void conn_read_callback(evutil_socket_t fd, short event, void *_conn); +static void conn_write_callback(evutil_socket_t fd, short event, void *_conn); static void second_elapsed_callback(periodic_timer_t *timer, void *args); static int conn_close_if_marked(int i); static void connection_start_reading_from_linked_conn(connection_t *conn); @@ -158,7 +158,7 @@ int connection_add(connection_t *conn) { tor_assert(conn); - tor_assert(conn->s >= 0 || + tor_assert(SOCKET_OK(conn->s) || conn->linked || (conn->type == CONN_TYPE_AP && TO_EDGE_CONN(conn)->is_dns_request)); @@ -167,7 +167,7 @@ connection_add(connection_t *conn) conn->conn_array_index = smartlist_len(connection_array); smartlist_add(connection_array, conn); - if (conn->s >= 0 || conn->linked) { + if (SOCKET_OK(conn->s) || conn->linked) { conn->read_event = tor_event_new(tor_libevent_get_base(), conn->s, EV_READ|EV_PERSIST, conn_read_callback, conn); conn->write_event = tor_event_new(tor_libevent_get_base(), @@ -175,7 +175,7 @@ connection_add(connection_t *conn) } log_debug(LD_NET,"new conn type %s, socket %d, address %s, n_conns %d.", - conn_type_to_string(conn->type), conn->s, conn->address, + conn_type_to_string(conn->type), (int)conn->s, conn->address, smartlist_len(connection_array)); return 0; @@ -187,12 +187,12 @@ connection_unregister_events(connection_t *conn) { if (conn->read_event) { if (event_del(conn->read_event)) - log_warn(LD_BUG, "Error removing read event for %d", conn->s); + log_warn(LD_BUG, "Error removing read event for %d", (int)conn->s); tor_free(conn->read_event); } if (conn->write_event) { if (event_del(conn->write_event)) - log_warn(LD_BUG, "Error removing write event for %d", conn->s); + log_warn(LD_BUG, "Error removing write event for %d", (int)conn->s); tor_free(conn->write_event); } if (conn->dns_server_port) { @@ -213,7 +213,7 @@ connection_remove(connection_t *conn) tor_assert(conn); log_debug(LD_NET,"removing socket %d (type %s), n_conns now %d", - conn->s, conn_type_to_string(conn->type), + (int)conn->s, conn_type_to_string(conn->type), smartlist_len(connection_array)); tor_assert(conn->conn_array_index >= 0); @@ -344,7 +344,7 @@ connection_stop_reading(connection_t *conn) if (event_del(conn->read_event)) log_warn(LD_NET, "Error from libevent setting read event state for %d " "to unwatched: %s", - conn->s, + (int)conn->s, tor_socket_strerror(tor_socket_errno(conn->s))); } } @@ -364,7 +364,7 @@ connection_start_reading(connection_t *conn) if (event_add(conn->read_event, NULL)) log_warn(LD_NET, "Error from libevent setting read event state for %d " "to watched: %s", - conn->s, + (int)conn->s, tor_socket_strerror(tor_socket_errno(conn->s))); } } @@ -394,7 +394,7 @@ connection_stop_writing(connection_t *conn) if (event_del(conn->write_event)) log_warn(LD_NET, "Error from libevent setting write event state for %d " "to unwatched: %s", - conn->s, + (int)conn->s, tor_socket_strerror(tor_socket_errno(conn->s))); } } @@ -415,7 +415,7 @@ connection_start_writing(connection_t *conn) if (event_add(conn->write_event, NULL)) log_warn(LD_NET, "Error from libevent setting write event state for %d " "to watched: %s", - conn->s, + (int)conn->s, tor_socket_strerror(tor_socket_errno(conn->s))); } } @@ -501,13 +501,13 @@ close_closeable_connections(void) /** Libevent callback: this gets invoked when (connection_t*)conn has * some data to read. */ static void -conn_read_callback(int fd, short event, void *_conn) +conn_read_callback(evutil_socket_t fd, short event, void *_conn) { connection_t *conn = _conn; (void)fd; (void)event; - log_debug(LD_NET,"socket %d wants to read.",conn->s); + log_debug(LD_NET,"socket %d wants to read.",(int)conn->s); /* assert_connection_ok(conn, time(NULL)); */ @@ -516,7 +516,7 @@ conn_read_callback(int fd, short event, void *_conn) #ifndef MS_WINDOWS log_warn(LD_BUG,"Unhandled error on read for %s connection " "(fd %d); removing", - conn_type_to_string(conn->type), conn->s); + conn_type_to_string(conn->type), (int)conn->s); tor_fragile_assert(); #endif if (CONN_IS_EDGE(conn)) @@ -533,13 +533,14 @@ conn_read_callback(int fd, short event, void *_conn) /** Libevent callback: this gets invoked when (connection_t*)conn has * some data to write. */ static void -conn_write_callback(int fd, short events, void *_conn) +conn_write_callback(evutil_socket_t fd, short events, void *_conn) { connection_t *conn = _conn; (void)fd; (void)events; - LOG_FN_CONN(conn, (LOG_DEBUG, LD_NET, "socket %d wants to write.",conn->s)); + LOG_FN_CONN(conn, (LOG_DEBUG, LD_NET, "socket %d wants to write.", + (int)conn->s)); /* assert_connection_ok(conn, time(NULL)); */ @@ -548,7 +549,7 @@ conn_write_callback(int fd, short events, void *_conn) /* this connection is broken. remove it. */ log_fn(LOG_WARN,LD_BUG, "unhandled error on write for %s connection (fd %d); removing", - conn_type_to_string(conn->type), conn->s); + conn_type_to_string(conn->type), (int)conn->s); tor_fragile_assert(); if (CONN_IS_EDGE(conn)) { /* otherwise we cry wolf about duplicate close */ @@ -589,8 +590,9 @@ conn_close_if_marked(int i) assert_connection_ok(conn, now); /* assert_all_pending_dns_resolves_ok(); */ - log_debug(LD_NET,"Cleaning up connection (fd %d).",conn->s); - if ((conn->s >= 0 || conn->linked_conn) && connection_wants_to_flush(conn)) { + log_debug(LD_NET,"Cleaning up connection (fd %d).",(int)conn->s); + if ((SOCKET_OK(conn->s) || conn->linked_conn) + && connection_wants_to_flush(conn)) { /* s == -1 means it's an incomplete edge connection, or that the socket * has already been closed as unflushable. */ ssize_t sz = connection_bucket_write_limit(conn, now); @@ -599,7 +601,7 @@ conn_close_if_marked(int i) "Conn (addr %s, fd %d, type %s, state %d) marked, but wants " "to flush %d bytes. (Marked at %s:%d)", escaped_safe_str_client(conn->address), - conn->s, conn_type_to_string(conn->type), conn->state, + (int)conn->s, conn_type_to_string(conn->type), conn->state, (int)conn->outbuf_flushlen, conn->marked_for_close_file, conn->marked_for_close); if (conn->linked_conn) { @@ -630,7 +632,7 @@ conn_close_if_marked(int i) if (retval > 0) { LOG_FN_CONN(conn, (LOG_INFO,LD_NET, "Holding conn (fd %d) open for more flushing.", - conn->s)); + (int)conn->s)); conn->timestamp_lastwritten = now; /* reset so we can flush more */ } return 0; @@ -652,7 +654,7 @@ conn_close_if_marked(int i) "(fd %d, type %s, state %d, marked at %s:%d).", (int)buf_datalen(conn->outbuf), escaped_safe_str_client(conn->address), - conn->s, conn_type_to_string(conn->type), conn->state, + (int)conn->s, conn_type_to_string(conn->type), conn->state, conn->marked_for_close_file, conn->marked_for_close); } @@ -759,7 +761,7 @@ run_connection_housekeeping(int i, time_t now) (!DIR_CONN_IS_SERVER(conn) && conn->timestamp_lastread + DIR_CONN_MAX_STALL < now))) { log_info(LD_DIR,"Expiring wedged directory conn (fd %d, purpose %d)", - conn->s, conn->purpose); + (int)conn->s, conn->purpose); /* This check is temporary; it's to let us know whether we should consider * parsing partial serverdesc responses. */ if (conn->purpose == DIR_PURPOSE_FETCH_SERVERDESC && @@ -787,7 +789,7 @@ run_connection_housekeeping(int i, time_t now) * mark it now. */ log_info(LD_OR, "Expiring non-used OR connection to fd %d (%s:%d) [Too old].", - conn->s, conn->address, conn->port); + (int)conn->s, conn->address, conn->port); if (conn->state == OR_CONN_STATE_CONNECTING) connection_or_connect_failed(TO_OR_CONN(conn), END_OR_CONN_REASON_TIMEOUT, @@ -798,7 +800,7 @@ run_connection_housekeeping(int i, time_t now) if (past_keepalive) { /* We never managed to actually get this connection open and happy. */ log_info(LD_OR,"Expiring non-open OR connection to fd %d (%s:%d).", - conn->s,conn->address, conn->port); + (int)conn->s,conn->address, conn->port); connection_mark_for_close(conn); } } else if (we_are_hibernating() && !or_conn->n_circuits && @@ -806,14 +808,14 @@ run_connection_housekeeping(int i, time_t now) /* We're hibernating, there's no circuits, and nothing to flush.*/ log_info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) " "[Hibernating or exiting].", - conn->s,conn->address, conn->port); + (int)conn->s,conn->address, conn->port); connection_mark_for_close(conn); conn->hold_open_until_flushed = 1; } else if (!or_conn->n_circuits && now >= or_conn->timestamp_last_added_nonpadding + IDLE_OR_CONN_TIMEOUT) { log_info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) " - "[idle %d].", conn->s,conn->address, conn->port, + "[idle %d].", (int)conn->s,conn->address, conn->port, (int)(now - or_conn->timestamp_last_added_nonpadding)); connection_mark_for_close(conn); conn->hold_open_until_flushed = 1; @@ -823,7 +825,7 @@ run_connection_housekeeping(int i, time_t now) log_fn(LOG_PROTOCOL_WARN,LD_PROTOCOL, "Expiring stuck OR connection to fd %d (%s:%d). (%d bytes to " "flush; %d seconds since last write)", - conn->s, conn->address, conn->port, + (int)conn->s, conn->address, conn->port, (int)buf_datalen(conn->outbuf), (int)(now-conn->timestamp_lastwritten)); connection_mark_for_close(conn); @@ -1700,7 +1702,7 @@ dumpstats(int severity) int i = conn_sl_idx; log(severity, LD_GENERAL, "Conn %d (socket %d) type %d (%s), state %d (%s), created %d secs ago", - i, conn->s, conn->type, conn_type_to_string(conn->type), + i, (int)conn->s, conn->type, conn_type_to_string(conn->type), conn->state, conn_state_to_string(conn->type, conn->state), (int)(now - conn->timestamp_created)); if (!connection_is_listener(conn)) { diff --git a/src/or/or.h b/src/or/or.h index b9d8319ba5..ae56dcd9b4 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -970,7 +970,7 @@ typedef struct connection_t { unsigned int proxy_state:4; /** Our socket; -1 if this connection is closed, or has no socket. */ - evutil_socket_t s; + tor_socket_t s; int conn_array_index; /**< Index into the global connection array. */ struct event *read_event; /**< Libevent event structure. */ struct event *write_event; /**< Libevent event structure. */ From 95ac3ea5946f723ddab474fe229a872669aee47f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 17 May 2011 21:42:35 -0400 Subject: [PATCH 04/12] Don't build descriptors if ORPort auto is set and we have no OR listener This situation can happen easily if you set 'ORPort auto' and 'AccountingMax'. Doing so means that when you have no ORPort, you won't be able to set an ORPort in a descriptor, so instead you would just generate lots of invalid descriptors, freaking out all the time. Possible fix for 3216; fix on 0.2.2.26-beta. --- changes/bug3216 | 4 ++++ src/or/router.c | 4 ++++ 2 files changed, 8 insertions(+) create mode 100644 changes/bug3216 diff --git a/changes/bug3216 b/changes/bug3216 new file mode 100644 index 0000000000..599b5e162f --- /dev/null +++ b/changes/bug3216 @@ -0,0 +1,4 @@ + o Major bugfixes: + - Don't try to build descriptors if "ORPort auto" is set and we + don't know our actual ORPort yet. Fix for bug 3216; bugfix on + 0.2.2.26-beta. diff --git a/src/or/router.c b/src/or/router.c index 184715b750..464cba0dba 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -802,6 +802,8 @@ decide_to_advertise_dirport(or_options_t *options, uint16_t dir_port) return 0; if (!check_whether_dirport_reachable()) return 0; + if (!router_get_advertised_dir_port(options)) + return 0; /* Section two: reasons to publish or not publish that the user * might find surprising. These are generally config options that @@ -1136,6 +1138,8 @@ decide_if_publishable_server(void) return 0; if (authdir_mode(options)) return 1; + if (!router_get_advertised_or_port(options)) + return 0; return check_whether_orport_reachable(); } From 0e1fad648d22923d7455b94734df36585b8f624f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 24 May 2011 11:55:08 -0400 Subject: [PATCH 05/12] Don't try to build descriptors when router_get_advertised_or_port()==0 The previous attempt was incomplete: it told us not to publish a descriptor, but didn't stop us from generating one. Now we treat an absent OR port the same as not knowing our address. (This means that when we _do_ get an OR port, we need to mark the descriptor dirty.) More attempt to fix bug3216. --- src/or/connection.c | 26 +++++++++++++++++++------- src/or/router.c | 3 ++- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/or/connection.c b/src/or/connection.c index 4d3e897185..a0330e90d7 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -1949,37 +1949,40 @@ retry_all_listeners(smartlist_t *replaced_conns, smartlist_t *new_conns) { or_options_t *options = get_options(); + int retval = 0; + const uint16_t old_or_port = router_get_advertised_or_port(options); + const uint16_t old_dir_port = router_get_advertised_dir_port(options); if (retry_listeners(CONN_TYPE_OR_LISTENER, options->ORListenAddress, options->ORPort, "0.0.0.0", replaced_conns, new_conns, options->ClientOnly, AF_INET)<0) - return -1; + retval = -1; if (retry_listeners(CONN_TYPE_DIR_LISTENER, options->DirListenAddress, options->DirPort, "0.0.0.0", replaced_conns, new_conns, options->ClientOnly, AF_INET)<0) - return -1; + retval = -1; if (retry_listeners(CONN_TYPE_AP_LISTENER, options->SocksListenAddress, options->SocksPort, "127.0.0.1", replaced_conns, new_conns, 0, AF_INET)<0) - return -1; + retval = -1; if (retry_listeners(CONN_TYPE_AP_TRANS_LISTENER, options->TransListenAddress, options->TransPort, "127.0.0.1", replaced_conns, new_conns, 0, AF_INET)<0) - return -1; + retval = -1; if (retry_listeners(CONN_TYPE_AP_NATD_LISTENER, options->NATDListenAddress, options->NATDPort, "127.0.0.1", replaced_conns, new_conns, 0, AF_INET)<0) - return -1; + retval = -1; if (retry_listeners(CONN_TYPE_AP_DNS_LISTENER, options->DNSListenAddress, options->DNSPort, "127.0.0.1", replaced_conns, new_conns, 0, AF_INET)<0) - return -1; + retval = -1; if (retry_listeners(CONN_TYPE_CONTROL_LISTENER, options->ControlListenAddress, options->ControlPort, "127.0.0.1", @@ -1993,7 +1996,16 @@ retry_all_listeners(smartlist_t *replaced_conns, AF_UNIX)<0) return -1; - return 0; + if (old_or_port != router_get_advertised_or_port(options) || + old_dir_port != router_get_advertised_dir_port(options)) { + /* Our chosen ORPort or DirPort is not what it used to be: the + * descriptor we had (if any) should be regenerated. (We won't + * automatically notice this because of changes in the option, + * since the value could be "auto".) */ + mark_my_descriptor_dirty("Chosen Or/DirPort changed"); + } + + return retval; } /** Return 1 if we should apply rate limiting to conn, diff --git a/src/or/router.c b/src/or/router.c index 464cba0dba..5fd7b06afa 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1423,7 +1423,8 @@ router_rebuild_descriptor(int force) if (desc_clean_since && !force) return 0; - if (router_pick_published_address(options, &addr) < 0) { + if (router_pick_published_address(options, &addr) < 0 || + router_get_advertised_or_port(options) == 0) { /* Stop trying to rebuild our descriptor every second. We'll * learn that it's time to try again when ip_address_changed() * marks it dirty. */ From 112d204fadab74426cc802e6d998a53f938a5248 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Sun, 29 May 2011 08:03:41 -0700 Subject: [PATCH 06/12] Set timestamp_dirty on HS circuits as circuit_expire_building requires Fixes part of #1297; bugfix on 48e0228f1e031a709c1deb149c7dfd187c3609cf, when circuit_expire_building was changed to assume that timestamp_dirty was set when a circuit changed purpose to _C_REND_READY. (It wasn't.) --- changes/bug1297a | 8 ++++++++ src/or/or.h | 5 +++++ src/or/rendclient.c | 11 +++++++++++ 3 files changed, 24 insertions(+) create mode 100644 changes/bug1297a diff --git a/changes/bug1297a b/changes/bug1297a new file mode 100644 index 0000000000..459ef6534d --- /dev/null +++ b/changes/bug1297a @@ -0,0 +1,8 @@ + o Major bugfixes: + - Apply circuit timeouts to opened hidden-service-related circuits + based on the correct start time. Previously, we would apply the + circuit build timeout based on time since the circuit's + creation; it was supposed to be applied based on time since the + circuit entered its current state. Bugfix on 0.0.6; fixes part + of bug 1297. + diff --git a/src/or/or.h b/src/or/or.h index 9cac5f3548..6ccd95727c 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2143,6 +2143,11 @@ typedef struct circuit_t { * in time in order to indicate that a circuit shouldn't be used for new * streams, but that it can stay alive as long as it has streams on it. * That's a kludge we should fix. + * + * XXX023 The CBT code uses this field to record when HS-related + * circuits entered certain states. This usage probably won't + * interfere with this field's primary purpose, but we should + * document it more thoroughly to make sure of that. */ time_t timestamp_dirty; diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 77e11c2a07..a5423394f7 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -275,6 +275,10 @@ rend_client_send_introduction(origin_circuit_t *introcirc, /* Now, we wait for an ACK or NAK on this circuit. */ introcirc->_base.purpose = CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT; + /* Set timestamp_dirty, because circuit_expire_building expects it + * to specify when a circuit entered the _C_INTRODUCE_ACK_WAIT + * state. */ + introcirc->_base.timestamp_dirty = time(NULL); return 0; perm_err: @@ -329,6 +333,10 @@ rend_client_introduction_acked(origin_circuit_t *circ, circ->rend_data->onion_address, CIRCUIT_PURPOSE_C_REND_READY); if (rendcirc) { /* remember the ack */ rendcirc->_base.purpose = CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED; + /* Set timestamp_dirty, because circuit_expire_building expects + * it to specify when a circuit entered the + * _C_REND_READY_INTRO_ACKED state. */ + rendcirc->_base.timestamp_dirty = time(NULL); } else { log_info(LD_REND,"...Found no rend circ. Dropping on the floor."); } @@ -674,6 +682,9 @@ rend_client_rendezvous_acked(origin_circuit_t *circ, const uint8_t *request, log_info(LD_REND,"Got rendezvous ack. This circuit is now ready for " "rendezvous."); circ->_base.purpose = CIRCUIT_PURPOSE_C_REND_READY; + /* Set timestamp_dirty, because circuit_expire_building expects it + * to specify when a circuit entered the _C_REND_READY state. */ + circ->_base.timestamp_dirty = time(NULL); /* XXXX023 This is a pretty brute-force approach. It'd be better to * attach only the connections that are waiting on this circuit, rather * than trying to attach them all. See comments bug 743. */ From 8fc02a8e0ca431e4b61e54ed0aa10e7f0215c340 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Sun, 29 May 2011 08:21:43 -0700 Subject: [PATCH 07/12] Use the normal four-hop CBT for client intro circuits Fixes another part of bug 1297. --- changes/bug1297a | 8 ++++++++ src/or/circuituse.c | 11 ----------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/changes/bug1297a b/changes/bug1297a index 459ef6534d..140b94e3b0 100644 --- a/changes/bug1297a +++ b/changes/bug1297a @@ -5,4 +5,12 @@ creation; it was supposed to be applied based on time since the circuit entered its current state. Bugfix on 0.0.6; fixes part of bug 1297. + - Use the same circuit timeout for client-side introduction + circuits as for other four-hop circuits. Previously, + client-side introduction circuits were closed after the same + timeout as single-hop directory-fetch circuits; this was + appropriate with the static circuit build timeout in 0.2.1.x and + earlier, but caused many hidden service access attempts to fail + with the adaptive CBT introduced in 0.2.2.2-alpha. Bugfix on + 0.2.2.2-alpha; fixes another part of bug 1297. diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 7289aa5c11..97ac5600a2 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -288,7 +288,6 @@ circuit_expire_building(void) struct timeval general_cutoff, begindir_cutoff, fourhop_cutoff, cannibalize_cutoff, close_cutoff, extremely_old_cutoff; struct timeval now; - struct timeval introcirc_cutoff; cpath_build_state_t *build_state; tor_gettimeofday(&now); @@ -307,8 +306,6 @@ circuit_expire_building(void) SET_CUTOFF(close_cutoff, circ_times.close_ms); SET_CUTOFF(extremely_old_cutoff, circ_times.close_ms*2 + 1000); - introcirc_cutoff = begindir_cutoff; - while (next_circ) { struct timeval cutoff; victim = next_circ; @@ -325,8 +322,6 @@ circuit_expire_building(void) cutoff = fourhop_cutoff; else if (TO_ORIGIN_CIRCUIT(victim)->has_opened) cutoff = cannibalize_cutoff; - else if (victim->purpose == CIRCUIT_PURPOSE_C_INTRODUCING) - cutoff = introcirc_cutoff; else if (victim->purpose == CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT) cutoff = close_cutoff; else @@ -337,12 +332,6 @@ circuit_expire_building(void) #if 0 /* some debug logs, to help track bugs */ - if (victim->purpose == CIRCUIT_PURPOSE_C_INTRODUCING && - victim->timestamp_created <= introcirc_cutoff && - victim->timestamp_created > general_cutoff) - log_info(LD_REND|LD_CIRC, "Timing out introduction circuit which we " - "would not have done if it had been a general circuit."); - if (victim->purpose >= CIRCUIT_PURPOSE_C_INTRODUCING && victim->purpose <= CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED) { if (!victim->timestamp_dirty) From f08f0e9dde8e8910de5dc42d567629ff410c52f4 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 23 May 2011 16:38:35 -0400 Subject: [PATCH 08/12] Reinit keys at the start of options_act(). Previously we did this nearer to the end (in the old_options && transition_affects_workers() block). But other stuff cares about keys being consistent with options... particularly anything which tries to access a key, which can die in assert_identity_keys_ok(). Fixes bug 3228; bugfix on 0.2.2.18-alpha. --- changes/bug3228 | 3 +++ src/or/config.c | 23 +++++++++++++++-------- 2 files changed, 18 insertions(+), 8 deletions(-) create mode 100644 changes/bug3228 diff --git a/changes/bug3228 b/changes/bug3228 new file mode 100644 index 0000000000..4aca810d3c --- /dev/null +++ b/changes/bug3228 @@ -0,0 +1,3 @@ + o Major bugfixes: + - Resolve a crash that occured when setting BridgeRelay to 1 with + accounting enabled. Fixes bug 3228; bugfix on 0.2.2.18-alpha. diff --git a/src/or/config.c b/src/or/config.c index 46b0cef413..117925549e 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1172,12 +1172,26 @@ options_act(or_options_t *old_options) or_options_t *options = get_options(); int running_tor = options->command == CMD_RUN_TOR; char *msg; + const int transition_affects_workers = + old_options && options_transition_affects_workers(old_options, options); if (running_tor && !have_lockfile()) { if (try_locking(options, 1) < 0) return -1; } + /* We want to reinit keys as needed before we do much of anything else: + keys are important, and other things can depend on them. */ + if (running_tor && + (transition_affects_workers || + (options->V3AuthoritativeDir && (!old_options || + !old_options->V3AuthoritativeDir)))) { + if (init_keys() < 0) { + log_warn(LD_BUG,"Error initializing keys; exiting"); + return -1; + } + } + if (consider_adding_dir_authorities(options, old_options) < 0) return -1; @@ -1346,14 +1360,10 @@ options_act(or_options_t *old_options) } } - if (options_transition_affects_workers(old_options, options)) { + if (transition_affects_workers) { log_info(LD_GENERAL, "Worker-related options changed. Rotating workers."); - if (init_keys() < 0) { - log_warn(LD_BUG,"Error initializing keys; exiting"); - return -1; - } if (server_mode(options) && !server_mode(old_options)) { ip_address_changed(0); if (can_complete_circuit || !any_predicted_circuits(time(NULL))) @@ -1367,9 +1377,6 @@ options_act(or_options_t *old_options) return -1; } - if (options->V3AuthoritativeDir && !old_options->V3AuthoritativeDir) - init_keys(); - if (options->PerConnBWRate != old_options->PerConnBWRate || options->PerConnBWBurst != old_options->PerConnBWBurst) connection_or_update_token_buckets(get_connection_array(), options); From 3c0d944b076aa552216ca598a6cdc50e77ad1a58 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 30 May 2011 15:15:10 -0400 Subject: [PATCH 09/12] Improve comments and defensive programming for 3045 The comment fixes are trivial. The defensive programming trick is to tolerate receiving NULL inputs on the describe functions. That should never actually happen, but it seems like the likeliest mistake for us to make in the future. --- src/or/router.c | 13 +++++++++++-- src/or/router.h | 7 +++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/or/router.c b/src/or/router.c index a11d3a5923..0bd4c55026 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2223,8 +2223,7 @@ is_legal_hexdigest(const char *s) * and address addr or addr32h. * * The nickname and addr fields are optional and may be set to - * NULL. The addr32h field is optional and may be set to - * addr32h. + * NULL. The addr32h field is optional and may be set to 0. * * Return a pointer to the front of buf. */ @@ -2237,6 +2236,10 @@ format_node_description(char *buf, uint32_t addr32h) { char *cp; + + if (!buf) + return ""; + buf[0] = '$'; base16_encode(buf+1, HEX_DIGEST_LEN+1, id_digest, DIGEST_LEN); cp = buf+1+HEX_DIGEST_LEN; @@ -2268,6 +2271,8 @@ format_node_description(char *buf, const char * router_get_description(char *buf, const routerinfo_t *ri) { + if (!ri) + return ""; return format_node_description(buf, ri->cache_info.identity_digest, ri->is_named, @@ -2284,6 +2289,8 @@ router_get_description(char *buf, const routerinfo_t *ri) const char * routerstatus_get_description(char *buf, const routerstatus_t *rs) { + if (!rs) + return ""; return format_node_description(buf, rs->identity_digest, rs->is_named, @@ -2300,6 +2307,8 @@ routerstatus_get_description(char *buf, const routerstatus_t *rs) const char * extend_info_get_description(char *buf, const extend_info_t *ei) { + if (!ei) + return ""; return format_node_description(buf, ei->identity_digest, 0, diff --git a/src/or/router.h b/src/or/router.h index 6cabfd56e8..a285a3e773 100644 --- a/src/or/router.h +++ b/src/or/router.h @@ -86,6 +86,13 @@ int is_legal_nickname(const char *s); int is_legal_nickname_or_hexdigest(const char *s); int is_legal_hexdigest(const char *s); +/** + * Longest allowed output of format_node_description, plus 1 character for + * NUL. This allows space for: + * "$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF~xxxxxxxxxxxxxxxxxxx at" + * " [ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255]" + * plus a terminating NUL. + */ #define NODE_DESC_BUF_LEN (MAX_VERBOSE_NICKNAME_LEN+4+TOR_ADDR_BUF_LEN) const char *format_node_description(char *buf, const char *id_digest, From 7f0fb8e6083128ff20b30d8cd29c220e9427d95e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 30 May 2011 15:21:06 -0400 Subject: [PATCH 10/12] whitespace fixes --- src/common/compat_libevent.h | 5 +---- src/common/util.h | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/common/compat_libevent.h b/src/common/compat_libevent.h index 1decc8d5f9..89b256396b 100644 --- a/src/common/compat_libevent.h +++ b/src/common/compat_libevent.h @@ -9,15 +9,12 @@ struct event; struct event_base; - #ifdef HAVE_EVENT2_EVENT_H #include -#else -#ifndef EVUTIL_SOCKET_DEFINED +#elif !defined(EVUTIL_SOCKET_DEFINED) #define EVUTIL_SOCKET_DEFINED #define evutil_socket_t int #endif -#endif void configure_libevent_logging(void); void suppress_libevent_log_msg(const char *msg); diff --git a/src/common/util.h b/src/common/util.h index ebbcf78bdc..d657db674e 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -276,7 +276,7 @@ typedef struct ratelim_t { char *rate_limit_log(ratelim_t *lim, time_t now); /* File helpers */ -ssize_t write_all(tor_socket_t fd, const char *buf, size_t count, int isSocket); +ssize_t write_all(tor_socket_t fd, const char *buf, size_t count,int isSocket); ssize_t read_all(tor_socket_t fd, char *buf, size_t count, int isSocket); /** Return values from file_status(); see that function's documentation From f118dc80e0ef2eeb62ae52ed107af3384e2a1afd Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Mon, 30 May 2011 15:31:06 -0400 Subject: [PATCH 11/12] minor cleanups while reviewing 3216 --- src/or/config.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/or/config.c b/src/or/config.c index 117925549e..f97e9b1bea 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -211,7 +211,7 @@ static config_var_t _option_vars[] = { V(ControlPortFileGroupReadable,BOOL, "0"), V(ControlPortWriteToFile, FILENAME, NULL), V(ControlSocket, LINELIST, NULL), - V(ControlSocketsGroupWritable, BOOL, "0"), + V(ControlSocketsGroupWritable, BOOL, "0"), V(CookieAuthentication, BOOL, "0"), V(CookieAuthFileGroupReadable, BOOL, "0"), V(CookieAuthFile, STRING, NULL), @@ -2064,6 +2064,7 @@ get_assigned_option(config_format_t *fmt, void *options, escape_val = 0; break; } + /* fall through */ case CONFIG_TYPE_INTERVAL: case CONFIG_TYPE_UINT: /* This means every or_options_t uint or bool element From ca538290af507833c7cd5df7f663507c3540448d Mon Sep 17 00:00:00 2001 From: Sebastian Hahn Date: Sun, 29 May 2011 00:54:59 +0200 Subject: [PATCH 12/12] Warn when two hs use the same directory This simple implementation has a few issues, but it should do for 0.2.2.x. We will want to revisit this later and make it smarter. --- changes/bug3289 | 5 +++++ src/or/rendservice.c | 25 +++++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 changes/bug3289 diff --git a/changes/bug3289 b/changes/bug3289 new file mode 100644 index 0000000000..c469796d6e --- /dev/null +++ b/changes/bug3289 @@ -0,0 +1,5 @@ + o Minor bugfixes: + - Warn when the user configures two HiddenServiceDir lines that point + to the same directory. Bugfix on 0.0.6 (the version introducing + HiddenServiceDir); fixes bug 3289. + diff --git a/src/or/rendservice.c b/src/or/rendservice.c index edcf59d870..a10e43fead 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -181,6 +181,31 @@ rend_add_service(rend_service_t *service) log_warn(LD_CONFIG, "Hidden service with no ports configured; ignoring."); rend_service_free(service); } else { + int dupe = 0; + /* XXX This duplicate check has two problems: + * + * a) It's O(n^2), but the same comment from the bottom of + * rend_config_services() should apply. + * + * b) We only compare directory paths as strings, so we can't + * detect two distinct paths that specify the same directory + * (which can arise from symlinks, case-insensitivity, bind + * mounts, etc.). + * + * It also can't detect that two separate Tor instances are trying + * to use the same HiddenServiceDir; for that, we would need a + * lock file. But this is enough to detect a simple mistake that + * at least one person has actually made. + */ + SMARTLIST_FOREACH(rend_service_list, rend_service_t*, ptr, + dupe = dupe || + !strcmp(ptr->directory, service->directory)); + if (dupe) { + log_warn(LD_REND, "Another hidden service is already configured for " + "directory %s, ignoring.", service->directory); + rend_service_free(service); + return; + } smartlist_add(rend_service_list, service); log_debug(LD_REND,"Configuring service with directory \"%s\"", service->directory);