[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #17975 [Core Tor/Tor]: Introduce OutboundExitAddress to enable exit-only traffic to go via a different IP address



#17975: Introduce OutboundExitAddress to enable exit-only traffic to go via a
different IP address
-------------------------------------------------+-------------------------
 Reporter:  naif                                 |          Owner:
     Type:  enhancement                          |         Status:
                                                 |  needs_revision
 Priority:  Low                                  |      Milestone:  Tor:
                                                 |  0.3.0.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  lorax, yawning, isaremoved review-   |  Actual Points:
  group-13                                       |
Parent ID:                                       |         Points:  1
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by teor):

 Code review (reading only, not patching or running):

 Thanks, this is getting better, I think we are almost there!

 Design:

 parsing addresses:
 * the way this code turns OutboundBindAddress into OutboundBindAddressOR
 and OutboundBindAddressExit means that when the options are written out
 (for example, by a SETCONF), the torrc will change. We avoid changing the
 user's torrc.

 verify_and_store_address:
 * please turn verify_and_store_address into a setter function which take a
 family and an enum for Exit/OR/Both, and then get/set
 options->OutboundBindAddress{OR,Exit,}IPv{4,6}_. It would really simplify
 parse_outbound_addresses.
   * this would also fix the use of hard-coded constants when passing
 adrCount to verify_and_store_address, and avoid the bug-prone pointer
 twiddling
 * once you have this abstraction, you can parse each group of lines using
 the same function: it takes lines, a family, and an enum for Exit/OR/Both.

 conn_get_outbound_address:
 * please don't fall back from OutboundBindAddressExit to
 OutboundBindAddressOR, it's confusing and undocumented. Multihomed relays
 might have good reasons to use a particular IP for their OR connections,
 but allow Exit traffic to use the best route for the destination IP.
 * Update the man page: s/This option cannot be used together with
 **OutboundBindAddress**, unless they specify a different protocol./This
 option overrides **OutboundBindAddress** for the same IP version./
 * configure_nameservers should use conn_get_outbound_address

 policies_copy_outbound_addresses_to_smartlist:
 * this list of addresses is used to block all addresses on the local
 machine. It must include both the OR and Exit addresses.


 Standardisation:
 * address families are typically `sa_family_t family` in tor

 Nitpicks:
 * please put a newline at the end of the changes file
 * `const int` is somewhat redundant, we typically only use const on
 pointers, and on variables in functions

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/17975#comment:17>
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