[tor-commits] [obfsproxy/master] Improved shared secret support based on the comments of my Mentor in #3202.
nickm at torproject.org
nickm at torproject.org
Sun May 29 01:33:38 UTC 2011
commit 536e014a102d283f668dbfd75bdd56de2728bc87
Author: George Kadianakis <desnacked at 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(¶ms->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
More information about the tor-commits
mailing list