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

[tor-commits] [obfsproxy/master] Protocol API sanitization



commit 4896bd7e4609f74780dbe047ac568283848d06f2
Author: Zack Weinberg <zackw@xxxxxxxxx>
Date:   Tue Jul 12 17:04:21 2011 -0700

    Protocol API sanitization
    
    * Embed the protocol name in the protocol vtable
    * Protocol vtables are now statically allocated
    * Return objects rather than error codes when that makes sense
    * Use pseudo-inheritance rather than void pointers for protocol-
      specific, per-connection state
    * Tidy up the protocol.h upward interface a little
    * Constification
---
 src/main.c                |   47 +++----
 src/network.c             |    2 +-
 src/protocol.c            |  109 +++++++-------
 src/protocol.h            |  120 +++++++++------
 src/protocols/dummy.c     |  104 +++++++-------
 src/protocols/dummy.h     |   12 +-
 src/protocols/obfs2.c     |  212 ++++++++++-----------------
 src/protocols/obfs2.h     |   29 ++--
 src/test/unittest_obfs2.c |  363 +++++++++++++++++++--------------------------
 9 files changed, 448 insertions(+), 550 deletions(-)

diff --git a/src/main.c b/src/main.c
index 48d0d37..f41d502 100644
--- a/src/main.c
+++ b/src/main.c
@@ -25,10 +25,6 @@
 static void usage(void) __attribute__((noreturn));
 static int handle_obfsproxy_args(const char **argv);
 
-/* protocol.c */
-extern char *supported_protocols[];
-extern int n_supported_protocols;
-
 static struct event_base *the_event_base=NULL;
 
 /**
@@ -44,7 +40,7 @@ usage(void)
           SEPARATOR);
   /* this is awful. */
   for (i=0;i<n_supported_protocols;i++)
-    fprintf(stderr,"[%s] ", supported_protocols[i]);
+    fprintf(stderr,"[%s] ", supported_protocols[i]->name);
   fprintf(stderr, "\n* Available arguments:\n"
           "--log-file=<file> ~ set logfile\n"
           "--log-min-severity=warn|info|debug ~ set minimum logging severity\n"
@@ -93,7 +89,7 @@ handle_signal_cb(evutil_socket_t fd, short what, void *arg)
    Stops obfsproxy's event loop.
 
    Final cleanup happens in main().
-*/ 
+*/
 void
 finish_shutdown(void)
 {
@@ -105,8 +101,8 @@ finish_shutdown(void)
    and writes them in 'options_string'.
 */
 static void
-populate_options(char **options_string, 
-                 const char **argv, int n_options) 
+populate_options(char **options_string,
+                 const char **argv, int n_options)
 {
   int g;
   for (g=0;g<=n_options-1;g++)
@@ -114,14 +110,14 @@ populate_options(char **options_string,
 }
 
 /**
-   Return 0 if 'name' is the nmae of a supported protocol, otherwise
+   Return 0 if 'name' is the name of a supported protocol, otherwise
    return -1.
 */
 static int
 is_supported_protocol(const char *name) {
   int f;
   for (f=0;f<n_supported_protocols;f++) {
-    if (!strcmp(name,supported_protocols[f])) 
+    if (!strcmp(name,supported_protocols[f]->name))
       return 0;
   }
   return -1;
@@ -130,7 +126,7 @@ is_supported_protocol(const char *name) {
 /**
    Receives argv[1] as 'argv' and scans from thereafter for any
    obfsproxy optional arguments and tries to set them in effect.
-   
+
    If it succeeds it returns the number of argv arguments its caller
    should skip to get past the optional arguments we already handled.
    If it fails, it exits obfsproxy.
@@ -142,14 +138,14 @@ handle_obfsproxy_args(const char **argv)
   int logsev_set=0;
   int i=0;
 
-  while (argv[i] && 
+  while (argv[i] &&
          !strncmp(argv[i],"--",2)) {
     if (!strncmp(argv[i], "--log-file=", 11)) {
       if (logmethod_set) {
-        log_warn("You've already set a log file!"); 
+        log_warn("You've already set a log file!");
         exit(1);
       }
-      if (log_set_method(LOG_METHOD_FILE, 
+      if (log_set_method(LOG_METHOD_FILE,
                          (char *)argv[i]+11) < 0) {
         log_warn("Failed creating logfile.");
         exit(1);
@@ -161,7 +157,7 @@ handle_obfsproxy_args(const char **argv)
         exit(1);
       }
       if (log_set_min_severity((char *)argv[i]+19) < 0) {
-        log_warn("Error at setting logging severity"); 
+        log_warn("Error at setting logging severity");
         exit(1);
       }
       logsev_set=1;
@@ -171,7 +167,7 @@ handle_obfsproxy_args(const char **argv)
           exit(1);
         }
         if (log_set_method(LOG_METHOD_NULL, NULL) < 0) {
-          printf("Error at setting logging severity.\n"); 
+          printf("Error at setting logging severity.\n");
           exit(1);
         }
         logsev_set=1;
@@ -212,7 +208,7 @@ main(int argc, const char **argv)
      managed to recognize, by their protocol name.  Of course it's not
      the *actual* actual_protocols since some of them could have wrong
      options or arguments, but this will be resolved per-protocol by
-     set_up_protocol(). */
+     proto_params_init(). */
   int actual_protocols=0;
 
   int start;
@@ -300,7 +296,7 @@ main(int argc, const char **argv)
 
     /* First option should be protocol_name. See if we support it. */
     if (is_supported_protocol(argv[start])<0) {
-      log_warn("We don't support protocol: %s", argv[start]); 
+      log_warn("We don't support protocol: %s", argv[start]);
       continue;
     }
 
@@ -308,14 +304,14 @@ main(int argc, const char **argv)
 
     /* Allocate space for the array carrying the options of this
        protocol. */
-    protocol_options[actual_protocols-1] = 
+    protocol_options[actual_protocols-1] =
       calloc(sizeof(char*), (n_options));
     if (!protocol_options[actual_protocols-1])
       die_oom();
 
     /* Write the number of options to the correct place in n_options_array[]. */
     n_options_array[actual_protocols-1] = n_options;
-  
+
     /* Finally! Let's fill protocol_options. */
     populate_options(protocol_options[actual_protocols-1],
                      &argv[start], n_options);
@@ -343,7 +339,7 @@ main(int argc, const char **argv)
     log_warn("Can't initialize evdns; failing");
     return 1;
   }
-  
+
   /* Handle signals */
 #ifdef SIGPIPE
    signal(SIGPIPE, SIG_IGN);
@@ -357,7 +353,7 @@ main(int argc, const char **argv)
     return 1;
   }
 
-  /*Let's open a new listener for each protocol. */ 
+  /*Let's open a new listener for each protocol. */
   int h;
   listener_t *temp_listener;
   int n_listeners=0;
@@ -366,10 +362,9 @@ main(int argc, const char **argv)
     log_debug("Spawning listener %d!", h+1);
 
     /** normally free'd in listener_free() */
-    proto_params = calloc(1, sizeof(protocol_params_t));
-    if (set_up_protocol(n_options_array[h],protocol_options[h],
-                        proto_params)<0) {
-      free(proto_params);
+    proto_params = proto_params_init(n_options_array[h],
+                                     (const char *const *)protocol_options[h]);
+    if (!proto_params) {
       free(protocol_options[h]);
       continue;
     }
diff --git a/src/network.c b/src/network.c
index 6a8afa6..365c61a 100644
--- a/src/network.c
+++ b/src/network.c
@@ -202,7 +202,7 @@ simple_listener_cb(struct evconnlistener *evcl,
 
   conn->mode = lsn->proto_params->mode;
 
-  conn->proto = proto_new(lsn->proto_params);
+  conn->proto = proto_create(lsn->proto_params);
   if (!conn->proto) {
     log_warn("Creation of protocol object failed! Closing connection.");
     goto err;
diff --git a/src/protocol.c b/src/protocol.c
index 282db10..85b975f 100644
--- a/src/protocol.c
+++ b/src/protocol.c
@@ -15,8 +15,13 @@
     All supported protocols should be put in this array.
     It's used by main.c.
 */
-char *supported_protocols[] = { "obfs2", "dummy" };
-int n_supported_protocols = 2;
+const protocol_vtable *const supported_protocols[] =
+{
+  &dummy_vtable,
+  &obfs2_vtable,
+};
+const size_t n_supported_protocols =
+  sizeof(supported_protocols)/sizeof(supported_protocols[0]);
 
 /**
    This function figures out which protocol we want to set up, and
@@ -25,16 +30,33 @@ int n_supported_protocols = 2;
    This function is called once for every listener through the runtime
    of obfsproxy.
 */
-int
-set_up_protocol(int n_options, char **options,
-                struct protocol_params_t *params)
+struct protocol_params_t *
+proto_params_init(int n_options, const char *const *options)
 {
-  if (!strcmp(*options,"dummy"))
-    return dummy_init(n_options, options, params);
-  else if (!strcmp(*options,"obfs2"))
-    return obfs2_init(n_options, options, params);
-  else
-    return -1;
+  size_t i;
+  for (i = 0; i < n_supported_protocols; i++)
+    if (!strcmp(*options, supported_protocols[i]->name))
+      return supported_protocols[i]->init(n_options, options);
+
+  return NULL;
+}
+
+/**
+   This function destroys 'params'.
+   It's called everytime we free a listener.
+*/
+void
+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);
 }
 
 /**
@@ -44,17 +66,12 @@ set_up_protocol(int n_options, char **options,
    Return a 'protocol_t' if successful, NULL otherwise.
 */
 struct protocol_t *
-proto_new(protocol_params_t *params) {
-  struct protocol_t *proto = calloc(1, sizeof(struct protocol_t));
-  if (!proto)
-    return NULL;
-
-  if (params->proto == OBFS2_PROTOCOL)
-    proto->state = obfs2_new(proto, params);
-  else if (params->proto == DUMMY_PROTOCOL)
-    proto->state = dummy_new(proto, NULL);
-
-  return proto->state ? proto : NULL;
+proto_create(protocol_params_t *params)
+{
+  assert(params);
+  assert(params->vtable);
+  assert(params->vtable->create);
+  return params->vtable->create(params);
 }
 
 /**
@@ -64,10 +81,9 @@ proto_new(protocol_params_t *params) {
 int
 proto_handshake(struct protocol_t *proto, void *buf) {
   assert(proto);
-  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;
+  assert(proto->vtable);
+  assert(proto->vtable->handshake);
+  return proto->vtable->handshake(proto, buf);
 }
 
 /**
@@ -76,10 +92,9 @@ proto_handshake(struct protocol_t *proto, void *buf) {
 int
 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
-    return -1;
+  assert(proto->vtable);
+  assert(proto->vtable->send);
+  return proto->vtable->send(proto, source, dest);
 }
 
 /**
@@ -88,10 +103,9 @@ proto_send(struct protocol_t *proto, void *source, void *dest) {
 enum recv_ret
 proto_recv(struct protocol_t *proto, void *source, void *dest) {
   assert(proto);
-  if (proto->vtable->recv)
-    return proto->vtable->recv(proto->state, source, dest);
-  else
-    return -1;
+  assert(proto->vtable);
+  assert(proto->vtable->recv);
+  return proto->vtable->recv(proto, source, dest);
 }
 
 /**
@@ -101,28 +115,7 @@ proto_recv(struct protocol_t *proto, void *source, void *dest) {
 void
 proto_destroy(struct protocol_t *proto) {
   assert(proto);
-  assert(proto->state);
-
-  if (proto->vtable->destroy)
-    proto->vtable->destroy(proto->state);
-
-  free(proto);
-}
-
-/**
-   This function destroys 'params'.
-   It's called everytime we free a listener.
-*/
-void
-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);
+  assert(proto->vtable);
+  assert(proto->vtable->destroy);
+  proto->vtable->destroy(proto);
 }
diff --git a/src/protocol.h b/src/protocol.h
index 6d5dcfe..476d447 100644
--- a/src/protocol.h
+++ b/src/protocol.h
@@ -11,17 +11,15 @@
 struct evbuffer;
 struct sockaddr;
 
-#define DUMMY_PROTOCOL      1
-#define OBFS2_PROTOCOL      2
-
 /**
-  This struct defines parameters of the protocol per-listener basis.
+  This struct defines parameters of a protocol on a 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.
 */
 typedef struct protocol_params_t {
+  const struct protocol_vtable *vtable;
   struct sockaddr *target_address;
   struct sockaddr *listen_address;
   char *shared_secret;
@@ -30,61 +28,91 @@ typedef struct protocol_params_t {
   size_t listen_address_len;
   int is_initiator;
   int mode;
-  int proto; /* Protocol that this listener can speak. */
 } protocol_params_t;
 
+/**
+   This protocol specific struct defines the state of the protocol
+   on a per-connection basis.
+
+   By 'protocol specific' I mean that every protocol has its own
+   state struct. (for example, obfs2 has obfs2_state_t).  A protocol_t
+   struct is always the first member of this struct, and vtable->create
+   returns that member (standard fake-inheritance-in-C technique).
+   All data other than the vtable is hidden from everything but the
+   protocol implementation.
+
+   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.
+ */
 struct protocol_t {
-  /* protocol vtable */
-  struct protocol_vtable *vtable;
-
-  /* This protocol specific struct defines the state of the protocol
-     per-connection basis.
-
-     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;
+  const struct protocol_vtable *vtable;
 };
-int set_up_protocol(int n_options, char **options,
-                    struct protocol_params_t *params);
-struct protocol_t *proto_new(struct protocol_params_t *params);
-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);
-enum recv_ret proto_recv(struct protocol_t *proto, void *source, void *dest);
-
-void proto_params_free(protocol_params_t *params);
-
-typedef struct protocol_vtable {
-  /* Initialization function: Fills in the protocol vtable. */
-  int (*init)(int n_options, char **options,
-              struct protocol_params_t *params);
-
-  /* Destructor: Destroys the protocol state.  */
-  void (*destroy)(void *state);
 
-  /* Constructor: Creates a protocol object. */
-  void *(*create)(struct protocol_t *proto_params,
-                  struct protocol_params_t *parameters);
-
-  /* does handshake. Not all protocols have a handshake. */
-  int (*handshake)(void *state,
+/**
+   This struct defines a protocol and its methods; note that not all
+   of them are methods on the same object in the C++ sense.
+
+   A filled-in, statically allocated protocol_vtable object is the
+   principal interface between each individual protocol and generic
+   code.  At present there is a static list of these objects in protocol.c.
+ */
+typedef struct protocol_vtable
+{
+  /** The short name of this protocol. */
+  const char *name;
+
+  /** Initialization function: Allocate a 'protocol_params_t' object
+      and fill it in from the provided 'options' array. */
+  struct protocol_params_t *(*init)(int n_options,
+                                    const char *const *options);
+
+  /** Constructor: Allocates per-connection, protocol-specific state. */
+  struct protocol_t *(*create)(struct protocol_params_t *params);
+
+  /** Destructor: Destroys per-connection, protocol-specific state.  */
+  void (*destroy)(struct protocol_t *state);
+
+  /** Perform a connection handshake. Not all protocols have a handshake. */
+  int (*handshake)(struct protocol_t *state,
                    struct evbuffer *buf);
 
-  /* send data function */
-  int (*send)(void *state,
+  /** Send data coming downstream from 'source' along to 'dest'. */
+  int (*send)(struct protocol_t *state,
               struct evbuffer *source,
               struct evbuffer *dest);
 
-  /* receive data function */
-  enum recv_ret (*recv)(void *state,
+  /** Receive data from 'source' and pass it upstream to 'dest'. */
+  enum recv_ret (*recv)(struct protocol_t *state,
                         struct evbuffer *source,
                         struct evbuffer *dest);
 
 } protocol_vtable;
 
+/**
+   Use this macro to define protocol_vtable objects; it ensures all
+   the methods are in the correct order and enforces a consistent
+   naming convention on protocol implementations.
+ */
+#define DEFINE_PROTOCOL_VTABLE(name)                    \
+  const struct protocol_vtable name##_vtable = {        \
+    #name,                                              \
+    name##_init, name##_create, name##_destroy,         \
+    name##_handshake, name##_send, name##_recv          \
+  }
+
+struct protocol_params_t *proto_params_init(int n_options,
+                                            const char *const *options);
+void proto_params_free(protocol_params_t *params);
+
+struct protocol_t *proto_create(struct protocol_params_t *params);
+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);
+enum recv_ret proto_recv(struct protocol_t *proto, void *source, void *dest);
+
+extern const protocol_vtable *const supported_protocols[];
+extern const size_t n_supported_protocols;
+
 #endif
diff --git a/src/protocols/dummy.c b/src/protocols/dummy.c
index 3b31e12..7db48d7 100644
--- a/src/protocols/dummy.c
+++ b/src/protocols/dummy.c
@@ -7,22 +7,16 @@
 #include "../util.h"
 
 #include <assert.h>
-#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
 #include <event2/buffer.h>
 
-static int dummy_send(void *nothing,
-                      struct evbuffer *source, struct evbuffer *dest);
-static enum recv_ret dummy_recv(void *nothing, struct evbuffer *source,
-                                struct evbuffer *dest);
 static void usage(void);
-static int parse_and_set_options(int n_options, char **options,
+static int parse_and_set_options(int n_options,
+                                 const char *const *options,
                                  struct protocol_params_t *params);
 
-static protocol_vtable *vtable=NULL;
-
 /**
    This function populates 'params' according to 'options' and sets up
    the protocol vtable.
@@ -30,34 +24,28 @@ static protocol_vtable *vtable=NULL;
    'options' is an array like this:
    {"dummy","socks","127.0.0.1:6666"}
 */
-int
-dummy_init(int n_options, char **options,
-           struct protocol_params_t *params)
+static struct protocol_params_t *
+dummy_init(int n_options, const char *const *options)
 {
-  if (parse_and_set_options(n_options,options,params) < 0) {
+  struct protocol_params_t *params
+    = calloc(1, sizeof(struct protocol_params_t));
+  if (!params)
+    return NULL;
+
+  if (parse_and_set_options(n_options, options, params) < 0) {
+    free(params);
     usage();
-    return -1;
+    return NULL;
   }
 
-  /* XXX memleak. */
-  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 0;
+  return params;
 }
 
 /**
    Helper: Parses 'options' and fills 'params'.
-*/ 
+*/
 static int
-parse_and_set_options(int n_options, char **options,
+parse_and_set_options(int n_options, const char *const *options,
                       struct protocol_params_t *params)
 {
   const char* defport;
@@ -66,7 +54,6 @@ parse_and_set_options(int n_options, char **options,
     return -1;
 
   assert(!strcmp(options[0],"dummy"));
-  params->proto = DUMMY_PROTOCOL;
 
   if (!strcmp(options[1], "client")) {
     defport = "48988"; /* bf5c */
@@ -87,7 +74,8 @@ parse_and_set_options(int n_options, char **options,
     return -1;
   }
 
-  return 1;
+  params->vtable = &dummy_vtable;
+  return 0;
 }
 
 /**
@@ -96,31 +84,34 @@ parse_and_set_options(int n_options, char **options,
 static void
 usage(void)
 {
-  printf("Great... You can't even form a dummy protocol line:\n"
-         "dummy syntax:\n"
-         "\tdummy dummy_opts\n"
-         "\t'dummy_opts':\n"
-         "\t\tmode ~ server|client|socks\n"
-         "\t\tlisten address ~ host:port\n"
-         "Example:\n"
-         "\tobfsproxy dummy socks 127.0.0.1:5000\n");
+  log_warn("Great... You can't even form a dummy protocol line:\n"
+           "dummy syntax:\n"
+           "\tdummy dummy_opts\n"
+           "\t'dummy_opts':\n"
+           "\t\tmode ~ server|client|socks\n"
+           "\t\tlisten address ~ host:port\n"
+           "Example:\n"
+           "\tobfsproxy dummy socks 127.0.0.1:5000");
 }
 
 /*
   This is called everytime we get a connection for the dummy
   protocol.
-
-  It sets up the protocol vtable in 'proto_struct'.
 */
-void *
-dummy_new(struct protocol_t *proto_struct,
-          struct protocol_params_t *params)
+
+static struct protocol_t *
+dummy_create(struct protocol_params_t *params)
 {
-  proto_struct->vtable = vtable;
+  /* Dummy needs no per-connection protocol-specific state. */
+  struct protocol_t *proto = calloc(1, sizeof(struct protocol_t));
+  proto->vtable = &dummy_vtable;
+  return proto;
+}
 
-  /* Dodging state check.
-     This is terrible I know.*/
-  return (void *)666U;
+static void
+dummy_destroy(struct protocol_t *proto)
+{
+  free(proto);
 }
 
 /**
@@ -129,10 +120,16 @@ dummy_new(struct protocol_t *proto_struct,
    The dummy protocol just puts the data of 'source' in 'dest'.
 */
 static int
-dummy_send(void *nothing,
-           struct evbuffer *source, struct evbuffer *dest) {
-  (void)nothing;
+dummy_handshake(struct protocol_t *proto __attribute__((unused)),
+                struct evbuffer *buf __attribute__((unused)))
+{
+  return 0;
+}
 
+static int
+dummy_send(struct protocol_t *proto __attribute__((unused)),
+           struct evbuffer *source, struct evbuffer *dest)
+{
   return evbuffer_add_buffer(dest,source);
 }
 
@@ -142,12 +139,13 @@ dummy_send(void *nothing,
   The dummy protocol just puts the data of 'source' into 'dest'.
 */
 static enum recv_ret
-dummy_recv(void *nothing,
-           struct evbuffer *source, struct evbuffer *dest) {
-  (void)nothing;
-
+dummy_recv(struct protocol_t *proto __attribute__((unused)),
+           struct evbuffer *source, struct evbuffer *dest)
+{
   if (evbuffer_add_buffer(dest,source)<0)
     return RECV_BAD;
   else
     return RECV_GOOD;
 }
+
+DEFINE_PROTOCOL_VTABLE(dummy);
diff --git a/src/protocols/dummy.h b/src/protocols/dummy.h
index a3fa32d..e7dbd2a 100644
--- a/src/protocols/dummy.h
+++ b/src/protocols/dummy.h
@@ -1,14 +1,10 @@
 /* Copyright 2011 Nick Mathewson, George Kadianakis
    See LICENSE for other credits and copying information
 */
-#ifndef DUMMY_H
-#define DUMMY_H
+#ifndef PROTOCOL_DUMMY_H
+#define PROTOCOL_DUMMY_H
 
-struct protocol_t;
-struct protocol_params_t;
-
-int dummy_init(int n_options, char **options, struct protocol_params_t *lsn);
-void *dummy_new(struct protocol_t *proto_struct,
-                struct protocol_params_t *params);
+struct protocol_vtable;
+extern const struct protocol_vtable dummy_vtable;
 
 #endif
diff --git a/src/protocols/obfs2.c b/src/protocols/obfs2.c
index 27e9272..cc208ed 100644
--- a/src/protocols/obfs2.c
+++ b/src/protocols/obfs2.c
@@ -2,34 +2,28 @@
    See LICENSE for other credits and copying information
 */
 
-#define CRYPT_PROTOCOL_PRIVATE
+#define PROTOCOL_OBFS2_PRIVATE
 #include "obfs2.h"
 
-#include "../protocol.h"
 #include "../util.h"
 
 #include <assert.h>
-#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
 #include <event2/buffer.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 enum recv_ret obfs2_recv(void *state, struct evbuffer *source,
-                                struct evbuffer *dest);
-static void *obfs2_state_new(protocol_params_t *params);
-static int obfs2_state_set_shared_secret(void *s,
-                                         const char *secret,
-                                         size_t secretlen);
-static int set_up_vtable(void);
 static void usage(void);
+static int parse_and_set_options(int n_options,
+                                 const char *const *options,
+                                 struct protocol_params_t *params);
 
-static protocol_vtable *vtable=NULL;
+static inline obfs2_protocol_t *
+downcast(struct protocol_t *proto)
+{
+  return (obfs2_protocol_t *)
+    ((char *)proto - offsetof(obfs2_protocol_t, super));
+}
 
 /*
    This function parses 'options' and fills the protocol parameters
@@ -38,31 +32,34 @@ static protocol_vtable *vtable=NULL;
 
    Returns 0 on success, -1 on fail.
 */
-int
-obfs2_init(int n_options, char **options,
-           struct protocol_params_t *params)
+static struct protocol_params_t *
+obfs2_init(int n_options, const char *const *options)
 {
-  if (parse_and_set_options(n_options,options,params) < 0) {
+  struct protocol_params_t *params
+    = calloc(1, sizeof(struct protocol_params_t));
+  if (!params)
+    return NULL;
+
+  if (parse_and_set_options(n_options, options, params) < 0) {
     usage();
-    return -1;
+    free(params);
+    return NULL;
   }
 
-  if (set_up_vtable() < 0)
-    return -1;
-
   if (initialize_crypto() < 0) {
-    fprintf(stderr, "Can't initialize crypto; failing\n");
-    return -1;
+    log_warn("Can't initialize crypto; failing");
+    free(params);
+    return NULL;
   }
 
-  return 0;
+  return params;
 }
 
 /**
    Helper: Parses 'options' and fills 'params'.
 */
 int
-parse_and_set_options(int n_options, char **options,
+parse_and_set_options(int n_options, const char *const *options,
                       struct protocol_params_t *params)
 {
   int got_dest=0;
@@ -75,7 +72,6 @@ parse_and_set_options(int n_options, char **options,
   }
 
   assert(!strcmp(*options,"obfs2"));
-  params->proto = OBFS2_PROTOCOL;
   options++;
 
   /* Now parse the optional arguments */
@@ -136,6 +132,8 @@ parse_and_set_options(int n_options, char **options,
     }
 
     log_debug("%s(): Parsed obfs2 options nicely!", __func__);
+
+    params->vtable = &obfs2_vtable;
     return 0;
 }
 
@@ -159,28 +157,6 @@ usage(void)
          "\tobfs2 server 127.0.0.1:1026");
 }
 
-/**
-   Helper: Allocates space for the protocol vtable and populates it's
-   function pointers.
-   Returns 0 on success, -1 on fail.
-*/
-static int
-set_up_vtable(void)
-{
-  /* XXX memleak. */
-  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;
-
-  return 0;
-}
-
 /** Return true iff the OBFUSCATE_SEED_LENGTH-byte seed in 'seed' is nonzero */
 static int
 seed_nonzero(const uchar *seed)
@@ -195,7 +171,7 @@ seed_nonzero(const uchar *seed)
 static crypt_t *
 derive_key(void *s, const char *keytype)
 {
-  obfs2_state_t *state = s;
+  obfs2_protocol_t *state = s;
   crypt_t *cryptstate;
   uchar buf[SHA256_LENGTH];
   digest_t *c = digest_new();
@@ -235,7 +211,7 @@ static crypt_t *
 derive_padding_key(void *s, const uchar *seed,
                    const char *keytype)
 {
-  obfs2_state_t *state = s;
+  obfs2_protocol_t *state = s;
 
   crypt_t *cryptstate;
   uchar buf[SHA256_LENGTH];
@@ -274,95 +250,84 @@ derive_padding_key(void *s, const uchar *seed,
    to create and return a protocol state according to the protocol
    parameters 'params'.
 */
-void *
-obfs2_new(struct protocol_t *proto_struct,
-          protocol_params_t *params)
+static struct protocol_t *
+obfs2_create(protocol_params_t *params)
 {
-  assert(vtable);
-  proto_struct->vtable = vtable;
-
-  return obfs2_state_new(params);
-}
-
-/**
-   Returns an obfs2 state according to the protocol parameters
-   'params'. If something goes wrong it returns NULL.
- */
-static void *
-obfs2_state_new(protocol_params_t *params)
-{
-  obfs2_state_t *state = calloc(1, sizeof(obfs2_state_t));
+  obfs2_protocol_t *proto = calloc(1, sizeof(obfs2_protocol_t));
   uchar *seed;
   const char *send_pad_type;
 
-  if (!state)
+  if (!proto)
     return NULL;
-  state->state = ST_WAIT_FOR_KEY;
-  state->we_are_initiator = params->is_initiator;
-  if (state->we_are_initiator) {
+  proto->state = ST_WAIT_FOR_KEY;
+  proto->we_are_initiator = params->is_initiator;
+  if (proto->we_are_initiator) {
     send_pad_type = INITIATOR_PAD_TYPE;
-    seed = state->initiator_seed;
+    seed = proto->initiator_seed;
   } else {
     send_pad_type = RESPONDER_PAD_TYPE;
-    seed = state->responder_seed;
+    seed = proto->responder_seed;
   }
 
   /* Generate our seed */
   if (random_bytes(seed, OBFUSCATE_SEED_LENGTH) < 0) {
-    free(state);
+    free(proto);
     return NULL;
   }
 
-  if (params->shared_secret)
-    if (obfs2_state_set_shared_secret(state,
-                                      params->shared_secret,
-                                      params->shared_secret_len)<0)
+  if (params->shared_secret) {
+    /* ASN we must say in spec that we hash command line shared secret. */
+    digest_t *c = digest_new();
+    if (!c) {
+      free(proto);
       return NULL;
+    }
+    digest_update(c, (uchar*)params->shared_secret, params->shared_secret_len);
+    digest_getdigest(c, proto->secret_seed, SHARED_SECRET_LENGTH);
+    digest_free(c);
+  }
 
   /* Derive the key for what we're sending */
-  state->send_padding_crypto = derive_padding_key(state, seed, send_pad_type);
-  if (state->send_padding_crypto == NULL) {
-    free(state);
+  proto->send_padding_crypto = derive_padding_key(proto, seed, send_pad_type);
+  if (proto->send_padding_crypto == NULL) {
+    free(proto);
     return NULL;
   }
 
-  return state;
+  proto->super.vtable = &obfs2_vtable;
+  return &proto->super;
 }
 
 /**
-    Sets the shared 'secret' to be used, on the protocol state 's'.
+    Frees obfs2 state 's'
 */
-static int
-obfs2_state_set_shared_secret(void *s, const char *secret,
-                              size_t secretlen)
+static void
+obfs2_destroy(struct protocol_t *s)
 {
-  assert(secret);
-  assert(secretlen);
-
-  uchar buf[SHARED_SECRET_LENGTH];
-  obfs2_state_t *state = s;
-
-  /* ASN we must say in spec that we hash command line shared secret. */
-  digest_t *c = digest_new();
-  digest_update(c, (uchar*)secret, secretlen);
-  digest_getdigest(c, buf, sizeof(buf));
-
-  memcpy(state->secret_seed, buf, SHARED_SECRET_LENGTH);
-
-  memset(buf,0,SHARED_SECRET_LENGTH);
-  digest_free(c);
-
-  return 0;
+  obfs2_protocol_t *state = downcast(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_protocol_t));
+  free(state);
 }
 
+
 /**
    Write the initial protocol setup and padding message for state 's' to
    the evbuffer 'buf'.  Return 0 on success, -1 on failure.
  */
 static int
-obfs2_send_initial_message(void *s, struct evbuffer *buf)
+obfs2_handshake(struct protocol_t *s, struct evbuffer *buf)
 {
-  obfs2_state_t *state = s;
+  obfs2_protocol_t *state = downcast(s);
 
   uint32_t magic = htonl(OBFUSCATE_MAGIC_VALUE), plength, send_plength;
   uchar msg[OBFUSCATE_MAX_PADDING + OBFUSCATE_SEED_LENGTH + 8];
@@ -426,10 +391,10 @@ crypt_and_transmit(crypt_t *crypto,
    using the state in 'state'.  Returns 0 on success, -1 on failure.
  */
 static int
-obfs2_send(void *s,
+obfs2_send(struct protocol_t *s,
           struct evbuffer *source, struct evbuffer *dest)
 {
-  obfs2_state_t *state = s;
+  obfs2_protocol_t *state = downcast(s);
 
   if (state->send_crypto) {
     /* First of all, send any data that we've been waiting to send. */
@@ -460,7 +425,7 @@ obfs2_send(void *s,
 static int
 init_crypto(void *s)
 {
-  obfs2_state_t *state = s;
+  obfs2_protocol_t *state = s;
 
   const char *send_keytype;
   const char *recv_keytype;
@@ -503,10 +468,10 @@ init_crypto(void *s)
  *  our callers that they must call obfs2_send() immediately.
  */
 static enum recv_ret
-obfs2_recv(void *s, struct evbuffer *source,
+obfs2_recv(struct protocol_t *s, struct evbuffer *source,
            struct evbuffer *dest)
 {
-  obfs2_state_t *state = s;
+  obfs2_protocol_t *state = downcast(s);
   enum recv_ret r=0;
 
   if (state->state == ST_WAIT_FOR_KEY) {
@@ -587,23 +552,4 @@ obfs2_recv(void *s, struct evbuffer *source,
   return r;
 }
 
-/**
-    Frees obfs2 state 's'
-*/
-static void
-obfs2_state_free(void *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);
-}
+DEFINE_PROTOCOL_VTABLE(obfs2);
diff --git a/src/protocols/obfs2.h b/src/protocols/obfs2.h
index a9ebfb1..e93fbcd 100644
--- a/src/protocols/obfs2.h
+++ b/src/protocols/obfs2.h
@@ -2,27 +2,19 @@
    See LICENSE for other credits and copying information
 */
 
-#ifndef OBFS2_H
-#define OBFS2_H
+#ifndef PROTOCOL_OBFS2_H
+#define PROTOCOL_OBFS2_H
 
-typedef struct obfs2_state_t obfs2_state_t;
-struct evbuffer;
-struct protocol_t;
-struct protocol_params_t;
-struct listener_t;
+struct protocol_vtable;
+extern const struct protocol_vtable obfs2_vtable;
 
-int obfs2_init(int n_options, char **options, struct protocol_params_t *params);
-void *obfs2_new(struct protocol_t *proto_struct,
-                struct protocol_params_t *params);
-int parse_and_set_options(int n_options, char **options,
-                          struct protocol_params_t *params);
-
-#ifdef CRYPT_PROTOCOL_PRIVATE
+#ifdef PROTOCOL_OBFS2_PRIVATE
 
 #include "../crypt.h"
+#include "../protocol.h"
 
 /* ==========
-   These definitions are not part of the crypt_protocol interface.
+   These definitions are not part of the obfs2_protocol interface.
    They're exposed here so that the unit tests can use them.
    ==========
 */
@@ -43,7 +35,9 @@ int parse_and_set_options(int n_options, char **options,
 
 #define SHARED_SECRET_LENGTH SHA256_LENGTH
 
-struct obfs2_state_t {
+typedef struct obfs2_protocol_t {
+  struct protocol_t super;
+
   /** Current protocol state.  We start out waiting for key information.  Then
       we have a key and wait for padding to arrive.  Finally, we are sending
       and receiving bytes on the connection.
@@ -76,7 +70,8 @@ struct obfs2_state_t {
 
   /** Number of padding bytes to read before we get to real data */
   int padding_left_to_read;
-};
+} obfs2_protocol_t;
+
 #endif
 
 #endif
diff --git a/src/test/unittest_obfs2.c b/src/test/unittest_obfs2.c
index 0b2054b..594c36c 100644
--- a/src/test/unittest_obfs2.c
+++ b/src/test/unittest_obfs2.c
@@ -5,12 +5,11 @@
 #include "tinytest.h"
 #include "tinytest_macros.h"
 
-#define CRYPT_PROTOCOL_PRIVATE
+#define PROTOCOL_OBFS2_PRIVATE
 #define CRYPT_PRIVATE
 #include "../protocols/obfs2.h"
 #include "../crypt.h"
 #include "../util.h"
-#include "../protocol.h"
 
 #include <stdio.h>
 #include <stdlib.h>
@@ -18,86 +17,60 @@
 
 #include <event2/buffer.h>
 
+#define ALEN(x) (sizeof x/sizeof x[0])
+#define OPTV(name) static const char *const name[]
+
+static inline obfs2_protocol_t *
+downcast(struct protocol_t *proto)
+{
+  return (obfs2_protocol_t *)
+    ((char *)proto - offsetof(obfs2_protocol_t, super));
+}
 
 static void
 test_proto_option_parsing(void *data)
 {
-  protocol_params_t *proto_params = calloc(1, sizeof(protocol_params_t));
-  char *options[] = {"obfs2", "--shared-secret=a", "socks", "127.0.0.1:0"};
-  int n_options = 4;
-
   /* Suppress logs for the duration of this function. */
   log_set_method(LOG_METHOD_NULL, NULL);
 
-  tt_assert(set_up_protocol(n_options, options,
-                            proto_params) == 0);
+  /** good option list */
+  OPTV(options1) = {"obfs2", "--shared-secret=a", "socks", "127.0.0.1:0"};
+  tt_assert(proto_params_init(ALEN(options1), options1) != NULL);
 
   /** two --dest. */
-  char *options2[] = {"obfs2", "--dest=127.0.0.1:5555", "--dest=a",
-                      "server", "127.0.0.1:5552"};
-  n_options = 5;
-
-  tt_assert(set_up_protocol(n_options, options2,
-                            proto_params) == -1);
+  OPTV(options2) = {"obfs2", "--dest=127.0.0.1:5555", "--dest=a",
+                    "server", "127.0.0.1:5552"};
+  tt_assert(proto_params_init(ALEN(options2), options2) == NULL);
 
   /** unknown arg */
-  char *options3[] = {"obfs2", "--gabura=a",
-                      "server", "127.0.0.1:5552"};
-  n_options = 4;
-
-  tt_assert(set_up_protocol(n_options, options3,
-                            proto_params) == -1);
+  OPTV(options3) = {"obfs2", "--gabura=a", "server", "127.0.0.1:5552"};
+  tt_assert(proto_params_init(ALEN(options3), options3) == NULL)
 
   /** too many args */
-  char *options4[] = {"obfs2", "1", "2", "3", "4", "5" };
-  n_options = 6;
-
-  tt_assert(set_up_protocol(n_options, options4,
-                            proto_params) == -1);
+  OPTV(options4) = {"obfs2", "1", "2", "3", "4", "5" };
+  tt_assert(proto_params_init(ALEN(options4), options4) == NULL)
 
   /** wrong mode  */
-  char *options5[] = {"obfs2", "--dest=1:1",
-                      "gladiator", "127.0.0.1:5552"};
-  n_options = 4;
-
-  tt_assert(set_up_protocol(n_options, options5,
-                            proto_params) == -1);
-
-  /** stupid listen addr.  */
-  char *options6[] = {"obfs2", "--dest=1:1",
-                      "server", "127.0.0.1:a"};
-  n_options = 4;
+  OPTV(options5) = {"obfs2", "--dest=1:1", "gladiator", "127.0.0.1:5552"};
+  tt_assert(proto_params_init(ALEN(options5), options5) == NULL)
 
-  tt_assert(set_up_protocol(n_options, options6,
-                            proto_params) == -1);
+  /** bad listen addr.  */
+  OPTV(options6) = {"obfs2", "--dest=1:1", "server", "127.0.0.1:a"};
+  tt_assert(proto_params_init(ALEN(options6), options6) == NULL)
 
-  /** stupid dest addr.  */
-  char *options7[] = {"obfs2", "--dest=1:b",
-                      "server", "127.0.0.1:1"};
-  n_options = 4;
-
-  tt_assert(set_up_protocol(n_options, options7,
-                            proto_params) == -1);
+  /** bad dest addr.  */
+  OPTV(options7) = {"obfs2", "--dest=1:b", "server", "127.0.0.1:1"};
+  tt_assert(proto_params_init(ALEN(options7), options7) == NULL)
 
   /** socks with dest.  */
-  char *options8[] = {"obfs2", "--dest=1:2",
-                      "socks", "127.0.0.1:1"};
-  n_options = 4;
-
-  tt_assert(set_up_protocol(n_options, options8,
-                            proto_params) == -1);
+  OPTV(options8) = {"obfs2", "--dest=1:2", "socks", "127.0.0.1:1"};
+  tt_assert(proto_params_init(ALEN(options8), options8) == NULL)
 
   /** socks with dest.  */
-  char *options9[] = {"obfs2", "--shared-secret=a",
-                      "server", "127.0.0.1:1"};
-  n_options = 4;
-
-  tt_assert(set_up_protocol(n_options, options9,
-                            proto_params) == -1);
+  OPTV(options9) = {"obfs2", "--shared-secret=a", "server", "127.0.0.1:1"};
+  tt_assert(proto_params_init(ALEN(options9), options9) == NULL)
 
  end:
-  if (proto_params)
-    free(proto_params);
   /* Unsuspend logging */
   log_set_method(LOG_METHOD_STDOUT, NULL);
 }
@@ -108,36 +81,36 @@ test_proto_setup(void *data)
 {
   struct protocol_t *client_proto = NULL;
   struct protocol_t *server_proto = NULL;
+  struct protocol_params_t *proto_params_client = NULL;
+  struct protocol_params_t *proto_params_server = NULL;
 
-  protocol_params_t *proto_params_client = calloc(1, sizeof(protocol_params_t));
-  char *options_client[] = {"obfs2", "--shared-secret=hahaha",
-                            "socks", "127.0.0.1:1800"};
-  int n_options_client = 4;
-
-  protocol_params_t *proto_params_serv = calloc(1, sizeof(protocol_params_t));
-  char *options_server[] = {"obfs2", "--shared-secret=hahaha",
-                            "--dest=127.0.0.1:1500",
-                           "server", "127.0.0.1:1800"};
-  int n_options_server = 5;
-
-  tt_assert(set_up_protocol(n_options_client, options_client,
-                            proto_params_client) >= 0);
-  tt_assert(set_up_protocol(n_options_server, options_server,
-                            proto_params_serv) >= 0);
+  OPTV(options_client) = {"obfs2", "--shared-secret=hahaha",
+                          "socks", "127.0.0.1:1800"};
+  proto_params_client = proto_params_init(ALEN(options_client), options_client);
+  tt_assert(proto_params_client);
 
-  client_proto = proto_new(proto_params_serv);
-  server_proto = proto_new(proto_params_client);
+  OPTV(options_server) = {"obfs2", "--shared-secret=hahaha",
+                          "--dest=127.0.0.1:1500",
+                          "server", "127.0.0.1:1800"};
+  proto_params_server = proto_params_init(ALEN(options_server), options_server);
+  tt_assert(proto_params_server);
 
+  client_proto = proto_create(proto_params_client);
   tt_assert(client_proto);
+
+  server_proto = proto_create(proto_params_server);
   tt_assert(server_proto);
-  tt_assert(client_proto->state);
-  tt_assert(server_proto->state);
 
- end:
+ end:;
   if (client_proto)
-    proto_destroy(client_proto);
+      proto_destroy(client_proto);
   if (server_proto)
-    proto_destroy(server_proto);
+      proto_destroy(server_proto);
+
+  if (proto_params_client)
+    proto_params_free(proto_params_client);
+  if (proto_params_server)
+    proto_params_free(proto_params_server);
 }
 
 static void
@@ -150,33 +123,28 @@ test_proto_handshake(void *data)
 
   struct protocol_t *client_proto = NULL;
   struct protocol_t *server_proto = NULL;
+  struct protocol_params_t *proto_params_client = NULL;
+  struct protocol_params_t *proto_params_server = NULL;
 
-  protocol_params_t *proto_params_client = calloc(1, sizeof(protocol_params_t));
-  char *options_client[] = {"obfs2", "--shared-secret=hahaha",
-                            "socks", "127.0.0.1:1800"};
-  int n_options_client = 4;
-
-  protocol_params_t *proto_params_serv = calloc(1, sizeof(protocol_params_t));
-  char *options_server[] = {"obfs2", "--shared-secret=hahaha",
-                            "--dest=127.0.0.1:1500",
-                            "server", "127.0.0.1:1800"};
-  int n_options_server = 5;
-
-  tt_assert(set_up_protocol(n_options_client, options_client,
-                            proto_params_client) >= 0);
-  tt_assert(set_up_protocol(n_options_server, options_server,
-                            proto_params_serv) >= 0);
+  OPTV(options_client) = {"obfs2", "--shared-secret=hahaha",
+                          "socks", "127.0.0.1:1800"};
+  proto_params_client = proto_params_init(ALEN(options_client), options_client);
+  tt_assert(proto_params_client);
 
-  client_proto = proto_new(proto_params_client);
-  server_proto = proto_new(proto_params_serv);
+  OPTV(options_server) = {"obfs2", "--shared-secret=hahaha",
+                          "--dest=127.0.0.1:1500",
+                          "server", "127.0.0.1:1800"};
+  proto_params_server = proto_params_init(ALEN(options_server), options_server);
+  tt_assert(proto_params_server);
 
+  client_proto = proto_create(proto_params_client);
   tt_assert(client_proto);
+
+  server_proto = proto_create(proto_params_server);
   tt_assert(server_proto);
-  tt_assert(client_proto->state);
-  tt_assert(server_proto->state);
 
-  obfs2_state_t *client_state = client_proto->state;
-  obfs2_state_t *server_state = server_proto->state;
+  obfs2_protocol_t *client_state = downcast(client_proto);
+  obfs2_protocol_t *server_state = downcast(server_proto);
 
   /* We create a client handshake message and pass it to output_buffer */
   tt_int_op(0, <=, proto_handshake(client_proto, output_buffer));
@@ -209,9 +177,9 @@ test_proto_handshake(void *data)
       proto_destroy(server_proto);
 
   if (proto_params_client)
-    free(proto_params_client);
-  if (proto_params_serv)
-    free(proto_params_serv);
+    proto_params_free(proto_params_client);
+  if (proto_params_server)
+    proto_params_free(proto_params_server);
 
   if (output_buffer)
     evbuffer_free(output_buffer);
@@ -229,30 +197,25 @@ test_proto_transfer(void *data)
 
   struct protocol_t *client_proto = NULL;
   struct protocol_t *server_proto = NULL;
+  struct protocol_params_t *proto_params_client = NULL;
+  struct protocol_params_t *proto_params_server = NULL;
 
-  protocol_params_t *proto_params_client = calloc(1, sizeof(protocol_params_t));
-  char *options_client[] = {"obfs2", "--shared-secret=hahaha",
-                            "socks", "127.0.0.1:1800"};
-  int n_options_client = 4;
-
-  protocol_params_t *proto_params_serv = calloc(1, sizeof(protocol_params_t));
-  char *options_server[] = {"obfs2", "--shared-secret=hahaha",
-                            "--dest=127.0.0.1:1500",
-                            "server", "127.0.0.1:1800"};
-  int n_options_server = 5;
-
-  tt_assert(set_up_protocol(n_options_client, options_client,
-                            proto_params_client) >= 0);
-  tt_assert(set_up_protocol(n_options_server, options_server,
-                            proto_params_serv) >= 0);
+  OPTV(options_client) = {"obfs2", "--shared-secret=hahaha",
+                          "socks", "127.0.0.1:1800"};
+  proto_params_client = proto_params_init(ALEN(options_client), options_client);
+  tt_assert(proto_params_client);
 
-  client_proto = proto_new(proto_params_client);
-  server_proto = proto_new(proto_params_serv);
+  OPTV(options_server) = {"obfs2", "--shared-secret=hahaha",
+                          "--dest=127.0.0.1:1500",
+                          "server", "127.0.0.1:1800"};
+  proto_params_server = proto_params_init(ALEN(options_server), options_server);
+  tt_assert(proto_params_server);
 
+  client_proto = proto_create(proto_params_client);
   tt_assert(client_proto);
+
+  server_proto = proto_create(proto_params_server);
   tt_assert(server_proto);
-  tt_assert(client_proto->state);
-  tt_assert(server_proto->state);
 
   int n;
   struct evbuffer_iovec v[2];
@@ -302,9 +265,9 @@ test_proto_transfer(void *data)
     proto_destroy(server_proto);
 
   if (proto_params_client)
-    free(proto_params_client);
-  if (proto_params_serv)
-    free(proto_params_serv);
+    proto_params_free(proto_params_client);
+  if (proto_params_server)
+    proto_params_free(proto_params_server);
 
   if (output_buffer)
     evbuffer_free(output_buffer);
@@ -325,8 +288,8 @@ test_proto_transfer(void *data)
 static void
 test_proto_split_handshake(void *data)
 {
-  obfs2_state_t *client_state = NULL;
-  obfs2_state_t *server_state = NULL;
+  obfs2_protocol_t *client_state = NULL;
+  obfs2_protocol_t *server_state = NULL;
 
   struct evbuffer *output_buffer = NULL;
   struct evbuffer *dummy_buffer = NULL;
@@ -335,33 +298,28 @@ test_proto_split_handshake(void *data)
 
   struct protocol_t *client_proto = NULL;
   struct protocol_t *server_proto = NULL;
+  struct protocol_params_t *proto_params_client = NULL;
+  struct protocol_params_t *proto_params_server = NULL;
 
-  protocol_params_t *proto_params_client = calloc(1, sizeof(protocol_params_t));
-  char *options_client[] = {"obfs2", "--shared-secret=hahaha",
-                            "socks", "127.0.0.1:1800"};
-  int n_options_client = 4;
-
-  protocol_params_t *proto_params_serv = calloc(1, sizeof(protocol_params_t));
-  char *options_server[] = {"obfs2", "--shared-secret=hahaha",
-                            "--dest=127.0.0.1:1500",
-                            "server", "127.0.0.1:1800"};
-  int n_options_server = 5;
-
-  tt_assert(set_up_protocol(n_options_client, options_client,
-                            proto_params_client) >= 0);
-  tt_assert(set_up_protocol(n_options_server, options_server,
-                            proto_params_serv) >= 0);
+  OPTV(options_client) = {"obfs2", "--shared-secret=hahaha",
+                          "socks", "127.0.0.1:1800"};
+  proto_params_client = proto_params_init(ALEN(options_client), options_client);
+  tt_assert(proto_params_client);
 
-  client_proto = proto_new(proto_params_client);
-  server_proto = proto_new(proto_params_serv);
+  OPTV(options_server) = {"obfs2", "--shared-secret=hahaha",
+                          "--dest=127.0.0.1:1500",
+                          "server", "127.0.0.1:1800"};
+  proto_params_server = proto_params_init(ALEN(options_server), options_server);
+  tt_assert(proto_params_server);
 
+  client_proto = proto_create(proto_params_client);
   tt_assert(client_proto);
+
+  server_proto = proto_create(proto_params_server);
   tt_assert(server_proto);
-  tt_assert(client_proto->state);
-  tt_assert(server_proto->state);
 
-  client_state = client_proto->state;
-  server_state = server_proto->state;
+  client_state = downcast(client_proto);
+  server_state = downcast(server_proto);
 
   uint32_t magic = htonl(OBFUSCATE_MAGIC_VALUE);
   uint32_t plength1, plength1_msg1, plength1_msg2, send_plength1;
@@ -474,9 +432,9 @@ test_proto_split_handshake(void *data)
     proto_destroy(server_proto);
 
   if (proto_params_client)
-    free(proto_params_client);
-  if (proto_params_serv)
-    free(proto_params_serv);
+    proto_params_free(proto_params_client);
+  if (proto_params_server)
+    proto_params_free(proto_params_server);
 
   if (output_buffer)
     evbuffer_free(output_buffer);
@@ -491,8 +449,8 @@ test_proto_split_handshake(void *data)
 static void
 test_proto_wrong_handshake_magic(void *data)
 {
-  obfs2_state_t *client_state = NULL;
-  obfs2_state_t *server_state = NULL;
+  obfs2_protocol_t *client_state = NULL;
+  obfs2_protocol_t *server_state = NULL;
 
   struct evbuffer *output_buffer = NULL;
   struct evbuffer *dummy_buffer = NULL;
@@ -501,33 +459,28 @@ test_proto_wrong_handshake_magic(void *data)
 
   struct protocol_t *client_proto = NULL;
   struct protocol_t *server_proto = NULL;
+  struct protocol_params_t *proto_params_client = NULL;
+  struct protocol_params_t *proto_params_server = NULL;
 
-  protocol_params_t *proto_params_client = calloc(1, sizeof(protocol_params_t));
-  char *options_client[] = {"obfs2", "--shared-secret=hahaha",
-                            "socks", "127.0.0.1:1800"};
-  int n_options_client = 4;
-
-  protocol_params_t *proto_params_serv = calloc(1, sizeof(protocol_params_t));
-  char *options_server[] = {"obfs2", "--shared-secret=hahaha",
-                            "--dest=127.0.0.1:1500",
-                            "server", "127.0.0.1:1800"};
-  int n_options_server = 5;
+  OPTV(options_client) = {"obfs2", "--shared-secret=hahaha",
+                          "socks", "127.0.0.1:1800"};
+  proto_params_client = proto_params_init(ALEN(options_client), options_client);
+  tt_assert(proto_params_client);
 
-  tt_assert(set_up_protocol(n_options_client, options_client,
-                            proto_params_client) >= 0);
-  tt_assert(set_up_protocol(n_options_server, options_server,
-                            proto_params_serv) >= 0);
-
-  client_proto = proto_new(proto_params_client);
-  server_proto = proto_new(proto_params_serv);
+  OPTV(options_server) = {"obfs2", "--shared-secret=hahaha",
+                          "--dest=127.0.0.1:1500",
+                          "server", "127.0.0.1:1800"};
+  proto_params_server = proto_params_init(ALEN(options_server), options_server);
+  tt_assert(proto_params_server);
 
+  client_proto = proto_create(proto_params_client);
   tt_assert(client_proto);
+
+  server_proto = proto_create(proto_params_server);
   tt_assert(server_proto);
-  tt_assert(client_proto->state);
-  tt_assert(server_proto->state);
 
-  client_state = client_proto->state;
-  server_state = server_proto->state;
+  client_state = downcast(client_proto);
+  server_state = downcast(server_proto);
 
   uint32_t wrong_magic = 0xD15EA5E;
 
@@ -561,9 +514,9 @@ test_proto_wrong_handshake_magic(void *data)
     proto_destroy(server_proto);
 
   if (proto_params_client)
-    free(proto_params_client);
-  if (proto_params_serv)
-    free(proto_params_serv);
+    proto_params_free(proto_params_client);
+  if (proto_params_server)
+    proto_params_free(proto_params_server);
 
   if (output_buffer)
     evbuffer_free(output_buffer);
@@ -577,8 +530,8 @@ test_proto_wrong_handshake_magic(void *data)
 static void
 test_proto_wrong_handshake_plength(void *data)
 {
-  obfs2_state_t *client_state = NULL;
-  obfs2_state_t *server_state = NULL;
+  obfs2_protocol_t *client_state = NULL;
+  obfs2_protocol_t *server_state = NULL;
 
   struct evbuffer *output_buffer = NULL;
   struct evbuffer *dummy_buffer = NULL;
@@ -587,34 +540,28 @@ test_proto_wrong_handshake_plength(void *data)
 
   struct protocol_t *client_proto = NULL;
   struct protocol_t *server_proto = NULL;
+  struct protocol_params_t *proto_params_client = NULL;
+  struct protocol_params_t *proto_params_server = NULL;
 
+  OPTV(options_client) = {"obfs2", "--shared-secret=hahaha",
+                          "socks", "127.0.0.1:1800"};
+  proto_params_client = proto_params_init(ALEN(options_client), options_client);
+  tt_assert(proto_params_client);
 
-  protocol_params_t *proto_params_client = calloc(1, sizeof(protocol_params_t));
-  char *options_client[] = {"obfs2", "--shared-secret=hahaha",
-                            "socks", "127.0.0.1:1800"};
-  int n_options_client = 4;
-
-  protocol_params_t *proto_params_serv = calloc(1, sizeof(protocol_params_t));
-  char *options_server[] = {"obfs2", "--shared-secret=hahaha",
-                            "--dest=127.0.0.1:1500",
-                            "server", "127.0.0.1:1800"};
-  int n_options_server = 5;
-
-  tt_assert(set_up_protocol(n_options_client, options_client,
-                            proto_params_client) >= 0);
-  tt_assert(set_up_protocol(n_options_server, options_server,
-                            proto_params_serv) >= 0);
-
-  client_proto = proto_new(proto_params_client);
-  server_proto = proto_new(proto_params_serv);
+  OPTV(options_server) = {"obfs2", "--shared-secret=hahaha",
+                          "--dest=127.0.0.1:1500",
+                          "server", "127.0.0.1:1800"};
+  proto_params_server = proto_params_init(ALEN(options_server), options_server);
+  tt_assert(proto_params_server);
 
+  client_proto = proto_create(proto_params_client);
   tt_assert(client_proto);
+
+  server_proto = proto_create(proto_params_server);
   tt_assert(server_proto);
-  tt_assert(client_proto->state);
-  tt_assert(server_proto->state);
 
-  client_state = client_proto->state;
-  server_state = server_proto->state;
+  client_state = downcast(client_proto);
+  server_state = downcast(server_proto);
 
   uchar msg[OBFUSCATE_MAX_PADDING + OBFUSCATE_SEED_LENGTH + 8 + 1];
   uint32_t magic = htonl(OBFUSCATE_MAGIC_VALUE);
@@ -641,15 +588,15 @@ test_proto_wrong_handshake_plength(void *data)
   tt_assert(server_state->state == ST_WAIT_FOR_KEY);
 
  end:
-  if (client_state)
+  if (client_proto)
     proto_destroy(client_proto);
-  if (server_state)
+  if (server_proto)
     proto_destroy(server_proto);
 
   if (proto_params_client)
-    free(proto_params_client);
-  if (proto_params_serv)
-    free(proto_params_serv);
+    proto_params_free(proto_params_client);
+  if (proto_params_server)
+    proto_params_free(proto_params_server);
 
   if (output_buffer)
     evbuffer_free(output_buffer);



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