[tor-bugs] #28525 [Core Tor/Tor]: Make tor_addr_is_internal_() aware of RFC 6598 (Carrier Grade NAT/Large Scale NAT) IPv4 Ranges

[Tor Bug Tracker & Wiki]

#28525: Make tor_addr_is_internal_() aware of RFC 6598 (Carrier Grade NAT/Large
Scale NAT) IPv4 Ranges
--------------------------+------------------------------------
 Reporter:  neel          |          Owner:  neel
     Type:  enhancement   |         Status:  needs_revision
 Priority:  Medium        |      Milestone:  Tor: 0.4.0.x-final
Component:  Core Tor/Tor  |        Version:
 Severity:  Normal        |     Resolution:
 Keywords:  ipv6          |  Actual Points:
Parent ID:                |         Points:
 Reviewer:  nickm         |        Sponsor:
--------------------------+------------------------------------
Changes (by nickm):

 * status:  needs_review => needs_revision
 * reviewer:  mikeperry => nickm


Comment:

 This patch does what it is supposed to do.  It would be good to have a
 test.

 One problem here is that I'm not sure that this changed behavior is
 correct.  If you have an address inside a carrier NAT, you have the worst
 of both worlds: it is an address that the public internet cannot reach,
 but it is an address that other random people on your internet provider
 can still connect to.  In other words, these addresses are not useful
 enough to call them public, but not safe enough to call them private. So
 we need to treat these addresses as internal for the purpose of "can this
 address go onto the public tor network", but we need to treat them as non-
 internal for the purpose of "is it safe to have a socksport/extorport/etc
 here."

 The main purpose of the rest of my review here is to see what else we
 would need to change to make sure this change is safe.  I'm going to do
 this by looking at all the users of tor_addr_is_internal in the codebase.

    * In warn_nonlocal_client_ports(), we will stop warning about binding a
 socksport to one of these addresses.  Is this a problem?  I need more
 guidance from others.
    * In warn_nonlocal_ext_orports(), we will stop warning about binding an
 extorport to one of these addresses.  (same note as above)
    * In connection_is_rate_limited(), we no longer count connections to or
 from one of these addresses as having any rate limits.
    * In channeltls.c [which calls tor_addr_is_internal via
 is_local_addr()], we count any OR connections to these addresses as
 "local", which seems unwise.

 But all the other cases that I could find seemed like an improvement.

 Maybe what we need here is to replace the `for_listening` argument with a
 more general set of bitflags?

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