Add exit-policy/reject-private so stem can discover ExitPolicyRejectPrivate rules
Add controller getinfo exit-policy/reject-private for the reject rules added by ExitPolicyRejectPrivate. This makes it easier for stem to display exit policies. Add unit tests for getinfo exit-policy/*.
Activity
changed milestone to %Tor: 0.2.8.x-final in legacy/trac
added 028-triaged in Legacy / Trac TorCoreTeam201511 in Legacy / Trac component::core tor/tor in Legacy / Trac milestone::Tor: 0.2.8.x-final in Legacy / Trac points::small in Legacy / Trac priority::medium in Legacy / Trac resolution::implemented in Legacy / Trac severity::normal in Legacy / Trac status::closed in Legacy / Trac stem in Legacy / Trac type::enhancement in Legacy / Trac version::tor unspecified in Legacy / Trac labels
See my branch
getinfo-private-exitpolicyat https://github.com/teor2345/tor.gitTrac:
Status: new to needs_review
Quick review:
- Looks mostly good; glad to see tests.
- we never use strcat in Tor: too many linkers and security tools shout about it. Instead I'd just suggest you use
smartlist_add_asprintf()in the inner loop. It's waaay less error-prone. - There needs to be a corresponding control-spec.txt patch.
Trac:
Status: needs_review to needs_revisionI wonder if we should add the addresses from legacy/trac#17027 (moved) while doing this. Possibly, they belong under another getinfo: exit-policy/reject-relay ????
I think a good design is:
- exit-policy/reject-private - all ExitPolicyRejectPrivate addresses
- exit-policy/reject-private/networks - ExitPolicyRejectPrivate private network addresses excluded by default
- exit-policy/reject-private/mine - ExitPolicyRejectPrivate private network addresses excluded because they are:
- configured as addresses on the relay's tor instance
- configured on the local network interfaces on the relay's OS
I think I've done what atagar wants for stem now.
See my branch getinfo-private-exitpolicy-v2 in https://github.com/teor2345/tor.git
I've updated the code based on feedback, added new GETINFO questions, and updated the unit tests.
I've added:
- exit-policy/reject-private/default - private network addresses rejected by default (compiled-in)
- exit-policy/reject-private/relay - private network addresses rejected because they are:
- published as addresses in the relay's descriptor
- configured as outbound connection bind addresses
- configured as ports (for example, ORPort, DirPort)
- configured on the local network interfaces on the relay's OS
Which replace:
- exit-policy/reject-private
The existing items are:
- exit-policy/ipv4 - IPv4 exit policy
- exit-policy/ipv6 - IPv4 exit policy
- exit-policy/full - IPv4 and IPv6 exit policies
- exit-policy/default - default exit policy (compiled-in)
I can easily split exit-policy/reject-private/default and exit-policy/reject-private/relay into ipv4, ipv6, and full if needed. I could also combine them into exit-policy/reject-private ipv4, ipv6, and full, which would be a little more difficult. (This depends on exactly what atagar wants for stem.)
I've also tested these over stem's tor-prompt. (They block a temporary relay's autodiscovered IP address, which is exactly what I want to happen.)
I'll do the control-spec changes tomorrow.
Trac:
Status: needs_revision to needs_reviewThe control-spec changes are in legacy/trac#17611 (moved).
I've created a new branch getinfo-private-exitpolicy-v3 which cherry-picks the latest fix to legacy/trac#17027 (moved), and then passes a list of configured addresses to policies_parse_exit_policy_reject_private.
Replying to nickm:
re 236d74869ff68fd88553ce170f4784b3a9d761a7 : it looks like this logic is duplicated with the code that actually builds the list of addresses rejected in the exit policies. Should it be unified?
I thought they were different enough that I should leave them alone. But I think they could at least benefit from a utility "check this address before adding it" function. I'll look into this tomorrow.
Replying to nickm:
re 236d74869ff68fd88553ce170f4784b3a9d761a7 : it looks like this logic is duplicated with the code that actually builds the list of addresses rejected in the exit policies. Should it be unified?
Please see my branch getinfo-private-exitpolicy-v4.
I squashed the branch (including the squash in bug17027-reject-private-027-v6), and left a standard fixup implementing these changes as the final commit. (If you've reviewed the v3 branch, you only need review the final commit.)
It creates 3 utility functions for adding addresses from a tor_addr_t*, uint32 ipv4h, and or_options (outbound bind addresses). Naturally, these functions call each other as appropriate.
policies_parse_exit_policy_from_options and getinfo_helper_policies then call these functions as appropriate to add addresses to the smartlist.
This branch needed two fixes. I have added fixups to getinfo-private-exitpolicy-v4 for these issues, which also affect legacy/trac#17027 (moved).
policies_parse_exit_policy_reject_private Unit Tests
The unit tests for policies_parse_exit_policy_reject_private assumed too much about the interface addresses on the local system. This meant that tests failed on some machines.
I modified the unit tests to assume less, and used a mocked version of get_interface_address6_list to provide a range of addresses for testing.
getinfo exit-policy/reject-private/relay Implementation & Unit Tests
The implementation was ignoring ExitPolicyRejectPrivate and had a memory leak. The unit tests didn't set ExitPolicyRejectPrivate, and were missing a static on a mock function declaration.
I fixed these issues.
Trac:
Status: closed to reopened
Resolution: implemented to N/A-
I cherry-picked one of these to master as a09e7cd3. The other had already been applied? Please let me know if I missed a commit though.
Trac:
Status: needs_review to closed
Resolution: N/A to fixed