Private address warning on public network too quiet

In #40208 (closed) the behaviour for when a relay doesn't publish due to an RFC1918 IP address is addressed, but in the initial report there is this line:

*\[info\] address_can_be_used(): Address '127.0.0.1' is a private IP address. Tor relays that use the default DirAuthorities must have public IP addresses.*

which is only logged at info but is the only thing that provides a hint why the relay stopped publishing. It should probably be louder and explain both ways to fix the issue.

changed milestone to %Tor: 0.4.6.x-freeze

added Backlog Backport labels

assigned to @nickm

It looks like this message comes from the address_can_be_used() function, which takes a severity level as an argument. That severity level seems to come from calls to find_my_address(). I wonder why we're passing it INFO in this case.

added Next label and removed Backlog label

I confirm that you can reproduce this with e.g. Address 127.0.0.1 ORPort 9999

changed time estimate to 1h

added Doing label and removed Next label

It looks like the only call site I hit for find_my_address is relay_find_addr_to_publish() in relay_find_addr.c.

relay_find_addr_to_publish() gets hit a lot, so we probably wouldn't want to make it spam the logs, but surely there is some other place that we can try to warn. Perhaps something called from options_act() would be better?

Interestingly, I don't think that this problem is new with 0.4.5.

@dgoulet can you suggest a reasonable place to call find_my_address() with LOG_WARN?

Yeah I think we could use it in option validate as a way to validate the config.

However, a failure of that function doesn't mean tor won't work since we do address discovery through authorities as well. Furthermore, that function changes tor's state and thus I would avoid using it for config validation tbh :S

We could make it take a "validate only" but it might complexify an already very complex function or we could think of decoupling it into a validation part and "act on the option" part but again, possibly some more complex work.

Unclear to me here what is best approach. Address discovery is quite a complex algorithm already done constantly at different stage in the tor process so maybe a validation function would be the way to go...

added 30m of time spent at 2021-03-08

changed milestone to %Tor: 0.4.6.x-stable

@dgoulet hm, maybe we want a "warn once" behavior, where we warn the first time, but no subsequent times?

Maybe something like !347 (closed) ?

added Next label and removed Doing label

mentioned in merge request !347 (closed)

Assigning to @dgoulet since he did a patch here

added Needs Review label

assigned to @dgoulet and unassigned @nickm

removed Next label

!347 (closed) is squashed and merged in 0.4.6 and forward.

closed