r11895@catbus: nickm | 2007-02-23 15:12:58 -0500

Make remap stream events have a souce; make them generated every time we get a successful connected or resolved cell.  Also change reported stream target address to IP consistently when we get the IP from an exit node.


svn:r9624

ChangeLog

@@ -20,6 +20,8 @@ Changes in version 0.1.2.8-alpha - 2007-??-??
     - Report events where a resolve succeeded or where we got a socks
       protocol error correctly, rather than calling both of them
       "INTERNAL".
+    - Change reported stream target addresses to IP consistently when
+      we finally get the IP from an exit node.
 
   o Minor bugfixes (other):
     - Display correct results when reporting which versions are
@@ -61,6 +63,9 @@ Changes in version 0.1.2.8-alpha - 2007-??-??
     - Rate-limit SIGNEWNYM events in response to controllers that impolitely
       generate them for every single stream.  (Patch from mwenge; closes bug
       394.)
+    - Make REMAP stream events have a SOURCE (cache or exit), and make them
+      generated in every case where we get a successful connected or resolved
+      cell.
 
   o Minor bugfixes (performance):
     - Call router_have_min_dir_info half as often.  (This is showing up in

doc/TODO

@@ -72,8 +72,8 @@ R   - actually cause the directory.c functions to know about or_port
         should abandon.
       - update dir-spec with what we decided for each of these
 
-N - Controller fixes
-    - Make REMAP stream events have a SOURCE (cache or exit), and make them
+  o Controller fixes
+    o Make REMAP stream events have a SOURCE (cache or exit), and make them
       generated in every case where we get a connected or resolved cell.
 
 N - Libevent fixes

doc/spec/control-spec.txt

@@ -828,7 +828,8 @@ $Id$
     The syntax is:
 
       "650" SP "STREAM" SP StreamID SP StreamStatus SP CircID SP Target
-          [SP "REASON=" Reason [ SP "REMOTE_REASON=" Reason ]] CRLF
+          [SP "REASON=" Reason [ SP "REMOTE_REASON=" Reason ]]
+          [SP "SOURCE=" Source] CRLF
 
       StreamStatus =
                "NEW"          / ; New request to connect
@@ -858,12 +859,20 @@ $Id$
 
       END          (We received a RELAY_END cell from the other side of thise
                     stream.)
+      [XXXX document more.]
 
    The "REMOTE_REASON" field is provided only when we receive a RELAY_END
    cell, and only if extended events are enabled.  It contains the actual
    reason given by the remote OR for closing the stream. Clients MUST accept
    reasons not listed above.  Reasons are as listed in tor-spec.txt.
 
+   "REMAP" events include a Source if extended events are enabled:
+      Source = "CACHE" / "EXIT"
+   Clients MUST accept sources not listed above.  "CACHE" is given if
+   the Tor client decided to remap the address because of a cached
+   answer, and "EXIT" is given if the remote node we queried gave us
+   the new address as a response.
+
 4.1.3. OR Connection status changed
 
   The syntax is:
@@ -930,9 +939,6 @@ $Id$
   [XXX We should rename this to ADDRESSMAP. -RD]
     [Why? Surely it can't be worth the compatibility issues. -NM]
 
-  [FFF We should add a SOURCE=%s argument for extended events,
-   which specifies what exit node told us this addressmap. -RD]
-
 4.1.8. Descriptors uploaded to us in our role as authoritative dirserver
 
   Syntax:

src/or/connection_edge.c

@@ -1212,7 +1212,8 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn,
   } else {
     /* For address map controls, remap the address */
     if (addressmap_rewrite(socks->address, sizeof(socks->address))) {
-      control_event_stream_status(conn, STREAM_EVENT_REMAP, 0);
+      control_event_stream_status(conn, STREAM_EVENT_REMAP,
+                                  REMAP_STREAM_SOURCE_CACHE);
     }
   }
 

src/or/control.c

@@ -3309,6 +3309,19 @@ control_event_stream_status(edge_connection_t *conn, stream_status_event_t tp,
         tor_snprintf(reason_buf, sizeof(reason_buf),
                      "REASON=%s", reason_str);
       tor_free(r);
+    } else if (reason_code && tp == STREAM_EVENT_REMAP) {
+      switch (reason_code) {
+        case REMAP_STREAM_SOURCE_CACHE:
+          strlcpy(reason_buf, "SOURCE=CACHE", sizeof(reason_buf));
+          break;
+        case REMAP_STREAM_SOURCE_EXIT:
+          strlcpy(reason_buf, "SOURCE=EXIT", sizeof(reason_buf));
+          break;
+        default:
+          tor_snprintf(reason_buf, sizeof(reason_buf), "REASON=UNKNOWN_%d",
+                       reason_code);
+          break;
+      }
     }
     circ = circuit_get_by_edge_conn(conn);
     if (circ && CIRCUIT_IS_ORIGIN(circ))

src/or/or.h

@@ -562,6 +562,13 @@ typedef enum {
  * connection_mark_unattached_ap(). */
 #define END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED 2048
 
+/** Reason for remapping an AP connection's address: we have a cached
+ * answer. */
+#define REMAP_STREAM_SOURCE_CACHE 1
+/** Reason for remapping an AP connection's address: the exit node told us an
+ * answer. */
+#define REMAP_STREAM_SOURCE_EXIT 2
+
 /* 'type' values to use in RESOLVED cells.  Specified in tor-spec.txt */
 #define RESOLVED_TYPE_HOSTNAME 0
 #define RESOLVED_TYPE_IPV4 4

src/or/relay.c

@@ -829,6 +829,21 @@ connection_edge_process_end_not_open(
   return 0;
 }
 
+/** Helper: change the socks_request->address field on conn to the dotted-quad
+ * representation of <b>new_addr</b> (given in host order), and send an
+ * appropriate REMAP event. */
+static void
+remap_event_helper(edge_connection_t *conn, uint32_t new_addr)
+{
+  struct in_addr in;
+
+  in.s_addr = htonl(new_addr);
+  tor_inet_ntoa(&in, conn->socks_request->address,
+                sizeof(conn->socks_request->address));
+  control_event_stream_status(conn, STREAM_EVENT_REMAP,
+                              REMAP_STREAM_SOURCE_EXIT);
+}
+
 /** An incoming relay cell has arrived from circuit <b>circ</b> to
  * stream <b>conn</b>.
  *
@@ -858,7 +873,7 @@ connection_edge_process_relay_cell_not_open(
              "Got 'connected' while not in state connect_wait. Dropping.");
       return 0;
     }
-//    log_fn(LOG_INFO,"Connected! Notifying application.");
+    // log_fn(LOG_INFO,"Connected! Notifying application.");
     conn->_base.state = AP_CONN_STATE_OPEN;
     log_info(LD_APP,"'connected' received after %d seconds.",
              (int)(time(NULL) - conn->_base.timestamp_lastread));
@@ -879,6 +894,8 @@ connection_edge_process_relay_cell_not_open(
         ttl = -1;
       client_dns_set_addressmap(conn->socks_request->address, addr,
                                 conn->chosen_exit_name, ttl);
+
+      remap_event_helper(conn, addr);
     }
     circuit_log_path(LOG_INFO,LD_APP,TO_ORIGIN_CIRCUIT(circ));
     /* don't send a socks reply to transparent conns */
@@ -896,6 +913,7 @@ connection_edge_process_relay_cell_not_open(
       rh->command == RELAY_COMMAND_RESOLVED) {
     int ttl;
     int answer_len;
+    uint8_t answer_type;
     if (conn->_base.state != AP_CONN_STATE_RESOLVE_WAIT) {
       log_fn(LOG_PROTOCOL_WARN, LD_APP, "Got a 'resolved' cell while "
              "not in state resolve_wait. Dropping.");
@@ -914,11 +932,17 @@ connection_edge_process_relay_cell_not_open(
                                   2+answer_len));
     else
       ttl = -1;
+
+    answer_type = cell->payload[RELAY_HEADER_SIZE];
     connection_ap_handshake_socks_resolved(conn,
-                   cell->payload[RELAY_HEADER_SIZE], /*answer_type*/
+                   answer_type,
                    cell->payload[RELAY_HEADER_SIZE+1], /*answer_len*/
                    cell->payload+RELAY_HEADER_SIZE+2, /*answer*/
                    ttl);
+    if (answer_type == RESOLVED_TYPE_IPV4) {
+      uint32_t addr = ntohl(get_uint32(cell->payload+RELAY_HEADER_SIZE+2));
+      remap_event_helper(conn, addr);
+    }
     connection_mark_unattached_ap(conn,
                               END_STREAM_REASON_DONE |
                               END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED);