April 2018 - tor-commits - lists.torproject.org

[tor/master] test: Add testing module and some unittests for bridges.c.
by nickm＠torproject.org 10 Apr '18

10 Apr '18

commit c2c5b13e5d8a77eeee36028940175f182fda1ec9 Author: Isis Lovecruft <isis(a)torproject.org> Date: Mon Mar 5 20:43:26 2018 +0000 test: Add testing module and some unittests for bridges.c. This roughly doubles our test coverage of the bridges.c module. * ADD new testing module, .../src/test/test_bridges.c. * CHANGE a few function declarations from `static` to `STATIC`. * CHANGE one function in transports.c, transport_get_by_name(), to be mockable. * CLOSES #25425: https://bugs.torproject.org/25425 --- src/or/bridges.c | 10 +- src/or/bridges.h | 13 +- src/or/transports.c | 6 +- src/or/transports.h | 5 +- src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_bridges.c | 618 ++++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 646 insertions(+), 9 deletions(-) diff --git a/src/or/bridges.c b/src/or/bridges.c index d88d6c643..4ee355004 100644 --- a/src/or/bridges.c +++ b/src/or/bridges.c @@ -11,6 +11,8 @@ * Bridges are fixed entry nodes, used for censorship circumvention. **/ +#define TOR_BRIDGES_PRIVATE + #include "or.h" #include "bridges.h" #include "circuitbuild.h" @@ -93,7 +95,7 @@ sweep_bridge_list(void) } /** Initialize the bridge list to empty, creating it if needed. */ -static void +STATIC void clear_bridge_list(void) { if (!bridge_list) @@ -156,7 +158,7 @@ bridge_get_addr_port(const bridge_info_t *bridge) * bridge with no known digest whose address matches any of the * tor_addr_port_t's in orports, return that bridge. Else return * NULL. */ -static bridge_info_t * +STATIC bridge_info_t * get_configured_bridge_by_orports_digest(const char *digest, const smartlist_t *orports) { @@ -350,7 +352,7 @@ bridge_has_digest(const bridge_info_t *bridge, const char *digest) * existing bridge with the same address and port, and warn the user as * appropriate. */ -static void +STATIC void bridge_resolve_conflicts(const tor_addr_t *addr, uint16_t port, const char *digest, const char *transport_name) { @@ -472,7 +474,7 @@ bridge_add_from_config(bridge_line_t *bridge_line) } /** If digest is one of our known bridges, return it. */ -bridge_info_t * +STATIC bridge_info_t * find_bridge_by_digest(const char *digest) { if (! bridge_list) diff --git a/src/or/bridges.h b/src/or/bridges.h index 54a625025..3108eb555 100644 --- a/src/or/bridges.h +++ b/src/or/bridges.h @@ -20,7 +20,6 @@ typedef struct bridge_info_t bridge_info_t; void mark_bridge_list(void); void sweep_bridge_list(void); const smartlist_t *bridge_list_get(void); -bridge_info_t *find_bridge_by_digest(const char *digest); const uint8_t *bridge_get_rsa_id_digest(const bridge_info_t *bridge); const tor_addr_port_t * bridge_get_addr_port(const bridge_info_t *bridge); bridge_info_t *get_configured_bridge_by_addr_port_digest( @@ -65,5 +64,17 @@ MOCK_DECL(download_status_t *, get_bridge_dl_status_by_id, void bridges_free_all(void); +#ifdef TOR_BRIDGES_PRIVATE +STATIC void clear_bridge_list(void); +STATIC bridge_info_t *find_bridge_by_digest(const char *digest); +STATIC bridge_info_t *get_configured_bridge_by_orports_digest( + const char *digest, + const smartlist_t *orports); +STATIC void bridge_resolve_conflicts(const tor_addr_t *addr, + uint16_t port, + const char *digest, + const char *transport_name); +#endif /* defined(TOR_BRIDGES_PRIVATE) */ + #endif /* !defined(TOR_BRIDGES_H) */ diff --git a/src/or/transports.c b/src/or/transports.c index b08dcd161..fb6c29de7 100644 --- a/src/or/transports.c +++ b/src/or/transports.c @@ -135,7 +135,7 @@ static smartlist_t *transport_list = NULL; /** Returns a transport_t struct for a transport proxy supporting the protocol name listening at addr:port using SOCKS version socks_ver. */ -static transport_t * +STATIC transport_t * transport_new(const tor_addr_t *addr, uint16_t port, const char *name, int socks_ver, const char *extra_info_args) @@ -222,8 +222,8 @@ transport_copy(const transport_t *transport) /** Returns the transport in our transport list that has the name name. * Else returns NULL. */ -transport_t * -transport_get_by_name(const char *name) +MOCK_IMPL(transport_t *, +transport_get_by_name,(const char *name)) { tor_assert(name); diff --git a/src/or/transports.h b/src/or/transports.h index 1b2786472..022b926a0 100644 --- a/src/or/transports.h +++ b/src/or/transports.h @@ -38,7 +38,7 @@ MOCK_DECL(int, transport_add_from_config, void transport_free_(transport_t *transport); #define transport_free(tr) FREE_AND_NULL(transport_t, transport_free_, (tr)) -transport_t *transport_get_by_name(const char *name); +MOCK_DECL(transport_t*, transport_get_by_name, (const char *name)); MOCK_DECL(void, pt_kickstart_proxy, (const smartlist_t *transport_list, char **proxy_argv, @@ -113,6 +113,9 @@ typedef struct { smartlist_t *transports; } managed_proxy_t; +STATIC transport_t *transport_new(const tor_addr_t *addr, uint16_t port, + const char *name, int socks_ver, + const char *extra_info_args); STATIC int parse_cmethod_line(const char *line, managed_proxy_t *mp); STATIC int parse_smethod_line(const char *line, managed_proxy_t *mp); diff --git a/src/test/include.am b/src/test/include.am index 1a49367c6..7a3d05661 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -88,6 +88,7 @@ src_test_test_SOURCES = \ src/test/test_addr.c \ src/test/test_address.c \ src/test/test_address_set.c \ + src/test/test_bridges.c \ src/test/test_buffers.c \ src/test/test_cell_formats.c \ src/test/test_cell_queue.c \ diff --git a/src/test/test.c b/src/test/test.c index 2712de4ed..c9c646890 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1185,6 +1185,7 @@ struct testgroup_t testgroups[] = { { "addr/", addr_tests }, { "address/", address_tests }, { "address_set/", address_set_tests }, + { "bridges/", bridges_tests }, { "buffer/", buffer_tests }, { "cellfmt/", cell_format_tests }, { "cellqueue/", cell_queue_tests }, diff --git a/src/test/test.h b/src/test/test.h index 26139fc5f..91c8d3ebd 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -178,6 +178,7 @@ extern struct testcase_t accounting_tests[]; extern struct testcase_t addr_tests[]; extern struct testcase_t address_tests[]; extern struct testcase_t address_set_tests[]; +extern struct testcase_t bridges_tests[]; extern struct testcase_t buffer_tests[]; extern struct testcase_t cell_format_tests[]; extern struct testcase_t cell_queue_tests[]; diff --git a/src/test/test_bridges.c b/src/test/test_bridges.c new file mode 100644 index 000000000..aada363d9 --- /dev/null +++ b/src/test/test_bridges.c @@ -0,0 +1,618 @@ +/* Copyright (c) 2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file test_bridges.c + * \brief Unittests for code in src/or/bridges.c + **/ + +#define TOR_BRIDGES_PRIVATE +#define PT_PRIVATE /* Only needed for the mock_* items below */ + +#include <stdbool.h> + +#include "or.h" +#include "address.h" +#include "bridges.h" +#include "config.h" +#include "container.h" +#include "transports.h" +#include "util.h" + +/* Test suite stuff */ +#include "test.h" + +/** + * A mocked transport_t, constructed via mock_transport_get_by_name(). + */ +static transport_t *mock_transport = NULL; + +/** + * Mock transport_get_by_name() to simply return a transport_t for the + * transport name that was input to it. + */ +static transport_t * +mock_transport_get_by_name(const char *name) +{ + tor_addr_t *addr = tor_malloc(sizeof(tor_addr_t)); + uint16_t port = 9999; + int socksv = 9; + char *args = tor_strdup("foo=bar"); + + if (!mock_transport) { + tor_addr_parse(addr, "99.99.99.99"); + mock_transport = transport_new(addr, port, name, socksv, args); + } + + tor_free(addr); + tor_free(args); + + return mock_transport; +} + +#undef PT_PRIVATE /* defined(PT_PRIVATE) */ + +/** + * Test helper: Add a variety of bridges to our global bridgelist. + */ +static void +helper_add_bridges_to_bridgelist(void *arg) +{ + /* Note: the two bridges which do not have specified fingerprints will be + * internally stored as both having the same fingerprint of all-zero bytes. + */ + + (void)arg; + char *bridge0 = tor_strdup("6.6.6.6:6666"); + char *bridge1 = tor_strdup("6.6.6.7:6667 " + "A10C4F666D27364036B562823E5830BC448E046A"); + char *bridge2 = tor_strdup("obfs4 198.245.60.51:443 " + "752CF7825B3B9EA6A98C83AC41F7099D67007EA5 " + "cert=xpmQtKUqQ/6v5X7ijgYE/f03+l2/EuQ1dexjyUhh16wQlu/" + "cpXUGalmhDIlhuiQPNEKmKw iat-mode=0"); + char *bridge3 = tor_strdup("banana 5.5.5.5:5555 " + "9D6AE1BD4FDF39721CE908966E79E16F9BFCCF2F"); + char *bridge4 = tor_strdup("obfs4 1.2.3.4:1234 " + "foo=abcdefghijklmnopqrstuvwxyz"); + char *bridge5 = tor_strdup("apple 4.4.4.4:4444 " + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA " + "foo=abcdefghijklmnopqrstuvwxyz"); + + mark_bridge_list(); + +#define ADD_BRIDGE(bridge) \ + bridge_line_t *bridge_line_ ##bridge = parse_bridge_line(bridge); \ + if (!bridge_line_ ##bridge) { \ + printf("Unparseable bridge line: '%s'", #bridge); \ + } else { \ + bridge_add_from_config(bridge_line_ ##bridge); \ + } \ + tor_free(bridge); + + ADD_BRIDGE(bridge0); + ADD_BRIDGE(bridge1); + ADD_BRIDGE(bridge2); + ADD_BRIDGE(bridge3); + ADD_BRIDGE(bridge4); + ADD_BRIDGE(bridge5); +#undef ADD_BRIDGES + + sweep_bridge_list(); +} + +/** + * Make sure our test helper works too. + */ +static void +test_bridges_helper_func_add_bridges_to_bridgelist(void *arg) +{ + helper_add_bridges_to_bridgelist(arg); + tt_int_op(0, OP_EQ, 0); + + done: + mark_bridge_list(); + sweep_bridge_list(); +} + +/** + * Calling bridge_list_get() should create a new bridgelist if we + * didn't have one before. + */ +static void +test_bridges_bridge_list_get_creates_new_bridgelist(void *arg) +{ + const smartlist_t *bridgelist = bridge_list_get(); + + (void)arg; + + tt_ptr_op(bridgelist, OP_NE, NULL); + + done: + return; +} + +/** + * Calling clear_bridge_list() should remove all bridges from the bridgelist. + */ +static void +test_bridges_clear_bridge_list(void *arg) +{ + const smartlist_t *bridgelist; + const smartlist_t *bridgelist_after; + const bridge_info_t *bridge; + const bridge_info_t *bridge_after; + + helper_add_bridges_to_bridgelist(arg); + bridgelist = bridge_list_get(); + tt_ptr_op(bridgelist, OP_NE, NULL); + + bridge = smartlist_get(bridgelist, 0); + tt_ptr_op(bridge, OP_NE, NULL); + + clear_bridge_list(); + bridgelist_after = bridge_list_get(); + tt_ptr_op(bridgelist_after, OP_NE, NULL); + + bridge_after = smartlist_get(bridgelist, 0); + // There now shouldn't be a first bridge + tt_ptr_op(bridge_after, OP_EQ, NULL); + + done: + return; +} + +/** + * Calling bridge_get_addrport() should give me the address and port + * of the bridge. In this case, we sort the smartlist of bridges on + * fingerprints and choose the first one. + */ +static void +test_bridges_bridge_get_addrport(void *arg) +{ + smartlist_t *bridgelist; + const bridge_info_t *bridge; + const tor_addr_port_t *addrport; + + helper_add_bridges_to_bridgelist(arg); + bridgelist = (smartlist_t*)bridge_list_get(); + tt_ptr_op(bridgelist, OP_NE, NULL); + + // This should be the bridge at 6.6.6.6:6666 with fingerprint + // 0000000000000000000000000000000000000000 + bridge = smartlist_get(bridgelist, 0); + tt_ptr_op(bridge, OP_NE, NULL); + + addrport = bridge_get_addr_port(bridge); + tt_int_op(addrport->port, OP_EQ, 6666); + + done: + mark_bridge_list(); + sweep_bridge_list(); +} + +/** + * Calling get_configured_bridge_by_orports_digest() with two + * configured bridge orports and an invalid digest should return the + * bridge of the first addrport in the list. + */ +static void +test_bridges_get_configured_bridge_by_orports_digest(void *arg) +{ + smartlist_t *orports = NULL; + const smartlist_t *bridgelist; + const bridge_info_t *bridge1; + const bridge_info_t *bridge2; + const bridge_info_t *ret; + tor_addr_port_t *addrport1; + tor_addr_port_t *addrport2; + const char *digest; + + helper_add_bridges_to_bridgelist(arg); + bridgelist = bridge_list_get(); + tt_ptr_op(bridgelist, OP_NE, NULL); + + // This should be the bridge at 6.6.6.6:6666 with fingerprint + // 0000000000000000000000000000000000000000 + bridge1 = smartlist_get(bridgelist, 0); + tt_ptr_op(bridge1, OP_NE, NULL); + // This should be the bridge at 6.6.6.7:6667 with fingerprint + // A10C4F666D27364036B562823E5830BC448E046A + bridge2 = smartlist_get(bridgelist, 1); + tt_ptr_op(bridge2, OP_NE, NULL); + + addrport1 = (tor_addr_port_t*)bridge_get_addr_port(bridge1); + tt_int_op(addrport1->port, OP_EQ, 6666); + addrport2 = (tor_addr_port_t*)bridge_get_addr_port(bridge2); + tt_int_op(addrport2->port, OP_EQ, 6667); + + orports = smartlist_new(); + smartlist_add(orports, addrport1); + smartlist_add(orports, addrport2); + + digest = "zzzzzzzzzzzzzzzz"; + + ret = get_configured_bridge_by_orports_digest(digest, orports); + tt_ptr_op(ret, OP_NE, NULL); + + tt_mem_op(addrport1, OP_EQ, bridge_get_addr_port(ret), sizeof(ret)); + + done: + smartlist_free(orports); + + mark_bridge_list(); + sweep_bridge_list(); +} + +/** + * Calling get_configured_bridge_by_addr_port_digest() with a digest that we do + * have and an addr:port pair we don't should return the bridge for that + * digest. + */ +static void +test_bridges_get_configured_bridge_by_addr_port_digest_digest_only(void *arg) +{ + char digest[DIGEST_LEN]; + bridge_info_t *bridge; + const char fingerprint[HEX_DIGEST_LEN] = + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + tor_addr_t *addr = tor_malloc(sizeof(tor_addr_t)); + char ret_addr[16]; + uint16_t port = 11111; + int ret; + + helper_add_bridges_to_bridgelist(arg); + + // We don't actually have a bridge with this addr:port pair + base16_decode(digest, DIGEST_LEN, fingerprint, HEX_DIGEST_LEN); + ret = tor_addr_parse(addr, "111.111.111.111"); + tt_int_op(ret, OP_EQ, 2); // it returns the address family on success + + bridge = get_configured_bridge_by_addr_port_digest(addr, port, digest); + tt_ptr_op(bridge, OP_NE, NULL); + + tor_addr_to_str(ret_addr, &bridge_get_addr_port(bridge)->addr, 16, 0); + tt_str_op("4.4.4.4", OP_EQ, ret_addr); + + done: + tor_free(addr); + + mark_bridge_list(); + sweep_bridge_list(); +} + +/** + * Calling get_configured_bridge_by_addr_port_digest() with only an + * addr:port (i.e. digest set to NULL) should return the bridge for + * that digest when there is such a bridge. + */ +static void +test_bridges_get_configured_bridge_by_addr_port_digest_address_only(void *arg) +{ + bridge_info_t *bridge; + tor_addr_t *addr = tor_malloc(sizeof(tor_addr_t)); + char ret_addr[16]; + uint16_t port = 6666; + int ret; + + helper_add_bridges_to_bridgelist(arg); + + ret = tor_addr_parse(addr, "6.6.6.6"); + tt_int_op(ret, OP_EQ, 2); // it returns the address family on success + + bridge = get_configured_bridge_by_addr_port_digest(addr, port, NULL); + tt_ptr_op(bridge, OP_NE, NULL); + + tor_addr_to_str(ret_addr, &bridge_get_addr_port(bridge)->addr, 16, 0); + tt_str_op("6.6.6.6", OP_EQ, ret_addr); + + done: + tor_free(addr); + + mark_bridge_list(); + sweep_bridge_list(); +} + +/** + * Calling get_configured_bridge_by_exact_addr_port_digest() with a digest that + * we do have, and an addr:port pair we don't have, should return NULL. + */ +static void +test_bridges_get_configured_bridge_by_exact_addr_port_digest_donly(void *arg) +{ + char digest[DIGEST_LEN]; + bridge_info_t *bridge; + const char fingerprint[HEX_DIGEST_LEN] = + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + tor_addr_t *addr = tor_malloc(sizeof(tor_addr_t)); + uint16_t port = 11111; + int ret; + + helper_add_bridges_to_bridgelist(arg); + + // We don't actually have a bridge with this addr:port pair + base16_decode(digest, DIGEST_LEN, fingerprint, HEX_DIGEST_LEN); + ret = tor_addr_parse(addr, "111.111.111.111"); + tt_int_op(ret, OP_EQ, 2); // it returns the address family on success + + bridge = get_configured_bridge_by_exact_addr_port_digest(addr, port, digest); + tt_ptr_op(bridge, OP_EQ, NULL); + + done: + tor_free(addr); + + mark_bridge_list(); + sweep_bridge_list(); +} + +/** + * Calling get_configured_bridge_by_exact_addr_port_digest() with a digest that + * we do have, and an addr:port pair we do have, should return the bridge. + */ +static void +test_bridges_get_configured_bridge_by_exact_addr_port_digest_both(void *arg) +{ + char digest[DIGEST_LEN]; + bridge_info_t *bridge; + const char fingerprint[HEX_DIGEST_LEN] = + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + tor_addr_t *addr = tor_malloc(sizeof(tor_addr_t)); + uint16_t port = 4444; + char ret_addr[16]; + int ret; + + helper_add_bridges_to_bridgelist(arg); + + base16_decode(digest, DIGEST_LEN, fingerprint, HEX_DIGEST_LEN); + ret = tor_addr_parse(addr, "4.4.4.4"); + tt_int_op(ret, OP_EQ, 2); // it returns the address family on success + + bridge = get_configured_bridge_by_exact_addr_port_digest(addr, port, digest); + tt_ptr_op(bridge, OP_NE, NULL); + + tor_addr_to_str(ret_addr, &bridge_get_addr_port(bridge)->addr, 16, 0); + tt_str_op("4.4.4.4", OP_EQ, ret_addr); + + done: + tor_free(addr); + + mark_bridge_list(); + sweep_bridge_list(); +} + +/** + * Calling get_configured_bridge_by_exact_addr_port_digest() with no digest, + * and an addr:port pair we do have, should return the bridge. + */ +static void +test_bridges_get_configured_bridge_by_exact_addr_port_digest_aonly(void *arg) +{ + bridge_info_t *bridge; + tor_addr_t *addr = tor_malloc(sizeof(tor_addr_t)); + uint16_t port = 4444; + char ret_addr[16]; + int ret; + + helper_add_bridges_to_bridgelist(arg); + + ret = tor_addr_parse(addr, "4.4.4.4"); + tt_int_op(ret, OP_EQ, 2); // it returns the address family on success + + bridge = get_configured_bridge_by_exact_addr_port_digest(addr, port, NULL); + tt_ptr_op(bridge, OP_NE, NULL); + + tor_addr_to_str(ret_addr, &bridge_get_addr_port(bridge)->addr, 16, 0); + tt_str_op("4.4.4.4", OP_EQ, ret_addr); + + done: + tor_free(addr); + + mark_bridge_list(); + sweep_bridge_list(); +} + +/** + * Calling find_bridge_by_digest() when we have a bridge with a known + * identity digest should return the bridge's information. + */ +static void +test_bridges_find_bridge_by_digest_known(void *arg) +{ + char digest1[DIGEST_LEN]; + bridge_info_t *bridge; + const char fingerprint[HEX_DIGEST_LEN] = + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + + helper_add_bridges_to_bridgelist(arg); + + base16_decode(digest1, DIGEST_LEN, fingerprint, HEX_DIGEST_LEN); + bridge = find_bridge_by_digest(digest1); + + tt_ptr_op(bridge, OP_NE, NULL); + + /* We have to call bridge_get_rsa_id_digest() here because the bridge_info_t + * struct is opaquely defined in bridges.h. */ + const uint8_t *digest2 = bridge_get_rsa_id_digest(bridge); + + tt_mem_op((char*)digest2, OP_EQ, digest1, DIGEST_LEN); + + done: + mark_bridge_list(); + sweep_bridge_list(); +} + +/** + * Calling find_bridge_by_digest() when we do NOT have a bridge with that + * identity digest should return NULL. + */ +static void +test_bridges_find_bridge_by_digest_unknown(void *arg) +{ + const char *fingerprint = "cccccccccccccccccccccccccccccccccccccccc"; + bridge_info_t *bridge; + + helper_add_bridges_to_bridgelist(arg); + + bridge = find_bridge_by_digest(fingerprint); + + tt_ptr_op(bridge, OP_EQ, NULL); + + done: + mark_bridge_list(); + sweep_bridge_list(); +} + +/** + * Calling bridge_resolve_conflicts() with an identical bridge to one we've + * already configure should mark the pre-configured bridge for removal. + */ +static void +test_bridges_bridge_resolve_conflicts(void *arg) +{ + tor_addr_t *addr = tor_malloc(sizeof(tor_addr_t)); + uint16_t port = 4444; + const char *digest = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + const char *transport = "apple"; + int ret; + + helper_add_bridges_to_bridgelist(arg); + + ret = tor_addr_parse(addr, "4.4.4.4"); + tt_int_op(ret, OP_EQ, 2); // it returns the address family on success + + bridge_resolve_conflicts((const tor_addr_t*)addr, port, digest, transport); + + /* The bridge should now be marked for removal, and removed when we sweep the + * bridge_list */ + sweep_bridge_list(); + ret = addr_is_a_configured_bridge((const tor_addr_t*)addr, port, digest); + tt_int_op(ret, OP_EQ, 0); + + done: + tor_free(addr); + + mark_bridge_list(); + sweep_bridge_list(); +} + +/** + * Calling transport_is_needed() with a transport we do need ("obfs4") and a + * bogus transport that we don't need should return 1 and 0, respectively. + */ +static void +test_bridges_transport_is_needed(void *arg) +{ + int ret; + + helper_add_bridges_to_bridgelist(arg); + + ret = transport_is_needed("obfs4"); + tt_int_op(ret, OP_EQ, 1); + + ret = transport_is_needed("apowefjaoewpaief"); + tt_int_op(ret, OP_EQ, 0); + + done: + mark_bridge_list(); + sweep_bridge_list(); +} + +/** + * Calling get_transport_by_bridge_addrport() with the address and port of a + * configured bridge which uses a pluggable transport when there is no global + * transport_list should return -1 and the transport_t should be NULL. + */ +static void +test_bridges_get_transport_by_bridge_addrport_no_ptlist(void *arg) +{ + transport_t *transport = NULL; + tor_addr_t *addr = tor_malloc(sizeof(tor_addr_t)); + uint16_t port = 1234; + int ret; + + helper_add_bridges_to_bridgelist(arg); + + ret = tor_addr_parse(addr, "1.2.3.4"); + tt_int_op(ret, OP_EQ, 2); // it returns the address family on success? + + /* This will fail because the global transport_list has nothing in it, and so + * transport_get_by_name() has nothing to return, even the the bridge *did* + * say it had an obfs4 transport. + */ + ret = get_transport_by_bridge_addrport((const tor_addr_t*)addr, port, + (const transport_t**)&transport); + tt_int_op(ret, OP_EQ, -1); // returns -1 on failure + tt_ptr_op(transport, OP_EQ, NULL); + + done: + tor_free(addr); + + mark_bridge_list(); + sweep_bridge_list(); +} + +#define PT_PRIVATE + +/** + * Calling get_transport_by_bridge_addrport() with the address and port of a + * configured bridge which uses a pluggable transport should return 0 and set + * appropriate transport_t. + */ +static void +test_bridges_get_transport_by_bridge_addrport(void *arg) +{ + transport_t *transport = NULL; + tor_addr_t *addr = tor_malloc(sizeof(tor_addr_t)); + uint16_t port = 1234; + int ret; + + helper_add_bridges_to_bridgelist(arg); + mark_transport_list(); // Also initialise our transport_list + + ret = tor_addr_parse(addr, "1.2.3.4"); + tt_int_op(ret, OP_EQ, 2); // it returns the address family on success? + + /* After we mock transport_get_by_name() to return a bogus transport_t with + * the name it was asked for, the call should succeed. + */ + MOCK(transport_get_by_name, mock_transport_get_by_name); + ret = get_transport_by_bridge_addrport((const tor_addr_t*)addr, port, + (const transport_t**)&transport); + tt_int_op(ret, OP_EQ, 0); // returns 0 on success + tt_ptr_op(transport, OP_NE, NULL); + tt_str_op(transport->name, OP_EQ, "obfs4"); + + done: + UNMOCK(transport_get_by_name); + + tor_free(addr); + transport_free(transport); + + mark_bridge_list(); + sweep_bridge_list(); +} + +#undef PT_PRIVATE /* defined(PT_PRIVATE) */ + +#define B_TEST(name, flags) \ + { #name, test_bridges_ ##name, (flags), NULL, NULL } + +struct testcase_t bridges_tests[] = { + B_TEST(helper_func_add_bridges_to_bridgelist, 0), + B_TEST(bridge_list_get_creates_new_bridgelist, 0), + B_TEST(clear_bridge_list, 0), + B_TEST(bridge_get_addrport, 0), + B_TEST(get_configured_bridge_by_orports_digest, 0), + B_TEST(get_configured_bridge_by_addr_port_digest_digest_only, 0), + B_TEST(get_configured_bridge_by_addr_port_digest_address_only, 0), + B_TEST(get_configured_bridge_by_exact_addr_port_digest_donly, 0), + B_TEST(get_configured_bridge_by_exact_addr_port_digest_both, 0), + B_TEST(get_configured_bridge_by_exact_addr_port_digest_aonly, 0), + B_TEST(find_bridge_by_digest_known, 0), + B_TEST(find_bridge_by_digest_unknown, 0), + B_TEST(bridge_resolve_conflicts, 0), + B_TEST(get_transport_by_bridge_addrport_no_ptlist, 0), + B_TEST(get_transport_by_bridge_addrport, 0), + B_TEST(transport_is_needed, 0), + END_OF_TESTCASES +}; +

1 0

[tor/master] tests: Make tt_finished() macro for tests without tt_*_op() calls.
by nickm＠torproject.org 10 Apr '18

10 Apr '18

commit 3ee7a8d3a545633c578e059687f7683c43108a10 Author: Isis Lovecruft <isis(a)torproject.org> Date: Thu Mar 29 23:27:42 2018 +0000 tests: Make tt_finished() macro for tests without tt_*_op() calls. --- src/test/test.h | 8 ++++++++ src/test/test_bridges.c | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/test/test.h b/src/test/test.h index 91c8d3ebd..1eb3b6a2e 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -72,6 +72,14 @@ I64_PRINTF_TYPE, I64_FORMAT, \ {print_ = (I64_PRINTF_TYPE) value_;}, {}, TT_EXIT_TEST_FUNCTION) +/** + * Declare that the test is done, even though no tt___op() calls were made. + * + * For use when you only want to test calling something, but not check + * any values/pointers/etc afterwards. + */ +#define tt_finished() TT_EXIT_TEST_FUNCTION + const char *get_fname(const char *name); const char *get_fname_rnd(const char *name); struct crypto_pk_t *pk_generate(int idx); diff --git a/src/test/test_bridges.c b/src/test/test_bridges.c index aada363d9..b5bd27e09 100644 --- a/src/test/test_bridges.c +++ b/src/test/test_bridges.c @@ -107,7 +107,7 @@ static void test_bridges_helper_func_add_bridges_to_bridgelist(void *arg) { helper_add_bridges_to_bridgelist(arg); - tt_int_op(0, OP_EQ, 0); + tt_finished(); done: mark_bridge_list();

1 0

[tor/master] Merge remote-tracking branch 'isis-github/bug25425_squashed2'
by nickm＠torproject.org 10 Apr '18

10 Apr '18

commit 6bdfaa8b244ca3e10e57f6d99616af1d5f68fa67 Merge: 6e467a7a3 4178642bf Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Apr 10 15:32:26 2018 -0400 Merge remote-tracking branch 'isis-github/bug25425_squashed2' changes/bug25425 | 4 + src/or/bridges.c | 10 +- src/or/bridges.h | 13 +- src/or/transports.c | 6 +- src/or/transports.h | 5 +- src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 9 + src/test/test_bridges.c | 618 ++++++++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 658 insertions(+), 9 deletions(-)

1 0

[tor/master] changes: Add changes file for 25425.
by nickm＠torproject.org 10 Apr '18

10 Apr '18

commit 4178642bf83736d154176623ba8dce845a5b412c Author: Isis Lovecruft <isis(a)torproject.org> Date: Tue Apr 10 19:26:46 2018 +0000 changes: Add changes file for 25425. --- changes/bug25425 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/changes/bug25425 b/changes/bug25425 new file mode 100644 index 000000000..41f1a47b1 --- /dev/null +++ b/changes/bug25425 @@ -0,0 +1,4 @@ + o Minor features (testing): + - A new unittests module specifically for testing the functions in the + (new-ish) bridges.c module has been created with new unittests, raising + the code coverage percentages. Closes 25425.

1 0

[tor/master] Remove PortForwarding options
by nickm＠torproject.org 10 Apr '18

10 Apr '18

commit 9df110cd722a383414584d5ff6494944c993bb6d Author: Neel Chauhan <neel(a)neelc.org> Date: Tue Mar 20 08:27:41 2018 -0400 Remove PortForwarding options Signed-off-by: Isis Lovecruft <isis(a)torproject.org> --- changes/ticket25409 | 6 + doc/tor.1.txt | 19 +-- src/common/util.c | 348 ----------------------------------------- src/common/util.h | 9 +- src/or/config.c | 11 -- src/or/main.c | 29 ---- src/or/or.h | 4 - src/test/test_options.c | 32 ---- src/tools/tor-fw-helper/README | 10 -- 9 files changed, 10 insertions(+), 458 deletions(-) diff --git a/changes/ticket25409 b/changes/ticket25409 new file mode 100644 index 000000000..f0006fbc5 --- /dev/null +++ b/changes/ticket25409 @@ -0,0 +1,6 @@ + o Code simplification and refactoring: + We remove the PortForwsrding and PortForwardingHelper options, related + functions, and the port_forwarding tests. These options were used by + the now-deprecated Vidalia to help ordinary users become Tor relays or + bridges. Closes ticket 25409. Patch by Neel Chauhan. + diff --git a/doc/tor.1.txt b/doc/tor.1.txt index ccf6f8777..3d0e51f55 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -805,10 +805,9 @@ GENERAL OPTIONS [[NoExec]] **NoExec** **0**|**1**:: If this option is set to 1, then Tor will never launch another - executable, regardless of the settings of PortForwardingHelper, - ClientTransportPlugin, or ServerTransportPlugin. Once this - option has been set to 1, it cannot be set back to 0 without - restarting Tor. (Default: 0) + executable, regardless of the settings of ClientTransportPlugin + or ServerTransportPlugin. Once this option has been set to 1, + it cannot be set back to 0 without restarting Tor. (Default: 0) [[Schedulers]] **Schedulers** **KIST**|**KISTLite**|**Vanilla**:: Specify the scheduler type that tor should use. The scheduler is @@ -2083,18 +2082,6 @@ is non-zero): For obvious reasons, NoAdvertise and NoListen are mutually exclusive, and IPv4Only and IPv6Only are mutually exclusive. -[[PortForwarding]] **PortForwarding** **0**|**1**:: - Attempt to automatically forward the DirPort and ORPort on a NAT router - connecting this Tor server to the Internet. If set, Tor will try both - NAT-PMP (common on Apple routers) and UPnP (common on routers from other - manufacturers). (Default: 0) - -[[PortForwardingHelper]] **PortForwardingHelper** __filename__|__pathname__:: - If PortForwarding is set, use this executable to configure the forwarding. - If set to a filename, the system path will be searched for the executable. - If set to a path, only the specified path will be executed. - (Default: tor-fw-helper) - [[PublishServerDescriptor]] **PublishServerDescriptor** **0**|**1**|**v3**|**bridge**,**...**:: This option specifies which descriptors Tor will publish when acting as a relay. You can diff --git a/src/common/util.c b/src/common/util.c index f63b12167..065b3245f 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -5111,30 +5111,6 @@ stream_status_to_string(enum stream_status stream_status) } } -/* DOCDOC */ -static void -log_portfw_spawn_error_message(const char *buf, - const char *executable, int *child_status) -{ - /* Parse error message */ - int retval, child_state, saved_errno; - retval = tor_sscanf(buf, SPAWN_ERROR_MESSAGE "%x/%x", - &child_state, &saved_errno); - if (retval == 2) { - log_warn(LD_GENERAL, - "Failed to start child process \"%s\" in state %d: %s", - executable, child_state, strerror(saved_errno)); - if (child_status) - *child_status = 1; - } else { - /* Failed to parse message from child process, log it as a - warning */ - log_warn(LD_GENERAL, - "Unexpected message from port forwarding helper \"%s\": %s", - executable, buf); - } -} - #ifdef _WIN32 /** Return a smartlist containing lines outputted from @@ -5254,42 +5230,6 @@ tor_get_lines_from_handle, (int fd, enum stream_status *stream_status_out)) return lines; } -/** Read from fd, and send lines to log at the specified log level. - * Returns 1 if stream is closed normally, -1 if there is a error reading, and - * 0 otherwise. Handles lines from tor-fw-helper and - * tor_spawn_background() specially. - */ -static int -log_from_pipe(int fd, int severity, const char *executable, - int *child_status) -{ - char buf[256]; - enum stream_status r; - - for (;;) { - r = get_string_from_pipe(fd, buf, sizeof(buf) - 1); - - if (r == IO_STREAM_CLOSED) { - return 1; - } else if (r == IO_STREAM_EAGAIN) { - return 0; - } else if (r == IO_STREAM_TERM) { - return -1; - } - - tor_assert(r == IO_STREAM_OKAY); - - /* Check if buf starts with SPAWN_ERROR_MESSAGE */ - if (strcmpstart(buf, SPAWN_ERROR_MESSAGE) == 0) { - log_portfw_spawn_error_message(buf, executable, child_status); - } else { - log_fn(severity, LD_GENERAL, "Port forwarding helper says: %s", buf); - } - } - - /* We should never get here */ - return -1; -} #endif /* defined(_WIN32) */ /** Reads from fd and stores input in buf_out making @@ -5332,294 +5272,6 @@ get_string_from_pipe(int fd, char *buf_out, size_t count) return IO_STREAM_OKAY; } -/** Parse a line from tor-fw-helper and issue an appropriate - * log message to our user. */ -static void -handle_fw_helper_line(const char *executable, const char *line) -{ - smartlist_t *tokens = smartlist_new(); - char *message = NULL; - char *message_for_log = NULL; - const char *external_port = NULL; - const char *internal_port = NULL; - const char *result = NULL; - int port = 0; - int success = 0; - - if (strcmpstart(line, SPAWN_ERROR_MESSAGE) == 0) { - /* We need to check for SPAWN_ERROR_MESSAGE again here, since it's - * possible that it got sent after we tried to read it in log_from_pipe. - * - * XXX Ideally, we should be using one of stdout/stderr for the real - * output, and one for the output of the startup code. We used to do that - * before cd05f35d2c. - */ - int child_status; - log_portfw_spawn_error_message(line, executable, &child_status); - goto done; - } - - smartlist_split_string(tokens, line, NULL, - SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, -1); - - if (smartlist_len(tokens) < 5) - goto err; - - if (strcmp(smartlist_get(tokens, 0), "tor-fw-helper") || - strcmp(smartlist_get(tokens, 1), "tcp-forward")) - goto err; - - external_port = smartlist_get(tokens, 2); - internal_port = smartlist_get(tokens, 3); - result = smartlist_get(tokens, 4); - - if (smartlist_len(tokens) > 5) { - /* If there are more than 5 tokens, they are part of [<message>]. - Let's use a second smartlist to form the whole message; - strncat loops suck. */ - int i; - int message_words_n = smartlist_len(tokens) - 5; - smartlist_t *message_sl = smartlist_new(); - for (i = 0; i < message_words_n; i++) - smartlist_add(message_sl, smartlist_get(tokens, 5+i)); - - tor_assert(smartlist_len(message_sl) > 0); - message = smartlist_join_strings(message_sl, " ", 0, NULL); - - /* wrap the message in log-friendly wrapping */ - tor_asprintf(&message_for_log, " ('%s')", message); - - smartlist_free(message_sl); - } - - port = atoi(external_port); - if (port < 1 || port > 65535) - goto err; - - port = atoi(internal_port); - if (port < 1 || port > 65535) - goto err; - - if (!strcmp(result, "SUCCESS")) - success = 1; - else if (!strcmp(result, "FAIL")) - success = 0; - else - goto err; - - if (!success) { - log_warn(LD_GENERAL, "Tor was unable to forward TCP port '%s' to '%s'%s. " - "Please make sure that your router supports port " - "forwarding protocols (like NAT-PMP). Note that if '%s' is " - "your ORPort, your relay will be unable to receive inbound " - "traffic.", external_port, internal_port, - message_for_log ? message_for_log : "", - internal_port); - } else { - log_info(LD_GENERAL, - "Tor successfully forwarded TCP port '%s' to '%s'%s.", - external_port, internal_port, - message_for_log ? message_for_log : ""); - } - - goto done; - - err: - log_warn(LD_GENERAL, "tor-fw-helper sent us a string we could not " - "parse (%s).", line); - - done: - SMARTLIST_FOREACH(tokens, char *, cp, tor_free(cp)); - smartlist_free(tokens); - tor_free(message); - tor_free(message_for_log); -} - -/** Read what tor-fw-helper has to say in its stdout and handle it - * appropriately */ -static int -handle_fw_helper_output(const char *executable, - process_handle_t *process_handle) -{ - smartlist_t *fw_helper_output = NULL; - enum stream_status stream_status = 0; - - fw_helper_output = - tor_get_lines_from_handle(tor_process_get_stdout_pipe(process_handle), - &stream_status); - if (!fw_helper_output) { /* didn't get any output from tor-fw-helper */ - /* if EAGAIN we should retry in the future */ - return (stream_status == IO_STREAM_EAGAIN) ? 0 : -1; - } - - /* Handle the lines we got: */ - SMARTLIST_FOREACH_BEGIN(fw_helper_output, char *, line) { - handle_fw_helper_line(executable, line); - tor_free(line); - } SMARTLIST_FOREACH_END(line); - - smartlist_free(fw_helper_output); - - return 0; -} - -/** Spawn tor-fw-helper and ask it to forward the ports in - * ports_to_forward. ports_to_forward contains strings - * of the form "<external port>:<internal port>", which is the format - * that tor-fw-helper expects. */ -void -tor_check_port_forwarding(const char *filename, - smartlist_t *ports_to_forward, - time_t now) -{ -/* When fw-helper succeeds, how long do we wait until running it again */ -#define TIME_TO_EXEC_FWHELPER_SUCCESS 300 -/* When fw-helper failed to start, how long do we wait until running it again - */ -#define TIME_TO_EXEC_FWHELPER_FAIL 60 - - /* Static variables are initialized to zero, so child_handle.status=0 - * which corresponds to it not running on startup */ - static process_handle_t *child_handle=NULL; - - static time_t time_to_run_helper = 0; - int stderr_status, retval; - int stdout_status = 0; - - tor_assert(filename); - - /* Start the child, if it is not already running */ - if ((!child_handle || child_handle->status != PROCESS_STATUS_RUNNING) && - time_to_run_helper < now) { - /*tor-fw-helper cli looks like this: tor_fw_helper -p :5555 -p 4555:1111 */ - const char **argv; /* cli arguments */ - int args_n, status; - int argv_index = 0; /* index inside 'argv' */ - - tor_assert(smartlist_len(ports_to_forward) > 0); - - /* check for overflow during 'argv' allocation: - (len(ports_to_forward)*2 + 2)*sizeof(char*) > SIZE_MAX == - len(ports_to_forward) > (((SIZE_MAX/sizeof(char*)) - 2)/2) */ - if ((size_t) smartlist_len(ports_to_forward) > - (((SIZE_MAX/sizeof(char*)) - 2)/2)) { - log_warn(LD_GENERAL, - "Overflow during argv allocation. This shouldn't happen."); - return; - } - /* check for overflow during 'argv_index' increase: - ((len(ports_to_forward)*2 + 2) > INT_MAX) == - len(ports_to_forward) > (INT_MAX - 2)/2 */ - if (smartlist_len(ports_to_forward) > (INT_MAX - 2)/2) { - log_warn(LD_GENERAL, - "Overflow during argv_index increase. This shouldn't happen."); - return; - } - - /* Calculate number of cli arguments: one for the filename, two - for each smartlist element (one for "-p" and one for the - ports), and one for the final NULL. */ - args_n = 1 + 2*smartlist_len(ports_to_forward) + 1; - argv = tor_calloc(args_n, sizeof(char *)); - - argv[argv_index++] = filename; - SMARTLIST_FOREACH_BEGIN(ports_to_forward, const char *, port) { - argv[argv_index++] = "-p"; - argv[argv_index++] = port; - } SMARTLIST_FOREACH_END(port); - argv[argv_index] = NULL; - - /* Assume tor-fw-helper will succeed, start it later*/ - time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_SUCCESS; - - if (child_handle) { - tor_process_handle_destroy(child_handle, 1); - child_handle = NULL; - } - -#ifdef _WIN32 - /* Passing NULL as lpApplicationName makes Windows search for the .exe */ - status = tor_spawn_background(NULL, argv, NULL, &child_handle); -#else - status = tor_spawn_background(filename, argv, NULL, &child_handle); -#endif /* defined(_WIN32) */ - - tor_free_((void*)argv); - argv=NULL; - - if (PROCESS_STATUS_ERROR == status) { - log_warn(LD_GENERAL, "Failed to start port forwarding helper %s", - filename); - time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_FAIL; - return; - } - - log_info(LD_GENERAL, - "Started port forwarding helper (%s) with pid '%d'", - filename, tor_process_get_pid(child_handle)); - } - - /* If child is running, read from its stdout and stderr) */ - if (child_handle && PROCESS_STATUS_RUNNING == child_handle->status) { - /* Read from stdout/stderr and log result */ - retval = 0; -#ifdef _WIN32 - stderr_status = log_from_handle(child_handle->stderr_pipe, LOG_INFO); -#else - stderr_status = log_from_pipe(child_handle->stderr_pipe, - LOG_INFO, filename, &retval); -#endif /* defined(_WIN32) */ - if (handle_fw_helper_output(filename, child_handle) < 0) { - log_warn(LD_GENERAL, "Failed to handle fw helper output."); - stdout_status = -1; - retval = -1; - } - - if (retval) { - /* There was a problem in the child process */ - time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_FAIL; - } - - /* Combine the two statuses in order of severity */ - if (-1 == stdout_status || -1 == stderr_status) - /* There was a failure */ - retval = -1; -#ifdef _WIN32 - else if (!child_handle || tor_get_exit_code(child_handle, 0, NULL) != - PROCESS_EXIT_RUNNING) { - /* process has exited or there was an error */ - /* TODO: Do something with the process return value */ - /* TODO: What if the process output something since - * between log_from_handle and tor_get_exit_code? */ - retval = 1; - } -#else /* !(defined(_WIN32)) */ - else if (1 == stdout_status || 1 == stderr_status) - /* stdout or stderr was closed, the process probably - * exited. It will be reaped by waitpid() in main.c */ - /* TODO: Do something with the process return value */ - retval = 1; -#endif /* defined(_WIN32) */ - else - /* Both are fine */ - retval = 0; - - /* If either pipe indicates a failure, act on it */ - if (0 != retval) { - if (1 == retval) { - log_info(LD_GENERAL, "Port forwarding helper terminated"); - child_handle->status = PROCESS_STATUS_NOTRUNNING; - } else { - log_warn(LD_GENERAL, "Failed to read from port forwarding helper"); - child_handle->status = PROCESS_STATUS_ERROR; - } - - /* TODO: The child might not actually be finished (maybe it failed or - closed stdout/stderr), so maybe we shouldn't start another? */ - } - } -} - /** Initialize the insecure RNG rng from a seed value seed. */ void tor_init_weak_random(tor_weak_rng_t *rng, unsigned seed) diff --git a/src/common/util.h b/src/common/util.h index aeed8e823..ae27e5f01 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -414,11 +414,6 @@ void start_daemon(void); void finish_daemon(const char *desired_cwd); int write_pidfile(const char *filename); -/* Port forwarding */ -void tor_check_port_forwarding(const char *filename, - struct smartlist_t *ports_to_forward, - time_t now); - void tor_disable_spawning_background_processes(void); typedef struct process_handle_t process_handle_t; @@ -457,9 +452,7 @@ void set_environment_variable_in_smartlist(struct smartlist_t *env_vars, void (*free_old)(void*), int free_p); -/* Values of process_handle_t.status. PROCESS_STATUS_NOTRUNNING must be - * 0 because tor_check_port_forwarding depends on this being the initial - * statue of the static instance of process_handle_t */ +/* Values of process_handle_t.status. */ #define PROCESS_STATUS_NOTRUNNING 0 #define PROCESS_STATUS_RUNNING 1 #define PROCESS_STATUS_ERROR -1 diff --git a/src/or/config.c b/src/or/config.c index 212c6c6b9..a0890e278 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -492,8 +492,6 @@ static config_var_t option_vars_[] = { V(TestingSigningKeySlop, INTERVAL, "1 day"), V(OptimisticData, AUTOBOOL, "auto"), - V(PortForwarding, BOOL, "0"), - V(PortForwardingHelper, FILENAME, "tor-fw-helper"), OBSOLETE("PreferTunneledDirConns"), V(ProtocolWarnings, BOOL, "0"), V(PublishServerDescriptor, CSV, "1"), @@ -3907,15 +3905,6 @@ options_validate(or_options_t *old_options, or_options_t *options, if (options->KeepalivePeriod < 1) REJECT("KeepalivePeriod option must be positive."); - if (options->PortForwarding && options->Sandbox) { - REJECT("PortForwarding is not compatible with Sandbox; at most one can " - "be set"); - } - if (options->PortForwarding && options->NoExec) { - COMPLAIN("Both PortForwarding and NoExec are set; PortForwarding will " - "be ignored."); - } - if (ensure_bandwidth_cap(&options->BandwidthRate, "BandwidthRate", msg) < 0) return -1; diff --git a/src/or/main.c b/src/or/main.c index a0d2ae075..0a208c2a5 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1336,7 +1336,6 @@ CALLBACK(retry_listeners); CALLBACK(expire_old_ciruits_serverside); CALLBACK(check_dns_honesty); CALLBACK(write_bridge_ns); -CALLBACK(check_fw_helper_app); CALLBACK(heartbeat); CALLBACK(clean_consdiffmgr); CALLBACK(reset_padding_counts); @@ -1372,7 +1371,6 @@ static periodic_event_item_t periodic_events[] = { CALLBACK(expire_old_ciruits_serverside), CALLBACK(check_dns_honesty), CALLBACK(write_bridge_ns), - CALLBACK(check_fw_helper_app), CALLBACK(heartbeat), CALLBACK(clean_consdiffmgr), CALLBACK(reset_padding_counts), @@ -2186,33 +2184,6 @@ write_bridge_ns_callback(time_t now, const or_options_t *options) return PERIODIC_EVENT_NO_UPDATE; } -/** - * Periodic callback: poke the tor-fw-helper app if we're using one. - */ -static int -check_fw_helper_app_callback(time_t now, const or_options_t *options) -{ - if (net_is_disabled() || - ! server_mode(options) || - ! options->PortForwarding || - options->NoExec) { - return PERIODIC_EVENT_NO_UPDATE; - } - /* 11. check the port forwarding app */ - -#define PORT_FORWARDING_CHECK_INTERVAL 5 - smartlist_t *ports_to_forward = get_list_of_ports_to_forward(); - if (ports_to_forward) { - tor_check_port_forwarding(options->PortForwardingHelper, - ports_to_forward, - now); - - SMARTLIST_FOREACH(ports_to_forward, char *, cp, tor_free(cp)); - smartlist_free(ports_to_forward); - } - return PORT_FORWARDING_CHECK_INTERVAL; -} - static int heartbeat_callback_first_time = 1; /** diff --git a/src/or/or.h b/src/or/or.h index 25fc6cdf3..8cdfe76a9 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -4204,10 +4204,6 @@ typedef struct { * testing our DNS server. */ int EnforceDistinctSubnets; /**< If true, don't allow multiple routers in the * same network zone in the same circuit. */ - int PortForwarding; /**< If true, use NAT-PMP or UPnP to automatically - * forward the DirPort and ORPort on the NAT device */ - char *PortForwardingHelper; /** < Filename or full path of the port - forwarding helper executable */ int AllowNonRFC953Hostnames; /**< If true, we allow connections to hostnames * with weird characters. */ /** If true, we try resolving hostnames with weird characters. */ diff --git a/src/test/test_options.c b/src/test/test_options.c index eaf503439..af349ed01 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -2422,37 +2422,6 @@ test_options_validate__circuits(void *ignored) } static void -test_options_validate__port_forwarding(void *ignored) -{ - (void)ignored; - int ret; - char *msg; - options_test_data_t *tdata = NULL; - - free_options_test_data(tdata); - tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES - "PortForwarding 1\nSandbox 1\n"); - ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, "PortForwarding is not compatible with Sandbox;" - " at most one can be set"); - tor_free(msg); - - free_options_test_data(tdata); - tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES - "PortForwarding 1\nSandbox 0\n"); - ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, 0); - tt_assert(!msg); - tor_free(msg); - - done: - free_options_test_data(tdata); - policies_free_all(); - tor_free(msg); -} - -static void test_options_validate__tor2web(void *ignored) { (void)ignored; @@ -4261,7 +4230,6 @@ struct testcase_t options_tests[] = { LOCAL_VALIDATE_TEST(path_bias), LOCAL_VALIDATE_TEST(bandwidth), LOCAL_VALIDATE_TEST(circuits), - LOCAL_VALIDATE_TEST(port_forwarding), LOCAL_VALIDATE_TEST(tor2web), LOCAL_VALIDATE_TEST(rend), LOCAL_VALIDATE_TEST(single_onion), diff --git a/src/tools/tor-fw-helper/README b/src/tools/tor-fw-helper/README deleted file mode 100644 index 6a1ecaa1e..000000000 --- a/src/tools/tor-fw-helper/README +++ /dev/null @@ -1,10 +0,0 @@ - -We no longer recommend the use of this tool. Instead, please use the -pure-Go version of tor-fw-helper available at - https://gitweb.torproject.org/tor-fw-helper.git - -Why? - -The C code here was fine, but frankly: we don't trust the underlying -libraries. They don't seem to have been written with network security -in mind, and we have very little faith in their safety.

1 0

[tor/master] changes: Add changes file for 25409.
by nickm＠torproject.org 10 Apr '18

10 Apr '18

commit 550037f67779fc52d08a12c5f4ffdd04d55841a9 Author: Isis Lovecruft <isis(a)torproject.org> Date: Tue Apr 10 19:21:10 2018 +0000 changes: Add changes file for 25409. --- changes/bug25409 | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/changes/bug25409 b/changes/bug25409 new file mode 100644 index 000000000..093d8f58b --- /dev/null +++ b/changes/bug25409 @@ -0,0 +1,12 @@ + o Removed features: + - The PortForwarding and PortForwardingHelper features have been + removed. The reasoning is, given that implementations of NAT traversal + protocols within common consumer grade routers are frequently buggy, and + that the target audience for a NAT punching feature is a perhaps + less-technically-inclined relay operator, when the helper fails to setup + traversal the problems are usually deep, ugly, and very router specific, + making them horrendously impossible for technical support to reliable + assist with, and thus resulting in frustration all around. Unfortunately, + relay operators who would like to run relays behind NATs will need to + become more familiar with the port forwarding configurations on their + local router. Closes 25409.

1 0

[tor/master] config: Obsolete PortForwarding and PortForwardingHelper options.
by nickm＠torproject.org 10 Apr '18

10 Apr '18

commit 65d6b66e99a1a441805402ba8f275a63bccbf0f5 Author: Isis Lovecruft <isis(a)torproject.org> Date: Tue Apr 10 19:08:57 2018 +0000 config: Obsolete PortForwarding and PortForwardingHelper options. * FIXES part of #25409: https://bugs.torproject.org/25409 --- src/or/config.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/or/config.c b/src/or/config.c index a0890e278..b2cb1f407 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -492,6 +492,8 @@ static config_var_t option_vars_[] = { V(TestingSigningKeySlop, INTERVAL, "1 day"), V(OptimisticData, AUTOBOOL, "auto"), + OBSOLETE("PortForwarding"), + OBSOLETE("PortForwardingHelper"), OBSOLETE("PreferTunneledDirConns"), V(ProtocolWarnings, BOOL, "0"), V(PublishServerDescriptor, CSV, "1"),

1 0

[tor/master] Merge remote-tracking branch 'isis-github/bug25409'
by nickm＠torproject.org 10 Apr '18

10 Apr '18

commit 6e467a7a340cdc0267206ebaf8dca54c1a4a7b0a Merge: ef16a11b9 550037f67 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Apr 10 15:27:09 2018 -0400 Merge remote-tracking branch 'isis-github/bug25409' changes/bug25409 | 12 ++ changes/ticket25409 | 6 + doc/tor.1.txt | 19 +-- src/common/util.c | 348 ----------------------------------------- src/common/util.h | 9 +- src/or/config.c | 13 +- src/or/main.c | 29 ---- src/or/or.h | 4 - src/test/test_options.c | 32 ---- src/tools/tor-fw-helper/README | 10 -- 10 files changed, 24 insertions(+), 458 deletions(-)

1 0

[translation/support-tbb] Update translations for support-tbb
by translation＠torproject.org 10 Apr '18

10 Apr '18

commit 199a98e572a7babaae90f9db4c4626bae436b8eb Author: Translation commit bot <translation(a)torproject.org> Date: Tue Apr 10 19:20:26 2018 +0000 Update translations for support-tbb --- ga.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ga.json b/ga.json index f76ecde4a..2f80ba01a 100644 --- a/ga.json +++ b/ga.json @@ -3,7 +3,7 @@ "id": "#tbb-1", "control": "tbb-1", "title": "Cad iad na fadhbanna is coitianta a bhaineann leis an leagan cobhsaí is déanaí de Bhrabhsálaí Tor?", - "description": "Nuair a fhoilsímid leagan cobhsaí nua de Bhrabhsálaí Tor, scríobhaimid postáil bhlag a dhéanann cur síos ar na gnéithe nua agus aon fhadhbanna ar ár n-eolas. Má tá fadhbanna agat a thosaigh tar éis duit Brabhsálaí Tor a nuashonrú, féach ar an bpostáil is déanaí faoin leagan cobhsaí ar <a href=\"https://blog.torproject.org\">https://blog.torproject.org</a> le féachaint an bhfuil d'fhadhb luaite." + "description": "Nuair a fhoilsímid leagan cobhsaí nua de Bhrabhsálaí Tor, scríobhaimid postáil bhlag a dhéanann cur síos ar na gnéithe nua agus ar aon fhadhbanna ar ár n-eolas. Má tá fadhbanna agat a thosaigh tar éis duit Brabhsálaí Tor a nuashonrú, féach ar an bpostáil is déanaí faoin leagan cobhsaí ar <a href=\"https://blog.torproject.org\">https://blog.torproject.org</a> le féachaint an bhfuil d'fhadhb luaite ann." }, "tbb-2": { "id": "#tbb-2", @@ -68,7 +68,7 @@ "tbb-11": { "id": "#tbb-11", "control": "tbb-11", - "title": "D'íoslódáil agus shuiteáil mé Brabhsálaí Tor ar Windows, ach ní fheicim ar mo ríomhaire é anois.", + "title": "D'íoslódáil agus shuiteáil mé Brabhsálaí Tor ar Windows, ach anois ní fheicim ar mo ríomhaire é.", "description": "The file you download and run prompts you for a destination. If you don't remember what this destination was, it's most likely your Downloads or Desktop folder. The default setting in the Windows installer also creates a shortcut for you on your Desktop, though be aware that you may have accidentally unticked the option to create a shortcut. If you can't find it in either of those folders, download it again and look for the prompt that asks you to choose a directory to download it in. Choose a directory location that you'll remember easily, and once the download finishes you should see a Tor Browser folder there." }, "tbb-12": { @@ -92,7 +92,7 @@ "tbb-15": { "id": "#tbb-15", "control": "tbb-15", - "title": "An féidir liom Brabhsálaí Tor a íoslódáil ar ChromeOS?", + "title": "An féidir liom Brabhsálaí Tor a úsáid ar ChromeOS?", "description": "Ní féidir, faraor. Níl aon leagan de Bhrabhsálaí Tor ar ChromeOS faoi láthair." }, "tbb-16": {

1 0

[stem/master] Don't retry downloading descriptors when we've timed out
by atagar＠torproject.org 10 Apr '18

10 Apr '18

commit e43a168fd8d8d3f561c3feb4f36af357ec32f848 Author: Damian Johnson <atagar(a)torproject.org> Date: Tue Apr 10 10:20:46 2018 -0700 Don't retry downloading descriptors when we've timed out Presently one of our dirauths (tor26) is having some health issues. In particular connection resets and timeouts. This has revealed a couple issues. First is that python doesn't always honor its socket timeouts. Though rare, our requests to tor26 sometimes hang for hours despite imposing a one minute timeout. Second, we retry requests even when our timeout has been reached (making callers effectively wait three times as long as they asked). The first of these issues is still baffling me. Unfortunately I've seen it, but can't reliably repro. The second issue however is pretty straight forward so fixing it. --- docs/change_log.rst | 1 + stem/descriptor/remote.py | 13 ++++++++----- test/unit/descriptor/remote.py | 14 +++++++++++--- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/docs/change_log.rst b/docs/change_log.rst index b0773382..9be7912d 100644 --- a/docs/change_log.rst +++ b/docs/change_log.rst @@ -57,6 +57,7 @@ The following are only available within Stem's `git repository * `Fallback directory v2 support <https://lists.torproject.org/pipermail/tor-dev/2017-December/012721.html>`_, which adds *nickname* and *extrainfo* * Added zstd and lzma compression support (:spec:`1cb56af`) * Added server descriptor's new is_hidden_service_dir attribute + * Don't retry downloading descriptors when we've timed out * Reduced maximum descriptors fetched by the remote module to match tor's new limit (:trac:`24743`) * Consensus **shared_randomness_*_reveal_count** attributes undocumented, and unavailable if retrieved before their corresponding shared_randomness_*_value attribute (:trac:`25046`) * Allow 'proto' line to have blank values (:spec:`a8455f4`) diff --git a/stem/descriptor/remote.py b/stem/descriptor/remote.py index 97fd1e5c..cf1b1628 100644 --- a/stem/descriptor/remote.py +++ b/stem/descriptor/remote.py @@ -430,7 +430,7 @@ class Query(object): self._downloader_thread = threading.Thread( name = 'Descriptor Query', target = self._download_descriptors, - args = (self.retries,) + args = (self.retries, self.timeout) ) self._downloader_thread.setDaemon(True) @@ -520,7 +520,7 @@ class Query(object): return 'http://%s:%i/%s' % (address, dirport, self.resource.lstrip('/')) - def _download_descriptors(self, retries): + def _download_descriptors(self, retries, timeout): try: use_authority = retries == 0 and self.fall_back_to_authority self.download_url = self._pick_url(use_authority) @@ -531,7 +531,7 @@ class Query(object): self.download_url, headers = {'Accept-Encoding': ', '.join(self.compression)}, ), - timeout = self.timeout, + timeout = timeout, ) data = response.read() @@ -562,9 +562,12 @@ class Query(object): except: exc = sys.exc_info()[1] - if retries > 0: + if timeout is not None: + timeout -= time.time() - self.start_time + + if retries > 0 and (timeout is None or timeout > 0): log.debug("Unable to download descriptors from '%s' (%i retries remaining): %s" % (self.download_url, retries, exc)) - return self._download_descriptors(retries - 1) + return self._download_descriptors(retries - 1, timeout) else: log.debug("Unable to download descriptors from '%s': %s" % (self.download_url, exc)) self.error = exc diff --git a/test/unit/descriptor/remote.py b/test/unit/descriptor/remote.py index 05cf1d57..cc4c3997 100644 --- a/test/unit/descriptor/remote.py +++ b/test/unit/descriptor/remote.py @@ -4,6 +4,7 @@ Unit tests for stem.descriptor.remote. import socket import tempfile +import time import unittest import stem.descriptor.remote @@ -273,19 +274,26 @@ class TestDescriptorDownloader(unittest.TestCase): @patch(URL_OPEN) def test_query_with_timeout(self, urlopen_mock): - urlopen_mock.side_effect = socket.timeout('connection timed out') + def urlopen_call(*args, **kwargs): + time.sleep(0.06) + raise socket.timeout('connection timed out') + + urlopen_mock.side_effect = urlopen_call query = stem.descriptor.remote.Query( TEST_RESOURCE, 'server-descriptor 1.0', endpoints = [('128.31.0.39', 9131)], fall_back_to_authority = False, - timeout = 5, + timeout = 0.1, validate = True, ) + # After two requests we'll have reached our total permissable timeout. + # Check that we don't make a third. + self.assertRaises(socket.timeout, query.run) - self.assertEqual(3, urlopen_mock.call_count) + self.assertEqual(2, urlopen_mock.call_count) @patch(URL_OPEN, _urlopen_mock(TEST_DESCRIPTOR)) def test_can_iterate_multiple_times(self):

1 0