Opened 5 months ago

Closed 3 months ago

#25928 closed defect (implemented)

Single DA in sandbox vs. PDS_ALLOW_SELF flag

Reported by: somlo Owned by:
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-dirauth, test-network
Cc: Actual Points:
Parent ID: Points:
Reviewer: teor Sponsor:

Description

I am running a TOR network simulation in a self-contained sandbox, and only
really need a single node to act as Directory Authority. The configuration
file looks as follows (the DA's fqdn is da.sandbox.local, and its IP is
12.34.56.78):

# common to all nodes:
RunAsDaemon 1
TestingTorNetwork 1
UseDefaultFallbackDirs 0
DataDirectory /var/lib/tor
PidFile /var/lib/tor/pid
Log info file /var/lib/tor/info.log
SafeLogging 0
DirAuthority orport=5000 v3ident=6542F7312EE19D39E9D389CCCB1DDD342A32E94D 12.34.56.78:7000 1B494B7382C8C2D2D13FB0B5175B0B3A14E54D69

# additionally, regular onion routers (incl. the DA):
ORPort 5000

# additionally, for the DA only:
DirPort 7000
Address da.sandbox.local
OutboundBindAddress da.sandbox.local
AuthoritativeDirectory 1
V3AuthoritativeDirectory 1
V3AuthVotingInterval 10
V3AuthVoteDelay 2
V3AuthDistDelay 2

When I start the DA, I get lots of log entries (in /var/lib/tor/info.log)
complaining about the absence of any reachable directory servers:

[info] router_pick_dirserver_generic(): No dirservers are reachable. Trying them all again.
[info] router_pick_directory_server(): No reachable router entries for dirservers. Trying them all again.
[info] directory_pick_generic_dirserver(): No router found for consensus network-status fetch; falling back to dirserver list.

While the single DA eventually appears to work properly, and publishes a
consensus file containing itself as a router entry, these warnings keep
showing up periodically in the log file on an ongoing basis.

Once the DA publishes its initial one-entry consensus, I can start further
ORs which are quickly added to the consensus, and any client nodes are
easily able to build circuits and operate correctly in my sandbox network.

In an attempt to silence the DA's dirserver reachability complaints I looked
through the TOR sources, and found that a DA's ability to pick itself as its
own directory server (in function router_pick_directory_server() in file
src/or/routerlist.c) is controlled by the PDS_ALLOW_SELF flag.

This flag was originally introduced in commit 02e7a83f9, and also appears
in subsequent commits b87a7760e, 74c2bff78, and b3a690749. The latter two
commits remove code that used to set the flag (haven't spent the time to
figure out if this would have helped my single-DA scenario, though).

Currently, there appears to be no code path that sets this flag, and also
no way to request it to be set via the command line or configuration file.

Would it make sense to assume the flag is *always* set (which would always
allow a DA to pick itself as its own DA), or rather would it be better to
provide some interface (config file entry) that allows setting the flag
explicitly (maybe only in testing mode)?

Child Tickets

Change History (13)

comment:1 Changed 5 months ago by gk

Component: - Select a componentCore Tor/Tor

comment:2 Changed 5 months ago by teor

Keywords: tor-dirauth test-network added
Milestone: Tor: unspecified

I think Tor should automatically set the flag if it is the directory authority that is the only relay in:

  • the set of preconfigured authorities, or
  • the set of authorities in the consensus

This will never happen in the public network, because there are 9 preconfigured authorities, and a majority for the consensus is 5.

As a workaround, please use two authorities until this issue is fixed. If we don't plan on fixing this soon, perhaps we should document the minimum supported number of authorities, and this workaround.

comment:3 Changed 5 months ago by teor

Summary: Summary: Single DA in sandbox vs. PDS_ALLOW_SELF flagSingle DA in sandbox vs. PDS_ALLOW_SELF flag

comment:4 Changed 5 months ago by somlo

I tried something like this:

diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 1bfbd9f67..0d75eb0f5 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1761,6 +1761,8 @@ trusteddirserver_get_by_v3_auth_digest, (const char *digest))
 const routerstatus_t *
 router_pick_trusteddirserver(dirinfo_type_t type, int flags)
 {
+  if (smartlist_len(trusted_dir_servers) <= 1)
+    flags |= PDS_ALLOW_SELF;
   return router_pick_dirserver_generic(trusted_dir_servers, type, flags);
 }
 

... and it seems to work so far. Is this overly simplistic? (I am, after all, unfamiliar with the code base... :) )

comment:5 Changed 5 months ago by teor

Status: newneeds_revision

That's part of the patch.

Here's what we need to do to make it complete:

  • add a condition to the if statement: "this relay must be the only configured directory authority"
  • refactor the if condition into its own function
  • add the same condition to the fallback dirserver selection function

I think you'll want to compare the relay fingerprint field from router_get_my_routerinfo(), to the relay fingerprint in the only entry in the list.

comment:6 Changed 4 months ago by somlo

OK, how about this, then:

diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 7603eb3ec..705e76120 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1786,6 +1786,9 @@ router_pick_dirserver_generic(smartlist_t *sourcelist,
   const routerstatus_t *choice;
   int busy = 0;

+  if (smartlist_len(sourcelist) == 1)
+    flags |= PDS_ALLOW_SELF;
+
   choice = router_pick_trusteddirserver_impl(sourcelist, type, flags, &busy);
   if (choice || !(flags & PDS_RETRY_IF_NO_SERVERS))
     return choice;

Both router_pick_trusteddirserver() and router_pick_fallback_dirserver() rely on router_pick_dirserver_generic(), which, in turn, uses router_pick_trusteddirserver_impl().

The latter contains the following relevant code:

...
  const int requireother = ! (flags & PDS_ALLOW_SELF);
...
    SMARTLIST_FOREACH_BEGIN(sourcelist, const dir_server_t *, d)
    {
...
      if (requireother && me && router_digest_is_me(d->digest))
        continue;
...

so it appears all of the points made above are being addressed. The alternative would be to move the check directly into router_pick_trusteddirserver_impl() and eliminate the PS_ALLOW_SELF flag altogether, although from my standpoint that'd be a lot more invasive...

comment:7 Changed 4 months ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.5.x-final

comment:9 Changed 3 months ago by nickm

Status: needs_revisionneeds_review

comment:10 in reply to:  5 ; Changed 3 months ago by teor

Reviewer: teor
Status: needs_reviewneeds_revision

Thanks for the patch, but the condition activates PDS_ALLOW_SELF on *any* relay or client with a single configured DirAuth.

We only want to activate PDS_ALLOW_SELF if *this* relay is the single dirauth:

Replying to teor:

Here's what we need to do to make it complete:

  • add a condition to the if statement: "this relay must be the only configured directory authority"


I think you'll want to compare the relay fingerprint field from router_get_my_routerinfo(), to the relay fingerprint in the only entry in the list.

comment:11 in reply to:  10 Changed 3 months ago by somlo

Replying to teor:

Thanks for the patch, but the condition activates PDS_ALLOW_SELF on *any* relay or client with a single configured DirAuth.

We only want to activate PDS_ALLOW_SELF if *this* relay is the single dirauth:

Replying to teor:

Here's what we need to do to make it complete:

  • add a condition to the if statement: "this relay must be the only configured directory authority"


I think you'll want to compare the relay fingerprint field from router_get_my_routerinfo(), to the relay fingerprint in the only entry in the list.

But that already happens, when router_pick_dirserver_generic() calls router_pick_trusteddirserver_impl(). The latter *consumes* the PDS_ALLOW_SELF flag to determine whether to allow "me && router_digest_is_me(d->digest)" to be a candidate DA, which IMHO takes care of your requirement above. Please check out comment #6 above, and tell me what I'm missing -- Thanks!

comment:12 Changed 3 months ago by teor

Status: needs_revisionmerge_ready

Ok, thanks for the explanation, and for being patient with me.

As this patch only affects single directory authority networks, I'm marking it for merge.

comment:13 Changed 3 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

Merged, and added a changes file and a comment. Thanks!

Note: See TracTickets for help on using tickets.