[tor-bugs] #7193 [Core Tor/Tor]: Tor's sybil protection doesn't consider IPv6

[Tor Bug Tracker & Wiki]

#7193: Tor's sybil protection doesn't consider IPv6
-------------------------------------------------+-------------------------
 Reporter:  asn                                  |          Owner:  (none)
     Type:  enhancement                          |         Status:
                                                 |  needs_review
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.4.4.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  ipv6, intro, tor-dirauth, security,  |  Actual Points:
  sybil, network-health, outreachy-ipv6,         |
  network-team-roadmap-2020Q1                    |
Parent ID:  #24403                               |         Points:  1
 Reviewer:  nickm                                |        Sponsor:
                                                 |  Sponsor55-can
-------------------------------------------------+-------------------------

Comment (by nickm):

 Hi again, and sorry about the delay!  The last week has been very intense.

 This new version of the code looks more workable than the last. I'd
 suggest these changes:
   * I'd suggest making a new patch that takes the new parts of
 `dirserv_generate_networkstatus_vote_obj` and turns it into a new
 function, for testing purposes.
   * Also, it seems like the code in
 `dirserv_generate_networkstatus_vote_obj` won't compare routers by IPv6
 addresses if they have any IPv4 address.  That seems wrong.
   * I don't think that removing the comment in that function about not
 using rl->routers is an improvement.
   * Our code style is not to write "if (a) b;" or "else c;" all on one
 line.  We should always have a new line for the code in the if and else
 blocks.
   * I think we could have a single "omit_as_sybil" digestmap, and add both
 sets of routers that we want to remove to it.  That would remove the need
 for duplication in other parts of the code.  (Also, it is not correct to
 call `dirserv_compute_performance_thresholds` twice in this way: it needs
 to be called exactly once per vote, with the complete list of sybils to
 omit.)
   * You don't need to malloc last_ipv6_addr in get_possible_sybil_list;
 you can just put it on the stack with `tor_addr_t last_ipv6_addr;`.
   * To copy `tor_addr_t`, use `tor_addr_copy`.
   * For the next version of this patch, could you please make a pull
 request?  That way, travis and appveyor will both run tests on the code to
 see if it's passing.

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