r11713@Kushana: nickm | 2006-12-24 22:42:08 -0500

 Better handling of internal addresses wrt X-Your-Address-Is (never believe them; never provide them.) Also, report something useful for X-Your-Address-Is with one-hop tunneled connections.


svn:r9191

6 files changed, 34 insertions, 5 deletions

diff --git a/ChangeLog b/ChangeLog
index c45988ee60..45f8c8b429 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -62,6 +62,9 @@ Changes in version 0.1.2.5-xxxx - 200?-??-??
     - When we get a 503 from a directory, and we're not a server, we don't
       count the failure against the total number of failures allowed for the
       thing we're trying to download.
+    - Report X-Your-Address-Is correctly from tunneled directory connections;
+      don't report X-Your-Address-Is is when it's an internal address; and
+      never believe reported remote addresses when they're internal.
 
   o Security bugfixes:
     - Stop sending the HttpProxyAuthenticator string to directory
diff --git a/doc/TODO b/doc/TODO
index 9742e58522..5b63e0d74b 100644
--- a/doc/TODO
+++ b/doc/TODO
@@ -63,6 +63,8 @@ R     - handle connect-dir streams that don't have a chosen_exit_name set.
           key=value syntax. so we could have a 'tor' version, but we
           could also have a 'conn' version, a 'dir' version, etc down
           the road. and one day maybe the 'tor' key would be deprecated.
+    o Give the right answer for X-Your-Address-Is on tunneled directory
+      connections.
 
   o Document .noconnect addresses...
     A new file 'address-spec.txt' that describes .exit, .onion,
diff --git a/doc/dir-spec.txt b/doc/dir-spec.txt
index c72cf331e8..207d4a5199 100644
--- a/doc/dir-spec.txt
+++ b/doc/dir-spec.txt
@@ -854,6 +854,10 @@ $Id$
 
   Servers MAY include an X-Your-Address-Is: header, whose value is the
   apparent IP address of the client connecting to them (as a dotted quad).
+  For directory connections tunneled over a BEGIN_DIR stream, servers SHOULD
+  report the IP from which the circuit carrying the BEGIN_DIR stream reached
+  them.  [Servers before version 0.1.2.5-alpha reported 127.0.0.1 for all
+  BEGIN_DIR-tunneled connections.]
 
   Servers SHOULD disable caching of multiple network statuses or multiple
   router descriptors.  Servers MAY enable caching of single descriptors,
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 5b4b9a245e..2c6f3205fb 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -1963,8 +1963,11 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
   char *address=NULL;
   uint16_t port;
   char end_payload[1];
+  or_circuit_t *or_circ = NULL;
 
   assert_circuit_ok(circ);
+  if (!CIRCUIT_IS_ORIGIN(circ))
+    or_circ = TO_OR_CIRCUIT(circ);
 
   relay_header_unpack(&rh, cell->payload);
 
@@ -2022,7 +2025,7 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
       return 0;
     }
 #endif
-    if (!CIRCUIT_IS_ORIGIN(circ) && TO_OR_CIRCUIT(circ)->is_first_hop) {
+    if (or_circ && or_circ->is_first_hop) {
       /* Don't let clients use us as a single-hop proxy; it attracts attackers
        * and users who'd be better off with, well, single-hop proxies.
        */
@@ -2043,7 +2046,10 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
                                    end_payload, 1, NULL);
       return 0;
     }
-    address = tor_strdup("127.0.0.1");
+    if (or_circ && or_circ->p_conn && or_circ->p_conn->_base.address)
+      address = tor_strdup(or_circ->p_conn->_base.address);
+    else
+      address = tor_strdup("127.0.0.1");
   } else {
     log_warn(LD_BUG, "Got an unexpected command %d", (int)rh.command);
     end_payload[0] = END_STREAM_REASON_INTERNAL;
@@ -2112,6 +2118,8 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
   log_debug(LD_EXIT,"about to start the dns_resolve().");
 
   if (rh.command == RELAY_COMMAND_BEGIN_DIR) {
+    if (or_circ && or_circ->p_conn && or_circ->p_conn->_base.addr)
+      n_stream->_base.addr = or_circ->p_conn->_base.addr;
     n_stream->next_stream = TO_OR_CIRCUIT(circ)->n_streams;
     n_stream->on_circuit = circ;
     TO_OR_CIRCUIT(circ)->n_streams = n_stream;
diff --git a/src/or/directory.c b/src/or/directory.c
index 39f2407a9f..27b90fdac1 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -1353,10 +1353,15 @@ write_http_response_header(dir_connection_t *conn, ssize_t length,
   format_rfc1123_time(date, now);
   cp = tmp;
   tor_snprintf(cp, sizeof(tmp),
-               "HTTP/1.0 200 OK\r\nDate: %s\r\nContent-Type: %s\r\n"
-               X_ADDRESS_HEADER "%s\r\n",
-               date, type, conn->_base.address);
+               "HTTP/1.0 200 OK\r\nDate: %s\r\nContent-Type: %s\r\n",
+               date, type);
   cp += strlen(tmp);
+  if (!is_internal_IP(conn->_base.addr, 0)) {
+    /* Don't report the source address for a localhost/private connection. */
+    tor_snprintf(cp, sizeof(tmp)-(cp-tmp),
+                 X_ADDRESS_HEADER "%s\r\n", conn->_base.address);
+    cp += strlen(cp);
+  }
   if (encoding) {
     tor_snprintf(cp, sizeof(tmp)-(cp-tmp),
                  "Content-Encoding: %s\r\n", encoding);
diff --git a/src/or/router.c b/src/or/router.c
index b0a3c5fef1..409dcf29bb 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -1034,7 +1034,14 @@ router_new_address_suggestion(const char *suggestion)
     last_guessed_ip = cur; /* store it in case we need it later */
     return;
   }
+  if (is_internal_IP(addr, 0)) {
+    /* Don't believe anybody who says our IP is, say, 127.0.0.1. */
+    return;
+  }
 
+  /* Okay.  We can't resolve our own address, and X-Your-Address-Is is giving
+   * us an answer different from what we had the last time we managed to
+   * resolve it. */
   if (last_guessed_ip != addr) {
     log_addr_has_changed(LOG_NOTICE, last_guessed_ip, addr);
     server_has_changed_ip();