Changes to tor-fw-helper, some based on Nick's review

 * MINIUPNPC rather than the generic UPNP
 * Nick suggested a better abstraction model for tor-fw-helper
 * Fix autoconf to build with either natpmp or miniupnpc
 * Add AM_PROG_CC_C_O to fix automake complaint
 * update spec to address nickm's concern
 * refactor nat-pmp to match upnp state
 * we prefer tor_snprintf to snprintf
 * link properlty for tor_snprintf
 * rename test_commandline_options to log_commandline_options
 * cast this uint as an int
 * detect possible FD_SETSIZE errors
 * make note about future enhancements for natpmp
 * add upnp enhancement note
 * ChangeLog entry
 * doxygen and check-spaces cleanup
 * create tor-fw-helper.1.txt

11 files changed, 557 insertions, 127 deletions

diff --git a/changes/tor-fw-helper b/changes/tor-fw-helper
new file mode 100644
index 000000000..f87d828b8
--- /dev/null
+++ b/changes/tor-fw-helper
@@ -0,0 +1,14 @@
+  o Major features:
+    - Tor now has the ability to wrangle NAT devices like a good network cowbot
+      with the tor-fw-helper tool. The tor-fw-helper tool supports Apple's
+      NAT-PMP protocol and the UPnP standard for TCP port mapping. This
+      optional tool may be enabled at compile time by configuring with
+      '--enable-upnp' or '--enable-natpmp' or with both. This tool may be
+      called by hand or by Tor.  By configuring the PortForwarding option, Tor
+      will launch the helper on a regular basis to ensure that the NAT mapping
+      is regularly updated.  Additionally, a user may also specify an
+      alternative helper by using the PortForwardingHelper option. The helper
+      may be specified by name or with the full path to the helper. The default
+      helper is named 'tor-fw-helper' and any alternative helper must take the
+      tor-fw-helper-spec.txt into account.
+
diff --git a/configure.in b/configure.in
index ab3689db2..18e312b7a 100644
--- a/configure.in
+++ b/configure.in
@@ -153,7 +153,10 @@ AC_PATH_PROG([A2X], [a2x], none)
 
 AM_CONDITIONAL(USE_ASCIIDOC, test x$asciidoc = xtrue)
 
-AM_CONDITIONAL(USE_FW_HELPER, test x$natpmp = xtrue || x$upnp = xtrue)
+AM_CONDITIONAL(USE_FW_HELPER, test x$natpmp = xtrue || test x$upnp = xtrue)
+AM_CONDITIONAL(NAT_PMP, test x$natpmp = xtrue)
+AM_CONDITIONAL(MINIUPNPC, test x$upnp = xtrue)
+AM_PROG_CC_C_O
 
 AC_PATH_PROG([SHA1SUM], [sha1sum], none)
 AC_PATH_PROG([OPENSSL], [openssl], none)
@@ -486,7 +489,7 @@ dnl Where do you live, libminiupnpc?  And how do we call you?
 dnl There are no packages for Debian or Redhat as of this patch
 
 if test "$upnp" = "true"; then
-    AC_DEFINE(UPNP, 1, [Define to 1 if we are building with UPnP.])
+    AC_DEFINE(MINIUPNPC, 1, [Define to 1 if we are building with UPnP.])
     TOR_SEARCH_LIBRARY(libminiupnpc, $trylibminiupnpcdir, [-lminiupnpc],
         [#include <miniupnpc/miniwget.h>
          #include <miniupnpc/miniupnpc.h>
diff --git a/doc/spec/tor-fw-helper-spec.txt b/doc/spec/tor-fw-helper-spec.txt
index 684f50fb1..0068b2655 100644
--- a/doc/spec/tor-fw-helper-spec.txt
+++ b/doc/spec/tor-fw-helper-spec.txt
@@ -33,12 +33,25 @@
     tor-fw-helper: FAILURE
 
  All informational messages are printed to standard output; all error messages
- are printed to standard error.
+ are printed to standard error. Messages other than SUCCESS and FAILURE
+ may be printed by any compliant tor-fw-helper.
+
+2.2 Output format stability
+
+ The above SUCCESS and FAILURE messages are the only stable output formats
+ provided by this specification. tor-fw-helper-spec compliant implementations
+ must return SUCCESS or FAILURE as defined above.
 
 3. Security Concerns
 
  It is probably best to hand configure port forwarding and in the process, we
- suggest disabling NAT-PMP and/or UPnP.
+ suggest disabling NAT-PMP and/or UPnP. This is of course absolutely confusing
+ to users and so we support automatic, non-authenticated NAT port mapping
+ protocols with compliant tor-fw-helper applications.
+
+ NAT should not be considered a security boundary. NAT-PMP and UPnP are hacks
+ to deal with the shortcomings of user education about TCP/IP, IPv4 shortages,
+ and of course, NAT devices that suffer from horrible user interface design.
 
 [0] http://en.wikipedia.org/wiki/NAT_Port_Mapping_Protocol
 [1] http://en.wikipedia.org/wiki/Universal_Plug_and_Play
diff --git a/doc/tor-fw-helper.1.txt b/doc/tor-fw-helper.1.txt
new file mode 100644
index 000000000..87607afb8
--- /dev/null
+++ b/doc/tor-fw-helper.1.txt
@@ -0,0 +1,68 @@
+// Copyright (c) The Tor Project, Inc.
+// See LICENSE for licensing information
+// This is an asciidoc file used to generate the manpage/html reference.
+// Learn asciidoc on http://www.methods.co.nz/asciidoc/userguide.html
+tor-fw-helper(1)
+==============
+Jacob Appelbaum
+
+NAME
+----
+tor-fw-helper - Manage upstream firewall/NAT devices
+
+SYNOPSIS
+--------
+**tor-fw-helper** [-h|--help] [-T|--test] [-v|--verbose] [-g|--fetch-public-ip]
+-i|--internal-or-port __TCP port__ [-e|--external-or-port _TCP port_]
+[-d|--internal-dir-port _TCP port_] [-p|--external-dir-port _TCP port_]
+
+DESCRIPTION
+-----------
+**tor-fw-helper** currently supports Apple's NAT-PMP protocol and the UPnP
+standard for TCP port mapping. It is written as the reference implementation of
+tor-fw-helper-spec.txt and conforms to that loose plugin API.  If your network
+supports either NAT-PMP or UPnP, tor-fw-helper will attempt to automatically
+map the required TCP ports for Tor's Or and Dir ports. +
+
+OPTIONS
+-------
+**-h** or **--help**::
+    Display help text and exit.
+
+**-v**::
+    Display verbose output.
+
+**-T** or **--test**::
+    Display test information and print the test information in
+    tor-fw-helper.log
+
+**-g** or **--fetch-public-ip**::
+    Fetch the the public ip address for each supported NAT helper method.
+
+**-i** or **--internal-or-port** __port__::
+    Inform **tor-fw-helper** of your internal OR port. This is the only
+    required argument.
+
+**-e** or **--external-or-port** __port__::
+    Inform **tor-fw-helper** of your external OR port.
+
+**-d** or **--internal-dir-port** __port__::
+    Inform **tor-fw-helper** of your internal Dir port.
+
+**-p** or **--external-dir-port** __port__::
+    Inform **tor-fw-helper** of your external Dir port.
+
+BUGS
+----
+This probably doesn't run on Windows. That's not a big issue, since we don't
+really want to deal with Windows before October 2010 anyway.
+
+SEE ALSO
+--------
+**tor**(1) +
+
+See also the "tor-fw-helper-spec.txt" file, distributed with Tor.
+
+AUTHORS
+-------
+    Jacob Appelbaum <jacob@torproject.org>, Steven J. Murdoch <Steven.Murdoch@cl.cam.ac.uk>
diff --git a/src/tools/tor-fw-helper/Makefile.am b/src/tools/tor-fw-helper/Makefile.am
index 272da0060..f01f94c25 100644
--- a/src/tools/tor-fw-helper/Makefile.am
+++ b/src/tools/tor-fw-helper/Makefile.am
@@ -7,6 +7,27 @@ endif
 tor_fw_helper_SOURCES = tor-fw-helper.c \
 	tor-fw-helper-natpmp.c tor-fw-helper-upnp.c
 tor_fw_helper_INCLUDES = tor-fw-helper.h tor-fw-helper-natpmp.h tor-fw-helper-upnp.h
-tor_fw_helper_LDFLAGS = @TOR_LDFLAGS_libnatpmp@ @TOR_LDFLAGS_libminiupnpc@
-tor_fw_helper_LDADD = -lnatpmp -lminiupnpc ../../common/libor.a @TOR_LIB_WS32@
-tor_fw_helper_CPPFLAGS = @TOR_CPPFLAGS_libnatpmp@ @TOR_CPPFLAGS_libminiupnpc@
+
+if NAT_PMP
+nat_pmp_ldflags = @TOR_LDFLAGS_libnatpmp@
+nat_pmp_ldadd  = -lnatpmp
+nat_pmp_cppflags = @TOR_CPPFLAGS_libnatpmp@
+else
+nat_pmp_ldflags =
+nat_pmp_ldadd  =
+nat_pmp_cppflags =
+endif
+
+if MINIUPNPC
+miniupnpc_ldflags = @TOR_LDFLAGS_libminiupnpc@
+miniupnpc_ldadd = -lminiupnpc -lm
+miniupnpc_cppflags = @TOR_CPPFLAGS_libminiupnpc@
+else
+miniupnpc_ldflags =
+miniupnpc_ldadd =
+miniupnpc_cppflags =
+endif
+
+tor_fw_helper_LDFLAGS = $(nat_pmp_ldflags) $(miniupnpc_ldflags)
+tor_fw_helper_LDADD = $(nat_pmp_ldadd) $(miniupnpc_ldadd) ../../common/libor.a @TOR_LIB_WS32@
+tor_fw_helper_CPPFLAGS = $(nat_pmp_cppflags) $(miniupnpc_cppflags)
diff --git a/src/tools/tor-fw-helper/tor-fw-helper-natpmp.c b/src/tools/tor-fw-helper/tor-fw-helper-natpmp.c
index ebcd9e61a..6e4e40404 100644
--- a/src/tools/tor-fw-helper/tor-fw-helper-natpmp.c
+++ b/src/tools/tor-fw-helper/tor-fw-helper-natpmp.c
@@ -2,51 +2,123 @@
  * Copyright (c) 2010, The Tor Project, Inc. */
 /* See LICENSE for licensing information */
 
+/**
+  * \file tor-fw-helper-natpmp.c
+  * \brief The implementation of our NAT-PMP firewall helper.
+  **/
+
+#include "orconfig.h"
+#ifdef NAT_PMP
 #include <stdint.h>
 #include <stdio.h>
 #include <string.h>
 #include <errno.h>
 #include <arpa/inet.h>
 
+// debugging stuff
+#include <assert.h>
+
 #include "tor-fw-helper.h"
 #include "tor-fw-helper-natpmp.h"
 
+/** This hooks NAT-PMP into our multi-backend API. */
+static tor_fw_backend_t tor_natpmp_backend = {
+    "natpmp",
+    sizeof(struct natpmp_state_t),
+    tor_natpmp_init,
+    tor_natpmp_cleanup,
+    tor_natpmp_fetch_public_ip,
+    tor_natpmp_add_tcp_mapping
+};
+
+/** Return the backend for NAT-PMP. */
+const tor_fw_backend_t *
+tor_fw_get_natpmp_backend(void)
+{
+    return &tor_natpmp_backend;
+}
+
+/** Initialize the NAT-PMP backend and store the results in
+ * <b>backend_state</b>.*/
+int
+tor_natpmp_init(tor_fw_options_t *tor_fw_options, void *backend_state)
+{
+    natpmp_state_t *state = (natpmp_state_t *) backend_state;
+    int r = 0;
+
+    memset(&(state->natpmp), 0, sizeof(natpmp_t));
+    memset(&(state->response), 0, sizeof(natpmpresp_t));
+    state->init = 0;
+    state->protocol = NATPMP_PROTOCOL_TCP;
+    state->lease = NATPMP_DEFAULT_LEASE;
+
+    if (tor_fw_options->verbose)
+        fprintf(stdout, "V: natpmp init...\n");
+
+    r = initnatpmp(&(state->natpmp));
+    if (r == 0) {
+        state->init = 1;
+        fprintf(stdout, "tor-fw-helper: natpmp initialized...\n");
+        return r;
+    } else {
+        fprintf(stderr, "tor-fw-helper: natpmp failed to initialize...\n");
+        return r;
+    }
+}
+
+/** Tear down the NAT-PMP connection stored in <b>backend_state</b>.*/
+int
+tor_natpmp_cleanup(tor_fw_options_t *tor_fw_options, void *backend_state)
+{
+    natpmp_state_t *state = (natpmp_state_t *) backend_state;
+    int r = 0;
+    if (tor_fw_options->verbose)
+        fprintf(stdout, "V: natpmp cleanup...\n");
+    r = closenatpmp(&(state->natpmp));
+    if (tor_fw_options->verbose)
+        fprintf(stdout, "V: closing natpmp socket: %d\n", r);
+    return r;
+}
+
+/** Add a TCP port mapping for a single port stored in <b>tor_fw_options</b>
+ * using the <b>natpmp_t</b> stored in <b>backend_state</b>. */
 int
-tor_natpmp_add_tcp_mapping(tor_fw_options_t *tor_fw_options)
+tor_natpmp_add_tcp_mapping(tor_fw_options_t *tor_fw_options,
+                           void *backend_state)
 {
+    natpmp_state_t *state = (natpmp_state_t *) backend_state;
     int r = 0;
     int x = 0;
     int sav_errno;
-    int protocol = NATPMP_PROTOCOL_TCP;
-    int lease = NATPMP_DEFAULT_LEASE;
-    natpmp_t natpmp;
-    natpmpresp_t response;
 
     fd_set fds;
     struct timeval timeout;
 
     if (tor_fw_options->verbose)
-        fprintf(stdout, "V: natpmp init...\n");
-    initnatpmp(&natpmp);
-
-    if (tor_fw_options->verbose)
         fprintf(stdout, "V: sending natpmp portmapping request...\n");
-    r = sendnewportmappingrequest(&natpmp, protocol,
+    r = sendnewportmappingrequest(&(state->natpmp), state->protocol,
                                   tor_fw_options->internal_port,
                                   tor_fw_options->external_port,
-                                  lease);
-    fprintf(stdout, "tor-fw-helper: NAT-PMP sendnewportmappingrequest returned"
-            " %d (%s)\n", r, r==12?"SUCCESS":"FAILED");
+                                  state->lease);
+    if (tor_fw_options->verbose)
+        fprintf(stdout, "tor-fw-helper: NAT-PMP sendnewportmappingrequest "
+                "returned %d (%s)\n", r, r==12?"SUCCESS":"FAILED");
 
     do {
         FD_ZERO(&fds);
-        FD_SET(natpmp.s, &fds);
-        getnatpmprequesttimeout(&natpmp, &timeout);
-        select(FD_SETSIZE, &fds, NULL, NULL, &timeout);
+        FD_SET(state->natpmp.s, &fds);
+        getnatpmprequesttimeout(&(state->natpmp), &timeout);
+        x = select(FD_SETSIZE, &fds, NULL, NULL, &timeout);
+        if (x == -1)
+        {
+            fprintf(stdout, "V: select failed in "
+                    "tor_natpmp_fetch_public_ip.\n");
+            return -1;
+        }
 
         if (tor_fw_options->verbose)
             fprintf(stdout, "V: attempting to readnatpmpreponseorretry...\n");
-        r = readnatpmpresponseorretry(&natpmp, &response);
+        r = readnatpmpresponseorretry(&(state->natpmp), &(state->response));
         sav_errno = errno;
 
         if (r<0 && r!=NATPMP_TRYAGAIN)
@@ -58,48 +130,70 @@ tor_natpmp_add_tcp_mapping(tor_fw_options_t *tor_fw_options)
 
     } while ( r == NATPMP_TRYAGAIN );
 
+    if (r != 0)
+    {
+        /* XXX TODO: NATPMP_* should be formatted into useful error strings */
+        fprintf(stderr, "E: NAT-PMP It appears that something went wrong:"
+                " %d\n", r);
+        if (r == -51)
+            fprintf(stderr, "E: NAT-PMP It appears that the request was "
+                    "unauthorized\n");
+        return r;
+    }
+
     if (r == NATPMP_SUCCESS) {
         fprintf(stdout, "tor-fw-helper: NAT-PMP mapped public port %hu to"
                 " localport %hu liftime %u\n",
-                response.pnu.newportmapping.mappedpublicport,
-                response.pnu.newportmapping.privateport,
-                response.pnu.newportmapping.lifetime);
+                (state->response).pnu.newportmapping.mappedpublicport,
+                (state->response).pnu.newportmapping.privateport,
+                (state->response).pnu.newportmapping.lifetime);
     }
 
-    x = closenatpmp(&natpmp);
-    if (tor_fw_options->verbose)
-        fprintf(stdout, "V: closing natpmp socket: %d\n", x);
+    tor_fw_options->nat_pmp_status = 1;
+
     return r;
 }
 
+/** Fetch our likely public IP from our upstream NAT-PMP enabled NAT device.
+ * Use the connection context stored in <b>backend_state</b>. */
 int
-tor_natpmp_fetch_public_ip(tor_fw_options_t *tor_fw_options)
+tor_natpmp_fetch_public_ip(tor_fw_options_t *tor_fw_options,
+                           void *backend_state)
 {
     int r = 0;
     int x = 0;
     int sav_errno;
-    natpmp_t natpmp;
-    natpmpresp_t response;
+    natpmp_state_t *state = (natpmp_state_t *) backend_state;
+
     struct timeval timeout;
     fd_set fds;
 
-    r = initnatpmp(&natpmp);
-    if (tor_fw_options->verbose)
-        fprintf(stdout, "V: NAT-PMP init: %d\n", r);
-
-    r = sendpublicaddressrequest(&natpmp);
+    r = sendpublicaddressrequest(&(state->natpmp));
     fprintf(stdout, "tor-fw-helper: NAT-PMP sendpublicaddressrequest returned"
             " %d (%s)\n", r, r==2?"SUCCESS":"FAILED");
 
     do {
+
+        if (state->natpmp.s >= FD_SETSIZE)
+        {
+            fprintf(stderr, "E: NAT-PMP FD_SETSIZE error %d\n",
+                    state->natpmp.s);
+            return -1;
+        }
         FD_ZERO(&fds);
-        FD_SET(natpmp.s, &fds);
-        getnatpmprequesttimeout(&natpmp, &timeout);
-        select(FD_SETSIZE, &fds, NULL, NULL, &timeout);
+        FD_SET(state->natpmp.s, &fds);
+        getnatpmprequesttimeout(&(state->natpmp), &timeout);
+        x = select(FD_SETSIZE, &fds, NULL, NULL, &timeout);
+        if (x == -1)
+        {
+            fprintf(stdout, "V: select failed in "
+                    "tor_natpmp_fetch_public_ip.\n");
+            return -1;
+        }
 
         if (tor_fw_options->verbose)
             fprintf(stdout, "V: NAT-PMP attempting to read reponse...\n");
-        r = readnatpmpresponseorretry(&natpmp, &response);
+        r = readnatpmpresponseorretry(&(state->natpmp), &(state->response));
         sav_errno = errno;
 
         if (tor_fw_options->verbose)
@@ -124,19 +218,18 @@ tor_natpmp_fetch_public_ip(tor_fw_options_t *tor_fw_options)
     }
 
     fprintf(stdout, "tor-fw-helper: ExternalIPAddress = %s\n",
-           inet_ntoa(response.pnu.publicaddress.addr));
-
-    x = closenatpmp(&natpmp);
+           inet_ntoa((state->response).pnu.publicaddress.addr));
+    tor_fw_options->public_ip_status = 1;
 
     if (tor_fw_options->verbose)
     {
         fprintf(stdout, "V: result = %u\n", r);
-        fprintf(stdout, "V: type = %u\n", response.type);
-        fprintf(stdout, "V: resultcode = %u\n", response.resultcode);
-        fprintf(stdout, "V: epoch = %u\n", response.epoch);
-        fprintf(stdout, "V: closing natpmp result: %d\n", r);
+        fprintf(stdout, "V: type = %u\n", (state->response).type);
+        fprintf(stdout, "V: resultcode = %u\n", (state->response).resultcode);
+        fprintf(stdout, "V: epoch = %u\n", (state->response).epoch);
     }
 
     return r;
 }
+#endif
 
diff --git a/src/tools/tor-fw-helper/tor-fw-helper-natpmp.h b/src/tools/tor-fw-helper/tor-fw-helper-natpmp.h
index 8fc1765f9..38448dffa 100644
--- a/src/tools/tor-fw-helper/tor-fw-helper-natpmp.h
+++ b/src/tools/tor-fw-helper/tor-fw-helper-natpmp.h
@@ -2,17 +2,46 @@
  * Copyright (c) 2010, The Tor Project, Inc. */
 /* See LICENSE for licensing information */
 
+/**
+  * \file tor-fw-helper-natpmp.h
+  **/
+
+#ifdef NAT_PMP
 #ifndef _TOR_FW_HELPER_NATPMP_H
 #define _TOR_FW_HELPER_NATPMP_H
 
 #include <natpmp.h>
 
+/** This is the default NAT-PMP lease time in seconds. */
 #define NATPMP_DEFAULT_LEASE 3600
+/** NAT-PMP has many codes for success; this is one of them. */
 #define NATPMP_SUCCESS 0
 
-int tor_natpmp_add_tcp_mapping(tor_fw_options_t *tor_fw_options);
+/** This is our NAT-PMP meta structure - it holds our request data, responses,
+ * various NAT-PMP parameters, and of course the status of the motion in the
+ * NAT-PMP ocean. */
+typedef struct natpmp_state_t {
+    natpmp_t natpmp;
+    natpmpresp_t response;
+    int fetch_public_ip;
+    int status;
+    int init; /**< Have we been initialized? */
+    int protocol; /**< This will only be TCP. */
+    int lease;
+} natpmp_state_t;
+
+const tor_fw_backend_t *tor_fw_get_natpmp_backend(void);
+
+int tor_natpmp_init(tor_fw_options_t *tor_fw_options, void *backend_state);
 
-int tor_natpmp_fetch_public_ip(tor_fw_options_t *tor_fw_options);
+int tor_natpmp_cleanup(tor_fw_options_t *tor_fw_options, void *backend_state);
 
+int tor_natpmp_add_tcp_mapping(tor_fw_options_t *tor_fw_options,
+                               void *backend_state);
+
+int tor_natpmp_fetch_public_ip(tor_fw_options_t *tor_fw_options,
+                               void *backend_state);
+
+#endif
 #endif
 
diff --git a/src/tools/tor-fw-helper/tor-fw-helper-upnp.c b/src/tools/tor-fw-helper/tor-fw-helper-upnp.c
index b471a6cd6..7cf6be916 100644
--- a/src/tools/tor-fw-helper/tor-fw-helper-upnp.c
+++ b/src/tools/tor-fw-helper/tor-fw-helper-upnp.c
@@ -2,17 +2,31 @@
  * Copyright (c) 2010, The Tor Project, Inc. */
 /* See LICENSE for licensing information */
 
+/**
+  * \file tor-fw-helper-upnp.c
+  * \brief The implementation of our UPnP firewall helper.
+  **/
+
+#include "orconfig.h"
+#ifdef MINIUPNPC
 #include <stdint.h>
 #include <string.h>
 #include <stdio.h>
 
+#include <assert.h>
+
+#include "compat.h"
 #include "tor-fw-helper.h"
 #include "tor-fw-helper-upnp.h"
 
+/** UPnP timeout value. */
 #define UPNP_DISCOVER_TIMEOUT 2000
-/* Description of the port mapping in the UPnP table */
+/** Description of the port mapping in the UPnP table. */
 #define UPNP_DESC "Tor relay"
 
+/* XXX TODO: We should print these as a useful user string when we return the
+ * number to a user */
+/** Magic numbers as miniupnpc return codes. */
 #define UPNP_ERR_SUCCESS 0
 #define UPNP_ERR_NODEVICESFOUND 1
 #define UPNP_ERR_NOIGDFOUND 2
@@ -24,14 +38,42 @@
 #define UPNP_ERR_OTHER 8
 #define UPNP_SUCCESS 1
 
+/** This hooks miniupnpc into our multi-backend API. */
+static tor_fw_backend_t tor_miniupnp_backend = {
+    "miniupnp",
+    sizeof(struct miniupnpc_state_t),
+    tor_upnp_init,
+    tor_upnp_cleanup,
+    tor_upnp_fetch_public_ip,
+    tor_upnp_add_tcp_mapping
+};
+
+/** Return the backend for miniupnp. */
+const tor_fw_backend_t *
+tor_fw_get_miniupnp_backend(void)
+{
+    return &tor_miniupnp_backend;
+}
+
+/** Initialize the UPnP backend and store the results in
+ * <b>backend_state</b>.*/
 int
-tor_upnp_init(miniupnpc_state_t *state)
+tor_upnp_init(tor_fw_options_t *options, void *backend_state)
 {
+  /*
+    This leaks the user agent from the client to the router - perhaps we don't
+    want to do that? eg:
+
+        User-Agent: Ubuntu/10.04, UPnP/1.0, MiniUPnPc/1.4
+
+  */
+  miniupnpc_state_t *state = (miniupnpc_state_t *) backend_state;
   struct UPNPDev *devlist;
   int r;
 
   memset(&(state->urls), 0, sizeof(struct UPNPUrls));
   memset(&(state->data), 0, sizeof(struct IGDdatas));
+  state->init = 0;
 
   devlist = upnpDiscover(UPNP_DISCOVER_TIMEOUT, NULL, NULL, 0);
   if (NULL == devlist) {
@@ -39,6 +81,7 @@ tor_upnp_init(miniupnpc_state_t *state)
     return UPNP_ERR_NODEVICESFOUND;
   }
 
+  assert(options);
   r = UPNP_GetValidIGD(devlist, &(state->urls), &(state->data),
                        state->lanaddr, UPNP_LANADDR_SZ);
   fprintf(stdout, "tor-fw-helper: UPnP GetValidIGD returned: %d (%s)\n", r,
@@ -53,9 +96,14 @@ tor_upnp_init(miniupnpc_state_t *state)
   return UPNP_ERR_SUCCESS;
 }
 
+/** Tear down the UPnP connection stored in <b>backend_state</b>.*/
 int
-tor_upnp_cleanup(miniupnpc_state_t *state)
+tor_upnp_cleanup(tor_fw_options_t *options, void *backend_state)
 {
+
+  miniupnpc_state_t *state = (miniupnpc_state_t *) backend_state;
+  assert(options);
+
   if (state->init)
     FreeUPNPUrls(&(state->urls));
   state->init = 0;
@@ -63,14 +111,17 @@ tor_upnp_cleanup(miniupnpc_state_t *state)
   return UPNP_ERR_SUCCESS;
 }
 
+/** Fetch our likely public IP from our upstream UPnP IGD enabled NAT device.
+* Use the connection context stored in <b>backend_state</b>. */
 int
-tor_upnp_fetch_public_ip(miniupnpc_state_t *state)
+tor_upnp_fetch_public_ip(tor_fw_options_t *options, void *backend_state)
 {
+  miniupnpc_state_t *state = (miniupnpc_state_t *) backend_state;
   int r;
   char externalIPAddress[16];
 
   if (!state->init) {
-    r = tor_upnp_init(state);
+    r = tor_upnp_init(options, state);
     if (r != UPNP_ERR_SUCCESS)
       return r;
   }
@@ -84,34 +135,41 @@ tor_upnp_fetch_public_ip(miniupnpc_state_t *state)
 
   if (externalIPAddress[0]) {
     fprintf(stdout, "tor-fw-helper: ExternalIPAddress = %s\n",
-            externalIPAddress); tor_upnp_cleanup(state);
+            externalIPAddress); tor_upnp_cleanup(options, state);
+    options->public_ip_status = 1;
     return UPNP_ERR_SUCCESS;
   } else
     goto err;
 
   err:
-    tor_upnp_cleanup(state);
+    tor_upnp_cleanup(options, state);
     return UPNP_ERR_GETEXTERNALIP;
 }
 
+/** Add a TCP port mapping for a single port stored in <b>tor_fw_options</b>
+ * and store the results in <b>backend_state</b>. */
 int
-tor_upnp_add_tcp_mapping(miniupnpc_state_t *state,
-    uint16_t internal_port, uint16_t external_port)
+tor_upnp_add_tcp_mapping(tor_fw_options_t *options, void *backend_state)
 {
+  miniupnpc_state_t *state = (miniupnpc_state_t *) backend_state;
   int r;
   char internal_port_str[6];
   char external_port_str[6];
 
   if (!state->init) {
-    r = tor_upnp_init(state);
+    r = tor_upnp_init(options, state);
     if (r != UPNP_ERR_SUCCESS)
       return r;
   }
 
-  snprintf(internal_port_str, sizeof(internal_port_str),
-           "%d", internal_port);
-  snprintf(external_port_str, sizeof(external_port_str),
-           "%d", external_port);
+  if (options->verbose)
+      fprintf(stdout, "V: internal port: %d, external port: %d\n",
+              (int)options->internal_port, (int)options->external_port);
+
+  tor_snprintf(internal_port_str, sizeof(internal_port_str),
+           "%d", (int)options->internal_port);
+  tor_snprintf(external_port_str, sizeof(external_port_str),
+           "%d", (int)options->external_port);
 
   r = UPNP_AddPortMapping(state->urls.controlURL,
                           state->data.first.servicetype,
@@ -120,6 +178,8 @@ tor_upnp_add_tcp_mapping(miniupnpc_state_t *state,
   if (r != UPNPCOMMAND_SUCCESS)
     return UPNP_ERR_ADDPORTMAPPING;
 
+  options->upnp_status = 1;
   return UPNP_ERR_SUCCESS;
 }
+#endif
 
diff --git a/src/tools/tor-fw-helper/tor-fw-helper-upnp.h b/src/tools/tor-fw-helper/tor-fw-helper-upnp.h
index aac745231..fdbd2dd18 100644
--- a/src/tools/tor-fw-helper/tor-fw-helper-upnp.h
+++ b/src/tools/tor-fw-helper/tor-fw-helper-upnp.h
@@ -2,6 +2,12 @@
  * Copyright (c) 2010, The Tor Project, Inc. */
 /* See LICENSE for licensing information */
 
+/**
+  * \file tor-fw-helper-upnp.h
+  * \brief The main header for our firewall helper.
+  **/
+
+#ifdef MINIUPNPC
 #ifndef _TOR_FW_HELPER_UPNP_H
 #define _TOR_FW_HELPER_UPNP_H
 
@@ -10,8 +16,11 @@
 #include <miniupnpc/upnpcommands.h>
 #include <miniupnpc/upnperrors.h>
 
+/** This is a magic number for miniupnpc lan address size. */
 #define UPNP_LANADDR_SZ 64
 
+/** This is our miniupnpc meta structure - it holds our request data,
+ * responses, and various miniupnpc parameters. */
 typedef struct miniupnpc_state_t {
   struct UPNPUrls urls;
   struct IGDdatas data;
@@ -19,14 +28,16 @@ typedef struct miniupnpc_state_t {
   int init;
 } miniupnpc_state_t;
 
-int tor_upnp_init(miniupnpc_state_t *state);
+const tor_fw_backend_t *tor_fw_get_miniupnp_backend(void);
+
+int tor_upnp_init(tor_fw_options_t *options, void *backend_state);
 
-int tor_upnp_cleanup(miniupnpc_state_t *state);
+int tor_upnp_cleanup(tor_fw_options_t *options, void *backend_state);
 
-int tor_upnp_fetch_public_ip(miniupnpc_state_t *state);
+int tor_upnp_fetch_public_ip(tor_fw_options_t *options, void *backend_state);
 
-int tor_upnp_add_tcp_mapping(miniupnpc_state_t *state,
-    uint16_t internal_port, uint16_t external_port);
+int tor_upnp_add_tcp_mapping(tor_fw_options_t *options, void *backend_state);
 
 #endif
+#endif
 
diff --git a/src/tools/tor-fw-helper/tor-fw-helper.c b/src/tools/tor-fw-helper/tor-fw-helper.c
index c7cc577e1..968f19f7f 100644
--- a/src/tools/tor-fw-helper/tor-fw-helper.c
+++ b/src/tools/tor-fw-helper/tor-fw-helper.c
@@ -2,6 +2,11 @@
  * Copyright (c) 2010, The Tor Project, Inc. */
 /* See LICENSE for licensing information */
 
+/**
+ * \file tor-fw-helper.c
+ * \brief The main wrapper around our firewall helper logic.
+ **/
+
 /*
  * tor-fw-helper is a tool for opening firewalls with NAT-PMP and UPnP; this
  * tool is designed to be called by hand or by Tor by way of a exec() at a
@@ -13,12 +18,76 @@
 #include <stdlib.h>
 #include <getopt.h>
 #include <time.h>
+#include <string.h>
 
 #include "orconfig.h"
 #include "tor-fw-helper.h"
+#ifdef NAT_PMP
 #include "tor-fw-helper-natpmp.h"
+#endif
+#ifdef MINIUPNPC
 #include "tor-fw-helper-upnp.h"
+#endif
+
+/** This is our meta storage type - it holds information about each helper
+  including the total number of helper backends, function pointers, and helper
+  state. */
+typedef struct backends_t {
+    /** The total number of backends */
+    int n_backends;
+    /** The backend functions as an array */
+    tor_fw_backend_t backend_ops[MAX_BACKENDS];
+    /** The internal backend state */
+    void *backend_state[MAX_BACKENDS];
+} backends_t;
+
+int
+init_backends(tor_fw_options_t *options, backends_t *backends);
 
+/** Initalize each backend helper with the user input stored in <b>options</b>
+ * and put the results in the <b>backends</b> struct. */
+int
+init_backends(tor_fw_options_t *options, backends_t *backends)
+{
+    int n_available = 0;
+    int i, r, n;
+    tor_fw_backend_t *backend_ops_list[MAX_BACKENDS];
+    void *data = NULL;
+    /* First, build a list of the working backends. */
+    n = 0;
+#ifdef MINIUPNPC
+    backend_ops_list[n++] = (tor_fw_backend_t *) tor_fw_get_miniupnp_backend();
+#endif
+#ifdef NAT_PMP
+    backend_ops_list[n++] = (tor_fw_backend_t *) tor_fw_get_natpmp_backend();
+#endif
+    n_available = n;
+
+    /* Now, for each backend that might work, try to initialize it.
+     * That's how we roll, initialized.
+     */
+    n = 0;
+    for (i=0; i<n_available; ++i) {
+        data = calloc(1, backend_ops_list[i]->state_len);
+        if (!data) {
+            perror("calloc");
+            exit(1);
+        }
+        r = backend_ops_list[i]->init(options, data);
+        if (r == 0) {
+            backends->backend_ops[n] = *backend_ops_list[i];
+            backends->backend_state[n] = data;
+            n++;
+        } else {
+            free(data);
+        }
+    }
+    backends->n_backends = n;
+
+    return n;
+}
+
+/** Return the proper commandline switches when the user needs information. */
 static void
 usage(void)
 {
@@ -33,9 +102,10 @@ usage(void)
            " [-p|--external-dir-port [TCP port]]]\n");
 }
 
-/* Log commandline options */
+/** Log commandline options to a hardcoded file <b>tor-fw-helper.log</b> in the
+ * current working directory. */
 static int
-test_commandline_options(int argc, char **argv)
+log_commandline_options(int argc, char **argv)
 {
   int i, retval;
   FILE *logfile;
@@ -71,71 +141,88 @@ test_commandline_options(int argc, char **argv)
     return -1;
 }
 
+/** Iterate over over each of the supported <b>backends</b> and attempt to
+ * fetch the public ip. */
 static void
 tor_fw_fetch_public_ip(tor_fw_options_t *tor_fw_options,
-                       miniupnpc_state_t *miniupnpc_state)
+                       backends_t *backends)
 {
+    int i;
     int r = 0;
-    r = tor_natpmp_fetch_public_ip(tor_fw_options);
-    if (tor_fw_options->verbose)
-        fprintf(stdout, "V: Attempts to fetch public ip (natpmp) resulted in: "
-                "%d\n", r);
-
-    if (r == 0)
-        tor_fw_options->public_ip_status = 1;
 
-    r = tor_upnp_fetch_public_ip(miniupnpc_state);
     if (tor_fw_options->verbose)
-        fprintf(stdout, "V: Attempts to fetch public ip (upnp) resulted in: "
-                "%d\n", r);
-
-    if (r == 0)
-        tor_fw_options->public_ip_status = 1;
+        fprintf(stdout, "V: tor_fw_fetch_public_ip\n");
+
+    for (i=0; i<backends->n_backends; ++i) {
+        if (tor_fw_options->verbose)
+        {
+            fprintf(stdout, "V: running backend_state now: %i\n", i);
+            fprintf(stdout, "V: size of backend state: %u\n",
+                   (int)(backends->backend_ops)[i].state_len);
+            fprintf(stdout, "V: backend state name: %s\n",
+                   (char *)(backends->backend_ops)[i].name);
+        }
+        r = ((backends->backend_ops)[i].fetch_public_ip(tor_fw_options,
+                                        (backends->backend_state)[i]));
+        fprintf(stdout, "tor-fw-helper: tor_fw_fetch_public_ip backend %s "
+                " returned: %i\n", (char *)(backends->backend_ops)[i].name, r);
+    }
 }
 
+/** Iterate over each of the supported <b>backends</b> and attempt to add a
+ * port forward for the OR port stored in <b>tor_fw_options</b>. */
 static void
-tor_fw_add_or_port(tor_fw_options_t *tor_fw_options, miniupnpc_state_t
-                   *miniupnpc_state)
+tor_fw_add_or_port(tor_fw_options_t *tor_fw_options,
+                       backends_t *backends)
 {
+    int i;
     int r = 0;
-    tor_fw_options->internal_port = tor_fw_options->private_or_port;
-    tor_fw_options->external_port = tor_fw_options->public_or_port;
-
-    r = tor_natpmp_add_tcp_mapping(tor_fw_options);
-    fprintf(stdout, "tor-fw-helper: Attempts to add ORPort mapping (natpmp)"
-            "resulted in: %d\n", r);
-
-    if (r == 0)
-        tor_fw_options->nat_pmp_status = 1;
 
-    r = tor_upnp_add_tcp_mapping(miniupnpc_state,
-                                 tor_fw_options->private_or_port,
-                                 tor_fw_options->public_or_port);
-    fprintf(stdout, "tor-fw-helper: Attempts to add ORPort mapping (upnp)"
-            "resulted in: %d\n", r);
-
-    if (r == 0)
-        tor_fw_options->upnp_status = 1;
+    if (tor_fw_options->verbose)
+        fprintf(stdout, "V: tor_fw_add_or_port\n");
+
+    for (i=0; i<backends->n_backends; ++i) {
+        if (tor_fw_options->verbose)
+        {
+            fprintf(stdout, "V: running backend_state now: %i\n", i);
+            fprintf(stdout, "V: size of backend state: %u\n",
+                    (int)(backends->backend_ops)[i].state_len);
+            fprintf(stdout, "V: backend state name: %s\n",
+                    (char *)(backends->backend_ops)[i].name);
+        }
+        r = ((backends->backend_ops)[i].add_tcp_mapping(tor_fw_options,
+                                        (backends->backend_state)[i]));
+        fprintf(stdout, "tor-fw-helper: tor_fw_add_or_port backend %s "
+                "returned: %i\n", (char *)(backends->backend_ops)[i].name, r);
+    }
 }
 
+/** Iterate over each of the supported <b>backends</b> and attempt to add a
+ * port forward for the Dir port stored in <b>tor_fw_options</b>. */
 static void
-tor_fw_add_dir_port(tor_fw_options_t *tor_fw_options, miniupnpc_state_t
-                    *miniupnpc_state)
+tor_fw_add_dir_port(tor_fw_options_t *tor_fw_options,
+                       backends_t *backends)
 {
+    int i;
     int r = 0;
-    tor_fw_options->internal_port = tor_fw_options->private_dir_port;
-    tor_fw_options->external_port = tor_fw_options->public_dir_port;
-
-    r = tor_natpmp_add_tcp_mapping(tor_fw_options);
-    fprintf(stdout, "V: Attempts to add DirPort mapping (natpmp) resulted in: "
-            "%d\n", r);
-
-    r = tor_upnp_add_tcp_mapping(miniupnpc_state,
-                                 tor_fw_options->private_or_port,
-                                 tor_fw_options->public_or_port);
-    fprintf(stdout, "V: Attempts to add DirPort mapping (upnp) resulted in: "
-            "%d\n",
-           r);
+
+    if (tor_fw_options->verbose)
+        fprintf(stdout, "V: tor_fw_add_dir_port\n");
+
+    for (i=0; i<backends->n_backends; ++i) {
+        if (tor_fw_options->verbose)
+        {
+            fprintf(stdout, "V: running backend_state now: %i\n", i);
+            fprintf(stdout, "V: size of backend state: %u\n",
+                    (int)(backends->backend_ops)[i].state_len);
+            fprintf(stdout, "V: backend state name: %s\n",
+                    (char *)(backends->backend_ops)[i].name);
+        }
+        r=((backends->backend_ops)[i].add_tcp_mapping(tor_fw_options,
+                                      (backends->backend_state)[i]));
+        fprintf(stdout, "tor-fw-helper: tor_fw_add_dir_port backend %s "
+                "returned: %i\n", (char *)(backends->backend_ops)[i].name, r);
+    }
 }
 
 int
@@ -145,9 +232,7 @@ main(int argc, char **argv)
    int c = 0;
 
    tor_fw_options_t tor_fw_options = {0,0,0,0,0,0,0,0,0,0,0,0,0};
-   miniupnpc_state_t miniupnpc_state;
-
-   miniupnpc_state.init = 0;
+   backends_t backend_state;
 
    while (1)
    {
@@ -204,7 +289,7 @@ main(int argc, char **argv)
    }
 
    if (tor_fw_options.test_commandline) {
-     return test_commandline_options(argc, argv);
+     return log_commandline_options(argc, argv);
    }
 
    /* At the very least, we require an ORPort;
@@ -256,19 +341,28 @@ main(int argc, char **argv)
                tor_fw_options.public_dir_port);
    }
 
+   // Initalize the various fw-helper backend helpers
+   r = init_backends(&tor_fw_options, &backend_state);
+   if (r)
+          printf("tor-fw-helper: %i NAT traversal helper(s) loaded\n", r);
+
    if (tor_fw_options.fetch_public_ip)
    {
-       tor_fw_fetch_public_ip(&tor_fw_options, &miniupnpc_state);
+       tor_fw_fetch_public_ip(&tor_fw_options, &backend_state);
    }
 
    if (tor_fw_options.private_or_port)
    {
-       tor_fw_add_or_port(&tor_fw_options, &miniupnpc_state);
+       tor_fw_options.internal_port = tor_fw_options.private_or_port;
+       tor_fw_options.external_port = tor_fw_options.private_or_port;
+       tor_fw_add_or_port(&tor_fw_options, &backend_state);
    }
 
    if (tor_fw_options.private_dir_port)
    {
-       tor_fw_add_dir_port(&tor_fw_options, &miniupnpc_state);
+       tor_fw_options.internal_port = tor_fw_options.private_dir_port;
+       tor_fw_options.external_port = tor_fw_options.private_dir_port;
+       tor_fw_add_dir_port(&tor_fw_options, &backend_state);
    }
 
    r = (((tor_fw_options.nat_pmp_status | tor_fw_options.upnp_status)
diff --git a/src/tools/tor-fw-helper/tor-fw-helper.h b/src/tools/tor-fw-helper/tor-fw-helper.h
index 03501459c..1af9c0c7a 100644
--- a/src/tools/tor-fw-helper/tor-fw-helper.h
+++ b/src/tools/tor-fw-helper/tor-fw-helper.h
@@ -2,6 +2,11 @@
  * Copyright (c) 2010, The Tor Project, Inc. */
 /* See LICENSE for licensing information */
 
+/**
+  * \file tor-fw-helper.h
+  * \brief The main header for our firewall helper.
+  **/
+
 #ifndef _TOR_FW_HELPER_H
 #define _TOR_FW_HELPER_H
 
@@ -11,8 +16,15 @@
 #include <getopt.h>
 #include <time.h>
 
+/** The current version of tor-fw-helper. */
 #define tor_fw_version "0.1"
 
+/** This is an arbitrary hard limit - We currently have two (NAT-PMP and UPnP).
+ We're likely going to add the Intel UPnP library but nothing else comes to
+ mind at the moment. */
+#define MAX_BACKENDS 23
+
+/** This is where we store parsed commandline options. */
 typedef struct {
     int verbose;
     int help;
@@ -29,5 +41,17 @@ typedef struct {
     int public_ip_status;
 } tor_fw_options_t;
 
+/** This is our main structure that defines our backend helper API; each helper
+ * must conform to these public methods if it expects to be handled in a
+ * non-special way. */
+typedef struct tor_fw_backend_t {
+    const char *name;
+    size_t state_len;
+    int (*init)(tor_fw_options_t *options, void *backend_state);
+    int (*cleanup)(tor_fw_options_t *options, void *backend_state);
+    int (*fetch_public_ip)(tor_fw_options_t *options, void *backend_state);
+    int (*add_tcp_mapping)(tor_fw_options_t *options, void *backend_state);
+} tor_fw_backend_t;
+
 #endif