Backport candidate: Fix a long-standing server-side DNS bug. When a

client asks us to resolve (not connect to) an address, and we have a cached answer, give them the cached answer. Previously, we would give them no answer at all. svn:r8478
2024-11-27 22:03:31 +01:00 · 2006-09-24 17:05:00 +00:00 · 2006-09-24 17:05:00 +00:00 · d3af1f2189
commit d3af1f2189
parent a951c015fe
5 changed files with 63 additions and 26 deletions
--- a/5
+++ b/5
@ -24,6 +24,11 @@ Changes in version 0.1.2.2-alpha - 2006-??-??
      descriptor for a named server with that name, we might return an old
      one.

+  o Major bugfixes
+    - When a client asks us to resolve (not connect to) an address,
+      and we have a cached answer, give them the cached answer.
+      Previously, we would give them no answer at all.
+
  o Minor Bugfixes
    - Small performance improvements on parsing descriptors (x2).
    - Major performance descriptor on inserting descriptors; change
--- a/doc/TODO
+++ b/doc/TODO
@ -111,7 +111,7 @@ d     - Special-case localhost?
      o Connect to resolve cells, server-side.
      o Add element to routerinfo to note routers that aren't using eventdns,
        so we can avoid sending them reverse DNS etc.
-      - Fix the bug with server-side caching, whatever is causing it.
+      o Fix the bug with server-side caching, whatever is causing it.
      . Add client-side interface
        o SOCKS interface: specify
        o SOCKS interface: implement
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@ -1899,7 +1899,7 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
  log_debug(LD_EXIT,"about to start the dns_resolve().");

  /* send it off to the gethostbyname farm */
-  switch (dns_resolve(n_stream)) {
+  switch (dns_resolve(n_stream, NULL)) {
    case 1: /* resolve worked */

      /* add it into the linked list of n_streams on this circuit */
@ -1912,6 +1912,7 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
      connection_exit_connect(n_stream);
      return 0;
    case -1: /* resolve failed */
+      /*  XXXX send back indication of failure for connect case? -NM*/
      /* n_stream got freed. don't touch it. */
      break;
    case 0: /* resolve added to pending list */
@ -1954,7 +1955,7 @@ connection_exit_begin_resolve(cell_t *cell, or_circuit_t *circ)
  dummy_conn->_base.purpose = EXIT_PURPOSE_RESOLVE;

  /* send it off to the gethostbyname farm */
-  switch (dns_resolve(dummy_conn)) {
+  switch (dns_resolve(dummy_conn, circ)) {
    case -1: /* Impossible to resolve; a resolved cell was sent. */
      /* Connection freed; don't touch it. */
      return 0;
--- a/src/or/dns.c
+++ b/src/or/dns.c
@ -113,8 +113,9 @@ static void purge_expired_resolves(time_t now);
 static void dns_found_answer(const char *address, int is_reverse,
                             uint32_t addr, const char *hostname, char outcome,
                             uint32_t ttl);
-static void send_resolved_cell(edge_connection_t *conn, uint8_t answer_type);
-static int launch_resolve(edge_connection_t *exitconn);
+static void send_resolved_cell(edge_connection_t *conn, or_circuit_t *circ,
+			       uint8_t answer_type);
+static int launch_resolve(edge_connection_t *exitconn, or_circuit_t *circ);
 #ifndef USE_EVENTDNS
 static void dnsworkers_rotate(void);
 static void dnsworker_main(void *data);
@ -385,9 +386,15 @@ purge_expired_resolves(time_t now)
 }

 /** Send a response to the RESOLVE request of a connection. answer_type must
- *  be one of RESOLVED_TYPE_(IPV4|ERROR|ERROR_TRANSIENT) */
+ * be one of RESOLVED_TYPE_(IPV4|ERROR|ERROR_TRANSIENT)
+ *
+ * If <b>circ</b> is provided, and we have a cached answer, send the
+ * answer back along circ; otherwise, send the answer back along *
+ * <b>exitconn</b>'s attached circuit.
+ */
 static void
-send_resolved_cell(edge_connection_t *conn, uint8_t answer_type)
+send_resolved_cell(edge_connection_t *conn, or_circuit_t *circ,
+		   uint8_t answer_type)
 {
  char buf[RELAY_PAYLOAD_SIZE];
  size_t buflen;
@ -421,7 +428,14 @@ send_resolved_cell(edge_connection_t *conn, uint8_t answer_type)
      return;
    }
  // log_notice(LD_EXIT, "Sending a regular RESOLVED reply: ");
-  connection_edge_send_command(conn, circuit_get_by_edge_conn(conn),
+  
+  if (!circ) {
+    circuit_t *tmp = circuit_get_by_edge_conn(conn);
+    if (! CIRCUIT_IS_ORIGIN(tmp))
+      circ = TO_OR_CIRCUIT(tmp);
+  }
+
+  connection_edge_send_command(conn, TO_CIRCUIT(circ),
                               RELAY_COMMAND_RESOLVED, buf, buflen,
                               conn->cpath_layer);
 }
@ -429,9 +443,14 @@ send_resolved_cell(edge_connection_t *conn, uint8_t answer_type)
 /** Send a response to the RESOLVE request of a connection for an in-addr.arpa
 * address on connection <b>conn</b> which yielded the result <b>hostname</b>.
 * The answer type will be RESOLVED_HOSTNAME.
+ *
+ * If <b>circ</b> is provided, and we have a cached answer, send the
+ * answer back along circ; otherwise, send the answer back along
+ * <b>exitconn</b>'s attached circuit.
 */
 static void
-send_resolved_hostname_cell(edge_connection_t *conn, const char *hostname)
+send_resolved_hostname_cell(edge_connection_t *conn, or_circuit_t *circ,
+			    const char *hostname)
 {
  char buf[RELAY_PAYLOAD_SIZE];
  size_t buflen;
@ -447,8 +466,14 @@ send_resolved_hostname_cell(edge_connection_t *conn, const char *hostname)
  set_uint32(buf+2+namelen, htonl(ttl));
  buflen = 2+namelen+4;

+  if (!circ) {
+    circuit_t *tmp = circuit_get_by_edge_conn(conn);
+    if (! CIRCUIT_IS_ORIGIN(tmp))
+      circ = TO_OR_CIRCUIT(tmp);
+  }
+
  // log_notice(LD_EXIT, "Sending a reply RESOLVED reply: %s", hostname);
-  connection_edge_send_command(conn, circuit_get_by_edge_conn(conn),
+  connection_edge_send_command(conn, TO_CIRCUIT(circ),
                               RELAY_COMMAND_RESOLVED, buf, buflen,
                               conn->cpath_layer);
  // log_notice(LD_EXIT, "Sent");
@ -500,6 +525,10 @@ parse_inaddr_arpa_address(const char *address, struct in_addr *in)
 * if resolve valid, put it into <b>exitconn</b>-\>addr and return 1.
 * If resolve failed, unlink exitconn if needed, free it, and return -1.
 *
+ * If <b>circ</b> is provided, and this is a resolve request, we have
+ * a cached answer, send the answer back along circ; otherwise, send
+ * the answer back along <b>exitconn</b>'s attached circuit.
+ *
 * Else, if seen before and pending, add conn to the pending list,
 * and return 0.
 *
@ -507,7 +536,7 @@ parse_inaddr_arpa_address(const char *address, struct in_addr *in)
 * dns farm, and return 0.
 */
 int
-dns_resolve(edge_connection_t *exitconn)
+dns_resolve(edge_connection_t *exitconn, or_circuit_t *oncirc)
 {
  cached_resolve_t *resolve;
  cached_resolve_t search;
@ -529,7 +558,7 @@ dns_resolve(edge_connection_t *exitconn)
    exitconn->_base.addr = ntohl(in.s_addr);
    exitconn->address_ttl = DEFAULT_DNS_TTL;
    if (is_resolve)
-      send_resolved_cell(exitconn, RESOLVED_TYPE_IPV4);
+      send_resolved_cell(exitconn, oncirc, RESOLVED_TYPE_IPV4);
    return 1;
  }

@ -566,7 +595,7 @@ dns_resolve(edge_connection_t *exitconn)
 #endif

      if (exitconn->_base.purpose == EXIT_PURPOSE_RESOLVE)
-        send_resolved_cell(exitconn, RESOLVED_TYPE_ERROR);
+        send_resolved_cell(exitconn, oncirc, RESOLVED_TYPE_ERROR);
      circ = circuit_get_by_edge_conn(exitconn);
      if (circ)
        circuit_detach_stream(circ, exitconn);
@ -602,19 +631,21 @@ dns_resolve(edge_connection_t *exitconn)
        exitconn->address_ttl = resolve->ttl;
        if (resolve->is_reverse) {
          tor_assert(is_resolve);
-          send_resolved_hostname_cell(exitconn, resolve->result.hostname);
+          send_resolved_hostname_cell(exitconn, oncirc,
+				      resolve->result.hostname);
        } else {
          exitconn->_base.addr = resolve->result.addr;
          if (is_resolve)
-            send_resolved_cell(exitconn, RESOLVED_TYPE_IPV4);
+            send_resolved_cell(exitconn, oncirc, RESOLVED_TYPE_IPV4);
        }
        return 1;
      case CACHE_STATE_CACHED_FAILED:
        log_debug(LD_EXIT,"Connection (fd %d) found cached error for %s",
                  exitconn->_base.s,
                  escaped_safe_str(exitconn->_base.address));
+	/*  XXXX send back indication of failure for connect case? -NM*/
        if (is_resolve)
-          send_resolved_cell(exitconn, RESOLVED_TYPE_ERROR);
+          send_resolved_cell(exitconn, oncirc, RESOLVED_TYPE_ERROR);
        circ = circuit_get_by_edge_conn(exitconn);
        if (circ)
          circuit_detach_stream(circ, exitconn);
@ -647,7 +678,7 @@ dns_resolve(edge_connection_t *exitconn)
  log_debug(LD_EXIT,"Launching %s.",
            escaped_safe_str(exitconn->_base.address));
  assert_cache_ok();
-  return launch_resolve(exitconn);
+  return launch_resolve(exitconn, oncirc);
 }

 /** Log an error and abort if conn is waiting for a DNS resolve.
@ -903,7 +934,7 @@ dns_found_answer(const char *address, int is_reverse, uint32_t addr,
        /* This detach must happen after we send the end cell. */
        circuit_detach_stream(circuit_get_by_edge_conn(pendconn), pendconn);
      } else {
-        send_resolved_cell(pendconn, RESOLVED_TYPE_ERROR);
+        send_resolved_cell(pendconn, NULL, RESOLVED_TYPE_ERROR);
        /* This detach must happen after we send the resolved cell. */
        circuit_detach_stream(circuit_get_by_edge_conn(pendconn), pendconn);
      }
@ -930,9 +961,9 @@ dns_found_answer(const char *address, int is_reverse, uint32_t addr,
         * but it does the right thing. */
        pendconn->_base.state = EXIT_CONN_STATE_RESOLVEFAILED;
        if (is_reverse)
-          send_resolved_hostname_cell(pendconn, hostname);
+          send_resolved_hostname_cell(pendconn, NULL, hostname);
        else
-          send_resolved_cell(pendconn, RESOLVED_TYPE_IPV4);
+          send_resolved_cell(pendconn, NULL, RESOLVED_TYPE_IPV4);
        circ = circuit_get_by_edge_conn(pendconn);
        tor_assert(circ);
        circuit_detach_stream(circ, pendconn);
@ -963,7 +994,7 @@ dns_found_answer(const char *address, int is_reverse, uint32_t addr,
 * <b>exitconn</b>-\>address; tell that dns worker to begin resolving.
 */
 static int
-launch_resolve(edge_connection_t *exitconn)
+launch_resolve(edge_connection_t *exitconn, or_circuit_t *circ)
 {
  connection_t *dnsconn;
  unsigned char len;
@ -983,7 +1014,7 @@ launch_resolve(edge_connection_t *exitconn)
  if (!dnsconn) {
    log_warn(LD_EXIT,"no idle dns workers. Failing.");
    if (exitconn->_base.purpose == EXIT_PURPOSE_RESOLVE)
-      send_resolved_cell(exitconn, RESOLVED_TYPE_ERROR_TRANSIENT);
+      send_resolved_cell(exitconn, circ, RESOLVED_TYPE_ERROR_TRANSIENT);
    goto err;
  }

@ -1533,7 +1564,7 @@ eventdns_callback(int result, char type, int count, int ttl, void *addresses,
 /** For eventdns: start resolving as necessary to find the target for
 * <b>exitconn</b> */
 static int
-launch_resolve(edge_connection_t *exitconn)
+launch_resolve(edge_connection_t *exitconn, or_circuit_t *circ)
 {
  char *addr = tor_strdup(exitconn->_base.address);
  struct in_addr in;
@ -1568,10 +1599,10 @@ launch_resolve(edge_connection_t *exitconn)
             escaped_safe_str(addr), r);
    if (exitconn->_base.purpose == EXIT_PURPOSE_RESOLVE) {
      if (eventdns_err_is_transient(r))
-        send_resolved_cell(exitconn, RESOLVED_TYPE_ERROR_TRANSIENT);
+        send_resolved_cell(exitconn, circ, RESOLVED_TYPE_ERROR_TRANSIENT);
      else {
        exitconn->address_ttl = DEFAULT_DNS_TTL;
-        send_resolved_cell(exitconn, RESOLVED_TYPE_ERROR);
+        send_resolved_cell(exitconn, circ, RESOLVED_TYPE_ERROR);
      }
    }
    dns_cancel_pending_resolve(addr); /* also sends end and frees */
--- a/src/or/or.h
+++ b/src/or/or.h
@ -2160,7 +2160,7 @@ void connection_dns_remove(edge_connection_t *conn);
 void assert_connection_edge_not_dns_pending(edge_connection_t *conn);
 void assert_all_pending_dns_resolves_ok(void);
 void dns_cancel_pending_resolve(const char *question);
-int dns_resolve(edge_connection_t *exitconn);
+int dns_resolve(edge_connection_t *exitconn, or_circuit_t *circ);
 void dns_launch_wildcard_checks(void);

 /********************************* hibernate.c **********************/