[tor-commits] [obfsproxy/master] This commit:

commit b46dde9aa8ebd779fe651c04704f8e253830f3eb Author: George Kadianakis desnacked@gmail.com Date: Mon Apr 11 02:18:39 2011 +0200

This commit: * Fixes many small bugs all around the code reported by Nick Mathewson. * Removes the nasty casts to (void *) on the vtable assignment. * Fixes BUG: * The BUG was caused by the fact that we initialized a single protocol_t in listener_new() for all connections. This means that multiple connections shared the same protocol_t and hence the same protocol state. So when a connection was freed, the state of other connections was also freed and overwritten. This was fixed: * By introducing a proto_init() function, which initializes the protocol. It's run only once; in the start of obfsproxy. * By using a proto_new() function which returns a protocol_t for every connection. * We now use a protocol_vtable structure which contains the function pointer table, and each protocol maintains a static vtable of it's own.

* TODO: Right now, both proto_new() and set_up_protocol() are protocol specific. proto_new() must be proto-agnostic, but I didn't have time to tinker and I wanted to see if my BUG solution would work so I left it like this. I'll fix it soonish. --- BUG | 21 ---------- src/main.c | 1 - src/network.c | 23 ++++------- src/plugins/dummy.c | 66 ++++++++++++++++--------------- src/plugins/dummy.h | 15 +------ src/plugins/obfs2.c | 105 ++++++++++++++++++++++++++++++++++---------------- src/plugins/obfs2.h | 13 +----- src/protocol.c | 66 ++++++++++++++++++++------------ src/protocol.h | 38 ++++++++++++------- src/socks.c | 2 - 10 files changed, 184 insertions(+), 166 deletions(-)

diff --git a/BUG b/BUG deleted file mode 100644 index 248b855..0000000 --- a/BUG +++ /dev/null @@ -1,21 +0,0 @@ -- -Program received signal SIGSEGV, Segmentation fault. -crypt_free (key=0xa0a0a0a0a0a0a0a) at src/plugins/obfs2_crypt.c:194 -194 memset(key, 0, sizeof(key)); -(gdb) backtrace -#0 crypt_free (key=0xa0a0a0a0a0a0a0a) at src/plugins/obfs2_crypt.c:194 -#1 0x0000000000403852 in obfs2_state_free (s=0x618230) at src/plugins/obfs2.c:357 -#2 0x0000000000401df2 in conn_free (conn=0x615bc0) at src/network.c:210 -#3 0x00007ffff7bb4ad8 in bufferevent_readcb (fd=<value optimized out>, event=<value optimized out>, arg=0x615f30) at bufferevent_sock.c:191 -#4 0x00007ffff7baa88c in event_process_active_single_queue (base=0x606220, flags=<value optimized out>) at event.c:1287 -#5 event_process_active (base=0x606220, flags=<value optimized out>) at event.c:1354 -#6 event_base_loop (base=0x606220, flags=<value optimized out>) at event.c:1551 -#7 0x0000000000401cc9 in main (argc=<value optimized out>, argv=<value optimized out>) at src/main.c:124 -(gdb) up -#1 0x0000000000403852 in obfs2_state_free (s=0x618230) at src/plugins/obfs2.c:357 -357 crypt_free(s->send_crypto); -(gdb) p s -$1 = (obfs2_state_t *) 0x618230 -(gdb) p s->send_crypto -$2 = (crypt_t *) 0xa0a0a0a0a0a0a0a -- diff --git a/src/main.c b/src/main.c index 4f29005..225c698 100644 --- a/src/main.c +++ b/src/main.c @@ -117,7 +117,6 @@ main(int argc, const char **argv) sigevent = evsignal_new(base, SIGINT, handle_signal_cb, (void*) base);

/* start an evconnlistener on the appropriate port(s) */ - /* ASN We hardcode OBFS2_PROTOCOL for now. */ listener = listener_new(base, mode, protocol, (struct sockaddr *)&ss_listen, sl_listen, diff --git a/src/network.c b/src/network.c index 3e23cdc..1199c96 100644 --- a/src/network.c +++ b/src/network.c @@ -27,7 +27,7 @@ struct listener_t { struct evconnlistener *listener; struct sockaddr_storage target_address; int target_address_len; - struct protocol_t *proto; /* Protocol that this listener can speak. */ + int proto; /* Protocol that this listener can speak. */ int mode; /* ASN */ /* char shared_secret[SHARED_SECRET_LENGTH]; @@ -63,13 +63,12 @@ listener_new(struct event_base *base, assert(mode == LSN_SIMPLE_CLIENT || mode == LSN_SIMPLE_SERVER || mode == LSN_SOCKS_CLIENT);

- struct protocol_t *proto = set_up_protocol(protocol); - if (!proto) { - printf("This is just terrible. We can't even set up a protocol! Seppuku time!\n"); - exit(-1); + if (set_up_protocol(protocol)<0) { + printf("This is just terrible. We can't even set up a protocol! Exiting.\n"); + exit(1); }

- lsn->proto = proto; + lsn->proto = protocol; lsn->mode = mode;

if (target_address) { @@ -79,6 +78,7 @@ listener_new(struct event_base *base, } else { assert(lsn->mode == LSN_SOCKS_CLIENT); } + /* ASN */ /* assert(shared_secret == NULL || shared_secret_len == SHARED_SECRET_LENGTH); @@ -123,16 +123,11 @@ simple_listener_cb(struct evconnlistener *evcl, dbg(("Got a connection\n"));

conn->mode = lsn->mode; - conn->proto = lsn->proto;

/* Will all protocols need to _init() here? Don't think so! */ - int is_initiator = (conn->mode != LSN_SIMPLE_SERVER) ? 1 : 0; - conn->proto->state = proto_init(conn->proto, &is_initiator); - - /* ASN Which means that all plugins need a state... */ - if (!conn->proto->state) - goto err; - + conn->proto = proto_new(lsn->proto, + conn->mode != LSN_SIMPLE_SERVER); + if (conn->mode == LSN_SOCKS_CLIENT) { /* Construct SOCKS state. */ conn->socks_state = socks_state_new(); diff --git a/src/plugins/dummy.c b/src/plugins/dummy.c index 957c30b..8fe722e 100644 --- a/src/plugins/dummy.c +++ b/src/plugins/dummy.c @@ -1,10 +1,3 @@ -/* Copyright 2011 Princess Peach Toadstool - - You may do anything with this work that copyright law would normally - restrict, so long as you retain the above notice(s) and this license - in all redistributed copies and derived works. There is no warranty. -*/ - #include <assert.h> #include <string.h> #include <stdlib.h> @@ -19,43 +12,52 @@ #include "../util.h" #include "../protocol.h"

+ +static int dummy_send(void *nothing, + struct evbuffer *source, struct evbuffer *dest); +static int dummy_recv(void *nothing, struct evbuffer *source, + struct evbuffer *dest); + +static protocol_vtable *vtable=NULL; + int -dummy_new(struct protocol_t *proto_struct) { - proto_struct->destroy = (void *)NULL; - proto_struct->init = (void *)dummy_init; - proto_struct->handshake = (void *)NULL; - proto_struct->send = (void *)dummy_send; - proto_struct->recv = (void *)dummy_recv; - - return 0; +dummy_init(void) { + vtable = calloc(1, sizeof(protocol_vtable)); + if (!vtable) + return -1; + + vtable->destroy = NULL; + vtable->create = dummy_new; + vtable->handshake = NULL; + vtable->send = dummy_send; + vtable->recv = dummy_recv; + + return 1; }

-int * -dummy_init(int *initiator) { - /* Dodging state check. */ - return initiator; +void * +dummy_new(struct protocol_t *proto_struct, int whatever) { + (void)whatever; + + proto_struct->vtable = vtable; + + /* Dodging state check. + This is terrible I know.*/ + return (void *)666U; }

-int +static int dummy_send(void *nothing, struct evbuffer *source, struct evbuffer *dest) { (void)nothing;

- /* ASN evbuffer_add_buffer() doesn't work for some reason. */ - while (1) { - int n = evbuffer_remove_buffer(source, dest, 1024); - if (n <= 0) - return 0; - } + return evbuffer_add_buffer(dest,source); }

-int +static int dummy_recv(void *nothing, struct evbuffer *source, struct evbuffer *dest) { (void)nothing; - while (1) { - int n = evbuffer_remove_buffer(source, dest, 1024); - if (n <= 0) - return 0; - } + + return evbuffer_add_buffer(dest,source); } diff --git a/src/plugins/dummy.h b/src/plugins/dummy.h index cf9342a..241366d 100644 --- a/src/plugins/dummy.h +++ b/src/plugins/dummy.h @@ -1,21 +1,10 @@ -/* Copyright 2011 Princess Peach Toadstool - - You may do anything with this work that copyright law would normally - restrict, so long as you retain the above notice(s) and this license - in all redistributed copies and derived works. There is no warranty. -*/ - #ifndef DUMMY_H #define DUMMY_H

struct protocol_t; struct evbuffer;

-int *dummy_init(int *initiator); -int dummy_send(void *nothing, - struct evbuffer *source, struct evbuffer *dest); -int dummy_recv(void *nothing, struct evbuffer *source, - struct evbuffer *dest); -int dummy_new(struct protocol_t *proto_struct); +int dummy_init(void); +void *dummy_new(struct protocol_t *proto_struct, int whatever);

#endif diff --git a/src/plugins/obfs2.c b/src/plugins/obfs2.c index ef8be8e..cac7bb2 100644 --- a/src/plugins/obfs2.c +++ b/src/plugins/obfs2.c @@ -20,17 +20,31 @@ #include "../util.h" #include "../protocol.h"

+static void obfs2_state_free(void *state); +static int obfs2_send_initial_message(void *state, struct evbuffer *buf); +static int obfs2_send(void *state, + struct evbuffer *source, struct evbuffer *dest); +static int obfs2_recv(void *state, struct evbuffer *source, + struct evbuffer *dest); +static void *obfs2_state_new(int initiator); + +static protocol_vtable *vtable=NULL; + /* Sets the function table for the obfs2 protocol and calls initialize_crypto(). Returns 0 on success, -1 on fail. */ int -obfs2_new(struct protocol_t *proto_struct) { - proto_struct->destroy = (void *)obfs2_state_free; - proto_struct->init = (void *)obfs2_state_new; - proto_struct->handshake = (void *)obfs2_send_initial_message; - proto_struct->send = (void *)obfs2_send; - proto_struct->recv = (void *)obfs2_recv; +obfs2_init(void) { + vtable = calloc(1, sizeof(protocol_vtable)); + if (!vtable) + return -1; + + vtable->destroy = obfs2_state_free; + vtable->create = obfs2_new; + vtable->handshake = obfs2_send_initial_message; + vtable->send = obfs2_send; + vtable->recv = obfs2_recv;

if (initialize_crypto() < 0) { fprintf(stderr, "Can't initialize crypto; failing\n"); @@ -52,8 +66,10 @@ seed_nonzero(const uchar *seed) 'state'. Returns NULL on failure. */ static crypt_t * -derive_key(obfs2_state_t *state, const char *keytype) +derive_key(void *s, const char *keytype) { + obfs2_state_t *state = s; + crypt_t *cryptstate; uchar buf[32]; digest_t *c = digest_new(); @@ -74,9 +90,11 @@ derive_key(obfs2_state_t *state, const char *keytype) }

static crypt_t * -derive_padding_key(obfs2_state_t *state, const uchar *seed, +derive_padding_key(void *s, const uchar *seed, const char *keytype) { + obfs2_state_t *state = s; + crypt_t *cryptstate; uchar buf[32]; digest_t *c = digest_new(); @@ -94,13 +112,21 @@ derive_padding_key(obfs2_state_t *state, const uchar *seed, return cryptstate; }

+void * +obfs2_new(struct protocol_t *proto_struct, int initiator) { + assert(vtable); + proto_struct->vtable = vtable; + + return obfs2_state_new(initiator); +} + /** Return a new object to handle protocol state. If 'initiator' is true, we're the handshake initiator. Otherwise, we're the responder. Return NULL on failure. */ -obfs2_state_t * -obfs2_state_new(int *initiator) +static void * +obfs2_state_new(int initiator) { obfs2_state_t *state = calloc(1, sizeof(obfs2_state_t)); uchar *seed; @@ -109,8 +135,8 @@ obfs2_state_new(int *initiator) if (!state) return NULL; state->state = ST_WAIT_FOR_KEY; - state->we_are_initiator = *initiator; - if (*initiator) { + state->we_are_initiator = initiator; + if (initiator) { send_pad_type = INITIATOR_PAD_TYPE; seed = state->initiator_seed; } else { @@ -136,9 +162,11 @@ obfs2_state_new(int *initiator)

/** Set the shared secret to be used with this protocol state. */ void -obfs2_state_set_shared_secret(obfs2_state_t *state, +obfs2_state_set_shared_secret(void *s, const char *secret, size_t secretlen) { + obfs2_state_t *state = s; + if (secretlen > SHARED_SECRET_LENGTH) secretlen = SHARED_SECRET_LENGTH; memcpy(state->secret_seed, secret, secretlen); @@ -148,9 +176,11 @@ obfs2_state_set_shared_secret(obfs2_state_t *state, Write the initial protocol setup and padding message for 'state' to the evbuffer 'buf'. Return 0 on success, -1 on failure. */ -int -obfs2_send_initial_message(obfs2_state_t *state, struct evbuffer *buf) +static int +obfs2_send_initial_message(void *s, struct evbuffer *buf) { + obfs2_state_t *state = s; + uint32_t magic = htonl(OBFUSCATE_MAGIC_VALUE), plength, send_plength; uchar msg[OBFUSCATE_MAX_PADDING + OBFUSCATE_SEED_LENGTH + 8]; const uchar *seed; @@ -213,10 +243,12 @@ crypt_and_transmit(crypt_t *crypto, obfuscated version. Copies and obfuscates data from 'source' into 'dest' using the state in 'state'. Returns 0 on success, -1 on failure. */ -int -obfs2_send(obfs2_state_t *state, +static int +obfs2_send(void *s, struct evbuffer *source, struct evbuffer *dest) { + obfs2_state_t *state = s; + if (state->send_crypto) { /* Our crypto is set up; just relay the bytes */ return crypt_and_transmit(state->send_crypto, source, dest); @@ -237,8 +269,10 @@ obfs2_send(obfs2_state_t *state, keys. Returns 0 on success, -1 on failure. */ static int -init_crypto(obfs2_state_t *state) +init_crypto(void *s) { + obfs2_state_t *state = s; + const char *send_keytype; const char *recv_keytype; const char *recv_pad_keytype; @@ -273,10 +307,12 @@ init_crypto(obfs2_state_t *state) * * Returns x for "don't call again till you have x bytes". 0 for "all ok". -1 * for "fail, close" */ -int -obfs2_recv(obfs2_state_t *state, struct evbuffer *source, +static int +obfs2_recv(void *s, struct evbuffer *source, struct evbuffer *dest) { + obfs2_state_t *state = s; + if (state->state == ST_WAIT_FOR_KEY) { /* We're waiting for the first OBFUSCATE_SEED_LENGTH+8 bytes to show up * so we can learn the partner's seed and padding length */ @@ -350,19 +386,20 @@ obfs2_recv(obfs2_state_t *state, struct evbuffer *source, return crypt_and_transmit(state->recv_crypto, source, dest); }

-void -obfs2_state_free(obfs2_state_t *s) +static void +obfs2_state_free(void *s) { - if (s->send_crypto) - crypt_free(s->send_crypto); - if (s->send_padding_crypto) - crypt_free(s->send_padding_crypto); - if (s->recv_crypto) - crypt_free(s->recv_crypto); - if (s->recv_padding_crypto) - crypt_free(s->recv_padding_crypto); - if (s->pending_data_to_send) - evbuffer_free(s->pending_data_to_send); - memset(s, 0x0a, sizeof(obfs2_state_t)); - free(s); + obfs2_state_t *state = s; + if (state->send_crypto) + crypt_free(state->send_crypto); + if (state->send_padding_crypto) + crypt_free(state->send_padding_crypto); + if (state->recv_crypto) + crypt_free(state->recv_crypto); + if (state->recv_padding_crypto) + crypt_free(state->recv_padding_crypto); + if (state->pending_data_to_send) + evbuffer_free(state->pending_data_to_send); + memset(state, 0x0a, sizeof(obfs2_state_t)); + free(state); } diff --git a/src/plugins/obfs2.h b/src/plugins/obfs2.h index 2d1d24e..3124bbc 100644 --- a/src/plugins/obfs2.h +++ b/src/plugins/obfs2.h @@ -16,17 +16,10 @@ struct protocol_t;

#define SHARED_SECRET_LENGTH 16

-obfs2_state_t *obfs2_state_new(int *initiator); -void obfs2_state_set_shared_secret(obfs2_state_t *state, +void obfs2_state_set_shared_secret(void *state, const char *secret, size_t secretlen); -void obfs2_state_free(obfs2_state_t *state); -int obfs2_send_initial_message(obfs2_state_t *state, struct evbuffer *buf); -int obfs2_send(obfs2_state_t *state, - struct evbuffer *source, struct evbuffer *dest); -int obfs2_recv(obfs2_state_t *state, struct evbuffer *source, - struct evbuffer *dest); - -int obfs2_new(struct protocol_t *proto_struct); +int obfs2_init(void); +void *obfs2_new(struct protocol_t *proto_struct, int initiator);

#ifdef CRYPT_PROTOCOL_PRIVATE diff --git a/src/protocol.c b/src/protocol.c index 339feae..b95ab11 100644 --- a/src/protocol.c +++ b/src/protocol.c @@ -9,30 +9,46 @@ #include "plugins/dummy.h"

/** - This function returns a protocol_t structure based on the mode - of obfsproxy + This function initializes <protocol>. + It's called once in the runtime of the program for each proto. */ -struct protocol_t * +int set_up_protocol(int protocol) { - struct protocol_t *proto = calloc(1, sizeof(struct protocol_t)); - if (protocol == OBFS2_PROTOCOL) - proto->new = &obfs2_new; + obfs2_init(); else if (protocol == DUMMY_PROTOCOL) - proto->new = &dummy_new; - /* elif { other protocols } */ - - if (proto->new(proto)>0) - printf("Protocol constructed\n"); + dummy_init(); + else + return -1;

- return proto; + return 1; }

-void * -proto_init(struct protocol_t *proto, void *arg) { - assert(proto); - if (proto->init) - return proto->init(arg); +/** + This function initializes a protocol. It creates a new + protocol_t structure and fills it's vtable etc. + Return the protocol_t if successful, NULL otherwise. +*/ +struct protocol_t * +proto_new(int protocol, int is_initiator) { + struct protocol_t *proto = calloc(1, sizeof(struct protocol_t)); + if (!proto) + return NULL; + + proto->vtable = calloc(1, sizeof(struct protocol_vtable)); + if (!proto->vtable) + return NULL; + + if (protocol == OBFS2_PROTOCOL) { + proto->proto = protocol; + proto->state = obfs2_new(proto, is_initiator); + } else if (protocol == DUMMY_PROTOCOL) { + proto->proto = protocol; + proto->state = dummy_new(proto, is_initiator); + } + + if (proto->state) + return proto; else return NULL; } @@ -40,8 +56,8 @@ proto_init(struct protocol_t *proto, void *arg) { int proto_handshake(struct protocol_t *proto, void *buf) { assert(proto); - if (proto->handshake) - return proto->handshake(proto->state, buf); + if (proto->vtable->handshake) + return proto->vtable->handshake(proto->state, buf); else /* It's okay with me, protocol didn't have a handshake */ return 0; } @@ -49,8 +65,8 @@ proto_handshake(struct protocol_t *proto, void *buf) { int proto_send(struct protocol_t *proto, void *source, void *dest) { assert(proto); - if (proto->send) - return proto->send(proto->state, source, dest); + if (proto->vtable->send) + return proto->vtable->send(proto->state, source, dest); else return -1; } @@ -58,8 +74,8 @@ proto_send(struct protocol_t *proto, void *source, void *dest) { int proto_recv(struct protocol_t *proto, void *source, void *dest) { assert(proto); - if (proto->recv) - return proto->recv(proto->state, source, dest); + if (proto->vtable->recv) + return proto->vtable->recv(proto->state, source, dest); else return -1; } @@ -68,6 +84,6 @@ void proto_destroy(struct protocol_t *proto) { assert(proto); assert(proto->state);

- if (proto->destroy) - proto->destroy(proto->state); + if (proto->vtable->destroy) + proto->vtable->destroy(proto->state); } diff --git a/src/protocol.h b/src/protocol.h index 781bde0..7ec430c 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -1,40 +1,50 @@ #ifndef PROTOCOL_H #define PROTOCOL_H

-/* ASN I'm gonna be calling crypt_protocol.c BRL_RPOTOCOL for now. Yes. */ -#define DUMMY_PROTOCOL 0 +#define DUMMY_PROTOCOL 0 #define OBFS2_PROTOCOL 1

+struct evbuffer;

-struct protocol_t *set_up_protocol(int protocol); -void *proto_init(struct protocol_t *proto, void *arg); +int set_up_protocol(int protocol); +struct protocol_t *proto_new(int protocol, int arg); void proto_destroy(struct protocol_t *proto); int proto_handshake(struct protocol_t *proto, void *buf); int proto_send(struct protocol_t *proto, void *source, void *dest); int proto_recv(struct protocol_t *proto, void *source, void *dest);

- -/* ASN Why the hell do half of them return int? FIXME */ -struct protocol_t { +typedef struct protocol_vtable { /* Constructor: creates the protocol; sets up functions etc. */ - int (*new)(struct protocol_t *self); + int (*init)(struct protocol_t *self); /* Destructor */ void (*destroy)(void *state);

/* does nessesary initiation steps; like build a proto state etc. */ - void *(*init)(void *arg); + void *(*create)(struct protocol_t *proto_struct, + int is_initiator);

/* does handshake. Supposedly all protocols have a handshake. */ - int (*handshake)(void *state, void *buf); + int (*handshake)(void *state, + struct evbuffer *buf);

/* send data function */ - int (*send)(void *state, void *source, - void *dest); + int (*send)(void *state, + struct evbuffer *source, + struct evbuffer *dest);

/* receive data function */ - int (*recv)(void *state, void *source, - void *dest); + int (*recv)(void *state, + struct evbuffer *source, + struct evbuffer *dest); +} protocol_vtable; + +struct protocol_t { + /* protocol */ + int proto; + + /* protocol vtable */ + protocol_vtable *vtable;

/* ASN do we need a proto_get_state()? */ void *state; diff --git a/src/socks.c b/src/socks.c index a3fb729..a1f794a 100644 --- a/src/socks.c +++ b/src/socks.c @@ -192,8 +192,6 @@ socks5_send_reply(struct evbuffer *reply_dest, socks_state_t *state, /* We either failed or succeded. Either way, we should send something back to the client */ p[0] = SOCKS5_VERSION; /* Version field */ - if (status == SOCKS5_REP_FAIL) - printf("Sending negative shit\n"); p[1] = (unsigned char) status; /* Reply field */ p[2] = 0; /* Reserved */ if (state->parsereq.af == AF_UNSPEC) {