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

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#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    |  Actual Points:
Parent ID:  #17811  |         Points:
  Sponsor:          |
--------------------+------------------------------------

Comment (by nickm):

 b6dda7afbd27 Fix *_get_all_orports to use ipv6_orport
  * Hmm. What if the md has an IPv6 address but the rs doesn't?
  * Also, even macros should have documentation.

 53b0788505ee Add ClientUseIPv4 and ClientPreferIPv6DirPort torrc options
  * wow this is a big one.
  * 'node_has_ipv6_addr' has a similar issue to the one from the previous
 patch -- what if the md has no ipv6 address but the rs does?
  * In the previous patch we looked at ports to see if the address was
 real; here we look at the address (in node_has_ipv6_addr).  Why? If
 there's a good reason let's document it.
  * Removing firewall_is_fascist_or() from choose_good_entry_server() makes
 us iterate over the whole nodelist needlessly sometimes.  Does that hurt
 performance ?
  *  The stuff in options_validate() to set reachable_knows_ipv6 seems
 pretty horrible.  We should be parsing these to see whether they're ipv6,
 right?  Doing strstr and strcmpstart seems like it's guaranteed to miss
 something.
  * nodelist_prefer_ipv6() seems a little icky; tristates can be confusing,
 especially when the name implies that the function returns a boolean.
 Maybe rename it to end with "_impl" to make it clear that this is an
 internal-use-only function that the other functions will refer to.
  * Also, it's against house style to say "refer to bug X" in the Tor code.
 If it's important enough to refer to in a comment, it's probably important
 enough to explain in the comment.
  * Why is nodelist_prefer_* a nodelist function?  It doesn't seem to use
 or manipulate nodes or the nodelist.
  * Can we remove the fascist_firewall_allows_... ri, rs, and md cases in
 favor of fascist_firewall_allows_node ?  Whenever an ri, rs, or md exists,
 a node should exist too.   At the very list, nobody should be using a raw
 md to refer to a node.
  * Same as above for choose_address...
  * The name fascist_firewall_... is less appropriate than it used to be.
 Let's add a new ticket to rename them though.

 1d81c63c85b4 Choose OR Entry Guards using IPv4/IPv6 preferences
   * LGTM
 4d453a7a9041 Choose directory servers by IPv4/IPv6 preferences
   * I'll return to this after I review all the little ones.
 dede8d405348 Log when IPv4/IPv6 restrictions or preferences weren't met
   * Seems worthwhile.
 a8a1cb70ce95 fixup! Add ClientUseIPv4 and ClientPreferIPv6DirPort torrc
 options
   * LGTM
 0204c9dca838 Choose bridge addresses by IPv4/IPv6 preferences
   * LGTM
 fb1cb1bd3046 Make entry_guard_set_status consistent with entry_is_live
   * Yup, lgtm.
 5ffe801801da Use fascist firewall and ClientUseIPv4 for bridge clients
   * I'll return to this too after I've reviewed all the little ones.

 Looks like I've got a couple more commits to review!

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