- tor-commits - lists.torproject.org

[tor/maint-0.2.2] Look at the right errno when sending reason for connect() failure
by nickm＠torproject.org 07 Apr '11

07 Apr '11

commit 6a5b94de6c51478ca638ec03c40a6d3c27f2d674 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 25 16:14:42 2011 -0400 Look at the right errno when sending reason for connect() failure In afe414 (tor-0.1.0.1-rc~173), when we moved to connection_edge_end_errno(), we used it in handling errors from connection_connect(). That's not so good, since by the time connection_connect() returns, the socket is no longer set, and we're supposed to be looking at the socket_errno return value from connection_connect() instead. So do what we should've done, and look at the socket_errno value that we get from connection_connect(). --- changes/connect_err_reporting | 6 ++++++ src/or/connection_edge.c | 8 ++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/changes/connect_err_reporting b/changes/connect_err_reporting new file mode 100644 index 0000000..61a46b6 --- /dev/null +++ b/changes/connect_err_reporting @@ -0,0 +1,6 @@ + o Minor bugfixes: + - Be more careful about reporting the correct error from a failed + connect() operation. Under some circumstances, it was possible to + look at an incorrect value for errno when sending the end reason. + Bugfix on Tor-0.1.0.1-rc. + diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 82b7139..c75c06b 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -2823,13 +2823,13 @@ connection_exit_connect(edge_connection_t *edge_conn) log_debug(LD_EXIT,"about to try connecting"); switch (connection_connect(conn, conn->address, addr, port, &socket_error)) { - case -1: - /* XXX022 use socket_error below rather than trying to piece things - * together from the current errno, which may have been clobbered. */ - connection_edge_end_errno(edge_conn); + case -1: { + int reason = errno_to_stream_end_reason(socket_error); + connection_edge_end(edge_conn, reason); circuit_detach_stream(circuit_get_by_edge_conn(edge_conn), edge_conn); connection_free(conn); return; + } case 0: conn->state = EXIT_CONN_STATE_CONNECTING;

1 0

[tor/maint-0.2.2] Fix some 'impossible' overflow bugs in byte counting
by nickm＠torproject.org 07 Apr '11

07 Apr '11

commit dddd333a80ee2e9bb731cb3c127ace3741d49673 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 25 16:45:25 2011 -0400 Fix some 'impossible' overflow bugs in byte counting The first was genuinely impossible, I think: it could only happen when the amount we read differed from the amount we wanted to read by more than INT_MAX. The second is just very unlikely: it would give incorrect results to the controller if you somehow wrote or read more than 4GB on one edge conn in one second. That one is a bugfix on 0.1.2.8-beta. --- changes/count_overflow | 5 +++++ src/or/connection.c | 36 ++++++++++++++++++++++-------------- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/changes/count_overflow b/changes/count_overflow new file mode 100644 index 0000000..f302ff2 --- /dev/null +++ b/changes/count_overflow @@ -0,0 +1,5 @@ + o Minor bugfixes: + - Correctly handle an "impossible" overflow cases in connection + byte counting, where we write or read more than 4GB on an edge + connection in single second. Bugfix on 0.1.2.8-beta. + diff --git a/src/or/connection.c b/src/or/connection.c index 7fa6cd9..084237d 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -51,7 +51,7 @@ static int connection_finished_flushing(connection_t *conn); static int connection_flushed_some(connection_t *conn); static int connection_finished_connecting(connection_t *conn); static int connection_reached_eof(connection_t *conn); -static int connection_read_to_buf(connection_t *conn, int *max_to_read, +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); @@ -2338,7 +2338,7 @@ connection_bucket_should_increase(int bucket, or_connection_t *conn) static int connection_handle_read_impl(connection_t *conn) { - int max_to_read=-1, try_to_read; + ssize_t max_to_read=-1, try_to_read; size_t before, n_read = 0; int socket_error = 0; @@ -2456,7 +2456,8 @@ connection_handle_read(connection_t *conn) * Return -1 if we want to break conn, else return 0. */ static int -connection_read_to_buf(connection_t *conn, int *max_to_read, int *socket_error) +connection_read_to_buf(connection_t *conn, ssize_t *max_to_read, + int *socket_error) { int result; ssize_t at_most = *max_to_read; @@ -2574,15 +2575,19 @@ connection_read_to_buf(connection_t *conn, int *max_to_read, int *socket_error) n_read = (size_t) result; } - if (n_read > 0) { /* change *max_to_read */ - /*XXXX022 check for overflow*/ - *max_to_read = (int)(at_most - n_read); - } + if (n_read > 0) { + /* change *max_to_read */ + *max_to_read = at_most - n_read; - if (conn->type == CONN_TYPE_AP) { - edge_connection_t *edge_conn = TO_EDGE_CONN(conn); - /*XXXX022 check for overflow*/ - edge_conn->n_read += (int)n_read; + /* Update edge_conn->n_read */ + if (conn->type == CONN_TYPE_AP) { + edge_connection_t *edge_conn = TO_EDGE_CONN(conn); + /* Check for overflow: */ + if (PREDICT_LIKELY(UINT32_MAX - edge_conn->n_read > n_read)) + edge_conn->n_read += (int)n_read; + else + edge_conn->n_read = UINT32_MAX; + } } connection_buckets_decrement(conn, approx_time(), n_read, n_written); @@ -2781,10 +2786,13 @@ connection_handle_write_impl(connection_t *conn, int force) n_written = (size_t) result; } - if (conn->type == CONN_TYPE_AP) { + if (n_written && conn->type == CONN_TYPE_AP) { edge_connection_t *edge_conn = TO_EDGE_CONN(conn); - /*XXXX022 check for overflow.*/ - edge_conn->n_written += (int)n_written; + /* Check for overflow: */ + if (PREDICT_LIKELY(UINT32_MAX - edge_conn->n_written > n_written)) + edge_conn->n_written += n_written; + else + edge_conn->n_written = UINT32_MAX; } connection_buckets_decrement(conn, approx_time(), n_read, n_written);

1 0

[tor/maint-0.2.2] Remove workaround code for bug539
by nickm＠torproject.org 07 Apr '11

07 Apr '11

commit 550749555cd57a1f82fe6c08e866ae14456ed439 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 25 17:11:04 2011 -0400 Remove workaround code for bug539 We fixed bug 539 (where directories would say "503" but send data anyway) back in 0.2.0.16-alpha/0.1.2.19. Because most directory versions were affected, we added workaround to make sure that we examined the contents of 503-replies to make sure there wasn't any data for them to find. But now that such routers are nonexistent, we can remove this code. (Even if somebody fired up an 0.1.2.19 directory cache today, it would still be fine to ignore data in its erroneous 503 replies.) --- changes/bug539_removal | 6 ++++++ src/or/directory.c | 33 +++++++++++++-------------------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/changes/bug539_removal b/changes/bug539_removal new file mode 100644 index 0000000..dbff43d --- /dev/null +++ b/changes/bug539_removal @@ -0,0 +1,6 @@ + o Removed code + - Removed workaround code to handle directory responses from + servers that had bug 539 (they would send HTTP status 503 + responses _and_ send a body too). Since only server versions + before 0.2.0.16-alpha/0.1.2.19 were affected, there is no longer + reason to keep the workaround in place. diff --git a/src/or/directory.c b/src/or/directory.c index 1347c8b..8f33a60 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -1539,26 +1539,19 @@ connection_dir_client_reached_eof(dir_connection_t *conn) (void) skewed; /* skewed isn't used yet. */ if (status_code == 503) { - if (body_len < 16) { - routerstatus_t *rs; - trusted_dir_server_t *ds; - log_info(LD_DIR,"Received http status code %d (%s) from server " - "'%s:%d'. I'll try again soon.", - status_code, escaped(reason), conn->_base.address, - conn->_base.port); - if ((rs = router_get_consensus_status_by_id(conn->identity_digest))) - rs->last_dir_503_at = now; - if ((ds = router_get_trusteddirserver_by_digest(conn->identity_digest))) - ds->fake_status.last_dir_503_at = now; + routerstatus_t *rs; + trusted_dir_server_t *ds; + log_info(LD_DIR,"Received http status code %d (%s) from server " + "'%s:%d'. I'll try again soon.", + status_code, escaped(reason), conn->_base.address, + conn->_base.port); + if ((rs = router_get_consensus_status_by_id(conn->identity_digest))) + rs->last_dir_503_at = now; + if ((ds = router_get_trusteddirserver_by_digest(conn->identity_digest))) + ds->fake_status.last_dir_503_at = now; - tor_free(body); tor_free(headers); tor_free(reason); - return -1; - } - /* XXXX022 Remove this once every server with bug 539 is obsolete. */ - log_info(LD_DIR, "Server at '%s:%d' sent us a 503 response, but included " - "a body anyway. We'll pretend it gave us a 200.", - conn->_base.address, conn->_base.port); - status_code = 200; + tor_free(body); tor_free(headers); tor_free(reason); + return -1; } plausible = body_is_plausible(body, body_len, conn->_base.purpose);

1 0

[tor/maint-0.2.2] Fix handling of StreamID exhaustion.
by nickm＠torproject.org 07 Apr '11

07 Apr '11

commit 432734279d3688fafb466a23f43585ff509ff693 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 25 17:57:15 2011 -0400 Fix handling of StreamID exhaustion. Since svn r1475/git 5b6099e8 in tor-0.0.6, we have responded to an exhaustion of all 65535 stream IDs on a circuit by marking that circuit for close. That's not the right response. Instead, we should mark the circuit as "too dirty for new circuits". Of course in reality this isn't really right either. If somebody has managed to cram 65535 streams onto a circuit, the circuit is probably not going to work well for any of those streams, so maybe we should be limiting the number of streams on an origin circuit concurrently. Also, closing the stream in this case is probably the wrong thing to do as well, but fixing that can also wait. --- changes/full_ap_circuits | 6 ++++++ src/or/connection_edge.c | 18 ++++++++++++++---- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/changes/full_ap_circuits b/changes/full_ap_circuits new file mode 100644 index 0000000..379a1a1 --- /dev/null +++ b/changes/full_ap_circuits @@ -0,0 +1,6 @@ + o Minor bugfixes + - When a client finds that an origin circuit has run out of 16-bit + stream IDs, we now mark it as unusable for new streams. + Previously, we would try to close the entire circuit. Bugfix on + Tor version 0.0.6. + diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index af0cfbe..72e2c8a 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -2164,9 +2164,14 @@ connection_ap_handshake_send_begin(edge_connection_t *ap_conn) ap_conn->stream_id = get_unique_stream_id_by_circ(circ); if (ap_conn->stream_id==0) { + /* XXXX023 Instead of closing this stream, we should make it get + * retried on another circuit. */ connection_mark_unattached_ap(ap_conn, END_STREAM_REASON_INTERNAL); - /*XXXX022 _close_ the circuit because it's full? That sounds dumb. */ - circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_RESOURCELIMIT); + + /* Mark this circuit "unusable for new streams". */ + /* XXXX023 this is a kludgy way to do this. */ + tor_assert(circ->_base.timestamp_dirty); + circ->_base.timestamp_dirty -= get_options()->MaxCircuitDirtiness; return -1; } @@ -2224,9 +2229,14 @@ connection_ap_handshake_send_resolve(edge_connection_t *ap_conn) ap_conn->stream_id = get_unique_stream_id_by_circ(circ); if (ap_conn->stream_id==0) { + /* XXXX023 Instead of closing this stream, we should make it get + * retried on another circuit. */ connection_mark_unattached_ap(ap_conn, END_STREAM_REASON_INTERNAL); - /*XXXX022 _close_ the circuit because it's full? That sounds dumb. */ - circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_RESOURCELIMIT); + + /* Mark this circuit "unusable for new streams". */ + /* XXXX023 this is a kludgy way to do this. */ + tor_assert(circ->_base.timestamp_dirty); + circ->_base.timestamp_dirty -= get_options()->MaxCircuitDirtiness; return -1; }

1 0

[tor/maint-0.2.2] Clarify some documentation and comments wrt resetting OR token buckets
by nickm＠torproject.org 07 Apr '11

07 Apr '11

commit d38030381b04263f76ce7f8ed2d6ceefc4ce363f Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 25 17:21:16 2011 -0400 Clarify some documentation and comments wrt resetting OR token buckets --- src/or/connection_or.c | 6 +++++- src/or/networkstatus.c | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/or/connection_or.c b/src/or/connection_or.c index ff863fd..4b932ec 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -353,6 +353,9 @@ connection_or_digest_is_known_relay(const char *id_digest) * per-conn limits that are big enough they'll never matter. But if it's * not a known relay, first check if we set PerConnBwRate/Burst, then * check if the consensus sets them, else default to 'big enough'. + * + * If <b>reset</b> is true, set the bucket to be full. Otherwise, just + * clip the bucket if it happens to be <em>too</em> full. */ static void connection_or_update_token_buckets_helper(or_connection_t *conn, int reset, @@ -392,7 +395,8 @@ connection_or_update_token_buckets_helper(or_connection_t *conn, int reset, } /** Either our set of relays or our per-conn rate limits have changed. - * Go through all the OR connections and update their token buckets. */ + * Go through all the OR connections and update their token buckets to make + * sure they don't exceed their maximum values. */ void connection_or_update_token_buckets(smartlist_t *conns, or_options_t *options) { diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 18cb477..4f6fe15 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1747,7 +1747,8 @@ networkstatus_set_current_consensus(const char *consensus, routerstatus_list_update_named_server_map(); cell_ewma_set_scale_factor(options, current_consensus); - /* XXX022 where is the right place to put this call? */ + /* XXXX023 this call might be unnecessary here: can changing the + * current consensus really alter our view of any OR's rate limits? */ connection_or_update_token_buckets(get_connection_array(), options); circuit_build_times_new_consensus_params(&circ_times, current_consensus);

1 0

[tor/maint-0.2.2] Add XXX023s for our timestamp_dirty abuse.
by nickm＠torproject.org 07 Apr '11

07 Apr '11

commit f3b89c114112987b74bb0632cfe47a3a17859adb Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 25 17:49:44 2011 -0400 Add XXX023s for our timestamp_dirty abuse. --- src/or/circuitlist.c | 3 +++ src/or/connection_edge.c | 2 ++ src/or/or.h | 10 ++++++++-- src/or/relay.c | 2 ++ 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index b4f5f45..521c21e 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -1023,6 +1023,7 @@ circuit_mark_all_unused_circs(void) * This is useful for letting the user change pseudonyms, so new * streams will not be linkable to old streams. */ +/* XXX023 this is a bad name for what this function does */ void circuit_expire_all_dirty_circs(void) { @@ -1033,6 +1034,8 @@ circuit_expire_all_dirty_circs(void) if (CIRCUIT_IS_ORIGIN(circ) && !circ->marked_for_close && circ->timestamp_dirty) + /* XXXX023 This is a screwed-up way to say "This is too dirty + * for new circuits. */ circ->timestamp_dirty -= options->MaxCircuitDirtiness; } } diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index c75c06b..af0cfbe 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -517,6 +517,7 @@ connection_ap_expire_beginning(void) /* kludge to make us not try this circuit again, yet to allow * current streams on it to survive if they can: make it * unattractive to use for new streams */ + /* XXXX023 this is a kludgy way to do this. */ tor_assert(circ->timestamp_dirty); circ->timestamp_dirty -= options->MaxCircuitDirtiness; /* give our stream another 'cutoff' seconds to try */ @@ -2164,6 +2165,7 @@ connection_ap_handshake_send_begin(edge_connection_t *ap_conn) ap_conn->stream_id = get_unique_stream_id_by_circ(circ); if (ap_conn->stream_id==0) { connection_mark_unattached_ap(ap_conn, END_STREAM_REASON_INTERNAL); + /*XXXX022 _close_ the circuit because it's full? That sounds dumb. */ circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_RESOURCELIMIT); return -1; } diff --git a/src/or/or.h b/src/or/or.h index 124892c..e44c626 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2127,8 +2127,14 @@ typedef struct circuit_t { char *n_conn_onionskin; time_t timestamp_created; /**< When was this circuit created? */ - time_t timestamp_dirty; /**< When the circuit was first used, or 0 if the - * circuit is clean. */ + /** When the circuit was first used, or 0 if the circuit is clean. + * + * XXXX023 Note that some code will artifically adjust this value backward + * 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. + */ + time_t timestamp_dirty; struct timeval highres_created; /**< When exactly was the circuit created? */ uint16_t marked_for_close; /**< Should we close this circuit at the end of diff --git a/src/or/relay.c b/src/or/relay.c index 076b46b..807cb3d 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -791,6 +791,8 @@ connection_ap_process_end_not_open( < MAX_RESOLVE_FAILURES) { /* We haven't retried too many times; reattach the connection. */ circuit_log_path(LOG_INFO,LD_APP,circ); + /* Mark this circuit "unusable for new streams". */ + /* XXXX023 this is a kludgy way to do this. */ tor_assert(circ->_base.timestamp_dirty); circ->_base.timestamp_dirty -= get_options()->MaxCircuitDirtiness;

1 0

[tor/master] Allow controllers a more up-to-date view of bridge usage.
by nickm＠torproject.org 07 Apr '11

07 Apr '11

commit 118d8ffdcb74137a36d22928ce6f46897809391e Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 15 22:39:22 2011 -0400 Allow controllers a more up-to-date view of bridge usage. Instead of answering GETINFO requests about our geoip usage only after running for 24 hours, this patch makes us answer GETINFO requests immediately. We still round and quantize as before. Implements bug2711. Also, refactor the heck out of the bridge usage formatting code. No longer should we need to do a generate-parse-and-regenerate cycle to get the controller string, and that lets us simplify the code a lot. --- changes/feature2711 | 4 + src/or/control.c | 4 +- src/or/geoip.c | 172 ++++++++++++++++++++++++++++----------------------- src/or/geoip.h | 2 +- 4 files changed, 101 insertions(+), 81 deletions(-) diff --git a/changes/feature2711 b/changes/feature2711 new file mode 100644 index 0000000..7cdcfbf --- /dev/null +++ b/changes/feature2711 @@ -0,0 +1,4 @@ + o Minor features + - Export GeoIP information on usage to bridge controller even if we have + not yet been running for 24 hours. + diff --git a/src/or/control.c b/src/or/control.c index fb9c1ae..8f3af0b 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -1785,12 +1785,12 @@ getinfo_helper_events(control_connection_t *control_conn, "information", question); } } else if (!strcmp(question, "status/clients-seen")) { - const char *bridge_stats = geoip_get_bridge_stats_controller(time(NULL)); + char *bridge_stats = geoip_get_bridge_stats_controller(time(NULL)); if (!bridge_stats) { *errmsg = "No bridge-client stats available"; return -1; } - *answer = tor_strdup(bridge_stats); + *answer = bridge_stats; } else { return 0; } diff --git a/src/or/geoip.c b/src/or/geoip.c index 654241c..28f570a 100644 --- a/src/or/geoip.c +++ b/src/or/geoip.c @@ -1083,14 +1083,14 @@ geoip_bridge_stats_term(void) start_of_bridge_stats_interval = 0; } -/** Parse the bridge statistics as they are written to extra-info - * descriptors for being returned to controller clients. Return the - * controller string if successful, or NULL otherwise. */ -static char * -parse_bridge_stats_controller(const char *stats_str, time_t now) +/** Validate a bridge statistics string as it would be written to a + * current extra-info descriptor. Return 1 if the string is valid and + * recent enough, or 0 otherwise. */ +static int +validate_bridge_stats(const char *stats_str, time_t now) { char stats_end_str[ISO_TIME_LEN+1], stats_start_str[ISO_TIME_LEN+1], - *controller_str, *eos, *eol, *summary; + *eos; const char *BRIDGE_STATS_END = "bridge-stats-end "; const char *BRIDGE_IPS = "bridge-ips "; @@ -1104,63 +1104,90 @@ parse_bridge_stats_controller(const char *stats_str, time_t now) "bridge-stats-end YYYY-MM-DD HH:MM:SS (N s)" */ tmp = find_str_at_start_of_line(stats_str, BRIDGE_STATS_END); if (!tmp) - return NULL; + return 0; tmp += strlen(BRIDGE_STATS_END); if (strlen(tmp) < ISO_TIME_LEN + 6) - return NULL; + return 0; strlcpy(stats_end_str, tmp, sizeof(stats_end_str)); if (parse_iso_time(stats_end_str, &stats_end_time) < 0) - return NULL; + return 0; if (stats_end_time < now - (25*60*60) || stats_end_time > now + (1*60*60)) - return NULL; + return 0; seconds = (int)strtol(tmp + ISO_TIME_LEN + 2, &eos, 10); if (!eos || seconds < 23*60*60) - return NULL; + return 0; format_iso_time(stats_start_str, stats_end_time - seconds); /* Parse: "bridge-ips CC=N,CC=N,..." */ tmp = find_str_at_start_of_line(stats_str, BRIDGE_IPS); - if (tmp) { - tmp += strlen(BRIDGE_IPS); - tmp = eat_whitespace_no_nl(tmp); - eol = strchr(tmp, '\n'); - if (eol) - summary = tor_strndup(tmp, eol-tmp); - else - summary = tor_strdup(tmp); - } else { + if (!tmp) { /* Look if there is an empty "bridge-ips" line */ tmp = find_str_at_start_of_line(stats_str, BRIDGE_IPS_EMPTY_LINE); if (!tmp) - return NULL; - summary = tor_strdup(""); + return 0; } - tor_asprintf(&controller_str, - "TimeStarted=\"%s\" CountrySummary=%s", - stats_start_str, summary); - tor_free(summary); - return controller_str; + return 1; } /** Most recent bridge statistics formatted to be written to extra-info * descriptors. */ static char *bridge_stats_extrainfo = NULL; -/** Most recent bridge statistics formatted to be returned to controller - * clients. */ -static char *bridge_stats_controller = NULL; +/** Return a newly allocated string holding our bridge usage stats by country + * in a format suitable for inclusion in an extrainfo document. Return NULL on + * failure. */ +static char * +format_bridge_stats_extrainfo(time_t now) +{ + char *out = NULL, *data = NULL; + long duration = now - start_of_bridge_stats_interval; + char written[ISO_TIME_LEN+1]; + + if (duration < 0) + return NULL; + + format_iso_time(written, now); + data = geoip_get_client_history(GEOIP_CLIENT_CONNECT); + + tor_asprintf(&out, + "bridge-stats-end %s (%ld s)\n" + "bridge-ips %s\n", + written, duration, + data ? data : ""); + tor_free(data); + + return out; +} + +/** Return a newly allocated string holding our bridge usage stats by country + * in a format suitable for the answer to a controller request. Return NULL on + * failure. */ +static char * +format_bridge_stats_controller(time_t now) +{ + char *out = NULL, *data = NULL; + char started[ISO_TIME_LEN+1]; + (void) now; + + format_iso_time(started, start_of_bridge_stats_interval); + data = geoip_get_client_history(GEOIP_CLIENT_CONNECT); + + tor_asprintf(&out, + "TimeStarted=\"%s\" CountrySummary=%s", + started, data ? data : ""); + tor_free(data); + return out; +} /** Write bridge statistics to $DATADIR/stats/bridge-stats and return * when we should next try to write statistics. */ time_t geoip_bridge_stats_write(time_t now) { - char *statsdir = NULL, *filename = NULL, *data = NULL, - written[ISO_TIME_LEN+1], *out = NULL, *controller_str; - size_t len; + char *filename = NULL, *val = NULL, *statsdir = NULL; /* Check if 24 hours have passed since starting measurements. */ if (now < start_of_bridge_stats_interval + WRITE_STATS_INTERVAL) @@ -1169,64 +1196,54 @@ geoip_bridge_stats_write(time_t now) /* Discard all items in the client history that are too old. */ geoip_remove_old_clients(start_of_bridge_stats_interval); + /* Generate formatted string */ + val = format_bridge_stats_extrainfo(now); + if (val == NULL) + goto done; + + /* Update the stored value. */ + tor_free(bridge_stats_extrainfo); + bridge_stats_extrainfo = val; + start_of_bridge_stats_interval = now; + + /* Write it to disk. */ statsdir = get_datadir_fname("stats"); if (check_private_dir(statsdir, CPD_CREATE) < 0) goto done; filename = get_datadir_fname2("stats", "bridge-stats"); - data = geoip_get_client_history(GEOIP_CLIENT_CONNECT); - format_iso_time(written, now); - len = strlen("bridge-stats-end (999999 s)\nbridge-ips \n") + - ISO_TIME_LEN + (data ? strlen(data) : 0) + 42; - out = tor_malloc(len); - if (tor_snprintf(out, len, "bridge-stats-end %s (%u s)\nbridge-ips %s\n", - written, (unsigned) (now - start_of_bridge_stats_interval), - data ? data : "") < 0) - goto done; - write_str_to_file(filename, out, 0); - controller_str = parse_bridge_stats_controller(out, now); - if (!controller_str) - goto done; - start_of_bridge_stats_interval = now; - tor_free(bridge_stats_extrainfo); - tor_free(bridge_stats_controller); - bridge_stats_extrainfo = out; - out = NULL; - bridge_stats_controller = controller_str; - control_event_clients_seen(controller_str); + + write_str_to_file(filename, bridge_stats_extrainfo, 0); + + /* Tell the controller, "hey, there are clients!" */ + { + char *controller_str = format_bridge_stats_controller(now); + if (controller_str) + control_event_clients_seen(controller_str); + tor_free(controller_str); + } done: tor_free(filename); tor_free(statsdir); - tor_free(data); - tor_free(out); - return start_of_bridge_stats_interval + - WRITE_STATS_INTERVAL; + + return start_of_bridge_stats_interval + WRITE_STATS_INTERVAL; } /** Try to load the most recent bridge statistics from disk, unless we - * have finished a measurement interval lately. */ + * have finished a measurement interval lately, and check whether they + * are still recent enough. */ static void load_bridge_stats(time_t now) { - char *statsdir, *fname=NULL, *contents, *controller_str; + char *fname, *contents; if (bridge_stats_extrainfo) return; - statsdir = get_datadir_fname("stats"); - if (check_private_dir(statsdir, CPD_CREATE) < 0) - goto done; + fname = get_datadir_fname2("stats", "bridge-stats"); contents = read_file_to_str(fname, RFTS_IGNORE_MISSING, NULL); - if (contents) { - controller_str = parse_bridge_stats_controller(contents, now); - if (controller_str) { - bridge_stats_extrainfo = contents; - bridge_stats_controller = controller_str; - } else { - tor_free(contents); - } - } - done: + if (contents && validate_bridge_stats(contents, now)) + bridge_stats_extrainfo = contents; + tor_free(fname); - tor_free(statsdir); } /** Return most recent bridge statistics for inclusion in extra-info @@ -1238,13 +1255,12 @@ geoip_get_bridge_stats_extrainfo(time_t now) return bridge_stats_extrainfo; } -/** Return most recent bridge statistics to be returned to controller - * clients, or NULL if we don't have recent bridge statistics. */ -const char * +/** Return a new string containing the recent bridge statistics to be returned + * to controller clients, or NULL if we don't have any bridge statistics. */ +char * geoip_get_bridge_stats_controller(time_t now) { - load_bridge_stats(now); - return bridge_stats_controller; + return format_bridge_stats_controller(now); } /** Start time of entry stats or 0 if we're not collecting entry diff --git a/src/or/geoip.h b/src/or/geoip.h index bafbeea..24f7c5b 100644 --- a/src/or/geoip.h +++ b/src/or/geoip.h @@ -51,7 +51,7 @@ void geoip_bridge_stats_init(time_t now); time_t geoip_bridge_stats_write(time_t now); void geoip_bridge_stats_term(void); const char *geoip_get_bridge_stats_extrainfo(time_t); -const char *geoip_get_bridge_stats_controller(time_t); +char *geoip_get_bridge_stats_controller(time_t); #endif

1 0

[tor/master] Merge remote-tracking branch 'origin/maint-0.2.2'
by nickm＠torproject.org 07 Apr '11

07 Apr '11

commit 07ab483e62457ee56187037304f25ec50cb0850a Merge: 09759a6 118d8ff Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Apr 7 11:55:17 2011 -0400 Merge remote-tracking branch 'origin/maint-0.2.2' changes/feature2711 | 4 + src/or/control.c | 4 +- src/or/geoip.c | 172 ++++++++++++++++++++++++++++----------------------- src/or/geoip.h | 2 +- 4 files changed, 101 insertions(+), 81 deletions(-)

1 0

[tor/maint-0.2.2] Allow controllers a more up-to-date view of bridge usage.
by nickm＠torproject.org 07 Apr '11

07 Apr '11

commit 118d8ffdcb74137a36d22928ce6f46897809391e Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 15 22:39:22 2011 -0400 Allow controllers a more up-to-date view of bridge usage. Instead of answering GETINFO requests about our geoip usage only after running for 24 hours, this patch makes us answer GETINFO requests immediately. We still round and quantize as before. Implements bug2711. Also, refactor the heck out of the bridge usage formatting code. No longer should we need to do a generate-parse-and-regenerate cycle to get the controller string, and that lets us simplify the code a lot. --- changes/feature2711 | 4 + src/or/control.c | 4 +- src/or/geoip.c | 172 ++++++++++++++++++++++++++++----------------------- src/or/geoip.h | 2 +- 4 files changed, 101 insertions(+), 81 deletions(-) diff --git a/changes/feature2711 b/changes/feature2711 new file mode 100644 index 0000000..7cdcfbf --- /dev/null +++ b/changes/feature2711 @@ -0,0 +1,4 @@ + o Minor features + - Export GeoIP information on usage to bridge controller even if we have + not yet been running for 24 hours. + diff --git a/src/or/control.c b/src/or/control.c index fb9c1ae..8f3af0b 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -1785,12 +1785,12 @@ getinfo_helper_events(control_connection_t *control_conn, "information", question); } } else if (!strcmp(question, "status/clients-seen")) { - const char *bridge_stats = geoip_get_bridge_stats_controller(time(NULL)); + char *bridge_stats = geoip_get_bridge_stats_controller(time(NULL)); if (!bridge_stats) { *errmsg = "No bridge-client stats available"; return -1; } - *answer = tor_strdup(bridge_stats); + *answer = bridge_stats; } else { return 0; } diff --git a/src/or/geoip.c b/src/or/geoip.c index 654241c..28f570a 100644 --- a/src/or/geoip.c +++ b/src/or/geoip.c @@ -1083,14 +1083,14 @@ geoip_bridge_stats_term(void) start_of_bridge_stats_interval = 0; } -/** Parse the bridge statistics as they are written to extra-info - * descriptors for being returned to controller clients. Return the - * controller string if successful, or NULL otherwise. */ -static char * -parse_bridge_stats_controller(const char *stats_str, time_t now) +/** Validate a bridge statistics string as it would be written to a + * current extra-info descriptor. Return 1 if the string is valid and + * recent enough, or 0 otherwise. */ +static int +validate_bridge_stats(const char *stats_str, time_t now) { char stats_end_str[ISO_TIME_LEN+1], stats_start_str[ISO_TIME_LEN+1], - *controller_str, *eos, *eol, *summary; + *eos; const char *BRIDGE_STATS_END = "bridge-stats-end "; const char *BRIDGE_IPS = "bridge-ips "; @@ -1104,63 +1104,90 @@ parse_bridge_stats_controller(const char *stats_str, time_t now) "bridge-stats-end YYYY-MM-DD HH:MM:SS (N s)" */ tmp = find_str_at_start_of_line(stats_str, BRIDGE_STATS_END); if (!tmp) - return NULL; + return 0; tmp += strlen(BRIDGE_STATS_END); if (strlen(tmp) < ISO_TIME_LEN + 6) - return NULL; + return 0; strlcpy(stats_end_str, tmp, sizeof(stats_end_str)); if (parse_iso_time(stats_end_str, &stats_end_time) < 0) - return NULL; + return 0; if (stats_end_time < now - (25*60*60) || stats_end_time > now + (1*60*60)) - return NULL; + return 0; seconds = (int)strtol(tmp + ISO_TIME_LEN + 2, &eos, 10); if (!eos || seconds < 23*60*60) - return NULL; + return 0; format_iso_time(stats_start_str, stats_end_time - seconds); /* Parse: "bridge-ips CC=N,CC=N,..." */ tmp = find_str_at_start_of_line(stats_str, BRIDGE_IPS); - if (tmp) { - tmp += strlen(BRIDGE_IPS); - tmp = eat_whitespace_no_nl(tmp); - eol = strchr(tmp, '\n'); - if (eol) - summary = tor_strndup(tmp, eol-tmp); - else - summary = tor_strdup(tmp); - } else { + if (!tmp) { /* Look if there is an empty "bridge-ips" line */ tmp = find_str_at_start_of_line(stats_str, BRIDGE_IPS_EMPTY_LINE); if (!tmp) - return NULL; - summary = tor_strdup(""); + return 0; } - tor_asprintf(&controller_str, - "TimeStarted=\"%s\" CountrySummary=%s", - stats_start_str, summary); - tor_free(summary); - return controller_str; + return 1; } /** Most recent bridge statistics formatted to be written to extra-info * descriptors. */ static char *bridge_stats_extrainfo = NULL; -/** Most recent bridge statistics formatted to be returned to controller - * clients. */ -static char *bridge_stats_controller = NULL; +/** Return a newly allocated string holding our bridge usage stats by country + * in a format suitable for inclusion in an extrainfo document. Return NULL on + * failure. */ +static char * +format_bridge_stats_extrainfo(time_t now) +{ + char *out = NULL, *data = NULL; + long duration = now - start_of_bridge_stats_interval; + char written[ISO_TIME_LEN+1]; + + if (duration < 0) + return NULL; + + format_iso_time(written, now); + data = geoip_get_client_history(GEOIP_CLIENT_CONNECT); + + tor_asprintf(&out, + "bridge-stats-end %s (%ld s)\n" + "bridge-ips %s\n", + written, duration, + data ? data : ""); + tor_free(data); + + return out; +} + +/** Return a newly allocated string holding our bridge usage stats by country + * in a format suitable for the answer to a controller request. Return NULL on + * failure. */ +static char * +format_bridge_stats_controller(time_t now) +{ + char *out = NULL, *data = NULL; + char started[ISO_TIME_LEN+1]; + (void) now; + + format_iso_time(started, start_of_bridge_stats_interval); + data = geoip_get_client_history(GEOIP_CLIENT_CONNECT); + + tor_asprintf(&out, + "TimeStarted=\"%s\" CountrySummary=%s", + started, data ? data : ""); + tor_free(data); + return out; +} /** Write bridge statistics to $DATADIR/stats/bridge-stats and return * when we should next try to write statistics. */ time_t geoip_bridge_stats_write(time_t now) { - char *statsdir = NULL, *filename = NULL, *data = NULL, - written[ISO_TIME_LEN+1], *out = NULL, *controller_str; - size_t len; + char *filename = NULL, *val = NULL, *statsdir = NULL; /* Check if 24 hours have passed since starting measurements. */ if (now < start_of_bridge_stats_interval + WRITE_STATS_INTERVAL) @@ -1169,64 +1196,54 @@ geoip_bridge_stats_write(time_t now) /* Discard all items in the client history that are too old. */ geoip_remove_old_clients(start_of_bridge_stats_interval); + /* Generate formatted string */ + val = format_bridge_stats_extrainfo(now); + if (val == NULL) + goto done; + + /* Update the stored value. */ + tor_free(bridge_stats_extrainfo); + bridge_stats_extrainfo = val; + start_of_bridge_stats_interval = now; + + /* Write it to disk. */ statsdir = get_datadir_fname("stats"); if (check_private_dir(statsdir, CPD_CREATE) < 0) goto done; filename = get_datadir_fname2("stats", "bridge-stats"); - data = geoip_get_client_history(GEOIP_CLIENT_CONNECT); - format_iso_time(written, now); - len = strlen("bridge-stats-end (999999 s)\nbridge-ips \n") + - ISO_TIME_LEN + (data ? strlen(data) : 0) + 42; - out = tor_malloc(len); - if (tor_snprintf(out, len, "bridge-stats-end %s (%u s)\nbridge-ips %s\n", - written, (unsigned) (now - start_of_bridge_stats_interval), - data ? data : "") < 0) - goto done; - write_str_to_file(filename, out, 0); - controller_str = parse_bridge_stats_controller(out, now); - if (!controller_str) - goto done; - start_of_bridge_stats_interval = now; - tor_free(bridge_stats_extrainfo); - tor_free(bridge_stats_controller); - bridge_stats_extrainfo = out; - out = NULL; - bridge_stats_controller = controller_str; - control_event_clients_seen(controller_str); + + write_str_to_file(filename, bridge_stats_extrainfo, 0); + + /* Tell the controller, "hey, there are clients!" */ + { + char *controller_str = format_bridge_stats_controller(now); + if (controller_str) + control_event_clients_seen(controller_str); + tor_free(controller_str); + } done: tor_free(filename); tor_free(statsdir); - tor_free(data); - tor_free(out); - return start_of_bridge_stats_interval + - WRITE_STATS_INTERVAL; + + return start_of_bridge_stats_interval + WRITE_STATS_INTERVAL; } /** Try to load the most recent bridge statistics from disk, unless we - * have finished a measurement interval lately. */ + * have finished a measurement interval lately, and check whether they + * are still recent enough. */ static void load_bridge_stats(time_t now) { - char *statsdir, *fname=NULL, *contents, *controller_str; + char *fname, *contents; if (bridge_stats_extrainfo) return; - statsdir = get_datadir_fname("stats"); - if (check_private_dir(statsdir, CPD_CREATE) < 0) - goto done; + fname = get_datadir_fname2("stats", "bridge-stats"); contents = read_file_to_str(fname, RFTS_IGNORE_MISSING, NULL); - if (contents) { - controller_str = parse_bridge_stats_controller(contents, now); - if (controller_str) { - bridge_stats_extrainfo = contents; - bridge_stats_controller = controller_str; - } else { - tor_free(contents); - } - } - done: + if (contents && validate_bridge_stats(contents, now)) + bridge_stats_extrainfo = contents; + tor_free(fname); - tor_free(statsdir); } /** Return most recent bridge statistics for inclusion in extra-info @@ -1238,13 +1255,12 @@ geoip_get_bridge_stats_extrainfo(time_t now) return bridge_stats_extrainfo; } -/** Return most recent bridge statistics to be returned to controller - * clients, or NULL if we don't have recent bridge statistics. */ -const char * +/** Return a new string containing the recent bridge statistics to be returned + * to controller clients, or NULL if we don't have any bridge statistics. */ +char * geoip_get_bridge_stats_controller(time_t now) { - load_bridge_stats(now); - return bridge_stats_controller; + return format_bridge_stats_controller(now); } /** Start time of entry stats or 0 if we're not collecting entry diff --git a/src/or/geoip.h b/src/or/geoip.h index bafbeea..24f7c5b 100644 --- a/src/or/geoip.h +++ b/src/or/geoip.h @@ -51,7 +51,7 @@ void geoip_bridge_stats_init(time_t now); time_t geoip_bridge_stats_write(time_t now); void geoip_bridge_stats_term(void); const char *geoip_get_bridge_stats_extrainfo(time_t); -const char *geoip_get_bridge_stats_controller(time_t); +char *geoip_get_bridge_stats_controller(time_t); #endif

1 0

[tor/master] Fix a compile warning on OS X 10.6
by nickm＠torproject.org 07 Apr '11

07 Apr '11

commit 286d44402e829488abe802efd1607eabbc98f1d1 Author: Sebastian Hahn <sebastian(a)torproject.org> Date: Thu Apr 7 05:30:49 2011 +0200 Fix a compile warning on OS X 10.6 --- src/or/circuituse.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 447ec84..3d5db98 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -291,7 +291,7 @@ circuit_expire_building(void) long ms = tor_lround(msec); \ struct timeval diff; \ diff.tv_sec = ms / 1000; \ - diff.tv_usec = (ms % 1000) * 1000; \ + diff.tv_usec = (int)((ms % 1000) * 1000); \ timersub(&now, &diff, &target); \ } while (0)

1 0