commit 9619c395ac7e2a8c2e57f55cd1885fe5f8e5f8b5 Merge: b1ca05d 2d4241d Author: Nick Mathewson nickm@torproject.org Date: Tue Nov 4 09:49:35 2014 -0500

Merge remote-tracking branch 'andrea/ticket6456'

Somewhat tricky conflicts: src/or/config.c

Also, s/test_assert/tt_assert in test_config.c

src/or/config.c | 287 ++++++++++++++++-------------------------------- src/or/config.h | 3 + src/or/entrynodes.c | 4 +- src/or/entrynodes.h | 2 +- src/or/transports.c | 12 +- src/or/transports.h | 10 +- src/test/test_config.c | 248 +++++++++++++++++++++++++++++++++++++++++ 7 files changed, 361 insertions(+), 205 deletions(-)

diff --cc src/or/config.c index 681955e,10b118c..cb27dac --- a/src/or/config.c +++ b/src/or/config.c @@@ -1410,26 -1421,24 +1404,26 @@@ options_act(const or_options_t *old_opt

mark_transport_list(); pt_prepare_proxy_list_for_config_read(); - if (options->ClientTransportPlugin) { - for (cl = options->ClientTransportPlugin; cl; cl = cl->next) { - if (parse_transport_line(options, cl->value, 0, 0) < 0) { - log_warn(LD_BUG, - "Previously validated ClientTransportPlugin line " - "could not be added!"); - return -1; + if (!options->DisableNetwork) { + if (options->ClientTransportPlugin) { + for (cl = options->ClientTransportPlugin; cl; cl = cl->next) { - if (parse_client_transport_line(options, cl->value, 0)<0) { ++ if (parse_transport_line(options, cl->value, 0, 0) < 0) { + log_warn(LD_BUG, + "Previously validated ClientTransportPlugin line " + "could not be added!"); + return -1; + } } } - }

- if (options->ServerTransportPlugin && server_mode(options)) { - for (cl = options->ServerTransportPlugin; cl; cl = cl->next) { - if (parse_transport_line(options, cl->value, 0, 1) < 0) { - log_warn(LD_BUG, - "Previously validated ServerTransportPlugin line " - "could not be added!"); - return -1; + if (options->ServerTransportPlugin && server_mode(options)) { + for (cl = options->ServerTransportPlugin; cl; cl = cl->next) { - if (parse_server_transport_line(options, cl->value, 0)<0) { ++ if (parse_transport_line(options, cl->value, 0, 1) < 0) { + log_warn(LD_BUG, + "Previously validated ServerTransportPlugin line " + "could not be added!"); + return -1; + } } } } @@@ -4844,40 -4846,49 +4852,57 @@@ parse_transport_line(const or_options_ goto err; }

- if (is_managed) { /* managed */ - if (!validate_only && is_useless_proxy) { - log_info(LD_GENERAL, "Pluggable transport proxy (%s) does not provide " - "any needed transports and will not be launched.", line); + if (is_managed) { + /* managed */ + + if (!server && !validate_only && is_useless_proxy) { - log_notice(LD_GENERAL, - "Pluggable transport proxy (%s) does not provide " - "any needed transports and will not be launched.", - line); ++ log_info(LD_GENERAL, ++ "Pluggable transport proxy (%s) does not provide " ++ "any needed transports and will not be launched.", ++ line); }

- /* If we are not just validating, use the rest of the line as the - argv of the proxy to be launched. Also, make sure that we are - only launching proxies that contribute useful transports. */ - if (!validate_only && !is_useless_proxy) { - proxy_argc = line_length-2; + /* + * If we are not just validating, use the rest of the line as the + * argv of the proxy to be launched. Also, make sure that we are + * only launching proxies that contribute useful transports. + */ + + if (!validate_only && (server || !is_useless_proxy)) { + proxy_argc = line_length - 2; tor_assert(proxy_argc > 0); - proxy_argv = tor_malloc_zero(sizeof(char *) * (proxy_argc + 1)); + proxy_argv = tor_calloc((proxy_argc + 1), sizeof(char *)); tmp = proxy_argv; - for (i=0;i<proxy_argc;i++) { /* store arguments */ + + for (i = 0; i < proxy_argc; i++) { + /* store arguments */ *tmp++ = smartlist_get(items, 2); smartlist_del_keeporder(items, 2); } - *tmp = NULL; /*terminated with NULL, just like execve() likes it*/ + *tmp = NULL; /* terminated with NULL, just like execve() likes it */

/* kickstart the thing */ - pt_kickstart_client_proxy(transport_list, proxy_argv); + if (server) { + pt_kickstart_server_proxy(transport_list, proxy_argv); + } else { + pt_kickstart_client_proxy(transport_list, proxy_argv); + } } - } else { /* external */ + } else { + /* external */ + + /* ClientTransportPlugins connecting through a proxy is managed only. */ - if (options->Socks4Proxy || options->Socks5Proxy || options->HTTPSProxy) { ++ if (!server && ++ (options->Socks4Proxy || options->Socks5Proxy || options->HTTPSProxy)) { + log_warn(LD_CONFIG, "You have configured an external proxy with another " + "proxy type. (Socks4Proxy|Socks5Proxy|HTTPSProxy)"); + goto err; + } + if (smartlist_len(transport_list) != 1) { - log_warn(LD_CONFIG, "You can't have an external proxy with " - "more than one transports."); + log_warn(LD_CONFIG, + "You can't have an external proxy with more than " + "one transport."); goto err; }

diff --cc src/test/test_config.c index 823d067,ae01fac..1ecb816 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@@ -552,6 -554,251 +554,251 @@@ test_config_parse_transport_options_lin } }

+ /* Mocks needed for the transport plugin line test */ + + static void pt_kickstart_proxy_mock(const smartlist_t *transport_list, + char **proxy_argv, int is_server); + static int transport_add_from_config_mock(const tor_addr_t *addr, + uint16_t port, const char *name, + int socks_ver); + static int transport_is_needed_mock(const char *transport_name); + + static int pt_kickstart_proxy_mock_call_count = 0; + static int transport_add_from_config_mock_call_count = 0; + static int transport_is_needed_mock_call_count = 0; + static int transport_is_needed_mock_return = 0; + + static void + pt_kickstart_proxy_mock(const smartlist_t *transport_list, + char **proxy_argv, int is_server) + { + ++pt_kickstart_proxy_mock_call_count; + } + + static int + transport_add_from_config_mock(const tor_addr_t *addr, + uint16_t port, const char *name, + int socks_ver) + { + ++transport_add_from_config_mock_call_count; + + return 0; + } + + static int + transport_is_needed_mock(const char *transport_name) + { + ++transport_is_needed_mock_call_count; + + return transport_is_needed_mock_return; + } + + /** + * Test parsing for the ClientTransportPlugin and ServerTransportPlugin config + * options. + */ + + static void + test_config_parse_transport_plugin_line(void *arg) + { + or_options_t *options = get_options_mutable(); + int r, tmp; + int old_pt_kickstart_proxy_mock_call_count; + int old_transport_add_from_config_mock_call_count; + int old_transport_is_needed_mock_call_count; + + /* Bad transport lines - too short */ + r = parse_transport_line(options, "bad", 1, 0); - test_assert(r < 0); ++ tt_assert(r < 0); + r = parse_transport_line(options, "bad", 1, 1); - test_assert(r < 0); ++ tt_assert(r < 0); + r = parse_transport_line(options, "bad bad", 1, 0); - test_assert(r < 0); ++ tt_assert(r < 0); + r = parse_transport_line(options, "bad bad", 1, 1); - test_assert(r < 0); ++ tt_assert(r < 0); + + /* Test transport list parsing */ + r = parse_transport_line(options, + "transport_1 exec /usr/bin/fake-transport", 1, 0); - test_assert(r == 0); ++ tt_assert(r == 0); + r = parse_transport_line(options, + "transport_1 exec /usr/bin/fake-transport", 1, 1); - test_assert(r == 0); ++ tt_assert(r == 0); + r = parse_transport_line(options, + "transport_1,transport_2 exec /usr/bin/fake-transport", 1, 0); - test_assert(r == 0); ++ tt_assert(r == 0); + r = parse_transport_line(options, + "transport_1,transport_2 exec /usr/bin/fake-transport", 1, 1); - test_assert(r == 0); ++ tt_assert(r == 0); + /* Bad transport identifiers */ + r = parse_transport_line(options, + "transport_* exec /usr/bin/fake-transport", 1, 0); - test_assert(r < 0); ++ tt_assert(r < 0); + r = parse_transport_line(options, + "transport_* exec /usr/bin/fake-transport", 1, 1); - test_assert(r < 0); ++ tt_assert(r < 0); + + /* Check SOCKS cases for client transport */ + r = parse_transport_line(options, + "transport_1 socks4 1.2.3.4:567", 1, 0); - test_assert(r == 0); ++ tt_assert(r == 0); + r = parse_transport_line(options, + "transport_1 socks5 1.2.3.4:567", 1, 0); - test_assert(r == 0); ++ tt_assert(r == 0); + /* Proxy case for server transport */ + r = parse_transport_line(options, + "transport_1 proxy 1.2.3.4:567", 1, 1); - test_assert(r == 0); ++ tt_assert(r == 0); + /* Multiple-transport error exit */ + r = parse_transport_line(options, + "transport_1,transport_2 socks5 1.2.3.4:567", 1, 0); - test_assert(r < 0); ++ tt_assert(r < 0); + r = parse_transport_line(options, + "transport_1,transport_2 proxy 1.2.3.4:567", 1, 1); + /* No port error exit */ + r = parse_transport_line(options, + "transport_1 socks5 1.2.3.4", 1, 0); - test_assert(r < 0); ++ tt_assert(r < 0); + r = parse_transport_line(options, + "transport_1 proxy 1.2.3.4", 1, 1); - test_assert(r < 0); ++ tt_assert(r < 0); + /* Unparsable address error exit */ + r = parse_transport_line(options, + "transport_1 socks5 1.2.3:6x7", 1, 0); - test_assert(r < 0); ++ tt_assert(r < 0); + r = parse_transport_line(options, + "transport_1 proxy 1.2.3:6x7", 1, 1); - test_assert(r < 0); ++ tt_assert(r < 0); + + /* "Strange {Client|Server}TransportPlugin field" error exit */ + r = parse_transport_line(options, + "transport_1 foo bar", 1, 0); - test_assert(r < 0); ++ tt_assert(r < 0); + r = parse_transport_line(options, + "transport_1 foo bar", 1, 1); - test_assert(r < 0); ++ tt_assert(r < 0); + + /* No sandbox mode error exit */ + tmp = options->Sandbox; + options->Sandbox = 1; + r = parse_transport_line(options, + "transport_1 exec /usr/bin/fake-transport", 1, 0); - test_assert(r < 0); ++ tt_assert(r < 0); + r = parse_transport_line(options, + "transport_1 exec /usr/bin/fake-transport", 1, 1); - test_assert(r < 0); ++ tt_assert(r < 0); + options->Sandbox = tmp; + + /* + * These final test cases cover code paths that only activate without + * validate_only, so they need mocks in place. + */ + MOCK(pt_kickstart_proxy, pt_kickstart_proxy_mock); + old_pt_kickstart_proxy_mock_call_count = + pt_kickstart_proxy_mock_call_count; + r = parse_transport_line(options, + "transport_1 exec /usr/bin/fake-transport", 0, 1); - test_assert(r == 0); - test_assert(pt_kickstart_proxy_mock_call_count == ++ tt_assert(r == 0); ++ tt_assert(pt_kickstart_proxy_mock_call_count == + old_pt_kickstart_proxy_mock_call_count + 1); + UNMOCK(pt_kickstart_proxy); + + /* This one hits a log line in the !validate_only case only */ + r = parse_transport_line(options, + "transport_1 proxy 1.2.3.4:567", 0, 1); - test_assert(r == 0); ++ tt_assert(r == 0); + + /* Check mocked client transport cases */ + MOCK(pt_kickstart_proxy, pt_kickstart_proxy_mock); + MOCK(transport_add_from_config, transport_add_from_config_mock); + MOCK(transport_is_needed, transport_is_needed_mock); + + /* Unnecessary transport case */ + transport_is_needed_mock_return = 0; + old_pt_kickstart_proxy_mock_call_count = + pt_kickstart_proxy_mock_call_count; + old_transport_add_from_config_mock_call_count = + transport_add_from_config_mock_call_count; + old_transport_is_needed_mock_call_count = + transport_is_needed_mock_call_count; + r = parse_transport_line(options, + "transport_1 exec /usr/bin/fake-transport", 0, 0); + /* Should have succeeded */ - test_assert(r == 0); ++ tt_assert(r == 0); + /* transport_is_needed() should have been called */ - test_assert(transport_is_needed_mock_call_count == ++ tt_assert(transport_is_needed_mock_call_count == + old_transport_is_needed_mock_call_count + 1); + /* + * pt_kickstart_proxy() and transport_add_from_config() should + * not have been called. + */ - test_assert(pt_kickstart_proxy_mock_call_count == ++ tt_assert(pt_kickstart_proxy_mock_call_count == + old_pt_kickstart_proxy_mock_call_count); - test_assert(transport_add_from_config_mock_call_count == ++ tt_assert(transport_add_from_config_mock_call_count == + old_transport_add_from_config_mock_call_count); + + /* Necessary transport case */ + transport_is_needed_mock_return = 1; + old_pt_kickstart_proxy_mock_call_count = + pt_kickstart_proxy_mock_call_count; + old_transport_add_from_config_mock_call_count = + transport_add_from_config_mock_call_count; + old_transport_is_needed_mock_call_count = + transport_is_needed_mock_call_count; + r = parse_transport_line(options, + "transport_1 exec /usr/bin/fake-transport", 0, 0); + /* Should have succeeded */ - test_assert(r == 0); ++ tt_assert(r == 0); + /* + * transport_is_needed() and pt_kickstart_proxy() should have been + * called. + */ - test_assert(pt_kickstart_proxy_mock_call_count == ++ tt_assert(pt_kickstart_proxy_mock_call_count == + old_pt_kickstart_proxy_mock_call_count + 1); - test_assert(transport_is_needed_mock_call_count == ++ tt_assert(transport_is_needed_mock_call_count == + old_transport_is_needed_mock_call_count + 1); + /* transport_add_from_config() should not have been called. */ - test_assert(transport_add_from_config_mock_call_count == ++ tt_assert(transport_add_from_config_mock_call_count == + old_transport_add_from_config_mock_call_count); + + /* proxy case */ + transport_is_needed_mock_return = 1; + old_pt_kickstart_proxy_mock_call_count = + pt_kickstart_proxy_mock_call_count; + old_transport_add_from_config_mock_call_count = + transport_add_from_config_mock_call_count; + old_transport_is_needed_mock_call_count = + transport_is_needed_mock_call_count; + r = parse_transport_line(options, + "transport_1 socks5 1.2.3.4:567", 0, 0); + /* Should have succeeded */ - test_assert(r == 0); ++ tt_assert(r == 0); + /* + * transport_is_needed() and transport_add_from_config() should have + * been called. + */ - test_assert(transport_add_from_config_mock_call_count == ++ tt_assert(transport_add_from_config_mock_call_count == + old_transport_add_from_config_mock_call_count + 1); - test_assert(transport_is_needed_mock_call_count == ++ tt_assert(transport_is_needed_mock_call_count == + old_transport_is_needed_mock_call_count + 1); + /* pt_kickstart_proxy() should not have been called. */ - test_assert(pt_kickstart_proxy_mock_call_count == ++ tt_assert(pt_kickstart_proxy_mock_call_count == + old_pt_kickstart_proxy_mock_call_count); + + /* Done with mocked client transport cases */ + UNMOCK(transport_is_needed); + UNMOCK(transport_add_from_config); + UNMOCK(pt_kickstart_proxy); + + done: + /* Make sure we undo all mocks */ + UNMOCK(pt_kickstart_proxy); + UNMOCK(transport_add_from_config); + UNMOCK(transport_is_needed); + + return; + } + // Tests if an options with MyFamily fingerprints missing '$' normalises // them correctly and also ensure it also works with multiple fingerprints static void