Opened 19 months ago

Closed 6 days ago

#21530 closed defect (implemented)

Make ExitRelay 0 the default when there is no exit policy

Reported by: teor Owned by: neel
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-exit tor-relay configuration usability expectations
Cc: neel@… Actual Points:
Parent ID: Points: 1
Reviewer: mikeperry Sponsor:

Description

We have logged a message saying we might do this for a while.

This caused chutney bug #17090.

Child Tickets

Change History (12)

comment:1 Changed 15 months ago by nickm

Keywords: tor-relay configuration usability expectations added

comment:2 Changed 4 weeks ago by neel

Cc: neel@… added
Owner: set to neel
Status: newassigned

Like with many other bugs, I am interested in taking on this.

I have a few questions:

  1. Should I make ExitRelay a bool and make its default value 0, or keep it an autobool and make it where the default value is based on whether ExitPolicy lines exist?
  2. If ExitRelay is 1, and there is no exit policy line, should I act as if ExitRelay is 0?
  3. If Question 2 is yes, can I implement a setting called DefaultExitPolicy with the default exit policy (that we normally have if we say we're an exit but don't have any specific exit policy) like we do with ReducedExitPolicy?

comment:3 in reply to:  2 Changed 4 weeks ago by teor

Replying to neel:

Like with many other bugs, I am interested in taking on this.

I have a few questions:

  1. Should I make ExitRelay a bool and make its default value 0, or keep it an autobool and make it where the default value is based on whether ExitPolicy lines exist?

You need to keep it an autobool, because changing it to a bool would break existing torrcs.

  1. If ExitRelay is 1, and there is no exit policy line, should I act as if ExitRelay is 0?

Edit: No, you should use the default exit policy.

  1. If Question 2 is yes, can I implement a setting called DefaultExitPolicy with the default exit policy (that we normally have if we say we're an exit but don't have any specific exit policy) like we do with ReducedExitPolicy?

Edit: The default exit policy is the default when ExitRelay is 1 and ReducedExitPolicy is 0. We discourage adding extra options, because you have to test all the option combinations.

Last edited 4 weeks ago by teor (previous) (diff)

comment:5 Changed 4 weeks ago by teor

Status: assignedneeds_review

comment:6 Changed 4 weeks ago by teor

Milestone: Tor: unspecifiedTor: 0.3.5.x-final

comment:7 Changed 2 weeks ago by asn

Reviewer: mikeperry

comment:8 Changed 7 days ago by mikeperry

Status: needs_reviewmerge_ready

This looks good to me. Nice and simple. Thanks Neel!

comment:9 Changed 6 days ago by nickm

I've asked Roger to comment on usability here. One possible improvement I can see is that it might be a good idea to give a different "You aren't an exit now" notice message for anybody who was expecting the old default behavior. But I'd like to hear what Roger (and others) think.

comment:10 Changed 6 days ago by arma

Right, hm. I think the main scenario we want to try to handle is the one where a relay operator intends to be running an exit relay, and even checked the exit policy on their relay and confirmed that it was what they wanted, but they haven't messed with the ExitRelay config option. In this case, when they upgrade, their exit policy will silently become something different than it used to be, and it would be smart for us to think through how they're supposed to learn about this surprise.

One option would be to make it very obvious in the ChangeLog, like turn it into a Major thing rather than a Minor thing. That's good but not enough imo.

Another option would be some log lines to help them know what's happening. I think there's a lot to be said for a notice-level log explaining *why* we decided to set the exit policy to reject-all.

We could imagine fancier approaches, like looking at the TorVersion line in the state file and giving them a warning if they have the right combination of config settings. But doing that warning only once (before the TorVersion in the state file gets updated, that is) is risky since it's so easy to miss warnings. So I think this approach wouldn't be worth building.

Another option would be to have some script that looks at the network for relays that used to be exits using the default exit policy, and then stopped being exits when they moved to this new version. Then we contact those people to let them know about the potential surprise. That option would be a winner except: what do we do about the people who don't set a usable ContactInfo?

Summary: my suggestion would be to add the notice-level log explaining why we're opting not to be an exit relay (that log line will be helpful to relay operators forever), and then also monitor the network and reach out to relays that look like they got hit with this surprise.

comment:11 Changed 6 days ago by neel

I have pushed the changes.

comment:12 Changed 6 days ago by nickm

Resolution: implemented
Status: merge_readyclosed

Looks good! I've merged, and tweaked a little:

  • I changed the log message to explain how to suppress it.
  • I made it so the log message only appears if you're running as a public relay. (If you're running as a client, you probably don't expect to be an exit.)
  • I tweaked the changes file's severity so it will be more prominent.

Thanks for the patch!

Note: See TracTickets for help on using tickets.