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

1 files changed, 74 insertions, 14 deletions

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 b471a6cd6f..7cf6be9160 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