[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #11264 [Tor]: Relay has Exit flag but short policy says reject *?
#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
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs