[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [obfsproxy/master] Improved shared secret support based on the comments of my Mentor in #3202.



commit 536e014a102d283f668dbfd75bdd56de2728bc87
Author: George Kadianakis <desnacked@xxxxxxxxx>
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



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits