- tor-commits - lists.torproject.org

[metrics-web/master] Add Roger's blog post to the papers page.
by karsten＠torproject.org 07 Apr '11

07 Apr '11

commit 2d1a8c13b918b5c411a3e2756e3172aed31dbd97 Author: Karsten Loesing <karsten.loesing(a)gmx.net> Date: Thu Apr 7 21:32:45 2011 +0200 Add Roger's blog post to the papers page. --- web/WEB-INF/papers.jsp | 21 +++++++++++++++++---- 1 files changed, 17 insertions(+), 4 deletions(-) diff --git a/web/WEB-INF/papers.jsp b/web/WEB-INF/papers.jsp index 1880483..50f8bda 100644 --- a/web/WEB-INF/papers.jsp +++ b/web/WEB-INF/papers.jsp @@ -12,10 +12,11 @@ <div class="main-column"> <h2>Tor Metrics Portal: Papers</h2> <br> - <p>The <a href="#papers">papers</a> and - <a href="#techreports">technical reports</a> listed on this page - originate from, or are based on, work performed in the Tor Metrics - Project.</p> + <p>The <a href="#papers">papers</a>, + <a href="#techreports">technical reports</a>, and + <a href="#blogposts">blog posts</a> listed on this page originate + from, are based on, or are related to work performed in the Tor + Metrics Project.</p> <br> <a name="papers"></a> <h3>Papers</h3> @@ -81,6 +82,18 @@ (<a href="papers/dirarch-2009-06-22.pdf">PDF</a>, 558K, last updated on June 22, 2009).</li> </ul> + <br> + <a name="blogposts"></a> + <h3>Blog posts</h3> + <br> + The following blog posts are either the results of metrics + research or describe new interesting research questions that can + (partly) be answered with metrics data. + <ul> + <li>Research problem: measuring the safety of the Tor network + (<a href="https://blog.torproject.org/blog/research-problem-measuring-safety-tor-netw…">link</a>, + February 5, 2011).</li> + </ul> </div> </div> <div class="bottom" id="bottom">

1 0

[torspec/master] add mike's triangle boy analysis
by arma＠torproject.org 07 Apr '11

07 Apr '11

commit 3b5b8804f64a4db7ec7fc0185ea1afb7a2713797 Author: Roger Dingledine <arma(a)torproject.org> Date: Thu Apr 7 12:35:28 2011 -0400 add mike's triangle boy analysis --- proposals/ideas/xxx-triangleboy-transport.txt | 194 +++++++++++++++++++++++++ 1 files changed, 194 insertions(+), 0 deletions(-) diff --git a/proposals/ideas/xxx-triangleboy-transport.txt b/proposals/ideas/xxx-triangleboy-transport.txt new file mode 100644 index 0000000..e09eaac --- /dev/null +++ b/proposals/ideas/xxx-triangleboy-transport.txt @@ -0,0 +1,194 @@ +[See also http://archives.seul.org/tor/dev/Feb-2011/msg00000.html and +its follow-ups.] + +Filename: ideas/xxx-triangleboy-transport.txt +Title: The Triangle Boy Transport +Author: Mike Perry +Created: 31-Jan-2011 +Status: Draft + + +Introduction + +In this proposal, we describe a transport plugin known as the Triangle +Boy transport: +http://www.webrant.com/safeweb_site/html/www/tboy_whitepaper.html + +The basic idea is to be able to utilize an asymmetric residential IP +with minimal CPU and memory resources, and limited upstream. We would +use the bridge for only half of the stream, avoiding the upstream +bottleneck of most residential connections. The return direction +will be spoofed by a supporting high-capacity Tor relay. + + +Overview + +The high level packet flow diagram would look like this. + + Client -> Client Plugin -> Transport Bridge -> Relay Plugin -> Relay + Client <- Client Plugin <--(spoof)--- Relay Plugin <- Relay + +Testing in the field has demonstrated that most residential +connections also perform egress filtering, where as many colo centers +do not. This means that the spoofing logic needs to be present at the +Relay endpoint, and not in the Transport Bridge (though the Transport +Bridge will still need raw sockets to listen for TCP fragments). + +It is this egress filter constraint that requires that this transport +use the pluggable-transport spec, as opposed to creating a very simple +spoofing Transport Bridge. + +There are two possible designs for the operation of the Transport +Bridge. The bridge can either be peered with a relay of its choosing, +or it can be peered with a client-specified relay. Since using a +client-specified relay in a censored location is prone to failure due +to unobservable changing network conditions, we will consider the case +where the Transport Bridge chooses which relay to forward the +connections to. + +The client will use the Transport Bridge IP with a bridge line like: + "bridge TriangleBoy address:port" + +On the client side, the TriangleBoy transport keyword would only mean +that the Tor client should not try to cache the identity key of the +bridge in question, nor should it perform any strict checks to ensure +that the IP reported by the relay is consistent or matches its +endpoint. + +Other than these basic loosened constraints, it should be able to use +the Transport Bridge IP as if it were any other bridge. This is +uniquely different from the what the pluggable-transport spec assumes. +Most of the complexity involved in supporting this transport will be +in the Transport Bridge, and in the Relay Plugin side of the equation. + + +Transport Bridge Operation + +The Transport Bridge will require raw sockets to operate. It will also +require a tor client to fetch and authenticate the Tor directory. + +The current pluggable transport spec suggests that extrainfo +descriptors be used to describe which transport plugins a bridge +supports. This would require that the Transport Bridge set +DownloadExtraInfo=1 in its torrc. + +The extrainfo document would need to list the following information: +TriangleBoyTransport ip:port +TriangleBoyCert <Certificate> + +The Transport Bridge would attempt to select an appropriate relay or +relays to forward all client connections. + +The forwarding operation by the Transport Bridge would involve a TLS +connection to the ip and port specified in the extrainfo document and +authenticated by the certificate there, and speak cells of the +following format: + + <data_len>: 2 bytes + <data>: data_len bytes + +Each new raw TCP packet that arrives at the transport bridge would be +packaged up according to this scheme. The data portion of the packet +would be the raw IP frame received by the transport bridge. + +The TLS connection can be reused to multiplex many different raw +datagrams for many different clients. + + +Relay Plugin Startup + +Not all relays can support the TriangleBoy transport. Many relays are +behind egress filters that will prevent the transmission of spoofed +packets. The transport plugin should be robust enough to be able to +determine if it can properly spoof the packets. This involves both +ensuring that the plugin process has sufficient permissions, and also +sending some spoofed test packets to a server to receive the response. + +If either of these tests fail, the plugin needs to inform Tor of this +fact so that Tor does *not* provide the lines for the TriangleBoy +transport in its extrainfo document. + +The relay would configure the plugin using its torrc: + +ServerTransportPlugin TriangleBoy /path/to/triangleboy ip=<ip> port=<port> \ + tor_ip=<ip> tor_port=<port> free_rfc1918=<ip> + +Tor would launch the triangleboy process, which would perform the +appropriate uid checks, and send test spoofed packets to $SERVER (the +bridgedb?). If the tests pass, it would reply with: + +SMETHOD: TriangleBoy <ip>:<port> \ + EXTRAINFO:TriangleBoyTransport=<ip>:<port>,TriangleBoyCert=<Certificate> +METHODS: DONE + +If there are any errors with permissions or spoofing, the transport +should reply with: + +METHODS: NONE + +and then exit. + + +Relay Plugin Operation + +The relay plugin will listen for TLS connections on its port, using +the certificate it created and sent to Tor during startup. + +It will perform some verification on the integrity of incoming +datagrams: ensuring that they are in fact TCP/IP, that their destination +IP was in fact the Transport Bridge, and that their source IP is in +publicly routable IP space. + +It would then perform a form of NAT in order to translate the raw IP +frames coming in to be sourced from the free_rfc1918 IP (specified in +the configuration line) and a random free port. It will maintain an +internal mapping between these randomized local source ports and the +originating Client IP and port (as well as the Transport Bridge IP). + +The Relay will then proceed believing that it has a TCP connection +from an unused, local rfc1918 ip and source port. This ip+port will +not actually exist, but instead will be handled by a raw socket +listening in the Relay Plugin. + +Upon receiving packets from the Tor process on its raw socket, the +Relay Plugin will use the destination port to look up the original +Client IP and port, and the Transport Bridge IP. It will rewrite the +packets to spoof the source IP as the Transport Bridge IP, and set the +destination IP and port as the one it found for the Client in the its +NAT table. + +In this way, the NAT will properly seamlessly translate packets for +the Relay, and the Client will communicate with what it believes to be +the Transport Bridge IP, while using the capacity of the supporting +Tor Relay for the downstream direction. + + +Appendix: List of key xxx-pluggable-transport.txt shortcomings + +1. This pluggable transport does not need any intelligence or process +launching on the Client side, aside from a way to tell Tor not to be +so pedantic about ensuring identity key and IP address consistency. + +2. The relay side needs to be able to detect if it has both the +permissions and the network ability to send spoofed packets. It needs +to communicate this fact with the Relay Plugin by responding with the +appropriate extrainfo lines, or with "METHODS: NONE" to indicate +error. This relay-side handshake should be specified in the +pluggable-transport spec. + +3. The relay plugin side needs some way to communicate EXTRAINFO lines +to be added to its extrainfo descriptor. In this proposal, we use the +SMETHOD reply to do this. + +4. Is extrainfo really the best place to keep this information? +Shouldn't it just be in the relay/bridge descriptor? Putting it in +extrainfo requires our TransportBridges to enable the wasteful +DownloadExtraInfo=1 torrc setting, which will consume more scarce +resources and RAM on what will probably be cheap routers with 32-64M +of RAM. + +5. How would we go about chaining an actual obfuscation mechanism with +this transport? Would we just create new and separate transport called +TriangleBoyOverHTTP, for example, or is there a better way to chain +different mechanisms? +

1 0

[tor/master] 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/master] Add a missing cast to silence the compiler
by nickm＠torproject.org 07 Apr '11

07 Apr '11

commit 8d81831d715adf9cb374100e23cc2b118efe3101 Author: Sebastian Hahn <sebastian(a)torproject.org> Date: Mon Mar 28 18:20:50 2011 +0200 Add a missing cast to silence the compiler --- src/or/connection.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/or/connection.c b/src/or/connection.c index 084237d..6e7bbd5 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -2790,7 +2790,7 @@ connection_handle_write_impl(connection_t *conn, int force) edge_connection_t *edge_conn = TO_EDGE_CONN(conn); /* Check for overflow: */ if (PREDICT_LIKELY(UINT32_MAX - edge_conn->n_written > n_written)) - edge_conn->n_written += n_written; + edge_conn->n_written += (int)n_written; else edge_conn->n_written = UINT32_MAX; }

1 0

[tor/master] 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] Improve a few comments
by nickm＠torproject.org 07 Apr '11

07 Apr '11

commit 9facf8918f959cc9d9c2674afbd0630718460dc3 Author: Sebastian Hahn <sebastian(a)torproject.org> Date: Mon Mar 28 19:28:04 2011 +0200 Improve a few comments --- src/common/address.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/common/address.c b/src/common/address.c index a089260..adc0ef0 100644 --- a/src/common/address.c +++ b/src/common/address.c @@ -51,11 +51,11 @@ /** Convert the tor_addr_t in <b>a</b>, with port in <b>port</b>, into a * sockaddr object in *<b>sa_out</b> of object size <b>len</b>. If not enough - * room is available in sa_out, or on error, return 0 On success, return + * room is available in sa_out, or on error, return 0. On success, return * the length of the sockaddr. * - * Interface note: ordinarly, we return -1 for error. We can't do that here, - * since socklen is unsigned on some platforms. + * Interface note: ordinarily, we return -1 for error. We can't do that here, + * since socklen_t is unsigned on some platforms. **/ socklen_t tor_addr_to_sockaddr(const tor_addr_t *a,

1 0

[tor/master] Send END_STREAM_REASON_NOROUTE: clients that didn't grok it are now obsolete
by nickm＠torproject.org 07 Apr '11

07 Apr '11

commit fc647832783cab352bebba63fe0210d7be395058 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 28 13:42:59 2011 -0400 Send END_STREAM_REASON_NOROUTE: clients that didn't grok it are now obsolete --- changes/noroute | 5 +++++ src/or/reasons.c | 8 +------- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/changes/noroute b/changes/noroute new file mode 100644 index 0000000..644deec --- /dev/null +++ b/changes/noroute @@ -0,0 +1,5 @@ + - Minor features + - Send END_STREAM_REASON_NOROUTE in response to EHOSTUNREACH errors. + Clients before 0.2.1.27 didn't handle NOROUTE correctly, but + such clients are already deprecated because of security bugs. + diff --git a/src/or/reasons.c b/src/or/reasons.c index 27c947e..319e6c0 100644 --- a/src/or/reasons.c +++ b/src/or/reasons.c @@ -174,13 +174,7 @@ errno_to_stream_end_reason(int e) S_CASE(ENETUNREACH): return END_STREAM_REASON_INTERNAL; S_CASE(EHOSTUNREACH): - /* XXXX023 - * The correct behavior is END_STREAM_REASON_NOROUTE, but older - * clients don't recognize it. So we're going to continue sending - * "MISC" until 0.2.1.27 or later is "well established". - */ - /* return END_STREAM_REASON_NOROUTE; */ - return END_STREAM_REASON_MISC; + return END_STREAM_REASON_NOROUTE; S_CASE(ECONNREFUSED): return END_STREAM_REASON_CONNECTREFUSED; S_CASE(ECONNRESET):

1 0

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

07 Apr '11

commit 67d88a7d6021e95a2d423a9f26811accd1da39b6 Merge: 07ab483 ba0cd80 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Apr 7 12:17:20 2011 -0400 Merge remote-tracking branch 'origin/maint-0.2.2' Conflicts: src/common/address.c src/common/compat_libevent.c src/common/memarea.c src/common/util.h src/or/buffers.c src/or/circuitbuild.c src/or/circuituse.c src/or/connection.c src/or/directory.c src/or/networkstatus.c src/or/or.h src/or/routerlist.c changes/bug539_removal | 6 ++++++ changes/connect_err_reporting | 6 ++++++ changes/count_overflow | 5 +++++ changes/full_ap_circuits | 6 ++++++ changes/kill_ftime | 7 +++++++ changes/noroute | 5 +++++ src/common/address.c | 20 ++++++++++++-------- src/common/address.h | 2 +- src/common/compat_libevent.c | 21 ++++++--------------- src/common/memarea.c | 6 ++++-- src/common/util.h | 18 ------------------ src/or/buffers.c | 7 +++++++ src/or/circuitbuild.c | 7 ++++--- src/or/circuitlist.c | 3 +++ src/or/circuituse.c | 5 +++-- src/or/config.c | 6 +++--- src/or/connection.c | 37 +++++++++++++++++++++++-------------- src/or/connection_edge.c | 30 +++++++++++++++++++++--------- src/or/connection_or.c | 6 +++++- src/or/directory.c | 38 +++++++++++++++----------------------- src/or/dirserv.c | 11 +++++++---- src/or/dns.c | 2 +- src/or/dnsserv.c | 2 +- src/or/eventdns.c | 2 +- src/or/geoip.c | 5 +++-- src/or/networkstatus.c | 11 ++++++++--- src/or/or.h | 12 +++++++++--- src/or/reasons.c | 8 +------- src/or/relay.c | 2 ++ src/or/rendclient.c | 4 ++-- src/or/rendcommon.c | 2 +- src/or/rephist.c | 2 +- src/or/routerlist.c | 16 ++++++++-------- src/or/routerparse.c | 2 +- 34 files changed, 188 insertions(+), 134 deletions(-) diff --cc src/or/circuituse.c index 5488eb7,cdf49e3..e58d5e0 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@@ -1277,10 -1284,11 +1277,11 @@@ circuit_get_open_circ_or_launch(edge_co return -1; } } else { - /* XXXX022 Duplicates checks in connection_ap_handshake_attach_circuit */ + /* XXXX023 Duplicates checks in connection_ap_handshake_attach_circuit: + * refactor into a single function? */ - routerinfo_t *router = router_get_by_nickname(conn->chosen_exit_name, 1); + const node_t *node = node_get_by_nickname(conn->chosen_exit_name, 1); int opt = conn->chosen_exit_optional; - if (router && !connection_ap_can_use_exit(conn, router, 0)) { + if (node && !connection_ap_can_use_exit(conn, node, 0)) { log_fn(opt ? LOG_INFO : LOG_WARN, LD_APP, "Requested exit point '%s' would refuse request. %s.", conn->chosen_exit_name, opt ? "Trying others" : "Closing"); diff --cc src/or/connection.c index 8b48b96,6e7bbd5..02ae7ee --- a/src/or/connection.c +++ b/src/or/connection.c @@@ -2746,15 -2575,19 +2747,19 @@@ connection_read_to_buf(connection_t *co 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 */ ++ /* 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); @@@ -3145,10 -2786,13 +3150,14 @@@ connection_handle_write_impl(connection 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 += (int)n_written; + else + edge_conn->n_written = UINT32_MAX; } connection_buckets_decrement(conn, approx_time(), n_read, n_written); diff --cc src/or/directory.c index 3b1fb02,8f33a60..6bef581 --- a/src/or/directory.c +++ b/src/or/directory.c @@@ -1602,27 -1539,19 +1602,19 @@@ connection_dir_client_reached_eof(dir_c (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); - rs = router_get_mutable_consensus_status_by_id(conn->identity_digest); - if (rs) - 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))) ++ if ((rs = router_get_mutable_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); diff --cc src/or/networkstatus.c index 6387126,4f6fe15..50982d7 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@@ -1807,11 -1765,15 +1808,15 @@@ networkstatus_set_current_consensus(con write_str_to_file(consensus_fname, consensus, 0); } - if (time_definitely_before(now, c->valid_after, 60)) { + /** If a consensus appears more than this many seconds before its declared + * valid-after time, declare that our clock is skewed. */ + #define EARLY_CONSENSUS_NOTICE_SKEW 60 + + if (now < current_consensus->valid_after - EARLY_CONSENSUS_NOTICE_SKEW) { char tbuf[ISO_TIME_LEN+1]; char dbuf[64]; - long delta = now - current_consensus->valid_after; - format_iso_time(tbuf, current_consensus->valid_after); + long delta = now - c->valid_after; + format_iso_time(tbuf, c->valid_after); format_time_interval(dbuf, sizeof(dbuf), delta); log_warn(LD_GENERAL, "Our clock is %s behind the time published in the " "consensus network status document (%s GMT). Tor needs an " diff --cc src/or/or.h index c134d7c,1688a08..56d701b --- a/src/or/or.h +++ b/src/or/or.h @@@ -2279,12 -2126,15 +2279,18 @@@ typedef struct circuit_t * length ONIONSKIN_CHALLENGE_LEN. */ char *n_conn_onionskin; - struct timeval timestamp_created; /**< When was the circuit created? */ + /** When was this circuit created? We keep this timestamp with a higher + * resolution than most so that the circuit-build-time tracking code can + * get millisecond resolution. */ + struct timeval timestamp_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; uint16_t marked_for_close; /**< Should we close this circuit at the end of * the main loop? (If true, holds the line number diff --cc src/or/routerlist.c index 1205fd1,4deff53..580763d --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@@ -1764,9 -1768,9 +1764,9 @@@ smartlist_choose_node_by_bandwidth_weig weighted_bw += weight*this_bw; if (is_me) sl_last_weighted_bw_of_me = weight*this_bw; - } + } SMARTLIST_FOREACH_END(node); - /* XXXX022 this is a kludge to expose these values. */ + /* XXXX023 this is a kludge to expose these values. */ sl_last_total_weighted_bw = weighted_bw; log_debug(LD_CIRC, "Choosing node for rule %s based on weights " @@@ -1874,21 -1883,20 +1874,21 @@@ smartlist_choose_node_by_bandwidth(smar int is_known = 1; int32_t flags = 0; uint32_t this_bw = 0; - if (statuses) { - status = smartlist_get(sl, i); - if (router_digest_is_me(status->identity_digest)) - me_idx = i; - router = router_get_by_digest(status->identity_digest); - is_exit = status->is_exit; - is_guard = status->is_possible_guard; - if (status->has_bandwidth) { - this_bw = kb_to_bytes(status->bandwidth); + i = node_sl_idx; + + if (router_digest_is_me(node->identity)) + me_idx = node_sl_idx; + + is_exit = node->is_exit; + is_guard = node->is_possible_guard; + if (node->rs) { + if (node->rs->has_bandwidth) { + this_bw = kb_to_bytes(node->rs->bandwidth); } else { /* guess */ - /* XXX022 once consensuses always list bandwidths, we can take + /* XXX023 once consensuses always list bandwidths, we can take * this guessing business out. -RD */ is_known = 0; - flags = status->is_fast ? 1 : 0; + flags = node->rs->is_fast ? 1 : 0; flags |= is_exit ? 2 : 0; flags |= is_guard ? 4 : 0; } @@@ -4686,13 -4606,10 +4686,13 @@@ update_router_descriptor_downloads(time if (directory_fetches_dir_info_early(options)) { update_router_descriptor_cache_downloads_v2(now); } + update_consensus_router_descriptor_downloads(now, 0, - networkstatus_get_reasonably_live_consensus(now)); + networkstatus_get_reasonably_live_consensus(now, FLAV_NS)); - /* XXXX021 we could be smarter here; see notes on bug 652. */ + /* XXXX023 we could be smarter here; see notes on bug 652. */ + /* XXXX NM Microdescs: if we're not fetching microdescriptors, we need + * to make something else invoke this. */ /* If we're a server that doesn't have a configured address, we rely on * directory fetches to learn when our address changes. So if we haven't * tried to get any routerdescs in a long time, try a dummy fetch now. */

1 0

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

07 Apr '11

commit ba0cd8094f8e6ae0113ad69958d9d0973bb1f2c3 Merge: 118d8ff fc64783 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Apr 7 12:03:04 2011 -0400 Merge remote-tracking branch 'public/xxx_fixups' into maint-0.2.2 Conflicts: src/or/or.h changes/bug539_removal | 6 +++ changes/connect_err_reporting | 6 +++ changes/count_overflow | 5 +++ changes/full_ap_circuits | 6 +++ changes/kill_ftime | 7 ++++ changes/noroute | 5 +++ src/common/address.c | 20 ++++++----- src/common/compat_libevent.c | 17 +++------ src/common/memarea.c | 7 +++- src/common/util.c | 77 ----------------------------------------- src/common/util.h | 10 ----- src/or/buffers.c | 6 ++-- src/or/circuitbuild.c | 9 +++-- src/or/circuitlist.c | 3 ++ src/or/circuituse.c | 5 ++- src/or/config.c | 6 ++-- src/or/connection.c | 36 ++++++++++++------- src/or/connection_edge.c | 30 +++++++++++----- src/or/connection_or.c | 6 +++- src/or/directory.c | 37 ++++++++------------ src/or/dirserv.c | 11 ++++-- src/or/dns.c | 2 +- src/or/dnsserv.c | 2 +- src/or/eventdns.c | 2 +- src/or/geoip.c | 5 ++- src/or/networkstatus.c | 11 ++++-- src/or/or.h | 12 +++++-- src/or/reasons.c | 8 +---- src/or/relay.c | 2 + src/or/rendclient.c | 4 +- src/or/rendcommon.c | 2 +- src/or/rephist.c | 2 +- src/or/routerlist.c | 18 +++++----- src/or/routerparse.c | 2 +- 34 files changed, 184 insertions(+), 203 deletions(-) diff --cc src/or/or.h index 3cadd31,e44c626..1688a08 --- a/src/or/or.h +++ b/src/or/or.h @@@ -2126,9 -2126,16 +2126,15 @@@ typedef struct circuit_t * length ONIONSKIN_CHALLENGE_LEN. */ char *n_conn_onionskin; - time_t timestamp_created; /**< When was this circuit created? */ + struct timeval timestamp_created; /**< When was the 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 * the main loop? (If true, holds the line number

1 0

[tor/master] Remove the "fuzzy time" code
by nickm＠torproject.org 07 Apr '11

07 Apr '11

commit 444e46d96d05c3e12eca02769512c74f2eb16b48 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 25 15:27:06 2011 -0400 Remove the "fuzzy time" code It was the start of a neat idea, but it only got used in 3 places, none of which really needed it. --- changes/kill_ftime | 7 ++++ src/common/util.c | 77 ------------------------------------------------ src/common/util.h | 10 ------ src/or/networkstatus.c | 6 +++- src/or/routerlist.c | 4 +- 5 files changed, 14 insertions(+), 90 deletions(-) diff --git a/changes/kill_ftime b/changes/kill_ftime new file mode 100644 index 0000000..47f4769 --- /dev/null +++ b/changes/kill_ftime @@ -0,0 +1,7 @@ + o Code simplification and refactoring + - Remove the old 'fuzzy time' logic. It was supposed to be used + for handling calculations where we have a known amount of clock + skew and an allowed amount of unknown skew. But we only used it + in three places, and we never adjusted the known/unknown skew + values. This is still something we might want to do someday, + but if we do, we'll want to do it differently. diff --git a/src/common/util.c b/src/common/util.c index abd87ea..38c0ad0 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1511,83 +1511,6 @@ update_approx_time(time_t now) #endif /* ===== - * Fuzzy time - * XXXX022 Use this consistently or rip most of it out. - * ===== */ - -/* In a perfect world, everybody would run NTP, and NTP would be perfect, so - * if we wanted to know "Is the current time before time X?" we could just say - * "time(NULL) < X". - * - * But unfortunately, many users are running Tor in an imperfect world, on - * even more imperfect computers. Hence, we need to track time oddly. We - * model the user's computer as being "skewed" from accurate time by - * -<b>ftime_skew</b> seconds, such that our best guess of the current time is - * time(NULL)+ftime_skew. We also assume that our measurements of time may - * have up to <b>ftime_slop</b> seconds of inaccuracy; IOW, our window of - * estimate for the current time is now + ftime_skew +/- ftime_slop. - */ -/** Our current estimate of our skew, such that we think the current time is - * closest to time(NULL)+ftime_skew. */ -static int ftime_skew = 0; -/** Tolerance during time comparisons, in seconds. */ -static int ftime_slop = 60; -/** Set the largest amount of sloppiness we'll allow in fuzzy time - * comparisons. */ -void -ftime_set_maximum_sloppiness(int seconds) -{ - tor_assert(seconds >= 0); - ftime_slop = seconds; -} -/** Set the amount by which we believe our system clock to differ from - * real time. */ -void -ftime_set_estimated_skew(int seconds) -{ - ftime_skew = seconds; -} -#if 0 -void -ftime_get_window(time_t now, ftime_t *ft_out) -{ - ft_out->earliest = now + ftime_skew - ftime_slop; - ft_out->latest = now + ftime_skew + ftime_slop; -} -#endif -/** Return true iff we think that <b>now</b> might be after <b>when</b>. */ -int -ftime_maybe_after(time_t now, time_t when) -{ - /* It may be after when iff the latest possible current time is after when */ - return (now + ftime_skew + ftime_slop) >= when; -} -/** Return true iff we think that <b>now</b> might be before <b>when</b>. */ -int -ftime_maybe_before(time_t now, time_t when) -{ - /* It may be before when iff the earliest possible current time is before */ - return (now + ftime_skew - ftime_slop) < when; -} -/** Return true if we think that <b>now</b> is definitely after <b>when</b>. */ -int -ftime_definitely_after(time_t now, time_t when) -{ - /* It is definitely after when if the earliest time it could be is still - * after when. */ - return (now + ftime_skew - ftime_slop) >= when; -} -/** Return true if we think that <b>now</b> is definitely before <b>when</b>. - */ -int -ftime_definitely_before(time_t now, time_t when) -{ - /* It is definitely before when if the latest time it could be is still - * before when. */ - return (now + ftime_skew + ftime_slop) < when; -} - -/* ===== * Rate limiting * ===== */ diff --git a/src/common/util.h b/src/common/util.h index 3736237..6b54856 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -247,16 +247,6 @@ time_t approx_time(void); void update_approx_time(time_t now); #endif -/* Fuzzy time. */ -void ftime_set_maximum_sloppiness(int seconds); -void ftime_set_estimated_skew(int seconds); -/* typedef struct ftime_t { time_t earliest; time_t latest; } ftime_t; */ -/* void ftime_get_window(time_t now, ftime_t *ft_out); */ -int ftime_maybe_after(time_t now, time_t when); -int ftime_maybe_before(time_t now, time_t when); -int ftime_definitely_after(time_t now, time_t when); -int ftime_definitely_before(time_t now, time_t when); - /* Rate-limiter */ /** A ratelim_t remembers how often an event is occurring, and how often diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 687ac03..155dffe 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1764,7 +1764,11 @@ networkstatus_set_current_consensus(const char *consensus, write_str_to_file(consensus_fname, consensus, 0); } - if (ftime_definitely_before(now, current_consensus->valid_after)) { +/** If a consensus appears more than this many seconds before its declared + * valid-after time, declare that our clock is skewed. */ +#define EARLY_CONSENSUS_NOTICE_SKEW 60 + + if (now < current_consensus->valid_after - EARLY_CONSENSUS_NOTICE_SKEW) { char tbuf[ISO_TIME_LEN+1]; char dbuf[64]; long delta = now - current_consensus->valid_after; diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 4421d5c..2dca578 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -328,7 +328,7 @@ trusted_dirs_remove_old_certs(void) time_t cert_published; if (newest == cert) continue; - expired = ftime_definitely_after(now, cert->expires); + expired = now > cert->expires; cert_published = cert->cache_info.published_on; /* Store expired certs for 48 hours after a newer arrives; */ @@ -520,7 +520,7 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now) continue; cl = get_cert_list(ds->v3_identity_digest); SMARTLIST_FOREACH(cl->certs, authority_cert_t *, cert, { - if (!ftime_definitely_after(now, cert->expires)) { + if (now < cert->expires) { /* It's not expired, and we weren't looking for something to * verify a consensus with. Call it done. */ download_status_reset(&cl->dl_status);

1 0