October 2017 - tor-commits - lists.torproject.org

[tor/master] Don't give "exitrelay may become 0 when no policy" warning on ReducedExitPolicy
by nickm＠torproject.org 31 Oct '17

31 Oct '17

commit 00e781142b919476fd1ea594ac36b08147c20ede Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 31 13:22:28 2017 -0400 Don't give "exitrelay may become 0 when no policy" warning on ReducedExitPolicy --- src/or/policies.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/policies.c b/src/or/policies.c index 0dfbd9209..1f8013071 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -1145,7 +1145,7 @@ validate_addr_policies(const or_options_t *options, char **msg) "to 1 to disable this warning, and for forward compatibility.", options->ExitPolicy == NULL ? " with the default exit policy" : ""); - if (options->ExitPolicy == NULL) { + if (options->ExitPolicy == NULL && options->ReducedExitPolicy == 0) { log_warn(LD_CONFIG, "In a future version of Tor, ExitRelay 0 may become the " "default when no ExitPolicy is given.");

1 0

[nyx/master] Specify installing right sqlite3 on FreeBSD
by atagar＠torproject.org 31 Oct '17

31 Oct '17

commit 93f20f9e8b7f276ba8aabfb7b224e3c71bb78aa6 Author: Damian Johnson <atagar(a)torproject.org> Date: Tue Oct 31 10:48:55 2017 -0700 Specify installing right sqlite3 on FreeBSD I was unsure if FreeBSD had sqlite3 ports for all the python versions but a reply on tor-relays@ seems to imply they do. Providing a command for whatever interpreter version the user's running. --- nyx/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nyx/__init__.py b/nyx/__init__.py index 0142c5d..f91ef07 100644 --- a/nyx/__init__.py +++ b/nyx/__init__.py @@ -74,8 +74,8 @@ emerge dev-lang/python with USE=sqlite. SQLITE_UNAVAILABLE_FREEBSD = """\ Python's sqlite3 module is unavailable. Please run... - sudo pkg install py27-sqlite3 -""" + sudo pkg install py%i%i-sqlite3 +""" % (sys.version_info[:2] try: import sqlite3

1 0

[nyx/master] Avoid os.putenv calls on FreeBSD
by atagar＠torproject.org 31 Oct '17

31 Oct '17

commit f4cfa06035a92c888588d1cb9c90b9ccc9836b80 Author: Damian Johnson <atagar(a)torproject.org> Date: Tue Oct 31 10:42:31 2017 -0700 Avoid os.putenv calls on FreeBSD Turns out even when we suppress the exceptions it causes the interpreter to print to stdout. Lets just avoid these calls entirely on FreeBSD. Dropping the catch clauses since we were only doing this due to FreeBSD. --- nyx/starter.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/nyx/starter.py b/nyx/starter.py index 2532a4a..62c8287 100644 --- a/nyx/starter.py +++ b/nyx/starter.py @@ -87,15 +87,14 @@ def main(config): _use_unicode() _set_process_name() - try: - os.putenv('LANG', 'C') # make subcommands (ps, netstat, etc) provide english results - except OSError: - pass + # These os.putenv calls fail on FreeBSD, and even attempting causes python to + # print the following to stdout... + # + # nyx: environment corrupt; missing value for - try: + if not stem.util.system.is_bsd(): + os.putenv('LANG', 'C') # make subcommands (ps, netstat, etc) provide english results os.putenv('ESCDELAY', '0') # make 'esc' take effect right away - except OSError: - pass try: nyx.curses.start(nyx.draw_loop, acs_support = config.get('acs_support', True), transparent_background = True, cursor = False)

1 0

[stem/master] Better messaging when unable to connect to tor on FreeBSD
by atagar＠torproject.org 31 Oct '17

31 Oct '17

commit c01d8c859d1272465e4261a10df8f598ea82e889 Author: Damian Johnson <atagar(a)torproject.org> Date: Tue Oct 31 10:36:34 2017 -0700 Better messaging when unable to connect to tor on FreeBSD FreeBSD ps output lists the full tor path rather than just the process name. Recognizing its default location so our connect() method provides better messaging when unable to connect. --- docs/change_log.rst | 3 +++ stem/connection.py | 10 +++++++--- stem/util/system.py | 15 ++++++++++++--- test/unit/util/system.py | 4 +++- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/docs/change_log.rst b/docs/change_log.rst index 4699c2b9..12a7dc8c 100644 --- a/docs/change_log.rst +++ b/docs/change_log.rst @@ -54,6 +54,7 @@ The following are only available within Stem's `git repository * Unable to use cookie auth when path includes wide characters (chinese, japanese, etc) * Tor change caused :func:`~stem.control.Controller.list_ephemeral_hidden_services` to provide empty strings if unset (:trac:`21329`) * Better error message when :func:`~stem.control.Controller.set_conf` fails due to an option being immutable + * :func:`~stem.control.Controller.get_ports` didn't provide ports for many representations of localhost (:trac:`24085`) * :func:`~stem.control.Controller.is_geoip_unavailable` now determines if database is available right away * Added the time attribute to :class:`~stem.response.events.StreamBwEvent` and :class:`~stem.response.events.CircuitBandwidthEvent` (:spec:`00b9daf`) * Added the consensus_content attribute to :class:`~stem.response.events.NewConsensusEvent` and deprecated its 'desc' @@ -65,6 +66,7 @@ The following are only available within Stem's `git repository * Resilient to 'Tor' prefix in 'GETINFO version' result (:spec:`c5ff1b1`) * Added a **all_extra** parameter to :class:`stem.version.Version` and support for multiple parenthetical entries (:trac:`22110`, :spec:`b50917d`) * Closing controller connection faster when under heavy event load + * Better messaging when unable to connect to tor on FreeBSD * More succinct trace level logging * **Descriptors** @@ -100,6 +102,7 @@ The following are only available within Stem's `git repository * Added a `'--run [command or path]' argument <tutorials/down_the_rabbit_hole.html#running-individual-commands>`_ to invoke specific commands (:trac:`21541`) * Allowing interpreter to continue after tor shutsdown (:trac:`22374`) + * Interpreter buffered an unbounded number of events, leaking memory over time * **Website** diff --git a/stem/connection.py b/stem/connection.py index cce00bf7..62309f0b 100644 --- a/stem/connection.py +++ b/stem/connection.py @@ -198,6 +198,12 @@ CONNECT_MESSAGES = { 'wrong_socket_type': WRONG_SOCKET_TYPE_MSG.strip(), } +COMMON_TOR_COMMANDS = ( + 'tor', + 'tor.real', # TBB command ran + '/usr/local/bin/tor', # FreeBSD expands the whole path, this is the default location +) + def connect(control_port = ('127.0.0.1', 'default'), control_socket = '/var/run/tor/control', password = None, password_prompt = False, chroot_path = None, controller = stem.control.Controller): """ @@ -273,12 +279,10 @@ def connect(control_port = ('127.0.0.1', 'default'), control_socket = '/var/run/ # If unable to connect to either a control socket or port then finally fail # out. If we only attempted to connect to one of them then provide the error # output from that. Otherwise we provide a more generic error message. - # - # We check for a 'tor.real' process name because that's what TBB uses. if not control_connection: if control_socket and control_port: - is_tor_running = stem.util.system.is_running('tor') or stem.util.system.is_running('tor.real') + is_tor_running = stem.util.system.is_running(COMMON_TOR_COMMANDS) error_msg = CONNECT_MESSAGES['no_control_port'] if is_tor_running else CONNECT_MESSAGES['tor_isnt_running'] print(error_msg) diff --git a/stem/util/system.py b/stem/util/system.py index fb799227..459cde4a 100644 --- a/stem/util/system.py +++ b/stem/util/system.py @@ -401,9 +401,10 @@ def is_running(command): Checks for if a process with a given name or pid is running. .. versionchanged:: 1.6.0 - Added support for pid arguments. + Added support for list and pid arguments. - :param str,int command: process name if a str or pid if an int to be checked + :param str,list,int command: process name if a str, multiple process names if + a list, or pid if an int to be checked :returns: **True** if the process is running, **False** if it's not among ps results, and **None** if ps can't be queried @@ -444,7 +445,15 @@ def is_running(command): if command_listing: command_listing = map(str_type.strip, command_listing) - return command in command_listing + + if isinstance(command, (bytes, unicode)): + command = [command] + + for cmd in command: + if cmd in command_listing: + return True + + return False return None diff --git a/test/unit/util/system.py b/test/unit/util/system.py index a957a23f..fd8fe2d9 100644 --- a/test/unit/util/system.py +++ b/test/unit/util/system.py @@ -137,9 +137,11 @@ class TestSystem(unittest.TestCase): self.assertTrue(system.is_running('irssi')) self.assertTrue(system.is_running('moc')) self.assertTrue(system.is_running('tor')) + self.assertTrue(system.is_running(['funky-tor', 'tor'])) self.assertTrue(system.is_running('ps')) self.assertTrue(system.is_running('firefox')) - self.assertEqual(False, system.is_running('something_else')) + self.assertFalse(system.is_running('something_else')) + self.assertFalse(system.is_running(['funky-tor', 'funkier-tor'])) # mock both calls failing

1 0

[stem/master] Recognize additional representations of 'localhost' in get_ports()
by atagar＠torproject.org 31 Oct '17

31 Oct '17

commit 66a9b77e165dfe47e3b32a84080b59f47cc85dc1 Author: Damian Johnson <atagar(a)torproject.org> Date: Tue Oct 31 10:01:35 2017 -0700 Recognize additional representations of 'localhost' in get_ports() Tim and Sebastian pointed out that we're still not doing correct localhost detection... https://trac.torproject.org/projects/tor/ticket/24085 --- stem/control.py | 14 +++++++++++++- test/unit/control/controller.py | 10 +++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/stem/control.py b/stem/control.py index 19619190..7c67cd0d 100644 --- a/stem/control.py +++ b/stem/control.py @@ -1323,7 +1323,19 @@ class Controller(BaseController): and no default was provided """ - return [port for (addr, port) in self.get_listeners(listener_type) if addr in ('127.0.0.1', '0.0.0.0')] + def is_localhost(address): + if stem.util.connection.is_valid_ipv4_address(address): + return address == '0.0.0.0' or address.startswith('127.') + elif stem.util.connection.is_valid_ipv6_address(address): + return stem.util.connection.expand_ipv6_address(address) in ( + '0000:0000:0000:0000:0000:0000:0000:0000', + '0000:0000:0000:0000:0000:0000:0000:0001', + ) + else: + log.info("Request for %s ports got an address that's neither IPv4 or IPv6: %s" % (listener_type, address)) + return False + + return [port for (addr, port) in self.get_listeners(listener_type) if is_localhost(addr)] @with_default() def get_listeners(self, listener_type, default = UNDEFINED): diff --git a/test/unit/control/controller.py b/test/unit/control/controller.py index a9a7e73d..929adcfc 100644 --- a/test/unit/control/controller.py +++ b/test/unit/control/controller.py @@ -227,7 +227,7 @@ class TestControl(unittest.TestCase): self.assertEqual([], self.controller.get_ports(Listener.CONTROL)) self.controller.clear_cache() - # Exercise via the GETINFO option. + # exercise via the GETINFO option get_info_mock.side_effect = None get_info_mock.return_value = '"127.0.0.1:1112" "127.0.0.1:1114"' @@ -240,6 +240,14 @@ class TestControl(unittest.TestCase): self.assertEqual([1112, 1114], self.controller.get_ports(Listener.CONTROL)) self.controller.clear_cache() + # with all localhost addresses, including a couple that aren't + + get_info_mock.side_effect = None + get_info_mock.return_value = '"27.4.4.1:1113" "127.0.0.5:1114" "0.0.0.0:1115" "[::]:1116" "[::1]:1117" "[10::]:1118"' + + self.assertEqual([1114, 1115, 1116, 1117], self.controller.get_ports(Listener.OR)) + self.controller.clear_cache() + # IPv6 address get_info_mock.return_value = '"0.0.0.0:9001" "[fe80:0000:0000:0000:0202:b3ff:fe1e:8329]:9001"'

1 0

[tor/release-0.3.2] hs-v3: Attempt descriptor refetch when dirinfo changes
by nickm＠torproject.org 31 Oct '17

31 Oct '17

commit 5dbcd48f0ee2f57557f6bcce6ee3ec11a76727e4 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Oct 4 16:22:49 2017 -0400 hs-v3: Attempt descriptor refetch when dirinfo changes When the directory information changes, callback to the HS client subsystem so it can check if any pending SOCKS connections are waiting for a descriptor. If yes, attempt a refetch for those. Fixes #23762 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/bug23762 | 4 +++ src/or/connection_edge.c | 9 +++---- src/or/hs_client.c | 59 ++++++++++++++++++++++++++++++++++++++++++ src/or/hs_client.h | 1 + src/or/nodelist.c | 2 ++ src/test/test_channelpadding.c | 12 ++++++--- src/test/test_entryconn.c | 5 ++-- 7 files changed, 81 insertions(+), 11 deletions(-) diff --git a/changes/bug23762 b/changes/bug23762 new file mode 100644 index 000000000..bf35e5ec1 --- /dev/null +++ b/changes/bug23762 @@ -0,0 +1,4 @@ + o Minor bugfixes (hidden service v3): + - Properly retry HSv3 descriptor fetches in the case where we were initially + missing required directory information. Closes ticket #23762; bugfix on + 0.3.2.1-alpha. diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 77dac62b0..b9d8eeaff 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1571,10 +1571,10 @@ connection_ap_handle_onion(entry_connection_t *conn, int ret = hs_client_refetch_hsdesc(&edge_conn->hs_ident->identity_pk); switch (ret) { case HS_CLIENT_FETCH_MISSING_INFO: - /* By going to the end, the connection is put in waiting for a circuit - * state which means that it will be retried and consider as a pending - * connection. */ - goto end; + /* Keeping the connection in descriptor wait state is fine because + * once we get enough dirinfo or a new live consensus, the HS client + * subsystem is notified and every connection in that state will + * trigger a fetch for the service key. */ case HS_CLIENT_FETCH_LAUNCHED: case HS_CLIENT_FETCH_PENDING: case HS_CLIENT_FETCH_HAVE_DESC: @@ -1591,7 +1591,6 @@ connection_ap_handle_onion(entry_connection_t *conn, /* We have the descriptor! So launch a connection to the HS. */ log_info(LD_REND, "Descriptor is here. Great."); - end: base_conn->state = AP_CONN_STATE_CIRCUIT_WAIT; /* We'll try to attach it at the next event loop, or whenever * we call connection_ap_attach_pending() */ diff --git a/src/or/hs_client.c b/src/or/hs_client.c index d66e56352..18b76e8b3 100644 --- a/src/or/hs_client.c +++ b/src/or/hs_client.c @@ -233,6 +233,55 @@ close_all_socks_conns_waiting_for_desc(const ed25519_public_key_t *identity_pk, smartlist_free(conns); } +/* Find all pending SOCKS connection waiting for a descriptor and retry them + * all. This is called when the directory information changed. */ +static void +retry_all_socks_conn_waiting_for_desc(void) +{ + smartlist_t *conns = + connection_list_by_type_state(CONN_TYPE_AP, AP_CONN_STATE_RENDDESC_WAIT); + + SMARTLIST_FOREACH_BEGIN(conns, connection_t *, base_conn) { + hs_client_fetch_status_t status; + const edge_connection_t *edge_conn = + ENTRY_TO_EDGE_CONN(TO_ENTRY_CONN(base_conn)); + + /* Ignore non HS or non v3 connection. */ + if (edge_conn->hs_ident == NULL) { + continue; + } + /* In this loop, we will possibly try to fetch a descriptor for the + * pending connections because we just got more directory information. + * However, the refetch process can cleanup all SOCKS request so the same + * service if an internal error happens. Thus, we can end up with closed + * connections in our list. */ + if (base_conn->marked_for_close) { + continue; + } + + /* XXX: There is an optimization we could do which is that for a service + * key, we could check if we can fetch and remember that decision. */ + + /* Order a refetch in case it works this time. */ + status = hs_client_refetch_hsdesc(&edge_conn->hs_ident->identity_pk); + if (BUG(status == HS_CLIENT_FETCH_HAVE_DESC)) { + /* This case is unique because it can NOT happen in theory. Once we get + * a new descriptor, the HS client subsystem is notified immediately and + * the connections waiting for it are handled which means the state will + * change from renddesc wait state. Log this and continue to next + * connection. */ + continue; + } + /* In the case of an error, either all SOCKS connections have been + * closed or we are still missing directory information. Leave the + * connection in renddesc wait state so when we get more info, we'll be + * able to try it again. */ + } SMARTLIST_FOREACH_END(base_conn); + + /* We don't have ownership of those objects. */ + smartlist_free(conns); +} + /* A v3 HS circuit successfully connected to the hidden service. Update the * stream state at <b>hs_conn_ident</b> appropriately. */ static void @@ -1529,3 +1578,13 @@ hs_client_purge_state(void) log_info(LD_REND, "Hidden service client state has been purged."); } +/* Called when our directory information has changed. */ +void +hs_client_dir_info_changed(void) +{ + /* We have possibly reached the minimum directory information or new + * consensus so retry all pending SOCKS connection in + * AP_CONN_STATE_RENDDESC_WAIT state in order to fetch the descriptor. */ + retry_all_socks_conn_waiting_for_desc(); +} + diff --git a/src/or/hs_client.h b/src/or/hs_client.h index 164accc0e..2523568ad 100644 --- a/src/or/hs_client.h +++ b/src/or/hs_client.h @@ -41,6 +41,7 @@ int hs_client_decode_descriptor( int hs_client_any_intro_points_usable(const ed25519_public_key_t *service_pk, const hs_descriptor_t *desc); int hs_client_refetch_hsdesc(const ed25519_public_key_t *identity_pk); +void hs_client_dir_info_changed(void); int hs_client_send_introduce1(origin_circuit_t *intro_circ, origin_circuit_t *rend_circ); diff --git a/src/or/nodelist.c b/src/or/nodelist.c index eae74e18b..fd5a8f0c0 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -48,6 +48,7 @@ #include "entrynodes.h" #include "geoip.h" #include "hs_common.h" +#include "hs_client.h" #include "main.h" #include "microdesc.h" #include "networkstatus.h" @@ -1973,6 +1974,7 @@ router_dir_info_changed(void) need_to_update_have_min_dir_info = 1; rend_hsdir_routers_changed(); hs_service_dir_info_changed(); + hs_client_dir_info_changed(); } /** Return a string describing what we're missing before we have enough diff --git a/src/test/test_channelpadding.c b/src/test/test_channelpadding.c index d54c9cc52..d5713688a 100644 --- a/src/test/test_channelpadding.c +++ b/src/test/test_channelpadding.c @@ -193,7 +193,8 @@ static void setup_mock_network(void) { routerstatus_t *relay; - connection_array = smartlist_new(); + if (!connection_array) + connection_array = smartlist_new(); relay1_relay2 = (channel_t*)new_fake_channeltls(2); relay1_relay2->write_cell = mock_channel_write_cell_relay1; @@ -280,7 +281,8 @@ test_channelpadding_timers(void *arg) tor_libevent_postfork(); - connection_array = smartlist_new(); + if (!connection_array) + connection_array = smartlist_new(); monotime_init(); monotime_enable_test_mocking(); @@ -570,7 +572,8 @@ test_channelpadding_consensus(void *arg) monotime_coarse_set_mock_time_nsec(1); timers_initialize(); - connection_array = smartlist_new(); + if (!connection_array) + connection_array = smartlist_new(); chan = (channel_t*)new_fake_channeltls(0); channel_timestamp_active(chan); @@ -928,7 +931,8 @@ test_channelpadding_decide_to_pad_channel(void *arg) */ channel_t *chan; int64_t new_time; - connection_array = smartlist_new(); + if (!connection_array) + connection_array = smartlist_new(); (void)arg; tor_libevent_postfork(); diff --git a/src/test/test_entryconn.c b/src/test/test_entryconn.c index 86c0c3dca..c29b1a712 100644 --- a/src/test/test_entryconn.c +++ b/src/test/test_entryconn.c @@ -790,8 +790,9 @@ test_entryconn_rewrite_onion_v3(void *arg) retval = connection_ap_handshake_rewrite_and_attach(conn, NULL, NULL); tt_int_op(retval, OP_EQ, 0); - /* Check connection state after rewrite */ - tt_int_op(ENTRY_TO_CONN(conn)->state, OP_EQ, AP_CONN_STATE_CIRCUIT_WAIT); + /* Check connection state after rewrite. It should be in waiting for + * descriptor state. */ + tt_int_op(ENTRY_TO_CONN(conn)->state, OP_EQ, AP_CONN_STATE_RENDDESC_WAIT); /* check that the address got rewritten */ tt_str_op(conn->socks_request->address, OP_EQ, "25njqamcweflpvkl73j4szahhihoc4xt3ktcgjnpaingr5yhkenl5sid");

1 0

[tor/master] hs-v3: Attempt descriptor refetch when dirinfo changes
by nickm＠torproject.org 31 Oct '17

31 Oct '17

commit 5dbcd48f0ee2f57557f6bcce6ee3ec11a76727e4 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Oct 4 16:22:49 2017 -0400 hs-v3: Attempt descriptor refetch when dirinfo changes When the directory information changes, callback to the HS client subsystem so it can check if any pending SOCKS connections are waiting for a descriptor. If yes, attempt a refetch for those. Fixes #23762 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/bug23762 | 4 +++ src/or/connection_edge.c | 9 +++---- src/or/hs_client.c | 59 ++++++++++++++++++++++++++++++++++++++++++ src/or/hs_client.h | 1 + src/or/nodelist.c | 2 ++ src/test/test_channelpadding.c | 12 ++++++--- src/test/test_entryconn.c | 5 ++-- 7 files changed, 81 insertions(+), 11 deletions(-) diff --git a/changes/bug23762 b/changes/bug23762 new file mode 100644 index 000000000..bf35e5ec1 --- /dev/null +++ b/changes/bug23762 @@ -0,0 +1,4 @@ + o Minor bugfixes (hidden service v3): + - Properly retry HSv3 descriptor fetches in the case where we were initially + missing required directory information. Closes ticket #23762; bugfix on + 0.3.2.1-alpha. diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 77dac62b0..b9d8eeaff 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1571,10 +1571,10 @@ connection_ap_handle_onion(entry_connection_t *conn, int ret = hs_client_refetch_hsdesc(&edge_conn->hs_ident->identity_pk); switch (ret) { case HS_CLIENT_FETCH_MISSING_INFO: - /* By going to the end, the connection is put in waiting for a circuit - * state which means that it will be retried and consider as a pending - * connection. */ - goto end; + /* Keeping the connection in descriptor wait state is fine because + * once we get enough dirinfo or a new live consensus, the HS client + * subsystem is notified and every connection in that state will + * trigger a fetch for the service key. */ case HS_CLIENT_FETCH_LAUNCHED: case HS_CLIENT_FETCH_PENDING: case HS_CLIENT_FETCH_HAVE_DESC: @@ -1591,7 +1591,6 @@ connection_ap_handle_onion(entry_connection_t *conn, /* We have the descriptor! So launch a connection to the HS. */ log_info(LD_REND, "Descriptor is here. Great."); - end: base_conn->state = AP_CONN_STATE_CIRCUIT_WAIT; /* We'll try to attach it at the next event loop, or whenever * we call connection_ap_attach_pending() */ diff --git a/src/or/hs_client.c b/src/or/hs_client.c index d66e56352..18b76e8b3 100644 --- a/src/or/hs_client.c +++ b/src/or/hs_client.c @@ -233,6 +233,55 @@ close_all_socks_conns_waiting_for_desc(const ed25519_public_key_t *identity_pk, smartlist_free(conns); } +/* Find all pending SOCKS connection waiting for a descriptor and retry them + * all. This is called when the directory information changed. */ +static void +retry_all_socks_conn_waiting_for_desc(void) +{ + smartlist_t *conns = + connection_list_by_type_state(CONN_TYPE_AP, AP_CONN_STATE_RENDDESC_WAIT); + + SMARTLIST_FOREACH_BEGIN(conns, connection_t *, base_conn) { + hs_client_fetch_status_t status; + const edge_connection_t *edge_conn = + ENTRY_TO_EDGE_CONN(TO_ENTRY_CONN(base_conn)); + + /* Ignore non HS or non v3 connection. */ + if (edge_conn->hs_ident == NULL) { + continue; + } + /* In this loop, we will possibly try to fetch a descriptor for the + * pending connections because we just got more directory information. + * However, the refetch process can cleanup all SOCKS request so the same + * service if an internal error happens. Thus, we can end up with closed + * connections in our list. */ + if (base_conn->marked_for_close) { + continue; + } + + /* XXX: There is an optimization we could do which is that for a service + * key, we could check if we can fetch and remember that decision. */ + + /* Order a refetch in case it works this time. */ + status = hs_client_refetch_hsdesc(&edge_conn->hs_ident->identity_pk); + if (BUG(status == HS_CLIENT_FETCH_HAVE_DESC)) { + /* This case is unique because it can NOT happen in theory. Once we get + * a new descriptor, the HS client subsystem is notified immediately and + * the connections waiting for it are handled which means the state will + * change from renddesc wait state. Log this and continue to next + * connection. */ + continue; + } + /* In the case of an error, either all SOCKS connections have been + * closed or we are still missing directory information. Leave the + * connection in renddesc wait state so when we get more info, we'll be + * able to try it again. */ + } SMARTLIST_FOREACH_END(base_conn); + + /* We don't have ownership of those objects. */ + smartlist_free(conns); +} + /* A v3 HS circuit successfully connected to the hidden service. Update the * stream state at <b>hs_conn_ident</b> appropriately. */ static void @@ -1529,3 +1578,13 @@ hs_client_purge_state(void) log_info(LD_REND, "Hidden service client state has been purged."); } +/* Called when our directory information has changed. */ +void +hs_client_dir_info_changed(void) +{ + /* We have possibly reached the minimum directory information or new + * consensus so retry all pending SOCKS connection in + * AP_CONN_STATE_RENDDESC_WAIT state in order to fetch the descriptor. */ + retry_all_socks_conn_waiting_for_desc(); +} + diff --git a/src/or/hs_client.h b/src/or/hs_client.h index 164accc0e..2523568ad 100644 --- a/src/or/hs_client.h +++ b/src/or/hs_client.h @@ -41,6 +41,7 @@ int hs_client_decode_descriptor( int hs_client_any_intro_points_usable(const ed25519_public_key_t *service_pk, const hs_descriptor_t *desc); int hs_client_refetch_hsdesc(const ed25519_public_key_t *identity_pk); +void hs_client_dir_info_changed(void); int hs_client_send_introduce1(origin_circuit_t *intro_circ, origin_circuit_t *rend_circ); diff --git a/src/or/nodelist.c b/src/or/nodelist.c index eae74e18b..fd5a8f0c0 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -48,6 +48,7 @@ #include "entrynodes.h" #include "geoip.h" #include "hs_common.h" +#include "hs_client.h" #include "main.h" #include "microdesc.h" #include "networkstatus.h" @@ -1973,6 +1974,7 @@ router_dir_info_changed(void) need_to_update_have_min_dir_info = 1; rend_hsdir_routers_changed(); hs_service_dir_info_changed(); + hs_client_dir_info_changed(); } /** Return a string describing what we're missing before we have enough diff --git a/src/test/test_channelpadding.c b/src/test/test_channelpadding.c index d54c9cc52..d5713688a 100644 --- a/src/test/test_channelpadding.c +++ b/src/test/test_channelpadding.c @@ -193,7 +193,8 @@ static void setup_mock_network(void) { routerstatus_t *relay; - connection_array = smartlist_new(); + if (!connection_array) + connection_array = smartlist_new(); relay1_relay2 = (channel_t*)new_fake_channeltls(2); relay1_relay2->write_cell = mock_channel_write_cell_relay1; @@ -280,7 +281,8 @@ test_channelpadding_timers(void *arg) tor_libevent_postfork(); - connection_array = smartlist_new(); + if (!connection_array) + connection_array = smartlist_new(); monotime_init(); monotime_enable_test_mocking(); @@ -570,7 +572,8 @@ test_channelpadding_consensus(void *arg) monotime_coarse_set_mock_time_nsec(1); timers_initialize(); - connection_array = smartlist_new(); + if (!connection_array) + connection_array = smartlist_new(); chan = (channel_t*)new_fake_channeltls(0); channel_timestamp_active(chan); @@ -928,7 +931,8 @@ test_channelpadding_decide_to_pad_channel(void *arg) */ channel_t *chan; int64_t new_time; - connection_array = smartlist_new(); + if (!connection_array) + connection_array = smartlist_new(); (void)arg; tor_libevent_postfork(); diff --git a/src/test/test_entryconn.c b/src/test/test_entryconn.c index 86c0c3dca..c29b1a712 100644 --- a/src/test/test_entryconn.c +++ b/src/test/test_entryconn.c @@ -790,8 +790,9 @@ test_entryconn_rewrite_onion_v3(void *arg) retval = connection_ap_handshake_rewrite_and_attach(conn, NULL, NULL); tt_int_op(retval, OP_EQ, 0); - /* Check connection state after rewrite */ - tt_int_op(ENTRY_TO_CONN(conn)->state, OP_EQ, AP_CONN_STATE_CIRCUIT_WAIT); + /* Check connection state after rewrite. It should be in waiting for + * descriptor state. */ + tt_int_op(ENTRY_TO_CONN(conn)->state, OP_EQ, AP_CONN_STATE_RENDDESC_WAIT); /* check that the address got rewritten */ tt_str_op(conn->socks_request->address, OP_EQ, "25njqamcweflpvkl73j4szahhihoc4xt3ktcgjnpaingr5yhkenl5sid");

1 0

[tor/release-0.3.2] make 23762 changes file pass check-changes
by nickm＠torproject.org 31 Oct '17

31 Oct '17

commit 652d22e608c10ee520a16fd86a4cdd90e5188025 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 31 12:19:39 2017 -0400 make 23762 changes file pass check-changes --- changes/bug23762 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes/bug23762 b/changes/bug23762 index bf35e5ec1..741a88e21 100644 --- a/changes/bug23762 +++ b/changes/bug23762 @@ -1,4 +1,4 @@ o Minor bugfixes (hidden service v3): - Properly retry HSv3 descriptor fetches in the case where we were initially - missing required directory information. Closes ticket #23762; bugfix on + missing required directory information. Fixes bug 23762; bugfix on 0.3.2.1-alpha.

1 0

[tor/master] hs-v3: Refactor client refetch function
by nickm＠torproject.org 31 Oct '17

31 Oct '17

commit 469ee6e6fe0dbc9bb7b3c8ceed8aa7c494fd4ac3 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Oct 4 15:49:25 2017 -0400 hs-v3: Refactor client refetch function Create a function that tells us if we can fetch or not the descriptor for the given service key. No behavior change. Mostly moving code but with a slight change so the function can properly work by returning a boolean and also a possible fetch status code. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/hs_client.c | 122 +++++++++++++++++++++++++++++++++-------------------- 1 file changed, 76 insertions(+), 46 deletions(-) diff --git a/src/or/hs_client.c b/src/or/hs_client.c index 93a913b34..d66e56352 100644 --- a/src/or/hs_client.c +++ b/src/or/hs_client.c @@ -1029,6 +1029,73 @@ handle_rendezvous2(origin_circuit_t *circ, const uint8_t *payload, return ret; } +/* Return true iff the client can fetch a descriptor for this service public + * identity key and status_out if not NULL is untouched. If the client can + * _not_ fetch the descriptor and if status_out is not NULL, it is set with + * the fetch status code. */ +static unsigned int +can_client_refetch_desc(const ed25519_public_key_t *identity_pk, + hs_client_fetch_status_t *status_out) +{ + hs_client_fetch_status_t status; + + tor_assert(identity_pk); + + /* Are we configured to fetch descriptors? */ + if (!get_options()->FetchHidServDescriptors) { + log_warn(LD_REND, "We received an onion address for a hidden service " + "descriptor but we are configured to not fetch."); + status = HS_CLIENT_FETCH_NOT_ALLOWED; + goto cannot; + } + + /* Without a live consensus we can't do any client actions. It is needed to + * compute the hashring for a service. */ + if (!networkstatus_get_live_consensus(approx_time())) { + log_info(LD_REND, "Can't fetch descriptor for service %s because we " + "are missing a live consensus. Stalling connection.", + safe_str_client(ed25519_fmt(identity_pk))); + status = HS_CLIENT_FETCH_MISSING_INFO; + goto cannot; + } + + if (!router_have_minimum_dir_info()) { + log_info(LD_REND, "Can't fetch descriptor for service %s because we " + "dont have enough descriptors. Stalling connection.", + safe_str_client(ed25519_fmt(identity_pk))); + status = HS_CLIENT_FETCH_MISSING_INFO; + goto cannot; + } + + /* Check if fetching a desc for this HS is useful to us right now */ + { + const hs_descriptor_t *cached_desc = NULL; + cached_desc = hs_cache_lookup_as_client(identity_pk); + if (cached_desc && hs_client_any_intro_points_usable(identity_pk, + cached_desc)) { + log_info(LD_GENERAL, "We would fetch a v3 hidden service descriptor " + "but we already have a usable descriptor."); + status = HS_CLIENT_FETCH_HAVE_DESC; + goto cannot; + } + } + + /* Don't try to refetch while we have a pending request for it. */ + if (directory_request_is_pending(identity_pk)) { + log_info(LD_REND, "Already a pending directory request. Waiting on it."); + status = HS_CLIENT_FETCH_PENDING; + goto cannot; + } + + /* Yes, client can fetch! */ + return 1; + cannot: + if (status_out) { + *status_out = status; + } + return 0; +} + /* ========== */ /* Public API */ /* ========== */ @@ -1134,62 +1201,25 @@ hs_client_any_intro_points_usable(const ed25519_public_key_t *service_pk, int hs_client_refetch_hsdesc(const ed25519_public_key_t *identity_pk) { - int ret; + hs_client_fetch_status_t status; tor_assert(identity_pk); - /* Are we configured to fetch descriptors? */ - if (!get_options()->FetchHidServDescriptors) { - log_warn(LD_REND, "We received an onion address for a hidden service " - "descriptor but we are configured to not fetch."); - return HS_CLIENT_FETCH_NOT_ALLOWED; - } - - /* Without a live consensus we can't do any client actions. It is needed to - * compute the hashring for a service. */ - if (!networkstatus_get_live_consensus(approx_time())) { - log_info(LD_REND, "Can't fetch descriptor for service %s because we " - "are missing a live consensus. Stalling connection.", - safe_str_client(ed25519_fmt(identity_pk))); - return HS_CLIENT_FETCH_MISSING_INFO; + if (!can_client_refetch_desc(identity_pk, &status)) { + return status; } - if (!router_have_minimum_dir_info()) { - log_info(LD_REND, "Can't fetch descriptor for service %s because we " - "dont have enough descriptors. Stalling connection.", - safe_str_client(ed25519_fmt(identity_pk))); - return HS_CLIENT_FETCH_MISSING_INFO; - } - - /* Check if fetching a desc for this HS is useful to us right now */ - { - const hs_descriptor_t *cached_desc = NULL; - cached_desc = hs_cache_lookup_as_client(identity_pk); - if (cached_desc && hs_client_any_intro_points_usable(identity_pk, - cached_desc)) { - log_info(LD_GENERAL, "We would fetch a v3 hidden service descriptor " - "but we already have a usable descriptor."); - return HS_CLIENT_FETCH_HAVE_DESC; - } - } - - /* Don't try to refetch while we have a pending request for it. */ - if (directory_request_is_pending(identity_pk)) { - log_info(LD_REND, "Already a pending directory request. Waiting on it."); - return HS_CLIENT_FETCH_PENDING; - } - - /* Try to fetch the desc and if we encounter an unrecoverable error, mark the - * desc as unavailable for now. */ - ret = fetch_v3_desc(identity_pk); - if (fetch_status_should_close_socks(ret)) { - close_all_socks_conns_waiting_for_desc(identity_pk, ret, + /* Try to fetch the desc and if we encounter an unrecoverable error, mark + * the desc as unavailable for now. */ + status = fetch_v3_desc(identity_pk); + if (fetch_status_should_close_socks(status)) { + close_all_socks_conns_waiting_for_desc(identity_pk, status, END_STREAM_REASON_RESOLVEFAILED); /* Remove HSDir fetch attempts so that we can retry later if the user * wants us to regardless of if we closed any connections. */ purge_hid_serv_request(identity_pk); } - return ret; + return status; } /* This is called when we are trying to attach an AP connection to these

1 0

[tor/master] Merge branch 'maint-0.3.2'
by nickm＠torproject.org 31 Oct '17

31 Oct '17

commit 4700ba6c3d67056fc271cd2355fc956c182af7ad Merge: 8b536ca59 652d22e60 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 31 12:19:42 2017 -0400 Merge branch 'maint-0.3.2' changes/bug23762 | 4 + src/or/connection_edge.c | 9 +- src/or/hs_client.c | 181 ++++++++++++++++++++++++++++++----------- src/or/hs_client.h | 1 + src/or/nodelist.c | 2 + src/test/test_channelpadding.c | 12 ++- src/test/test_entryconn.c | 5 +- 7 files changed, 157 insertions(+), 57 deletions(-)

1 0