commit 54336bf81d57b44c772fff24df21a058f301dd91 Author: Zack Weinberg <zackw@panix.com> Date: Fri Jul 8 14:27:39 2011 -0700 Avoid embedding struct sockaddr in protocol_params_t. resolve_address_port now heap-allocates a buffer of exactly the right size for the sockaddr coming back from evutil_getaddrinfo, and copies a *pointer* to it into an outparam. This removes the size limit, the need to include <sys/socket.h> in protocol.h, and code duplicated in all callers of resolve_address_port. It might make sense to have resolve_address_port return the entire evutil_addrinfo structure (eliminating the remaining copy) but let's not go there just yet. --- src/network.c | 6 +++--- src/protocol.c | 12 ++++++++---- src/protocol.h | 15 +++++++-------- src/protocols/dummy.c | 21 +++++++-------------- src/protocols/obfs2.c | 32 ++++++++++---------------------- src/util.c | 15 ++++++--------- src/util.h | 6 +++--- 7 files changed, 44 insertions(+), 63 deletions(-) diff --git a/src/network.c b/src/network.c index 0faf136..dfa086c 100644 --- a/src/network.c +++ b/src/network.c @@ -67,8 +67,8 @@ listener_new(struct event_base *base, lsn->listener = evconnlistener_new_bind(base, simple_listener_cb, lsn, flags, -1, - &lsn->proto_params->on_address, - lsn->proto_params->on_address_len); + lsn->proto_params->listen_address, + lsn->proto_params->listen_address_len); if (!lsn->listener) { log_warn("Failed to create listener!"); @@ -166,7 +166,7 @@ simple_listener_cb(struct evconnlistener *evcl, if (conn->mode == LSN_SIMPLE_SERVER || conn->mode == LSN_SIMPLE_CLIENT) { /* Launch the connect attempt. */ if (bufferevent_socket_connect(conn->output, - (struct sockaddr*)&lsn->proto_params->target_address, + lsn->proto_params->target_address, lsn->proto_params->target_address_len)<0) goto err; diff --git a/src/protocol.c b/src/protocol.c index 544dcef..e306c8e 100644 --- a/src/protocol.c +++ b/src/protocol.c @@ -28,7 +28,7 @@ int n_supported_protocols = 2; of obfsproxy. */ int -set_up_protocol(int n_options, char **options, +set_up_protocol(int n_options, char **options, struct protocol_params_t *params) { if (!strcmp(*options,"dummy")) @@ -41,7 +41,7 @@ set_up_protocol(int n_options, char **options, /** This function creates a protocol object. - It's called once per connection. + It's called once per connection. It creates a new protocol_t structure and fills it's vtable etc. Return a 'protocol_t' if successful, NULL otherwise. */ @@ -80,7 +80,7 @@ proto_send(struct protocol_t *proto, void *source, void *dest) { assert(proto); if (proto->vtable->send) return proto->vtable->send(proto->state, source, dest); - else + else return -1; } @@ -100,7 +100,7 @@ proto_recv(struct protocol_t *proto, void *source, void *dest) { This function destroys 'proto'. It's called everytime we close a connection. */ -void +void proto_destroy(struct protocol_t *proto) { assert(proto); assert(proto->state); @@ -120,6 +120,10 @@ proto_params_free(protocol_params_t *params) { assert(params); + if (params->target_address) + free(params->target_address); + if (params->listen_address) + free(params->listen_address); if (params->shared_secret) free(params->shared_secret); free(params); diff --git a/src/protocol.h b/src/protocol.h index 483bae2..1f0e7dd 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -5,10 +5,9 @@ #ifndef PROTOCOL_H #define PROTOCOL_H -#include <sys/socket.h> /* for sockaddr & sockaddr_storage - FIXME */ - struct evbuffer; struct listener_t; +struct sockaddr; #define DUMMY_PROTOCOL 1 #define OBFS2_PROTOCOL 2 @@ -21,15 +20,15 @@ struct listener_t; owns this protocol_params_t. */ typedef struct protocol_params_t { + struct sockaddr *target_address; + struct sockaddr *listen_address; + char *shared_secret; + size_t shared_secret_len; + size_t target_address_len; + size_t listen_address_len; int is_initiator; - struct sockaddr_storage target_address; - int target_address_len; - struct sockaddr on_address; - int on_address_len; int mode; int proto; /* Protocol that this listener can speak. */ - char *shared_secret; - size_t shared_secret_len; } protocol_params_t; struct protocol_t { diff --git a/src/protocols/dummy.c b/src/protocols/dummy.c index bdf97fc..8db5d51 100644 --- a/src/protocols/dummy.c +++ b/src/protocols/dummy.c @@ -57,13 +57,11 @@ dummy_init(int n_options, char **options, } static int -parse_and_set_options(int n_options, char **options, +parse_and_set_options(int n_options, char **options, struct protocol_params_t *params) { - struct sockaddr_storage ss_listen; - int sl_listen; const char* defport; - + if (n_options != 3) return -1; @@ -82,17 +80,13 @@ parse_and_set_options(int n_options, char **options, } else return -1; - if (resolve_address_port(options[2], 1, 1, - &ss_listen, &sl_listen, defport) < 0) { + if (resolve_address_port(options[2], 1, 1, + ¶ms->listen_address, + ¶ms->listen_address_len, defport) < 0) { log_warn("addr"); return -1; } - assert(sl_listen <= sizeof(struct sockaddr_storage)); - struct sockaddr *sa_listen=NULL; - sa_listen = (struct sockaddr *)&ss_listen; - memcpy(¶ms->on_address, sa_listen, sl_listen); - params->on_address_len = sl_listen; - + return 1; } @@ -108,7 +102,6 @@ usage(void) "Example:\n" "\tobfsproxy dummy socks 127.0.0.1:5000\n"); } - void * @@ -117,7 +110,7 @@ dummy_new(struct protocol_t *proto_struct, { proto_struct->vtable = vtable; - /* Dodging state check. + /* Dodging state check. This is terrible I know.*/ return (void *)666U; } diff --git a/src/protocols/obfs2.c b/src/protocols/obfs2.c index 676ddd7..57a695f 100644 --- a/src/protocols/obfs2.c +++ b/src/protocols/obfs2.c @@ -25,7 +25,7 @@ static int obfs2_send(void *state, struct evbuffer *source, struct evbuffer *dest); static enum recv_ret obfs2_recv(void *state, struct evbuffer *source, struct evbuffer *dest); -static void *obfs2_state_new(protocol_params_t *params); +static void *obfs2_state_new(protocol_params_t *params); static int obfs2_state_set_shared_secret(void *s, const char *secret, size_t secretlen); @@ -34,7 +34,7 @@ static void usage(void); static protocol_vtable *vtable=NULL; -/* +/* This function parses 'options' and fills the protocol parameters structure 'params'. It then fills the obfs2 vtable and initializes the crypto subsystem. @@ -42,7 +42,7 @@ static protocol_vtable *vtable=NULL; Returns 0 on success, -1 on fail. */ int -obfs2_init(int n_options, char **options, +obfs2_init(int n_options, char **options, struct protocol_params_t *params) { if (parse_and_set_options(n_options,options,params) < 0) { @@ -62,11 +62,9 @@ obfs2_init(int n_options, char **options, } int -parse_and_set_options(int n_options, char **options, +parse_and_set_options(int n_options, char **options, struct protocol_params_t *params) { - struct sockaddr_storage ss_listen; - int sl_listen; int got_dest=0; int got_ss=0; const char* defport; @@ -85,21 +83,15 @@ parse_and_set_options(int n_options, char **options, if (!strncmp(*options,"--dest=",7)) { if (got_dest) return -1; - struct sockaddr_storage ss_target; - struct sockaddr *sa_target=NULL; - int sl_target=0; if (resolve_address_port(*options+7, 1, 0, - &ss_target, &sl_target, NULL) < 0) + ¶ms->target_address, + ¶ms->target_address_len, NULL) < 0) return -1; - assert(sl_target <= sizeof(struct sockaddr_storage)); - sa_target = (struct sockaddr *)&ss_target; - memcpy(¶ms->target_address, sa_target, sl_target); - params->target_address_len = sl_target; got_dest=1; } else if (!strncmp(*options,"--shared-secret=",16)) { if (got_ss) return -1; - /* this is freed in protocol_params_free() */ + /* this is freed in proto_params_free() */ params->shared_secret = strdup(*options+16); params->shared_secret_len = strlen(*options+16); got_ss=1; @@ -127,14 +119,10 @@ parse_and_set_options(int n_options, char **options, params->is_initiator = (params->mode != LSN_SIMPLE_SERVER); - if (resolve_address_port(*options, 1, 1, - &ss_listen, &sl_listen, defport) < 0) + if (resolve_address_port(*options, 1, 1, + ¶ms->listen_address, + ¶ms->listen_address_len, defport) < 0) return -1; - assert(sl_listen <= sizeof(struct sockaddr_storage)); - struct sockaddr *sa_listen=NULL; - sa_listen = (struct sockaddr *)&ss_listen; - memcpy(¶ms->on_address, sa_listen, sl_listen); - params->on_address_len = sl_listen; /* Validate option selection. */ if (got_dest && (params->mode == LSN_SOCKS_CLIENT)) { diff --git a/src/util.c b/src/util.c index 70a6586..ce9a042 100644 --- a/src/util.c +++ b/src/util.c @@ -38,8 +38,8 @@ static void logv(int severity, const char *format, va_list ap); int resolve_address_port(const char *address, int nodns, int passive, - struct sockaddr_storage *addr_out, - int *addrlen_out, + struct sockaddr **addr_out, + size_t *addrlen_out, const char *default_port) { struct evutil_addrinfo *ai = NULL; @@ -78,13 +78,10 @@ resolve_address_port(const char *address, log_warn("No result for address %s", address); goto done; } - if (ai->ai_addrlen > sizeof(struct sockaddr_storage)) { - log_warn("Result for address %s too long", address); - goto done; - } - - memcpy(addr_out, ai->ai_addr, ai->ai_addrlen); - *addrlen_out = (int) ai->ai_addrlen; + struct sockaddr *addr = malloc(ai->ai_addrlen); + memcpy(addr, ai->ai_addr, ai->ai_addrlen); + *addr_out = addr; + *addrlen_out = ai->ai_addrlen; result = 0; done: diff --git a/src/util.h b/src/util.h index 90c55c2..9a8a577 100644 --- a/src/util.h +++ b/src/util.h @@ -8,7 +8,7 @@ #include <stdarg.h> /* for va_list */ #include <stddef.h> /* for size_t etc */ -struct sockaddr_storage; +struct sockaddr; struct event_base; struct evdns_base; @@ -16,8 +16,8 @@ struct evdns_base; int resolve_address_port(const char *address, int nodns, int passive, - struct sockaddr_storage *addr_out, - int *addrlen_out, + struct sockaddr **addr_out, + size_t *addrlen_out, const char *default_port); struct evdns_base *get_evdns_base(void);