policies_parse_exit_policy_internal should block all IPv4 and IPv6 local addresses

Currently it just handles a single IPv4 address, allowing IPv6 exits to be connected to on their IPv6 address, or multihomed IPv4 exits to be connected to on their other IPv4 addresses.

This is a potential security issue, as it allows connections to local ports on an exit.

changed milestone to %Tor: 0.2.8.x-final

added 027-backport TorCoreTeam201512 component::core tor/tor milestone::Tor: 0.2.8.x-final priority::high resolution::fixed security severity::normal status::closed type::defect version::tor unspecified labels

Trac:
Keywords: TorCoreTeam201509 security deleted, TorCoreTeam201509 security 026-backport added
Milestone: N/A to Tor: 0.2.7.x-final

Anything we do here will always have edge cases, like (multihomed) NAT, or NAT without an autodetected IP address.

That said, I think I can fix this by:

• refactoring get_interface_address6 as a wrapper for a new get_interface_address6_list, which produces a smartlist of local addresses. (Similarly, create get_interface_address_list, a wrapper which produces a list of IPv4 addresses.)
• blocking connections to all IPv4 addresses from get_interface_address_list, as long as rejectprivate and local_address are non-zero.
• blocking connections to all IPv6 addresses from get_interface_address6_list, as long as rejectprivate and local_address and ipv6_exit are non-zero. (Otherwise, we're blocking all IPv6 addresses anyway.)
• Creating or updating unit tests for get_interface_address6_list, get_interface_address_list, and policies_parse_exit_policy_internal.

Trac:
Milestone: Tor: 0.2.7.x-final to N/A
Keywords: TorCoreTeam201509 security 026-backport deleted, TorCoreTeam201509 security added

Didn't mean to revert those.

Trac:
Keywords: TorCoreTeam201509 security deleted, TorCoreTeam201509 security 026-backport added
Milestone: N/A to Tor: 0.2.7.x-final

We should also update the man page and torrcs with this change, as it changes the meaning of "private" to mean "all IPv4 and IPv6 addresses on all server interfaces", rather than "a single IPv4 address on one of the server's interfaces". (It's ambiguous in the man page at the moment.)

Further notes:

This is a patch on 42b8fb5a1523 (11 Nov 2007), released in 0.2.0.11-alpha.

This fix will automatically benefit from changes that find more interfaces/addresses, perhaps #12377 (moved) will do this for some platforms.

We should log an info-level (notice?) message for each address blocked Internal addresses are blocked anyway by reject private *:*, so this patch doesn't need to block them.

This change will include all addresses in non-internal blocks in the publicly available exit policy, but these addresses are typically globally visible on the Internet anyway. I believe the security benefits outweigh the small risk of leaking public server addresses from unusual configurations (and operators can always set ExitPolicyRejectPrivate 0 and block only the private and server addresses they want to block).

Trac:
Version: Tor: 0.2.7.2-alpha to Tor: unspecified

See my branch bug17027-reject-private-all-interfaces-squashed on https://github.com/teor2345/tor.git

Commits:

• Add get_interface_address[6]_list by refactoring get_interface_address6 (with unit tests)
• ExitPolicyRejectInternal rejects a relay's published IPv6 address (if we're an IPv6 Exit), and publicly routable IPv4 & IPv6 interface addresses (with unit tests)
• Log an info-level message for each local IP address blocked this way

The unit tests pass on OS X and Linux

This branch may require manual changes to merge correctly with #16069 (moved): this branch adds arguments to functions that #16069 (moved) adds calls to.

Trac:
Status: new to needs_review

Pushed a fixup commit that makes sure we don't reject addresses that are both the relay's published address, and the address of an interface. (These would have been caught by exit_policy_remove_redundancies, but it's best to avoid them in the first place.)

Added two fixups that update the documentation and torrcs

Looks good!

• Needs a changes file.
• I'm thinking this doesn't run us into trouble with bug #12497 (moved). Somebody should check my logic, though.
• get_interface_address6_list() can't return NULL, but its callers all check whether it does.

Replying to nickm:

Looks good!

• Needs a changes file.

It's there as changes/bug17027-reject-private-all-interfaces

• I'm thinking this doesn't run us into trouble with bug #12497 (moved). Somebody should check my logic, though.

This doesn't change the definition of private:*, instead, it appends explicit IP-based reject items to the ExitPolicy when ExitPolicyRejectPrivate is 1. The existing code adds a reject for the configured public IPv4 address, this new code does it for the configured public IPv6 address (if any), and any other public IPv4 or IPv6 addresses found on any interfaces.

• get_interface_address6_list() can't return NULL, but its callers all check whether it does.

Oops, fixed and squashed in bug17027-reject-private-all-interfaces-v2.

Please use my bug16069-bug17027 branch if you want to merge both #16069 (moved) and #17027 (moved). It resolves some merge conflicts and function argument changes, and includes the latest changes in both branches.

Merged

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

Reopened for possible backport to 026

Trac:
Status: closed to reopened
Resolution: fixed to N/A

Trac:
Milestone: Tor: 0.2.7.x-final to Tor: 0.2.6.x-final

IMO this is really potentially backportable to 0.2.6 as a security improvement.

Trac:
Sponsor: N/A to N/A

I agree, let's backport to 0.2.6, if the required function(s) or data structures exist to support each address discovery method. But can we make the following changes first?

In addition to blocking:

• the configured or autodiscovered IPv4 address (Address or resolve_my_address()),
• the configured IPv6 address (first IPv6 ORPort entry),
• the publicly routable IPv4 or IPv6 address(es) of every interface on the server, if available.

We could also block the following configured addresses by looking at OutboundBindAddressIPv4_/OutboundBindAddressIPv6_ and get_configured_ports():

• OutboundBindAddress
• ControlPort / ControlListenAddress
• SOCKSPort / SOCKSListenAddress
• TransPort / TransListenAddress
• NATDPort / NATDListenAddress
• DNSPort / DNSListenAddress
• ORPort / ORListenAddress (IPv4 entries or subsequent IPv6 entries)
• DirPort / DirListenAddress

Ideally, we'd do this out of two smartlists of unique IPv4 and IPv6 addresses, to avoid rejecting the same address multiple times. Duplicates will be removed by exit_policy_remove_redundancies() in any case.

Trac:
Status: reopened to needs_revision