commit 536e014a102d283f668dbfd75bdd56de2728bc87 Author: George Kadianakis desnacked@gmail.com Date: Tue May 24 16:06:33 2011 +0200

Improved shared secret support based on the comments of my Mentor in #3202.

Specifically: * We stop stupidly treating the shared secret as a C-string. We now instead hash it. We don't iteratively hash it yet. * Because of hashing the shared secret, we now allow shared secrets of arbitrary length. * We now pass protocol_params_t structs to the _new protocol methods. It looks/is much nicer. * I better defined the role of the protocol_params_t struct and is now attached to the listener_t struct.

This is not done yet. --- src/main.c | 2 +- src/network.c | 32 +++++++++++++++++++++----------- src/network.h | 2 +- src/protocol.c | 4 ++-- src/protocol.h | 33 +++++++++++++++++++++++++-------- src/protocols/dummy.c | 8 +++----- src/protocols/dummy.h | 3 ++- src/protocols/obfs2.c | 46 ++++++++++++++++++++++++++-------------------- src/protocols/obfs2.h | 7 +++++-- 9 files changed, 86 insertions(+), 51 deletions(-)

diff --git a/src/main.c b/src/main.c index 1c084d3..59b6909 100644 --- a/src/main.c +++ b/src/main.c @@ -128,7 +128,7 @@ main(int argc, const char **argv) mode, protocol, (struct sockaddr *)&ss_listen, sl_listen, sa_target, sl_target, - shared_secret); + shared_secret, strlen(shared_secret)); if (! listener) { printf("Couldn't create listener!\n"); return 4; diff --git a/src/network.c b/src/network.c index a32cb1d..07efbc4 100644 --- a/src/network.c +++ b/src/network.c @@ -27,10 +27,9 @@ struct listener_t { struct evconnlistener *listener; struct sockaddr_storage target_address; int target_address_len; - int proto; /* Protocol that this listener can speak. */ int mode; - const char *shared_secret; - unsigned int have_shared_secret : 1; + int proto; /* Protocol that this listener can speak. */ + struct protocol_params_t *proto_params; };

static void simple_listener_cb(struct evconnlistener *evcl, @@ -52,7 +51,7 @@ listener_new(struct event_base *base, int mode, int protocol, const struct sockaddr *on_address, int on_address_len, const struct sockaddr *target_address, int target_address_len, - const char *shared_secret) + const char *shared_secret, size_t shared_secret_len) { const unsigned flags = LEV_OPT_CLOSE_ON_FREE|LEV_OPT_CLOSE_ON_EXEC|LEV_OPT_REUSEABLE; @@ -68,8 +67,13 @@ listener_new(struct event_base *base,

lsn->proto = protocol; lsn->mode = mode; - lsn->shared_secret = shared_secret; - lsn->have_shared_secret = 1; + + struct protocol_params_t *proto_params = calloc(1, sizeof(struct protocol_params_t)); + proto_params->is_initiator = mode != LSN_SIMPLE_SERVER; + proto_params->shared_secret = shared_secret; + proto_params->shared_secret_len = shared_secret_len; + + lsn->proto_params = proto_params;

if (target_address) { assert(target_address_len <= sizeof(struct sockaddr_storage)); @@ -92,11 +96,21 @@ listener_new(struct event_base *base, return lsn; }

+static void +protocol_params_free(struct protocol_params_t *params) +{ + assert(params); + if (params->shared_secret) + free(&params->shared_secret); +} + void listener_free(listener_t *lsn) { if (lsn->listener) evconnlistener_free(lsn->listener); + if (lsn->proto_params) + protocol_params_free(lsn->proto_params); memset(lsn, 0xb0, sizeof(listener_t)); free(lsn); } @@ -115,12 +129,8 @@ simple_listener_cb(struct evconnlistener *evcl,

conn->mode = lsn->mode;

- struct protocol_params_t *proto_params = calloc(1, sizeof(struct protocol_params_t)); - proto_params->is_initiator = conn->mode != LSN_SIMPLE_SERVER; - proto_params->shared_secret = lsn->shared_secret; - conn->proto = proto_new(lsn->proto, - proto_params); + lsn->proto_params); if (!conn->proto) { printf("Creation of protocol object failed! Closing connection.\n"); goto err; diff --git a/src/network.h b/src/network.h index 1e13cb4..619e45f 100644 --- a/src/network.h +++ b/src/network.h @@ -28,7 +28,7 @@ listener_t *listener_new( int mode, int protocol, const struct sockaddr *on_address, int on_address_len, const struct sockaddr *target_address, int target_address_len, - const char *shared_secret); + const char *shared_secret, size_t shared_secret_len); void listener_free(listener_t *listener);

#ifdef NETWORK_PRIVATE diff --git a/src/protocol.c b/src/protocol.c index b371923..4e6ae81 100644 --- a/src/protocol.c +++ b/src/protocol.c @@ -42,10 +42,10 @@ proto_new(int protocol, struct protocol_params_t *params) {

if (protocol == OBFS2_PROTOCOL) { proto->proto = protocol; - proto->state = obfs2_new(proto, params->is_initiator, params->shared_secret); + proto->state = obfs2_new(proto, params); } else if (protocol == DUMMY_PROTOCOL) { proto->proto = protocol; - proto->state = dummy_new(proto, 666, NULL); + proto->state = dummy_new(proto, NULL); }

if (proto->state) diff --git a/src/protocol.h b/src/protocol.h index 7d0fb44..6b609ee 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -6,14 +6,37 @@

struct evbuffer;

+/** + This struct defines parameters of the protocol per-listener basis. + + By 'per-listener basis' I mean that the parameters defined here will + be inherited by *all* connections opened from the listener_t that + owns this protocol_params_t. +*/ +struct protocol_params_t { +int is_initiator; + + const char *shared_secret; + size_t shared_secret_len; +}; + struct protocol_t { /* protocol */ int proto;

/* protocol vtable */ struct protocol_vtable *vtable; + + /* This protocol specific struct defines the state of the protocol + per-connection basis.

- /* ASN do we need a proto_get_state()? */ + By 'protocol specific' I mean that every protocol has it's own + state struct. (for example, obfs2 has obfs2_state_t) + + By 'per-connection basis' I mean that the every connection has a + different protocol_t struct, and that's precisely the reason that + this struct is owned by the conn_t struct. + */ void *state; };

@@ -26,7 +49,7 @@ typedef struct protocol_vtable {

/* Constructor: Creates a protocol object. */ void *(*create)(struct protocol_t *proto_struct, - int is_initiator, const char *parameters); + struct protocol_params_t *parameters);

/* does handshake. Not all protocols have a handshake. */ int (*handshake)(void *state, @@ -44,12 +67,6 @@ typedef struct protocol_vtable {

} protocol_vtable;

-struct protocol_params_t { - int is_initiator; - - const char *shared_secret; -}; - int set_up_protocol(int protocol); struct protocol_t *proto_new(int protocol, struct protocol_params_t *params); diff --git a/src/protocols/dummy.c b/src/protocols/dummy.c index f5a7558..79d40e0 100644 --- a/src/protocols/dummy.c +++ b/src/protocols/dummy.c @@ -36,11 +36,9 @@ dummy_init(void) { }

void * -dummy_new(struct protocol_t *proto_struct, int whatever, - const char *whatever2) { - (void)whatever; - (void)whatever2; - +dummy_new(struct protocol_t *proto_struct, + struct protocol_params_t *params) +{ proto_struct->vtable = vtable;

/* Dodging state check. diff --git a/src/protocols/dummy.h b/src/protocols/dummy.h index 5ab39a4..1ee0e02 100644 --- a/src/protocols/dummy.h +++ b/src/protocols/dummy.h @@ -3,8 +3,9 @@

struct protocol_t; struct evbuffer; +struct protocol_params_t;

int dummy_init(void); -void *dummy_new(struct protocol_t *proto_struct, int whatever, const char* whatever2); +void *dummy_new(struct protocol_t *proto_struct, struct protocol_params_t *params);

#endif diff --git a/src/protocols/obfs2.c b/src/protocols/obfs2.c index fe50553..277ca63 100644 --- a/src/protocols/obfs2.c +++ b/src/protocols/obfs2.c @@ -26,9 +26,10 @@ 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, const char *shared_secret); +static void *obfs2_state_new(struct protocol_params_t *params); static int obfs2_state_set_shared_secret(void *s, - const char *secret); + const char *secret, + size_t secretlen);

static protocol_vtable *vtable=NULL;

@@ -115,11 +116,13 @@ derive_padding_key(void *s, const uchar *seed, }

void * -obfs2_new(struct protocol_t *proto_struct, int initiator, const char *shared_secret) { +obfs2_new(struct protocol_t *proto_struct, + struct protocol_params_t *params) +{ assert(vtable); proto_struct->vtable = vtable;

- return obfs2_state_new(initiator, shared_secret); + return obfs2_state_new(params); }

/** @@ -128,7 +131,7 @@ obfs2_new(struct protocol_t *proto_struct, int initiator, const char *shared_sec NULL on failure. */ static void * -obfs2_state_new(int initiator, const char *shared_secret) +obfs2_state_new(struct protocol_params_t *params) { obfs2_state_t *state = calloc(1, sizeof(obfs2_state_t)); uchar *seed; @@ -137,8 +140,8 @@ obfs2_state_new(int initiator, const char *shared_secret) if (!state) return NULL; state->state = ST_WAIT_FOR_KEY; - state->we_are_initiator = initiator; - if (initiator) { + state->we_are_initiator = params->is_initiator; + if (state->we_are_initiator) { send_pad_type = INITIATOR_PAD_TYPE; seed = state->initiator_seed; } else { @@ -152,8 +155,10 @@ obfs2_state_new(int initiator, const char *shared_secret) return NULL; }

- if (shared_secret) - if (obfs2_state_set_shared_secret(state, shared_secret)<0) + if (params->shared_secret) + if (obfs2_state_set_shared_secret(state, + params->shared_secret, + params->shared_secret_len)<0) return NULL;

/* Derive the key for what we're sending */ @@ -168,22 +173,23 @@ obfs2_state_new(int initiator, const char *shared_secret)

/** Set the shared secret to be used with this protocol state. */ static int -obfs2_state_set_shared_secret(void *s, - const char *secret) +obfs2_state_set_shared_secret(void *s, const char *secret, + size_t secretlen) { assert(secret); - obfs2_state_t *state = s; - size_t secretlen = strlen(secret); + assert(secretlen);

- if (secretlen < SHARED_SECRET_LENGTH) { - printf("Shared secrets must be at least 16 bytes.\n"); /* Why? */ - return -1; - } else if (secretlen > SHARED_SECRET_LENGTH) - secretlen = SHARED_SECRET_LENGTH; + uchar buf[SHARED_SECRET_LENGTH]; + obfs2_state_t *state = s;

- memcpy(state->secret_seed, secret, secretlen); + digest_t *c = digest_new(); + /* ASN do we like this cast here? */ + digest_update(c, (uchar*)secret, secretlen); + digest_getdigest(c, buf, sizeof(buf)); + memcpy(state->secret_seed, buf, SHARED_SECRET_LENGTH);

- /* free(secret); */ + memset(buf,0,SHARED_SECRET_LENGTH); + digest_free(c);

return 0; } diff --git a/src/protocols/obfs2.h b/src/protocols/obfs2.h index bcc423f..bc8a22d 100644 --- a/src/protocols/obfs2.h +++ b/src/protocols/obfs2.h @@ -13,11 +13,14 @@ typedef struct obfs2_state_t obfs2_state_t; struct evbuffer; struct protocol_t; +struct protocol_params_t;

-#define SHARED_SECRET_LENGTH 16 +#define SHA256_LENGTH 32 +#define SHARED_SECRET_LENGTH SHA256_LENGTH

int obfs2_init(void); -void *obfs2_new(struct protocol_t *proto_struct, int initiator, const char *parameters); +void *obfs2_new(struct protocol_t *proto_struct, + struct protocol_params_t *params);

#ifdef CRYPT_PROTOCOL_PRIVATE