From f772fc0c36578e61e01f8d54c95415554d5ef892 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 5 Jan 2009 20:52:14 +0000 Subject: [PATCH] apply a variant of rovv's bug 872 patch, and document that we want a better solution for 0.2.2.x. svn:r17924 --- ChangeLog | 5 +++++ doc/TODO.future | 7 +++++++ src/or/relay.c | 17 ++++++++++++----- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index 160acb9350..7ae496eaec 100644 --- a/ChangeLog +++ b/ChangeLog @@ -51,6 +51,11 @@ Changes in version 0.2.1.10-alpha - 2009-01-?? - Resume reporting accurate "stream end" reasons to the local control port. They were lost in the changes for Proposal 148. Bugfix on 0.2.1.9-alpha. + - When an exit resolves an address to a local IP, do not just keep + retrying that same exit over and over. Instead, just close + the connection. Addresses bug 872. Patch from rovv. + - If a hidden service sends us an END cell, do not consider + retrying the connection. Patch from rovv. o Deprecated and removed features: - The old "tor --version --version" command, which would spit out the diff --git a/doc/TODO.future b/doc/TODO.future index eaebde5940..8b5decd0d0 100644 --- a/doc/TODO.future +++ b/doc/TODO.future @@ -49,6 +49,13 @@ Later, unless people want to implement them now: - Make the timestamp granularity on logs configurable, with default of "1 second". This might make some kinds of after-the-fact attack harder. + - We should get smarter about handkling address resolve failures, or + addresses that resolve to local IPs. It would be neat to retry + them, since right now we just close the stream. But we need to + make sure we don't retry them on the same exit as before. But if + we mark the circuit, then any user who types "localhost" will + cycle through circuits till they run out of retries. See bug 872. + Can anybody remember why we wanted to do this and/or what it means? - config option __ControllerLimit that hangs up if there are a limit of controller connections already. diff --git a/src/or/relay.c b/src/or/relay.c index bc1b2a50ed..0c607f04f7 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -659,7 +659,9 @@ connection_ap_process_end_not_open( int control_reason = reason | END_STREAM_REASON_FLAG_REMOTE; (void) layer_hint; /* unused */ - if (rh->length > 0 && edge_reason_is_retriable(reason)) { + if (rh->length > 0 && edge_reason_is_retriable(reason) && + !connection_edge_is_rendezvous_stream(conn) /* avoid retry if rend */ + ) { log_info(LD_APP,"Address '%s' refused due to '%s'. Considering retrying.", safe_str(conn->socks_request->address), stream_end_reason_to_string(reason)); @@ -681,10 +683,15 @@ connection_ap_process_end_not_open( else ttl = -1; - if (!(get_options()->ClientDNSRejectInternalAddresses && - is_internal_IP(addr, 0))) - client_dns_set_addressmap(conn->socks_request->address, addr, - conn->chosen_exit_name, ttl); + if (get_options()->ClientDNSRejectInternalAddresses && + is_internal_IP(addr, 0)) { + log_info(LD_APP,"Address '%s' resolved to internal. Closing,", + safe_str(conn->socks_request->address)); + connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); + return 0; + } + client_dns_set_addressmap(conn->socks_request->address, addr, + conn->chosen_exit_name, ttl); } /* check if he *ought* to have allowed it */ if (exitrouter &&