Opened 12 years ago

Last modified 2 years ago

#449 accepted defect (None)

dns failures prevent legitimate options being set

Reported by: mwenge Owned by: nickm
Priority: Low Milestone: Tor: unspecified
Component: Core Tor/Tor Version: 0.2.0.2-alpha
Severity: Normal Keywords: dns, tor-hs, tor-client, tor-relay
Cc: mwenge, nickm, arma Actual Points:
Parent ID: Points: 4
Reviewer: Sponsor:

Description (last modified by arma)

Outright hostname lookup failures for previously configured hidden services prevent other options being set
while DNS is down.

For example, I configure a hidden service redirecting to google.com while DNS is working. DNS subsequently stops working,
e.g. nameserver becomes completely unreachable. If I then attempt to set a config option using the controller, it will
not get set as long as tor cannot resolve the hidden service name.

Rejection of hidden service configurations (and hence any subsequent or unrelated config change) made while tor is running
needs to be more tolerant of lookup failures.

The following attempts to validate the hidden service config currently in use (and previously validated when DNS was working).
If the validation fails, it must be because DNS is down, so the existing config is retained. If the user was attempting to add
a new hidden service config, then it doesn't get added.

Index: src/or/config.c
===================================================================
--- src/or/config.c     (revision 10545)
+++ src/or/config.c     (working copy)
@@ -963,10 +963,15 @@
     }
   }

-  if (running_tor && rend_config_services(options, 0)<0) {
-    log_warn(LD_BUG,
-       "Previously validated hidden services line could not be added!");
-    return -1;
+  if (running_tor && rend_config_services(options, 1)<0) {
+    log_warn(LD_CONFIG,
+       "Previously validated hidden services line no longer valid! Retaining existing hidden services config if there is one.");
+  }else{
+    if (rend_config_services(options, 0)<0){
+        log_warn(LD_BUG,
+           "Previously validated hidden services line could not be added!");
+        return -1;
+    }
   }

   if (running_tor) {
@@ -2920,9 +2925,10 @@
     }
   }

+/*
   if (rend_config_services(options, 1) < 0)
     REJECT("Failed to configure rendezvous options. See logs for details.");
-
+*/
   if (parse_virtual_addr_network(options->VirtualAddrNetwork, 1, NULL)<0)
     return -1;

[Automatically added by flyspray2trac: Operating System: All]

Child Tickets

Change History (15)

comment:1 Changed 12 years ago by mwenge

This one actually retains the existing configuration (so far as I can tell):

Index: src/or/config.c
===================================================================
--- src/or/config.c     (revision 10545)
+++ src/or/config.c     (working copy)
@@ -557,6 +557,9 @@
                *(uint32_t*)STRUCT_VAR_P(cfg,fmt->magic_offset));        \
   } while (0)

+static int config_assign(config_format_t *fmt, void *options,
+                   config_line_t *c, int use_defaults,
+                   int clear_first, char **msg);
 static void config_line_append(config_line_t **lst,
                                const char *key, const char *val);
 static void option_clear(config_format_t *fmt, or_options_t *options,
@@ -963,10 +966,16 @@
     }
   }

-  if (running_tor && rend_config_services(options, 0)<0) {
-    log_warn(LD_BUG,
-       "Previously validated hidden services line could not be added!");
-    return -1;
+  if (running_tor){
+    if (rend_config_services(options, 1)<0) {
+      log_warn(LD_CONFIG,
+         "Hidden services line could not be validated! Retaining existing hidden services config.");
+      config_assign(&options_format, options, old_options->RendConfigLines, 0, 1, &msg);
+    }else if (rend_config_services(options, 0)<0) {
+      log_warn(LD_CONFIG,
+         "Previously validated hidden services line could not be added!");
+      return -1;
+    }
   }

   if (running_tor) {
@@ -2919,10 +2928,10 @@
         REJECT("Bridge line did not parse. See logs for details.");
     }
   }
-
+/*
   if (rend_config_services(options, 1) < 0)
     REJECT("Failed to configure rendezvous options. See logs for details.");
-
+*/
   if (parse_virtual_addr_network(options->VirtualAddrNetwork, 1, NULL)<0)
     return -1;
Last edited 2 years ago by arma (previous) (diff)

comment:2 Changed 12 years ago by nickm

This looks like a consequence of the stuff we've done for bug 393.

comment:3 Changed 11 years ago by arma

This looks like a way for Tor to "silently" disappear when transient problems appear
on the machine.

My thought is that we should make Tor warn, but not exit, when a hidden service
specifies a hostname and it doesn't resolve. Even at boot.

And if somebody tries to access the hidserv while its destination doesn't resolve,
send back a stream-reason END_STREAM_REASON_EXITPOLICY. (This is currently what you
get if you ask for a hidserv port that isn't configured.)

comment:4 Changed 11 years ago by arma

Erm. My above comment should be taken for once we've fixed bug 393 too.

In the mean time, robert's patch above has two problems that I can
see:

a) in the first clause it needs to check if old_options is
defined before using it. (When we're starting Tor for the first time,
it's NULL.)

b) related, how does it handle the case where it doesn't resolve at boot?
Not very well it seems.

Oh, and c) there's still an (albeit rarer) case where the behavior Robert
doesn't like shows up, if the destination resolves flakily.

comment:5 Changed 11 years ago by nickm

see also bug 393

comment:6 Changed 9 years ago by nickm

Description: modified (diff)
Keywords: dns hidden service added
Milestone: post 0.2.1.xTor: unspecified
Status: assignedaccepted

comment:7 Changed 7 years ago by nickm

Keywords: tor-client added

comment:8 Changed 7 years ago by nickm

Component: Tor ClientTor

comment:9 Changed 4 years ago by teor

Cc: mwenge,nickm,armamwenge, nickm, arma

Related to #16372 - tor uses getaddrinfo even if DisableNetwork is set

comment:10 Changed 2 years ago by dgoulet

Keywords: tor-hs added; hidden service removed
Severity: Normal

comment:11 Changed 2 years ago by nickm

Keywords: tor-relay added
Points: 4

comment:12 Changed 2 years ago by nickm

So, what can we do here? One option is to cache the results of DNS lookups that we did as part of the torrc parse, and then later to treat a failed lookup for the same hostname as meaning that we should use the cached address.

I don't think we'd want to do this for DNS lookups in general -- we'd want to audit every place that we do a DNS lookup (except evdns.c) to see whether this is the behavior we want or not.

comment:13 Changed 2 years ago by arma

Description: modified (diff)

comment:14 Changed 2 years ago by arma

See also #2106 which we just closed as a duplicate.

comment:15 Changed 2 years ago by teor

#21900 is a similar issue in evdns itself (or how Tor interprets evdns return values).

Note: See TracTickets for help on using tickets.