[tor-commits] [obfsproxy/master] Avoid embedding struct sockaddr in protocol_params_t.
nickm at torproject.org
nickm at torproject.org
Thu Jul 14 16:28:32 UTC 2011
commit 54336bf81d57b44c772fff24df21a058f301dd91
Author: Zack Weinberg <zackw at 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);
More information about the tor-commits
mailing list