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

Re: [tor-bugs] #17840 [Tor]: Add a minimal implementation of ClientUseIPv4 so IPv6-only clients can bootstrap

#17840: Add a minimal implementation of ClientUseIPv4 so IPv6-only clients can
bootstrap
-------------------------------------+------------------------------------
 Reporter:  teor                     |          Owner:  teor
     Type:  defect                   |         Status:  needs_review
 Priority:  Medium                   |      Milestone:  Tor: 0.2.8.x-final
Component:  Tor                      |        Version:
 Severity:  Normal                   |     Resolution:
 Keywords:  ipv6, TorCoreTeam201601  |  Actual Points:
Parent ID:  #17811                   |         Points:
  Sponsor:                           |
-------------------------------------+------------------------------------
Changes (by teor):

 * keywords:  ipv6 => ipv6, TorCoreTeam201601
 * status:  needs_revision => needs_review


Comment:

 Replying to [comment:9 nickm]:
 > Returning to 4d453a7a9041 Choose directory servers by IPv4/IPv6
 preferences
 >  * directory_initiate_command_routerstatus_rend() -- it seems like the
 new code here might be better to extract into a new function?

 Done!

 >  * Can it ever be right to use the directory_initiate_command()
 interface any more, since it doesn't know about IPv6/IPv4 issues?

 We really should make callers specify a separate OR and Dir address here.
 Fixed that.

 I checked the cases where it's used when I was writing this patch:
 * In directory_get_from_dirserver(), bridge clients use it to request
 directory info from their bridge based on its routerinfo:
   * Since there may be no routerstatus for the bridge, they can't use
 directory_initiate_command_routerstatus_rend() directly.
   * My patch 0204c9dca838 in this branch chooses an appropriate IPv4/IPv6
 address before calling directory_initiate_command().
 * In launch_direct_bridge_descriptor_fetch(), bridge clients use the
 configured address for the bridge. There's only one configured address.
 It's in a bridge_info_t, not a routerstatus_t.
   * This is tried as a fallback mechanism after trying the bridge
 authority.
   * I added a notice-level log message in
 launch_direct_bridge_descriptor_fetch() if we try an unreachable bridge
 address. (Clients can still use the bridge without an up to date
 descriptor.)
 * In consider_testing_reachability(), relays use their IPv4 address for
 reachability testing.
   * We'll resolve the missing IPv6 reachability tests in #6939.
   * Relays aren't allowed to have a fascist firewall or similar config, so
 it's ok for the moment.

 >  * We used to have a notion of "anonymized connection not using
 begindir" -- we went to a  node and told it to make a connection to its
 own dirport.  Did we rip out all vestiges of this?  If not, the code here
 is a bit sketchy.

 I think it's all gone, but I'm not exactly sure what code you're talking
 about, I've never seen it. (Which likely means it was removed in or before
 0.2.6).

 DIRIND_ANON_DIRPORT is similar to what you're suggesting, but we only use
 that in consider_testing_reachability() for relay self-testing.

 > Returning to 5ffe801801da Use fascist firewall and ClientUseIPv4 for
 bridge clients
 >  * This whole thing is a bit scary.
 >  * In the autobool conversion, did you grep for ClientUseIPv4 and
 ClientUseIPv6 to make sure you found them all?

 Yes, all ClientUseIPv6 instances.
 ClientUseIPv4 was not changed in this patch.

 >  * In lieu of the majority of this patch, maybe it would be better to
 have the torrc's ClientUseIPv[46] options set or_options_t's
 ClientUseIPv[46]_option fields, and then have options_validate() or
 something set ClientUseIPv[46] based on that? Not sure.

 I agree it's messy.
 I think we should do what we already do for ClientPreferIPv6OR/DirPort,
 and create an accessor function fascist_firewall_use_ipv6(). Then the
 logic is all in one place if we ever need to change it. (Done!)

 Now the only references to ClientUseIPv6 are in config.c
 (options_validate) and policy.c (fascist_firewall_use_ipv6), in a log
 message in connection.c, and in or.h and the unit tests.

 ClientPreferIPv6OR/DirPort already uses accessor functions, except for a
 limited number of cases (which are the same as those for ClientUseIPv6).

 ClientUseIPv4 doesn't have an accessor function, but it is only used in a
 limited number of cases, similar to those above, with the addition of
 router_pick_directory_server_impl() and
 router_pick_trusteddirserver_impl() in routerlist.c.

 Further optimisation:

 Keeping firewall_is_fascist_or() (and adding firewall_is_fascist_dir())
 allows us to do some optimisations in the OR and Dir nodelist searches.
 I've added these as a final commit.

 Please see my branch feature17840-v11, which has these changes appended to
 it. (No reordering or rebases - the reviewed commits are all the same.)

 (I suspect it won't merge cleanly into master, and perhaps won't
 autosquash. I can rebase and squash it manually when it's ready for
 merge.)

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