Opened 4 weeks ago

Closed 3 weeks ago

#32123 closed enhancement (implemented)

Implement minimal --disable-relay-mode

Reported by: teor Owned by: teor
Priority: Medium Milestone: Tor: 0.4.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-design, network-team-roadmap-october
Cc: nickm Actual Points: 1.7
Parent ID: #31851 Points: 1
Reviewer: nickm Sponsor: Sponsor31-can

Description

Add:

  • --disable-relay-mode
    • Build tor with relay mode disabled: tor can not run as a relay, bridge, or authority. Implies --disable-dirauth-mode.
    • disable DirPort, DirCache, ORPort, and sets ClientOnly to 1
    • pick one quick module/function to disable

Update:

  • --disable-dirauth-mode
    • hidden alias --disable-module-dirauth
    • Build tor with authority mode disabled: tor can not run as a directory authority or bridge authority.

Child Tickets

TicketTypeStatusOwnerSummary
#32124defectclosedteorInterpret --disable-module-dirauth=no correctly

Change History (9)

comment:1 Changed 4 weeks ago by teor

I'm not sure if we should be using "mode" or "module" here. It's going to be weird if we disable some features that aren't modes. Like statistics.

comment:2 Changed 4 weeks ago by teor

Status: assignedneeds_review

See my draft branch, which sets up --disable-module-relay, and makes it imply --disable-module-dirauth:

Am I on the right track here?

comment:3 Changed 4 weeks ago by nickm

Status: needs_reviewassigned

This looks fine so far. I'd like it to prevent us from becoming a relay as part of the "minimum viable product", and it should also be listed when we say "--list-modules".

comment:4 Changed 4 weeks ago by teor

Actual Points: 1.5
Reviewer: nickm
Status: assignedneeds_review

This was a bit more complicated than I expected: a few of our check scripts expect relay mode.

Here's what this PR does:

  1. Depends on the fixes in #32124, to avoid conflicts
  2. Define --disable-module-relay, and make it imply --disable-module-dirauth
  3. Disables relay and dircache modes when --disable-module-relay is set
  4. Skips checks that don't work when --disable-module-relay is set
  5. Adds alternative outputs for disable module relay to the parseconf checks
    1. Adds extra parseconf checks for dirauth mode, with alternative outputs
  6. Adds a CI job for --disable-module-relay
  7. Updates doc/HACKING/Modules.md

comment:5 Changed 4 weeks ago by nickm

Status: needs_reviewneeds_revision

This looks pretty good; I've left some comments on the PR.

One of the OSX builders timed out; I've restarted travis CI to see if it will be happier now.

Travis is complaining that there is a merge conflict now that #32124 is in, but it looks easy enough to resolve.

Right now, this will fail silently to start as a relay if the user says "ORPort 9000" when the relay module is disabled. We should prioritize that as a next change, since it's a UI problem.

Edit: fix ticket number

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

comment:6 in reply to:  5 Changed 4 weeks ago by teor

Replying to nickm:

Travis is complaining that there is a merge conflict now that #32124 is in, but it looks easy enough to resolve.

It should be a simple rebase.

Right now, this will fail silently to start as a relay if the user says "ORPort 9000" when the relay module is disabled. We should prioritize that as a next change, since it's a UI problem.

That's #32139.

There is also a similar bug in the dirauth module. When it is disabled, dirauth options are still accepted, but tor silently launches as a relay instead:
https://github.com/torproject/tor/pull/1421/commits/c32b9f6c795735698a2185ad27a117702ebd55df#diff-8e153f33c8af8505093956dfa3a3e131

I'll do a minimal fix where we disable the essential options. We can disable all the options after we've split the dirauth and relay options out of the main options struct.

Fix: undo bad autocorrect

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

comment:7 Changed 4 weeks ago by teor

Status: needs_revisionneeds_review

Rebased to resolve conflicts, and added fixups in:

Squashed fixups so we can merge this PR:

comment:8 Changed 4 weeks ago by teor

Actual Points: 1.51.7

comment:9 Changed 3 weeks ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Looks okay now. Merged PR 1430 to master!

Note: See TracTickets for help on using tickets.