commit 4a5164fd86465ab4546ce4aad0ed8ec1b9e26cd1 Author: Andrea Shepard andrea@torproject.org Date: Mon Jul 28 19:32:23 2014 -0700

Replace all calls to parse_client_transport_line() or parse_server_transport_line() with new parse_transport_line() stub --- src/or/config.c | 291 ++++++++++++++++++++++++++---------------------- src/or/config.h | 7 +- src/test/test_config.c | 104 ++++++++--------- 3 files changed, 214 insertions(+), 188 deletions(-)

diff --git a/src/or/config.c b/src/or/config.c index b50834b..7911b5c 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -538,6 +538,10 @@ static int options_transition_affects_descriptor( static int check_nickname_list(char **lst, const char *name, char **msg); static char *get_bindaddr_from_transport_listen_line(const char *line, const char *transport); +static int parse_client_transport_line(const or_options_t *options, + const char *line, int validate_only); +static int parse_server_transport_line(const or_options_t *options, + const char *line, int validate_only); static int parse_dir_authority_line(const char *line, dirinfo_type_t required_type, int validate_only); @@ -1423,7 +1427,7 @@ options_act(const or_options_t *old_options) pt_prepare_proxy_list_for_config_read(); 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!"); @@ -1434,7 +1438,7 @@ options_act(const or_options_t *old_options)

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!"); @@ -3287,12 +3291,12 @@ options_validate(or_options_t *old_options, or_options_t *options, }

for (cl = options->ClientTransportPlugin; cl; cl = cl->next) { - if (parse_client_transport_line(options, cl->value, 1)<0) + if (parse_transport_line(options, cl->value, 1, 0) < 0) REJECT("Invalid client transport line. See logs for details."); }

for (cl = options->ServerTransportPlugin; cl; cl = cl->next) { - if (parse_server_transport_line(options, cl->value, 1)<0) + if (parse_transport_line(options, cl->value, 1, 1) < 0) REJECT("Invalid server transport line. See logs for details."); }

@@ -4748,6 +4752,29 @@ parse_bridge_line(const char *line) return bridge_line; }

+/** Read the contents of a ClientTransportPlugin or ServerTransportPlugin + * line from <b>line</b>, depending on the value of <b>server</b>. Return 0 + * if the line is well-formed, and -1 if it isn't. + * + * If <b>validate_only</b> is 0, the line is well-formed, and the transport is + * needed by some bridge: + * - If it's an external proxy line, add the transport described in the line to + * our internal transport list. + * - If it's a managed proxy line, launch the managed proxy. + */ + +STATIC int +parse_transport_line(const or_options_t *options, + const char *line, int validate_only, + int server) +{ + if (server) { + return parse_server_transport_line(options, line, validate_only); + } else { + return parse_client_transport_line(options, line, validate_only); + } +} + /** Read the contents of a ClientTransportPlugin line from * <b>line</b>. Return 0 if the line is well-formed, and -1 if it * isn't. @@ -4757,7 +4784,7 @@ parse_bridge_line(const char *line) * - If it's an external proxy line, add the transport described in the line to * our internal transport list. * - If it's a managed proxy line, launch the managed proxy. */ -STATIC int +static int parse_client_transport_line(const or_options_t *options, const char *line, int validate_only) { @@ -4901,6 +4928,133 @@ parse_client_transport_line(const or_options_t *options, return r; }

+/** Read the contents of a ServerTransportPlugin line from + * <b>line</b>. Return 0 if the line is well-formed, and -1 if it + * isn't. + * If <b>validate_only</b> is 0, the line is well-formed, and it's a + * managed proxy line, launch the managed proxy. */ +static int +parse_server_transport_line(const or_options_t *options, + const char *line, int validate_only) +{ + smartlist_t *items = NULL; + int r; + const char *transports=NULL; + smartlist_t *transport_list=NULL; + char *type=NULL; + char *addrport=NULL; + tor_addr_t addr; + uint16_t port = 0; + + /* managed proxy options */ + int is_managed=0; + char **proxy_argv=NULL; + char **tmp=NULL; + int proxy_argc,i; + + int line_length; + + items = smartlist_new(); + smartlist_split_string(items, line, NULL, + SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, -1); + + line_length = smartlist_len(items); + if (line_length < 3) { + log_warn(LD_CONFIG, "Too few arguments on ServerTransportPlugin line."); + goto err; + } + + /* Get the first line element, split it to commas into + transport_list (in case it's multiple transports) and validate + the transport names. */ + transports = smartlist_get(items, 0); + transport_list = smartlist_new(); + smartlist_split_string(transport_list, transports, ",", + SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); + SMARTLIST_FOREACH_BEGIN(transport_list, const char *, transport_name) { + if (!string_is_C_identifier(transport_name)) { + log_warn(LD_CONFIG, "Transport name is not a C identifier (%s).", + transport_name); + goto err; + } + } SMARTLIST_FOREACH_END(transport_name); + + type = smartlist_get(items, 1); + + if (!strcmp(type, "exec")) { + is_managed=1; + } else if (!strcmp(type, "proxy")) { + is_managed=0; + } else { + log_warn(LD_CONFIG, "Strange ServerTransportPlugin type '%s'", type); + goto err; + } + + if (is_managed && options->Sandbox) { + log_warn(LD_CONFIG, "Managed proxies are not compatible with Sandbox mode." + "(ServerTransportPlugin line was %s)", escaped(line)); + goto err; + } + + if (is_managed) { /* managed */ + if (!validate_only) { + proxy_argc = line_length-2; + tor_assert(proxy_argc > 0); + proxy_argv = tor_malloc_zero(sizeof(char*)*(proxy_argc+1)); + tmp = proxy_argv; + + 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*/ + + /* kickstart the thing */ + pt_kickstart_server_proxy(transport_list, proxy_argv); + } + } else { /* external */ + if (smartlist_len(transport_list) != 1) { + log_warn(LD_CONFIG, "You can't have an external proxy with " + "more than one transports."); + goto err; + } + + addrport = smartlist_get(items, 2); + + if (tor_addr_port_lookup(addrport, &addr, &port)<0) { + log_warn(LD_CONFIG, "Error parsing transport " + "address '%s'", addrport); + goto err; + } + if (!port) { + log_warn(LD_CONFIG, + "Transport address '%s' has no port.", addrport); + goto err; + } + + if (!validate_only) { + log_info(LD_DIR, "Server transport '%s' at %s.", + transports, fmt_addrport(&addr, port)); + } + } + + r = 0; + goto done; + + err: + r = -1; + + done: + SMARTLIST_FOREACH(items, char*, s, tor_free(s)); + smartlist_free(items); + if (transport_list) { + SMARTLIST_FOREACH(transport_list, char*, s, tor_free(s)); + smartlist_free(transport_list); + } + + return r; +} + /** Given a ServerTransportListenAddr <b>line</b>, return its * address:port string. Return NULL if the line was not * well-formed. @@ -5052,133 +5206,6 @@ get_options_for_server_transport(const char *transport) return NULL; }

-/** Read the contents of a ServerTransportPlugin line from - * <b>line</b>. Return 0 if the line is well-formed, and -1 if it - * isn't. - * If <b>validate_only</b> is 0, the line is well-formed, and it's a - * managed proxy line, launch the managed proxy. */ -STATIC int -parse_server_transport_line(const or_options_t *options, - const char *line, int validate_only) -{ - smartlist_t *items = NULL; - int r; - const char *transports=NULL; - smartlist_t *transport_list=NULL; - char *type=NULL; - char *addrport=NULL; - tor_addr_t addr; - uint16_t port = 0; - - /* managed proxy options */ - int is_managed=0; - char **proxy_argv=NULL; - char **tmp=NULL; - int proxy_argc,i; - - int line_length; - - items = smartlist_new(); - smartlist_split_string(items, line, NULL, - SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, -1); - - line_length = smartlist_len(items); - if (line_length < 3) { - log_warn(LD_CONFIG, "Too few arguments on ServerTransportPlugin line."); - goto err; - } - - /* Get the first line element, split it to commas into - transport_list (in case it's multiple transports) and validate - the transport names. */ - transports = smartlist_get(items, 0); - transport_list = smartlist_new(); - smartlist_split_string(transport_list, transports, ",", - SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); - SMARTLIST_FOREACH_BEGIN(transport_list, const char *, transport_name) { - if (!string_is_C_identifier(transport_name)) { - log_warn(LD_CONFIG, "Transport name is not a C identifier (%s).", - transport_name); - goto err; - } - } SMARTLIST_FOREACH_END(transport_name); - - type = smartlist_get(items, 1); - - if (!strcmp(type, "exec")) { - is_managed=1; - } else if (!strcmp(type, "proxy")) { - is_managed=0; - } else { - log_warn(LD_CONFIG, "Strange ServerTransportPlugin type '%s'", type); - goto err; - } - - if (is_managed && options->Sandbox) { - log_warn(LD_CONFIG, "Managed proxies are not compatible with Sandbox mode." - "(ServerTransportPlugin line was %s)", escaped(line)); - goto err; - } - - if (is_managed) { /* managed */ - if (!validate_only) { - proxy_argc = line_length-2; - tor_assert(proxy_argc > 0); - proxy_argv = tor_malloc_zero(sizeof(char*)*(proxy_argc+1)); - tmp = proxy_argv; - - 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*/ - - /* kickstart the thing */ - pt_kickstart_server_proxy(transport_list, proxy_argv); - } - } else { /* external */ - if (smartlist_len(transport_list) != 1) { - log_warn(LD_CONFIG, "You can't have an external proxy with " - "more than one transports."); - goto err; - } - - addrport = smartlist_get(items, 2); - - if (tor_addr_port_lookup(addrport, &addr, &port)<0) { - log_warn(LD_CONFIG, "Error parsing transport " - "address '%s'", addrport); - goto err; - } - if (!port) { - log_warn(LD_CONFIG, - "Transport address '%s' has no port.", addrport); - goto err; - } - - if (!validate_only) { - log_info(LD_DIR, "Server transport '%s' at %s.", - transports, fmt_addrport(&addr, port)); - } - } - - r = 0; - goto done; - - err: - r = -1; - - done: - SMARTLIST_FOREACH(items, char*, s, tor_free(s)); - smartlist_free(items); - if (transport_list) { - SMARTLIST_FOREACH(transport_list, char*, s, tor_free(s)); - smartlist_free(transport_list); - } - - return r; -} - /** Read the contents of a DirAuthority line from <b>line</b>. If * <b>validate_only</b> is 0, and the line is well-formed, and it * shares any bits with <b>required_type</b> or <b>required_type</b> diff --git a/src/or/config.h b/src/or/config.h index 31496c2..4097d37 100644 --- a/src/or/config.h +++ b/src/or/config.h @@ -140,10 +140,9 @@ STATIC int options_validate(or_options_t *old_options, or_options_t *options, or_options_t *default_options, int from_setconf, char **msg); -STATIC int parse_client_transport_line(const or_options_t *options, - const char *line, int validate_only); -STATIC int parse_server_transport_line(const or_options_t *options, - const char *line, int validate_only); +STATIC int parse_transport_line(const or_options_t *options, + const char *line, int validate_only, + int server); #endif

#endif diff --git a/src/test/test_config.c b/src/test/test_config.c index 5975e1e..ae01fac 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -608,84 +608,84 @@ test_config_parse_transport_plugin_line(void *arg) int old_transport_is_needed_mock_call_count;

/* Bad transport lines - too short */ - r = parse_client_transport_line(options, "bad", 1); + r = parse_transport_line(options, "bad", 1, 0); test_assert(r < 0); - r = parse_server_transport_line(options, "bad", 1); + r = parse_transport_line(options, "bad", 1, 1); test_assert(r < 0); - r = parse_client_transport_line(options, "bad bad", 1); + r = parse_transport_line(options, "bad bad", 1, 0); test_assert(r < 0); - r = parse_server_transport_line(options, "bad bad", 1); + r = parse_transport_line(options, "bad bad", 1, 1); test_assert(r < 0);

/* Test transport list parsing */ - r = parse_client_transport_line(options, - "transport_1 exec /usr/bin/fake-transport", 1); + r = parse_transport_line(options, + "transport_1 exec /usr/bin/fake-transport", 1, 0); test_assert(r == 0); - r = parse_server_transport_line(options, - "transport_1 exec /usr/bin/fake-transport", 1); + r = parse_transport_line(options, + "transport_1 exec /usr/bin/fake-transport", 1, 1); test_assert(r == 0); - r = parse_client_transport_line(options, - "transport_1,transport_2 exec /usr/bin/fake-transport", 1); + r = parse_transport_line(options, + "transport_1,transport_2 exec /usr/bin/fake-transport", 1, 0); test_assert(r == 0); - r = parse_server_transport_line(options, - "transport_1,transport_2 exec /usr/bin/fake-transport", 1); + r = parse_transport_line(options, + "transport_1,transport_2 exec /usr/bin/fake-transport", 1, 1); test_assert(r == 0); /* Bad transport identifiers */ - r = parse_client_transport_line(options, - "transport_* exec /usr/bin/fake-transport", 1); + r = parse_transport_line(options, + "transport_* exec /usr/bin/fake-transport", 1, 0); test_assert(r < 0); - r = parse_server_transport_line(options, - "transport_* exec /usr/bin/fake-transport", 1); + r = parse_transport_line(options, + "transport_* exec /usr/bin/fake-transport", 1, 1); test_assert(r < 0);

/* Check SOCKS cases for client transport */ - r = parse_client_transport_line(options, - "transport_1 socks4 1.2.3.4:567", 1); + r = parse_transport_line(options, + "transport_1 socks4 1.2.3.4:567", 1, 0); test_assert(r == 0); - r = parse_client_transport_line(options, - "transport_1 socks5 1.2.3.4:567", 1); + r = parse_transport_line(options, + "transport_1 socks5 1.2.3.4:567", 1, 0); test_assert(r == 0); /* Proxy case for server transport */ - r = parse_server_transport_line(options, - "transport_1 proxy 1.2.3.4:567", 1); + r = parse_transport_line(options, + "transport_1 proxy 1.2.3.4:567", 1, 1); test_assert(r == 0); /* Multiple-transport error exit */ - r = parse_client_transport_line(options, - "transport_1,transport_2 socks5 1.2.3.4:567", 1); + r = parse_transport_line(options, + "transport_1,transport_2 socks5 1.2.3.4:567", 1, 0); test_assert(r < 0); - r = parse_server_transport_line(options, - "transport_1,transport_2 proxy 1.2.3.4:567", 1); + r = parse_transport_line(options, + "transport_1,transport_2 proxy 1.2.3.4:567", 1, 1); /* No port error exit */ - r = parse_client_transport_line(options, - "transport_1 socks5 1.2.3.4", 1); + r = parse_transport_line(options, + "transport_1 socks5 1.2.3.4", 1, 0); test_assert(r < 0); - r = parse_server_transport_line(options, - "transport_1 proxy 1.2.3.4", 1); + r = parse_transport_line(options, + "transport_1 proxy 1.2.3.4", 1, 1); test_assert(r < 0); /* Unparsable address error exit */ - r = parse_client_transport_line(options, - "transport_1 socks5 1.2.3:6x7", 1); + r = parse_transport_line(options, + "transport_1 socks5 1.2.3:6x7", 1, 0); test_assert(r < 0); - r = parse_server_transport_line(options, - "transport_1 proxy 1.2.3:6x7", 1); + r = parse_transport_line(options, + "transport_1 proxy 1.2.3:6x7", 1, 1); test_assert(r < 0);

/* "Strange {Client|Server}TransportPlugin field" error exit */ - r = parse_client_transport_line(options, - "transport_1 foo bar", 1); + r = parse_transport_line(options, + "transport_1 foo bar", 1, 0); test_assert(r < 0); - r = parse_server_transport_line(options, - "transport_1 foo bar", 1); + r = parse_transport_line(options, + "transport_1 foo bar", 1, 1); test_assert(r < 0);

/* No sandbox mode error exit */ tmp = options->Sandbox; options->Sandbox = 1; - r = parse_client_transport_line(options, - "transport_1 exec /usr/bin/fake-transport", 1); + r = parse_transport_line(options, + "transport_1 exec /usr/bin/fake-transport", 1, 0); test_assert(r < 0); - r = parse_server_transport_line(options, - "transport_1 exec /usr/bin/fake-transport", 1); + r = parse_transport_line(options, + "transport_1 exec /usr/bin/fake-transport", 1, 1); test_assert(r < 0); options->Sandbox = tmp;

@@ -696,16 +696,16 @@ test_config_parse_transport_plugin_line(void *arg) MOCK(pt_kickstart_proxy, pt_kickstart_proxy_mock); old_pt_kickstart_proxy_mock_call_count = pt_kickstart_proxy_mock_call_count; - r = parse_server_transport_line(options, - "transport_1 exec /usr/bin/fake-transport", 0); + 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 == 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_server_transport_line(options, - "transport_1 proxy 1.2.3.4:567", 0); + r = parse_transport_line(options, + "transport_1 proxy 1.2.3.4:567", 0, 1); test_assert(r == 0);

/* Check mocked client transport cases */ @@ -721,8 +721,8 @@ test_config_parse_transport_plugin_line(void *arg) transport_add_from_config_mock_call_count; old_transport_is_needed_mock_call_count = transport_is_needed_mock_call_count; - r = parse_client_transport_line(options, - "transport_1 exec /usr/bin/fake-transport", 0); + r = parse_transport_line(options, + "transport_1 exec /usr/bin/fake-transport", 0, 0); /* Should have succeeded */ test_assert(r == 0); /* transport_is_needed() should have been called */ @@ -745,8 +745,8 @@ test_config_parse_transport_plugin_line(void *arg) transport_add_from_config_mock_call_count; old_transport_is_needed_mock_call_count = transport_is_needed_mock_call_count; - r = parse_client_transport_line(options, - "transport_1 exec /usr/bin/fake-transport", 0); + r = parse_transport_line(options, + "transport_1 exec /usr/bin/fake-transport", 0, 0); /* Should have succeeded */ test_assert(r == 0); /* @@ -769,8 +769,8 @@ test_config_parse_transport_plugin_line(void *arg) transport_add_from_config_mock_call_count; old_transport_is_needed_mock_call_count = transport_is_needed_mock_call_count; - r = parse_client_transport_line(options, - "transport_1 socks5 1.2.3.4:567", 0); + r = parse_transport_line(options, + "transport_1 socks5 1.2.3.4:567", 0, 0); /* Should have succeeded */ test_assert(r == 0); /*