[tor-bugs] #11264 [Tor]: Relay has Exit flag but short policy says reject *?

[Tor Bug Tracker & Wiki]

#11264: Relay has Exit flag but short policy says reject *?
------------------------+--------------------------------------------------
     Reporter:  arma    |      Owner:
         Type:  defect  |     Status:  needs_review
     Priority:  normal  |  Milestone:  Tor: 0.2.6.x-final
    Component:  Tor     |    Version:
   Resolution:          |   Keywords:  tor-auth, 026-triaged-1, nickm-patch
Actual Points:          |  Parent ID:
       Points:          |
------------------------+--------------------------------------------------

Comment (by teor):

 Nick, I worry this patch changes the definition of "Exit" to be more
 complex (adding ''more'' edge cases, not decreasing them), without
 changing the documentation.

 The directory specification defines the "Exit" flag as: [0]

 {{{
 1614    "Exit" if the router is more useful for building
 1615    general-purpose exit circuits than for relay circuits.
 }}}

 This patch substantially changes the specification of the Exit flag from:

 {{{
 1845    "Exit" -- A router is called an 'Exit' iff it allows exits to at
 1846    least two of the ports 80, 443, and 6667 and allows exits to at
 1847    least one /8 address space.
 }}}

 To:

 A router is called an 'Exit' iff it allows exits to at least two of the
 ports 80, 443, and 6667 and:
 * where the '''accept''' summary is shorter: '''there is at least one port
 that allows exits to the entire IPv4 or IPv6 address space;''' or,
 * where the '''reject''' summary is shorter: '''there is at least one port
 for which exits are not rejected to any non-private addresses'''
 (alternately, it is not the case that: every port is rejected to at least
 one non-private address); and allows exits to at least one /8 address
 space. (The /8 is not required in the "accept" case, because "entire
 address space" implies "at least one /8".)
 (I think - it's hard to tell exactly how policy_summarize() and
 exit_policy_is_general_exit() interact.)

 This change in Exit flag definition is caused by the patch depending on
 not only exit_policy_is_general_exit(), but the exact implementation of
 policy_summarize(), including the length comparison of accept and reject
 summaries, and the implementation of policy_summary_add_item():

 {{{
 Add a single exit policy item to our summary:
 If it is an accept ignore it unless it is for all IP addresses
 ("*"), i.e. it's prefixlen/maskbits is 0, else call
 policy_summary_accept().
 If it's a reject ignore it if it is about one of the private
 networks, else call policy_summary_reject().
 }}}

 This leaves a large number of corner cases where:

 * accept policies do not allow exiting to the entire address space; or
 * reject policies block all ports to parts of the address space (or, more
 precisely, block each port to at least one address); or
 * minor changes to policies flip the summary from accept to reject,
 causing instability;
 * and potentially other cases I haven't picked up on yet.

 In short, these cases are where policy_is_reject_star() is not the same as
 policy_summarize() == "reject 1-65535".

 I'd feel much more comfortable with some unit tests for these cases, to
 make sure we don't take away too many Exit flags with this change.

 In particular, I'd like to compare the results of:
  * !exit_policy_is_general_exit()
  * policy_summarize() == "reject 1-65535"
  * policy_is_reject_star()
 Ideally, we could do this for the entire consensus, and see how many Exits
 would be affected.

 And, of course, there's an awful lot of doco that would need to change
 with it, both in code comments and the dirspec.

 Sorry to rain on your patch.

 [0]: https://gitweb.torproject.org/torspec.git/blob/HEAD:/dir-spec.txt

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/11264#comment:13>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online