Opened 2 years ago

Closed 11 months ago

Last modified 10 months ago

#21900 closed defect (fixed)

evdns fails when resolv.conf is missing, but succeeds when resolv.conf is empty

Reported by: teor Owned by: rl1987
Priority: Medium Milestone: Tor: 0.4.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: technical-debt, dns, crash, tor-relay, macos, 032-unreached
Cc: Actual Points:
Parent ID: Points: 2
Reviewer: nickm Sponsor:

Description

When tor's ServerDNSResolvConfFile (default /etc/resolv.conf) is missing, evdns does not add any name servers, and therefore Exits do not allow any exit traffic (not even IP-based traffic):

[debug] configure_nameservers: stat()ing /etc/resolv.conf
[warn] Unable to stat resolver configuration in '/etc/resolv.conf': No such file or directory
[info] mark_my_descriptor_dirty: Decided to publish new relay descriptor: dns resolvers failed

This happens on macOS when the network is down. On macOS, /etc/resolv.conf is symlinked to /var/run/resolv.conf. When the network is down, macOS deletes /var/run/resolv.conf, so the stat() call on /etc/resolv.conf fails.

But when tor's ServerDNSResolvConfFile is empty, evdns adds a default name server (127.0.0.1:53 on my macOS), and therefore Exits allow exit traffic:

[info] eventdns: Parsing resolv.conf file /dev/null
[info] eventdns: Added nameserver 127.0.0.1:53 as 0x615000009e00
[info] mark_my_descriptor_dirty: Decided to publish new relay descriptor: dns resolvers failed
[info] eventdns: Parsing resolv.conf file /dev/null
...
[info] mark_my_descriptor_dirty: Decided to publish new relay descriptor: dns resolvers back

We should also stop the extra descriptor upload:

[info] mark_my_descriptor_dirty: Decided to publish new relay descriptor: dns resolvers failed

Child Tickets

TicketStatusOwnerSummaryComponent
#9990closedInternal errorCore Tor/Tor
#15421closednickmRelay crash when reloading and resolv.conf is emptyCore Tor/Tor

Change History (21)

comment:1 Changed 2 years ago by teor

Keywords: crash tor-relay macos added

Also, if you issue a SETCONF to tor while it's in this state, it will abort with:

[err] set_options: Bug: Acting on config options left us in a broken state. Dying. (on Tor 0.2.9.9 008f2846b9b53d52)

This is not such an important crash, because it only happens on platforms which remove resolv.conf (which as far as I know, is only macOS when the network is down). And I think it only happens on relays, not clients. But it would still be nice to fix it.

comment:2 Changed 2 years ago by arma

Sounds like the fix is to treat a missing resolv.conf as being the same as an empty resolv.conf?

comment:3 in reply to:  1 Changed 2 years ago by teor

Replying to teor:

Also, if you issue a SETCONF to tor while it's in this state, it will abort with:

[err] set_options: Bug: Acting on config options left us in a broken state. Dying. (on Tor 0.2.9.9 008f2846b9b53d52)

This is not such an important crash, because it only happens on platforms which remove resolv.conf (which as far as I know, is only macOS when the network is down). And I think it only happens on relays, not clients. But it would still be nice to fix it.

Tor Browser could trigger this crash by issuing a SETCONF when the network is down, but it doesn't use local DNS in the default config, so I think that's ok.

comment:4 Changed 2 years ago by teor

#449 is a similar issue with Hidden Services and DNS that doesn't resolve.

comment:5 Changed 2 years ago by nickm

See also #15421

comment:6 in reply to:  1 Changed 2 years ago by teor

Replying to teor:

This is not such an important crash, because it only happens on platforms which remove resolv.conf (which as far as I know, is only macOS when the network is down). And I think it only happens on relays, not clients. But it would still be nice to fix it.

Turns out this happens with some Linux configs, too.

comment:7 Changed 2 years ago by nickm

Keywords: 032-unreached added
Milestone: Tor: 0.3.2.x-finalTor: unspecified

Mark a large number of tickets that I do not think we will do for 0.3.2.

comment:8 Changed 14 months ago by rl1987

Owner: set to rl1987
Status: newaccepted

comment:9 Changed 13 months ago by rl1987

To reproduce, run:

src/app/tor -ORPort 4430 -ServerDNSResolvConfFile no_such_file -SOCKSPort 9053 -Log debug

and

src/app/tor -ORPort 4430 -ServerDNSResolvConfFile /dev/null -SOCKSPort 9053 -Log debug

The latter yields (on my macOS system):

Aug 13 17:07:34.000 [debug] configure_nameservers: stat()ing /dev/null
Aug 13 17:07:34.000 [info] configure_nameservers: Parsing resolver configuration in '/dev/null'
Aug 13 17:07:34.000 [info] eventdns: Parsing resolv.conf file /dev/null
Aug 13 17:07:34.000 [info] eventdns: Added nameserver 127.0.0.1:53 as 0x7fa1201bc490
Aug 13 17:07:34.000 [warn] Unable to parse '/dev/null', or no nameservers in '/dev/null' (6)
Aug 13 17:07:34.000 [info] mark_my_descriptor_dirty: Decided to publish new relay descriptor: dns resolvers failed
Aug 13 17:07:34.000 [warn] Couldn't set up any working nameservers. Network not up yet?  Will try again soon.

Note that empty resolv.conf is still treated as error condition - by libevent code, not by tor. Libevent also falls back to including 127.0.0.1 as default DNS server. See https://github.com/libevent/libevent/blob/master/evdns.c#L3649

So I suppose we should:

  • When resolv.conf is empty or non-existant:
    • Don't goto err
    • Add 127.0.0.1 to list of DNS servers.
    • Print a scary warning.
    • Mention the above behavior in manpage.

comment:10 Changed 13 months ago by rl1987

Status: acceptedneeds_review

comment:11 Changed 13 months ago by teor

Milestone: Tor: unspecifiedTor: 0.3.6.x-final

Assigning to 0.3.6, because this area of code is less tested, and has resulted in instability in the past.

comment:12 Changed 11 months ago by dgoulet

Reviewer: ahf

comment:13 Changed 11 months ago by dgoulet

Reviewer: ahfnickm

comment:14 Changed 11 months ago by nickm

Status: needs_reviewneeds_revision

Hi, looks nice! I've added a few comments to the codebase.

Also, have you tested this? It would be good to have tests to make sure this does what we expect.

comment:15 Changed 11 months ago by rl1987

Status: needs_revisionneeds_review

comment:16 Changed 11 months ago by nickm

Status: needs_reviewneeds_revision

Hi! I found one place where you need to check a return value. Otherwise this looks fine. Thanks!

comment:17 Changed 11 months ago by rl1987

Status: needs_revisionneeds_review

comment:18 Changed 11 months ago by nickm

Resolution: fixed
Status: needs_reviewclosed

LGTM; merging!

comment:19 Changed 11 months ago by teor

Commit b7edfcbf6b in this branch introduces a memory leak: tor_addr needs to be freed if it is not returned by the function. See #28257.

comment:20 Changed 11 months ago by teor

I ran this code on chutney on macOS with and without resolv.conf. It works both ways.

comment:21 Changed 10 months ago by nickm

Milestone: Tor: 0.3.6.x-finalTor: 0.4.0.x-final

Tor 0.3.6.x has been renamed to 0.4.0.x.

Note: See TracTickets for help on using tickets.