commit 8bffcf5549bbe27b95bff0ef357174c7028c3149 Author: Zack Weinberg <zackw@panix.com> Date: Mon Aug 1 16:50:21 2011 -0700 Take a chain saw to the overcomplicated argument segmenting code in main.c. Push the call to proto_params_init inside the call to create_listener. --- src/main.c | 289 ++++++++++++++++++------------------------------- src/network.c | 15 ++-- src/network.h | 2 +- src/test/tester.py.in | 4 +- src/util.c | 9 ++ src/util.h | 3 + 6 files changed, 127 insertions(+), 195 deletions(-) diff --git a/src/main.c b/src/main.c index 1459af3..14125f8 100644 --- a/src/main.c +++ b/src/main.c @@ -4,6 +4,7 @@ #include "util.h" +#include "container.h" #include "crypt.h" #include "network.h" #include "protocol.h" @@ -15,27 +16,18 @@ #include <event2/event.h> #include <event2/dns.h> -/* The character that seperates multiple listeners in the cli */ -#define SEPARATOR "+" -/* Totally arbitrary. */ -#define MAXPROTOCOLS 20 - -static void usage(void) ATTR_NORETURN; -static int handle_obfsproxy_args(const char **argv); - -static struct event_base *the_event_base=NULL; +static struct event_base *the_event_base; /** Prints the obfsproxy usage instructions then exits. */ -static void +static void ATTR_NORETURN usage(void) { int i; - fprintf(stderr, - "Usage: obfsproxy protocol_name [protocol_args] protocol_options %s protocol_name ...\n" - "* Available protocols:\n", - SEPARATOR); + fputs("Usage: obfsproxy protocol_name [protocol_args] protocol_options " + "protocol_name ...\n" + "* Available protocols:\n", stderr); /* this is awful. */ for (i=0;i<n_supported_protocols;i++) fprintf(stderr,"[%s] ", supported_protocols[i]->name); @@ -83,11 +75,7 @@ handle_signal_cb(evutil_socket_t fd, short what, void *arg) } } -/** - Stops obfsproxy's event loop. - - Final cleanup happens in main(). -*/ +/** Stop obfsproxy's event loop. Final cleanup happens in main(). */ void finish_shutdown(void) { @@ -95,36 +83,21 @@ finish_shutdown(void) event_base_loopexit(the_event_base, NULL); } -/** - This function visits 'n_options' command line arguments off 'argv' - and writes them in 'options_string'. -*/ -static void -populate_options(char **options_string, - const char **argv, int n_options) +/** Return 1 if 'name' is the name of a supported protocol, otherwise 0. */ +static int +is_supported_protocol(const char *name) { - int g; - for (g=0;g<=n_options-1;g++) - options_string[g] = (char*) argv[g]; -} + int i; + for (i = 0; i < n_supported_protocols; i++) + if (!strcmp(name, supported_protocols[i]->name)) + return 1; -/** - Return 0 if 'name' is the name of a supported protocol, otherwise - return -1. -*/ -static int -is_supported_protocol(const char *name) { - int f; - for (f=0;f<n_supported_protocols;f++) { - if (!strcmp(name,supported_protocols[f]->name)) - return 0; - } - return -1; + return 0; } /** - Receives argv[1] as 'argv' and scans from thereafter for any - obfsproxy optional arguments and tries to set them in effect. + Receives 'argv' and scans for any obfsproxy optional arguments and + tries to set them in effect. If it succeeds it returns the number of argv arguments its caller should skip to get past the optional arguments we already handled. @@ -135,7 +108,7 @@ handle_obfsproxy_args(const char **argv) { int logmethod_set=0; int logsev_set=0; - int i=0; + int i=1; while (argv[i] && !strncmp(argv[i],"--",2)) { @@ -186,121 +159,81 @@ main(int argc, const char **argv) struct event *sig_int; struct event *sig_term; - /* Yes, these are three stars right there. This is an array of - arrays of strings! Every element of the array is an array of - strings that contains all the options of a protocol. - At runtime it should look like this: - char protocol_options[<number of protocols>][<number of options>][<length of option>] - */ - char ***protocol_options = NULL; - /* This is an array of integers! Each integer is the number of - options of the respective protocol. */ - int *n_options_array = NULL; - /* This is an integer! It contains the number of protocols that we - managed to recognize, by their protocol name. Of course it's not - the *actual* actual_protocols since some of them could have wrong - options or arguments, but this will be resolved per-protocol by - proto_params_init(). */ - int actual_protocols=0; - - int start; - int end; - int n_options; - int i; + /* Array of argument counts, one per listener. */ + int *listener_argcs = NULL; - /* The number of protocols. */ - unsigned int n_protocols=1; - /* An array which holds the position in argv of the command line - options for each protocol. */ - unsigned int *protocols=NULL; - /* keeps track of allocated space for the protocols array */ - unsigned int n_alloc; + /* Array of pointers into argv. Each points to the beginning of a + sequence of options for a particular listener. */ + const char *const **listener_argvs = NULL; - if (argc < 2) { - usage(); - } + /* Total number of listeners requested on the command line. */ + unsigned int n_listeners; - /** "Points" to the first argv string after the optional obfsproxy - arguments. Normally this should be where the protocols start. */ - int start_of_protocols; - /** Handle optional obfsproxy arguments. */ - start_of_protocols = handle_obfsproxy_args(&argv[1]); - - protocols = xzalloc((n_protocols + 1) * sizeof(int)); - n_alloc = n_protocols+1; - - /* Populate protocols and calculate n_protocols. */ - for (i=start_of_protocols;i<argc;i++) { - if (!strcmp(argv[i],SEPARATOR)) { - protocols[n_protocols] = i; - n_protocols++; - - /* Do we need to expand the protocols array? */ - if (n_alloc <= n_protocols) { - n_alloc *= 2; - protocols = xrealloc(protocols, sizeof(int)*(n_alloc)); - } - } - } + /* Total number of listeners successfully created. */ + unsigned int n_good_listeners; - /* protocols[0] points right before the first option of the first - protocol. */ - protocols[0] = start_of_protocols; - /* protocols[n_protocols] points right after the last command line - option. */ - protocols[n_protocols] = argc; - - log_debug("Found %d protocol(s).", n_protocols); - - /* We now allocate enough space for our parsing adventures. - We first allocate space for n_protocols pointers in protocol_options, - that point to arrays carrying the options of the protocols. - Finally, we allocate enough space on the n_options_array so that - we can put the number of options there. - */ - protocol_options = xzalloc(n_protocols * sizeof(char**)); - n_options_array = xzalloc(n_protocols * sizeof(int)); - - /* Iterate through protocols. */ - for (i=0;i<n_protocols;i++) { - log_debug("Parsing protocol %d.", i+1); - /* This "points" to the first argument of this protocol in argv. */ - start = protocols[i]+1; - /* This "points" to the last argument of this protocol in argv. */ - end = protocols[i+1]-1; - /* This is the number of options of this protocol. */ - n_options = end-start+1; - - if (start >= end) { - log_warn("No protocol options were given on protocol %d.", i+1); - continue; - } + /* Index of the first argv string after the optional obfsproxy + arguments. Normally this should be where the listeners start. */ + int start_of_listeners; - /* First option should be protocol_name. See if we support it. */ - if (is_supported_protocol(argv[start])<0) { - log_warn("We don't support protocol: %s", argv[start]); - continue; - } + int cl, i; + + /* Handle optional obfsproxy arguments. */ + start_of_listeners = handle_obfsproxy_args(argv); - actual_protocols++; + if (!is_supported_protocol(argv[start_of_listeners])) + usage(); - /* Allocate space for the array carrying the options of this - protocol. */ - protocol_options[actual_protocols-1] = xzalloc(n_options * sizeof(char*)); + /* Count number of listeners and allocate space for the listener- + argument arrays. We already know there's at least one. */ + n_listeners = 1; + for (i = start_of_listeners+1; i < argc; i++) + if (is_supported_protocol(argv[i])) + n_listeners++; - /* Write the number of options to the correct place in n_options_array[]. */ - n_options_array[actual_protocols-1] = n_options; + log_debug("%d listener%s on command line.", + n_listeners, n_listeners == 1 ? "" : "s"); + listener_argcs = xzalloc(n_listeners * sizeof(int)); + listener_argvs = xzalloc(n_listeners * sizeof(char **)); + + /* Each listener's argument vector consists of the entries in argv + from its recognized protocol name, up to but not including + the next recognized protocol name. */ + cl = 1; + listener_argvs[0] = &argv[start_of_listeners]; + for (i = start_of_listeners + 1; i < argc; i++) + if (is_supported_protocol(argv[i])) { + listener_argcs[cl-1] = i - (listener_argvs[cl-1] - argv); + if (listener_argcs[cl-1] == 1) + log_warn("No arguments to listener %d", cl); + + listener_argvs[cl] = &argv[i]; + cl++; + } - /* Finally! Let's fill protocol_options. */ - populate_options(protocol_options[actual_protocols-1], - &argv[start], n_options); + listener_argcs[cl-1] = argc - (listener_argvs[cl-1] - argv); + if (listener_argcs[cl-1] == 1) + log_warn("No arguments to listener %d", cl); + + obfs_assert(cl == n_listeners); + + if (log_do_debug()) { + smartlist_t *s = smartlist_create(); + char *joined; + for (cl = 0; cl < n_listeners; cl++) { + smartlist_clear(s); + for (i = 0; i < listener_argcs[cl]; i++) + smartlist_add(s, (void *)listener_argvs[cl][i]); + joined = smartlist_join_strings(s, " ", 0, NULL); + log_debug("Listener %d: %s", cl+1, joined); + } + smartlist_free(s); } - /* Excellent. Now we should have protocol_options populated with all - the protocol options we got from the user. */ + /* argv has been chunked; proceed with initialization. */ - /* Ugly method to fix a Windows problem: - http://archives.seul.org/libevent/users/Oct-2010/msg00049.html */ + /* Ugly method to fix a Windows problem: + http://archives.seul.org/libevent/users/Oct-2010/msg00049.html */ #ifdef _WIN32 WSADATA wsaData; WSAStartup(0x101, &wsaData); @@ -308,24 +241,21 @@ main(int argc, const char **argv) /* Initialize crypto */ if (initialize_crypto() < 0) { - log_warn("Can't initialize crypto; failing"); - return 1; + log_error("Failed to initialize cryptography."); } /* Initialize libevent */ the_event_base = event_base_new(); if (!the_event_base) { - log_warn("Can't initialize Libevent; failing"); - return 1; + log_error("Failed to initialize networking."); } /* ASN should this happen only when SOCKS is enabled? */ - if (init_evdns_base(the_event_base) < 0) { - log_warn("Can't initialize evdns; failing"); - return 1; + if (init_evdns_base(the_event_base)) { + log_error("Failed to initialize DNS resolver."); } - /* Handle signals */ + /* Handle signals. */ #ifdef SIGPIPE signal(SIGPIPE, SIG_IGN); #endif @@ -334,37 +264,27 @@ main(int argc, const char **argv) sig_term = evsignal_new(the_event_base, SIGTERM, handle_signal_cb, NULL); if (event_add(sig_int,NULL) || event_add(sig_term,NULL)) { - log_warn("We can't even add events for signals! Exiting."); + log_error("Failed to initialize signal handling."); return 1; } - /*Let's open a new listener for each protocol. */ - int h; - int n_listeners=0; - protocol_params_t *proto_params=NULL; - for (h=0;h<actual_protocols;h++) { - log_debug("Spawning listener %d!", h+1); - - /** normally free'd in listener_free() */ - proto_params = proto_params_init(n_options_array[h], - (const char *const *)protocol_options[h]); - if (proto_params && create_listener(the_event_base, proto_params)) { - log_info("Succesfully created listener %d.", h+1); - n_listeners++; - } - - /** Free the space allocated for this protocol's options. */ - free(protocol_options[h]); - } + /* Open a new listener for each protocol. */ + n_good_listeners = 0; + for (cl = 0; cl < n_listeners; cl++) + if (create_listener(the_event_base, + listener_argcs[cl], listener_argvs[cl])) + n_good_listeners++; - log_debug("From the original %d protocols only %d " - "were parsed from main.c. In the end only " - "%d survived.", - n_protocols, actual_protocols,n_listeners); + /* If the number of usable listeners is not equal to the complete + set specified on the command line, we have a usage error. + Diagnostics have already been issued. */ + log_debug("%d recognized listener%s on command line, %d with valid config", + n_listeners, n_listeners == 1 ? "" : "s", n_good_listeners); + if (n_listeners != n_good_listeners) + return 2; - /* run the event loop if at least one listener was created. */ - if (n_listeners) - event_base_dispatch(the_event_base); + /* We are go for launch. */ + event_base_dispatch(the_event_base); log_info("Exiting."); @@ -377,9 +297,8 @@ main(int argc, const char **argv) cleanup_crypto(); close_obfsproxy_logfile(); - free(protocol_options); - free(n_options_array); - free(protocols); + free(listener_argvs); + free(listener_argcs); return 0; } diff --git a/src/network.c b/src/network.c index c9eafce..c327403 100644 --- a/src/network.c +++ b/src/network.c @@ -126,18 +126,19 @@ close_all_connections(void) /** This function spawns a listener configured according to the - provided 'protocol_params_t' object'. Returns 1 on success, 0 on - failure. (No, you can't have the listener object. It's private.) - - Regardless of success or failure, the protocol_params_t is consumed. + provided argument subvector. Returns 1 on success, 0 on failure. + (No, you can't have the listener object. It's private.) */ int -create_listener(struct event_base *base, protocol_params_t *params) +create_listener(struct event_base *base, int argc, const char *const *argv) { const unsigned flags = LEV_OPT_CLOSE_ON_FREE|LEV_OPT_CLOSE_ON_EXEC|LEV_OPT_REUSEABLE; evconnlistener_cb callback; - listener_t *lsn = xzalloc(sizeof(listener_t)); + listener_t *lsn; + protocol_params_t *params = proto_params_init(argc, argv); + if (!params) + return 0; switch (params->mode) { case LSN_SIMPLE_CLIENT: callback = simple_client_listener_cb; break; @@ -145,7 +146,7 @@ create_listener(struct event_base *base, protocol_params_t *params) case LSN_SOCKS_CLIENT: callback = socks_client_listener_cb; break; default: obfs_abort(); } - + lsn = xzalloc(sizeof(listener_t)); lsn->address = printable_address(params->listen_addr->ai_addr, params->listen_addr->ai_addrlen); lsn->proto_params = params; diff --git a/src/network.h b/src/network.h index 4ae96fc..54684a6 100644 --- a/src/network.h +++ b/src/network.h @@ -6,7 +6,7 @@ #define NETWORK_H /* returns 1 on success, 0 on failure */ -int create_listener(struct event_base *base, protocol_params_t *params); +int create_listener(struct event_base *base, int argc, const char *const *argv); void free_all_listeners(void); void start_shutdown(int barbaric); diff --git a/src/test/tester.py.in b/src/test/tester.py.in index 683724d..55d9477 100644 --- a/src/test/tester.py.in +++ b/src/test/tester.py.in @@ -356,7 +356,7 @@ class SocksBad(SocksTest, unittest.TestCase): # obfs_args = ("obfs2", # "--dest=127.0.0.1:%d" % EXIT_PORT, # "server", "127.0.0.1:%d" % SERVER_PORT, -# "+", "obfs2", +# "obfs2", # "--dest=127.0.0.1:%d" % SERVER_PORT, # "client", "127.0.0.1:%d" % ENTRY_PORT) @@ -364,7 +364,7 @@ class DirectDummy(DirectTest, unittest.TestCase): obfs_args = ("dummy", "server", "127.0.0.1:%d" % SERVER_PORT, "127.0.0.1:%d" % EXIT_PORT, - "+", "dummy", "client", + "dummy", "client", "127.0.0.1:%d" % ENTRY_PORT, "127.0.0.1:%d" % SERVER_PORT) diff --git a/src/util.c b/src/util.c index fa1a62c..3769aba 100644 --- a/src/util.c +++ b/src/util.c @@ -472,6 +472,15 @@ log_set_min_severity(const char* sev_string) return 0; } +/** True if the minimum log severity is "debug". Used in a few places + to avoid some expensive formatting work if we are going to ignore the + result. */ +int +log_do_debug(void) +{ + return logging_min_sev == LOG_SEV_DEBUG; +} + /** Logging worker function. Accepts a logging 'severity' and a 'format' string and logs the diff --git a/src/util.h b/src/util.h index 5108b62..d2bbea9 100644 --- a/src/util.h +++ b/src/util.h @@ -138,6 +138,9 @@ int log_set_method(int method, const char *filename); 'sev_string' may be "warn", "info", or "debug" (case-insensitively). */ int log_set_min_severity(const char* sev_string); +/** True if debug messages are being logged. */ +int log_do_debug(void); + /** Close the logfile if it's open. Ignores errors. */ void close_obfsproxy_logfile(void);