Opened 4 years ago

Closed 10 months ago

#13605 closed enhancement (implemented)

Create a client/relay-side ReducedExitPolicy

Reported by: mikeperry Owned by:
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-relay, easy, review-group-18, review-group-24
Cc: yawning, neel@… Actual Points: 3
Parent ID: Points: 3
Reviewer: Sponsor:

Description

We should have an "ExitPolicy Reduced" or "ReducedExitPolicy 1" torrc option for relay operators to more easily opt in to https://trac.torproject.org/projects/tor/wiki/doc/ReducedExitPolicy.

We have a lot of people doing this on an ad-hoc basis. We should make it official.

Child Tickets

Attachments (4)

0001-Add-code-for-letting-user-select-Reduced-Exit-Policy.patch (6.8 KB) - added by neel 21 months ago.
Patch for adding ReducedExitPolicy option to use Reduced Exit Policy
tor-patch-ReducedExitPolicy-001.patch (8.3 KB) - added by neel 13 months ago.
Updated patch to add ReducedExitPolicy option
tor-patch-ReducedExitPolicy-002.patch (8.4 KB) - added by neel 11 months ago.
Version 2 of patch to add ReducedExitPolicy option
tor-patch-ReducedExitPolicy-003.patch (11.3 KB) - added by neel 10 months ago.
Version 3 of patch to add ReducedExitPolicy option

Download all attachments as: .zip

Change History (32)

comment:1 Changed 4 years ago by mikeperry

Fwiw, we also have #6495 for a dirauth version of this, but that will require all clients+relays to upgrade before it can be used.

comment:2 Changed 3 years ago by nickm

Keywords: easy added
Milestone: Tor: 0.2.7.x-final

comment:3 Changed 3 years ago by nickm

Status: newassigned

comment:4 Changed 3 years ago by nickm

Keywords: lorax added
Milestone: Tor: 0.2.7.x-finalTor: 0.2.???

This got triaged out of 0.2.7 today.

If we do it, we need to make sure that we aren't promising 100% abuse-resistance to the servers who use it.

comment:5 Changed 3 years ago by nickm

Points: medium
Severity: Normal

comment:6 Changed 21 months ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

Changed 21 months ago by neel

Patch for adding ReducedExitPolicy option to use Reduced Exit Policy

comment:7 Changed 21 months ago by neel

Hi,

I have a patch to add a ReducedExitPolicy option, and to set the Exit Policy to the well known Reduced Exit Policy. Please tell me what you think about this patch.

Thank You,
Neel Chauhan

comment:8 Changed 20 months ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:9 Changed 15 months ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:10 Changed 15 months ago by nickm

Status: assignednew

Change the status of all assigned/accepted Tor tickets with owner="" to "new".

comment:11 Changed 14 months ago by nickm

Keywords: lorax removed
Milestone: Tor: unspecifiedTor: 0.3.2.x-final
Status: newneeds_review

comment:12 Changed 14 months ago by nickm

Oh hey, there's a patch here! We should review this for when the next merge window happens.

comment:13 Changed 14 months ago by nickm

Keywords: review-group-18 added

comment:14 Changed 14 months ago by mikeperry

Status: needs_reviewneeds_revision

Yay! Thank you so much for doing this neel. Sorry for taking so long to look at it.

The patch looks OK, but needs a couple things before merge:

  1. Can you add an entry for this option in the manpage? (doc/tor.1.txt)
  2. Can you create a changelog entry in a changes file for this (ie: changes/bug13605)

Otherwise I think this looks good. It appears to match the most recent reduced exit policy wiki page still, too.

Nick - do we have to have unit tests for torrc options like this, or can we consider this covered by the other exit policy tests?

comment:15 Changed 14 months ago by nickm

In an ideal world we'd have tests for everything, though for torrc options it might be better to wait until we have config.c factored in a much better way.

Changed 13 months ago by neel

Updated patch to add ReducedExitPolicy option

comment:16 Changed 13 months ago by neel

Sorry for the delay, but I have created a new patch with the filename tor-patch-ReducedExitPolicy-001.patch​.

comment:17 Changed 11 months ago by nickm

Milestone: Tor: 0.3.2.x-finalTor: unspecified

Mark some needs_revision tickets as unspecified. If/when the revisions happen, they can go back into a live milestone.

comment:18 Changed 11 months ago by teor

Apart from unit tests, these two things need to be changed:

The maximum exit policy is now EXIT_POLICY_ADD_REDUCED.

#define EXIT_POLICY_REJECT_LOCAL_INTERFACES  (1 << 3)
#define EXIT_POLICY_ADD_REDUCED              (1 << 4)
#define EXIT_POLICY_OPTION_MAX               EXIT_POLICY_REJECT_LOCAL_INTERFACES
/* All options set: used for unit testing */
#define EXIT_POLICY_OPTION_ALL             ((EXIT_POLICY_OPTION_MAX << 1) - 1)

Please check all uses of EXIT_POLICY_OPTION_ALL to make sure they still function as designed.

The last line of the changes file needs a newline, not an escape sequence:

operator to use a reduced exit policy rather than the default one. Closes\      ticket 13605.

comment:19 Changed 11 months ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.2.x-final

comment:20 Changed 11 months ago by nickm

Milestone: Tor: 0.3.2.x-finalTor: 0.3.3.x-final

Defer all needs_revision non-spec enhancements to 0.3.3.

comment:21 Changed 11 months ago by neel

Cc: neel@… added

Changed 11 months ago by neel

Version 2 of patch to add ReducedExitPolicy option

comment:22 Changed 11 months ago by neel

I have created a new patch tor-patch-ReducedExitPolicy-002.patch which includes the requested changes. It passes the regression test (at least on my machine).

comment:23 Changed 11 months ago by teor

Status: needs_revisionneeds_review

Looks great, but needs some more documentation.

The man page should talk about ReducedExitPolicy under ExitPolicy:

If you want to use a reduced exit policy rather than the default exit policy, set "ReducedExitPolicy 1". If you want to _replace_ the default exit policy with your custom exit policy, end your exit policy with either a reject *:* or an accept *:*. Otherwise, you’re _augmenting_ (prepending to) the default or reduced exit policy.

The man page should document exactly what the reduced exit policy is.

For example, here is the man page entry for the default exit policy:

    The default exit policy is:

    reject *:25
    reject *:119
    reject *:135-139
    reject *:445
    reject *:563
    reject *:1214
    reject *:4661-4666
    reject *:6346-6429
    reject *:6699
    reject *:6881-6999
    accept *:*

    Since the default exit policy uses accept/reject *, it applies to both IPv4 and IPv6 addresses.

Changed 10 months ago by neel

Version 3 of patch to add ReducedExitPolicy option

comment:24 Changed 10 months ago by neel

I have a new version of this patch. The filename is 'tor-patch-ReducedExitPolicy-003.patch' (without the quotes).

comment:25 Changed 10 months ago by nickm

Keywords: review-group-24 added

review-group-24 is now open.

comment:26 Changed 10 months ago by teor

Actual Points: 3
Points: medium3

This patch looks good to me.

I've not compiled it or run unit tests (are there existing unit tests for the exit policy code?).

We should do that before we merge.

comment:27 Changed 10 months ago by nickm

Status: needs_reviewmerge_ready

This is looking reasonable. Two changes to make before I merge (i can do these, no worries):

  • We should only add the reduced exit policy if BridgeMode is also 0.
  • The else goes on the same line as any } before it.

I'll take another look too while I'm at it. I'd do all this now, but I need to go afk

comment:28 Changed 10 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

I've put my changes (minor) in branch 13605_reduced_exit in my public repository; I've merged a squashed version to master. Thanks!

Note: See TracTickets for help on using tickets.