Re: [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_revision
 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, 044-must  |
Parent ID:  #24403   | Points:  1
 Reviewer:  nickm|Sponsor:
 |  Sponsor55-can
-+-
Changes (by nickm):

 * status:  needs_review => needs_revision


Comment:

 The patch looks pretty good; I've noted a couple more places it could use
 const.  Besides that and the CI issue, I think we're ok.

 Also I think the test file is probably fine as it is: they're often not
 too short.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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, 044-must  |
Parent ID:  #24403   | Points:  1
 Reviewer:  nickm|Sponsor:
 |  Sponsor55-can
-+-

Comment (by nickm):

 It looks like you're getting an error in travis CI:

 {{{
 src/test/test_dirvote.c:38:14: error: no previous extern declaration for
 non-static variable 'router_properties' [-Werror,-Wmissing-variable-
 declarations]

 digestmap_t *router_properties = NULL;
 }}}

 This error means that the compiler wants you either to declare
 `router_properties` as static, to make it clear that it can only be used
 inside of `test_dirvote.c`, or instead to make an extern declaration for
 `router_properties` in some header, to make it clear that other modules
 can use it too.

 Now I'm looking over the patch...

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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
-+-
Changes (by maurice_pibouin):

 * status:  needs_revision => needs_review


Comment:

 Finally managed to write the whole patch ! See
 https://github.com/torproject/tor/pull/1871
 I feel like the test file is a bit too long, do you have suggestions on
 how to factorize it better ?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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_revision
 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 MrSquanchee):

 Replying to [comment:41 maurice_pibouin]:
 > Not ready for review yet, the PR was just to see if it passed CI,
 switched PR to draft.

 You can enable travis-ci in your forked repository. That way you can
 submit your PR after completion.
 Thanks :)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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_revision
 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
-+-
Changes (by maurice_pibouin):

 * status:  needs_review => needs_revision


Comment:

 Not ready for review yet, the PR was just to see if it passed CI, switched
 PR to draft.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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
-+-
Changes (by teor):

 * status:  needs_revision => needs_review


Comment:

 I think this ticket needs review, I see a new PR here:
 https://github.com/torproject/tor/pull/1871

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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_revision
 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):

 Replying to [comment:38 maurice_pibouin]:
 > Thank you for the review !
 >
 >  * I'm not sure what you mean by new patch : a different PR, branch,
 issue, or just new commits ?

 New commits would be fine.

 >  * I didn't do a PR because I thought it was reserved to "finished"
 patches (ie that include tests), I will use a PR next time

 It's good to have a PR even if it's not ready.  That lets  you get CI
 results, and lets us comment directly on the code.

 >  * Comment removal was a mistake
 >  * I didn't see anything in `CodingStandards.md` about the if-else
 newline, is it just common good practice ?

 It's what we do in the rest of the code, except in a few places where we
 didn't catch it during initial code review.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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_revision
 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 maurice_pibouin):

 Thank you for the review !

  * I'm not sure what you mean by new patch : a different PR, branch,
 issue, or just new commits ?

  * I didn't do a PR because I thought it was reserved to "finished"
 patches (ie that include tests), I will use a PR next time
  * Comment removal was a mistake
  * I didn't see anything in `CodingStandards.md` about the if-else
 newline, is it just common good practice ?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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_revision
 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
-+-
Changes (by nickm):

 * status:  needs_review => needs_revision


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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
-+-
Changes (by teor):

 * milestone:  Tor: unspecified => Tor: 0.4.4.x-final


Comment:

 Hmm, maybe we missed the review on this one because it's not in the 0.4.4
 milestone.

 Also, last weekend was a 3-5 day weekend for a lot of people, so we're
 still catching up.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:
 |  unspecified
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
-+-
Changes (by maurice_pibouin):

 * status:  needs_revision => needs_review


Comment:

 I refactored everything according to your comments. Please tell me what
 you think of it and I'll write the tests (there are 5 functions to test
 now ...) once you are satisfied with my patch.
 Branch : ​https://github.com/vnepveu/tor/tree/ticket7193

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
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
-+-
Changes (by MrSquanchee):

 * cc: usuraj35@… (added)


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
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
-+-
Changes (by MrSquanchee):

 * cc: usuraj35@… (removed)


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
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 teor):

 I agree with Nick's answer to this question:
 > It probably makes sense for there to be two different variants of the
 function -- one that sorts by ipv6 and one that sorts by ipv4. The parts
 of the two functions that would be shared in common should turn into a
 third function that they both would call.

 I don't mind so much about the interface between get_possible_sybil_list()
 and dirserv_generate_networkstatus_vote_obj().

 It seems like a good idea to me to get separate lists of IPv4 and IPv6
 sybils. You could try to merge them before returning to
 dirserv_generate_networkstatus_vote_obj(), but that might be hard. Or you
 could just loop through each list separately, and call
 rep_hist_make_router_pessimal() on them all.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
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 maurice_pibouin):

 Replying to [comment:20 teor]:
 > There's a good description of how to implement this change in Proposal
 312, section 3.5.7:
 > https://gitweb.torproject.org/torspec.git/tree/proposals/312-relay-auto-
 ipv6-addr.txt#n

 I am trying to change the get_possible_sybil_list function.
 If adding the "family" argument is easy, it seems like it would require
 changes where the function is called (inside
 dirserv_generate_networkstatus_vote_obj, l4535 ), so that one call is made
 for IPv6 routers, and one is made for IPv4 routers. I was thinking of
 creating two routers list (one IPv4 , one IPv6) but that seems messy.
 Wouldn't it be better to filter the "family" directly inside
 "get_possible_sybil_list" so as to avoid having to change too many things
 ?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ipv6, intro, tor-dirauth, security,  |  Actual Points:
  sybil, network-health, outreachy-ipv6  |
Parent ID:  #24403   | Points:  1
 Reviewer:  nickm|Sponsor:
 |  Sponsor55-can
-+-

Comment (by maurice_pibouin):

 Thanks a lot for your comments, I will try to fix everything !

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ipv6, intro, tor-dirauth, security,  |  Actual Points:
  sybil, network-health, outreachy-ipv6  |
Parent ID:  #24403   | Points:  1
 Reviewer:  nickm|Sponsor:
 |  Sponsor55-can
-+-
Changes (by nickm):

 * status:  needs_review => needs_revision


Comment:

 Some notes on the content of the patch:
   * This like is incorrect `node_t *node_running =
 tor_malloc(sizeof(node_t *));`.  That "sizeof" is taking the size of a
 pointer, not the size of a node_t.  Because of that, you'll only allocate
 a few bytes, not a whole node_t.
   * It looks like the `mock_node_get*` functions have a memory leak -- you
 malloc node_running, but there is nothing that frees it.  Please remember
 that in C, every allocated object needs to have a corresponding free.  If
 you run configure with `--enable-fragile-hardening` it will turn on extra
 run-time checking that will catch this kind of problem for you.
   * When using strlcpy or strlcat, it's good style to use the size of the
 target buffer.
   * For the address comparison, could we use tor_addr_compare() ?
   * I don't understand why we're looking at the family type for the ipv6
 address -- if it's an IPv6 address, then the family should always be
 AF_INET6.  (And if the family is AF_INET, then it shouldn't be stored in a
 variable called ipv6_addr).
   * Most fundamentally: this function seems to be for sorting relays into
 a list by address and then by bandwidth.  But now it is trying to sort by
 ipv6 address _and_ by ipv4 address at the same time.  I don't think that
 makes sense -- a relay can have both kinds of address, but it wouldn't
 appear at more than one point in the list necessarily.

 It probably makes sense for there to be two different variants of the
 function -- one that sorts by ipv6 and one that sorts by ipv4.  The parts
 of the two functions that would be shared in common should turn into a
 third function that they both would call.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ipv6, intro, tor-dirauth, security,  |  Actual Points:
  sybil, network-health, outreachy-ipv6  |
Parent ID:  #24403   | Points:  1
 Reviewer:  nickm|Sponsor:
 |  Sponsor55-can
-+-
Changes (by gk):

 * cc: gk (added)


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ipv6, intro, tor-dirauth, security,  |  Actual Points:
  sybil, network-health, outreachy-ipv6  |
Parent ID:  #24403   | Points:  1
 Reviewer:  nickm|Sponsor:
 |  Sponsor55-can
-+-
Changes (by nickm):

 * reviewer:   => nickm


Comment:

 Thanks! I'll have a look on Monday.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ipv6, intro, tor-dirauth, security,  |  Actual Points:
  sybil, network-health, outreachy-ipv6  |
Parent ID:  #24403   | Points:  1
 Reviewer:   |Sponsor:
 |  Sponsor55-can
-+-
Changes (by maurice_pibouin):

 * status:  new => needs_review


Comment:

 Hi ! Managed to write a refactorization of compare_router_by_ip_and_bw_,
 aswell as the corresponding unit tests. Before submitting a pull request
 with the same thing for get_possible_sybil_list, I'd very much appreciate
 if you could point out any flaws in my current work.
 Branch : https://github.com/vnepveu/tor/tree/ticket7193

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ipv6, intro, tor-dirauth, security,  |  Actual Points:
  sybil, network-health, outreachy-ipv6  |
Parent ID:  #24403   | Points:  1
 Reviewer:   |Sponsor:
 |  Sponsor55-can
-+-

Comment (by nickm):

 You need to read the warnings carefully:
 {{{
 In file included from ./src/lib/container/map.h:15,
  from ./src/core/or/or.h:27,
  from src/feature/client/bridges.c:16:
 ./src/feature/nodelist/dirlist.h:30:16: error: redundant redeclaration of
 ‘router_digest_is_trusted_dir_type’ [-Werror=redundant-decls]
30 | MOCK_DECL(int, router_digest_is_trusted_dir_type,
   |^
 ./src/lib/testsupport/testsupport.h:128:6: note: in definition of macro
 ‘MOCK_DECL’
   128 |   rv funcname arglist
   |  ^~~~
 In file included from src/feature/client/bridges.c:28:
 ./src/feature/nodelist/dirlist.h:28:5: note: previous declaration of
 ‘router_digest_is_trusted_dir_type’ was here
28 | int router_digest_is_trusted_dir_type(const char *digest,
   | ^
 }}}

 This is telling you that the function `router_digest_is_trusted_dir_type`
 is getting declared twice: first on line 28, and then on line 30.

 If you remove the declaration on line 28, that warning will go away.

 There's still another warning in dirlist.c, since you're using MOCK_DECL
 there: when you are implementing a function, you use MOCK_IMPL instead.
 There should only be one implementation of the function in dirlist.c, and
 it should be decorated with MOCK_IMPL.

 When you actually want to declare a mock replacement for the function, you
 do that in the unit tests, and you give it a different name from the
 original function.  Then in the test code, you can use MOCK and UNMOCK to
 replace the original function.

 There's more documentation on mocking, along with examples, in
 `src/lib/testsupport/testsupport.h`,
 and in `doc/HACKING/WritingTests.md`



  -- the mock implementation doesn't belon

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ipv6, intro, tor-dirauth, security,  |  Actual Points:
  sybil, network-health, outreachy-ipv6  |
Parent ID:  #24403   | Points:  1
 Reviewer:   |Sponsor:
 |  Sponsor55-can
-+-

Comment (by maurice_pibouin):

 I am writing tests for my modified implementation of
 "compare_routerinfo_by_ip_and_bw_", and I have trouble using the MOCK_DECL
 and MOCK_IMPL macros. I am trying to MOCK the function
 "routerdigest_is_trusted_dir_type", so I put a MOCK_DECL statement in
 dirlist.h, and a MOCK_IMPL statement in dirlist.c . On compilation, it
 doesn't work : I get "redefinition of
 ‘router_digest_is_trusted_dir_type’". I also tried a bunch of different
 location for both statements.
 See my branch : https://github.com/vnepveu/tor/tree/ticket7193_exp

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ipv6, intro, tor-dirauth security|  Actual Points:
  sybil, network-health  |
Parent ID:  #24403   | Points:  1
 Reviewer:   |Sponsor:
 |  Sponsor55-can
-+-
Changes (by teor):

 * points:  small => 1
 * sponsor:   => Sponsor55-can


Comment:

 There's a good description of how to implement this change in Proposal
 312, section 3.5.7:
 https://gitweb.torproject.org/torspec.git/tree/proposals/312-relay-auto-
 ipv6-addr.txt#n

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ipv6, intro, tor-dirauth security|  Actual Points:
  sybil, network-health  |
Parent ID:  #24403   | Points:  small
 Reviewer:   |Sponsor:
-+-
Changes (by gk):

 * keywords:  ipv6, intro, tor-dirauth security sybil => ipv6, intro, tor-
 dirauth security sybil, network-health


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ipv6, intro, tor-dirauth security|  Actual Points:
  sybil  |
Parent ID:  #24403   | Points:  small
 Reviewer:   |Sponsor:
-+-

Old description:

> Some bugs:
>
> `get_possible_sybil_list()` doesn't consider IPv6 addresses at all.
>
> `clear_status_flags_on_sybil()` doesn't clear `ipv6_addr` (and maybe more
> flags).
>
> Also, maybe we could add a `log_notice` or `log_info` to mention if and
> which relays were found to be part of a Sybil attack.
>
> Finally (and this is a minor bug), in `get_possible_sybil_list()` we
> assume that `max_with_same_addr < max_with_same_addr_on_authority`, which
> is true in the current tor network, but maybe it shouldn't be an inherent
> property of the source code.

New description:

 Some bugs:

 `get_possible_sybil_list()` doesn't consider IPv6 addresses at all.

 ~~`clear_status_flags_on_sybil()` doesn't clear `ipv6_addr` (and maybe
 more flags).~~ Obsoleted by consensus method 24, because it requires the
 Running flag for a router to be in the consensus.

 Also, maybe we could add a `log_notice` or `log_info` to mention if and
 which relays were found to be part of a Sybil attack.

 ~~Finally (and this is a minor bug), in `get_possible_sybil_list()` we
 assume that `max_with_same_addr < max_with_same_addr_on_authority`, which
 is true in the current tor network, but maybe it shouldn't be an inherent
 property of the source code.~~ Obsoleted by #20960:
 max_with_same_addr_on_authority has been removed.

--

Comment (by teor):

 We've made some progress on this issue, via other tickets that obsolete
 some of the bugs listed here.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ipv6, intro, tor-dirauth security|  Actual Points:
  sybil  |
Parent ID:  #24403   | Points:  small
 Reviewer:   |Sponsor:
-+-
Changes (by teor):

 * parent:   => #24403


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ipv6, intro, tor-dirauth security|  Actual Points:
  sybil  |
Parent ID:   | Points:  small
 Reviewer:   |Sponsor:
-+-

Comment (by teor):

 We need to do #24393 before we do this.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ipv6, intro, tor-dirauth security|  Actual Points:
  sybil  |
Parent ID:   | Points:  small
 Reviewer:   |Sponsor:
-+-
Changes (by nickm):

 * keywords:  ipv6, intro, tor-dirauth => ipv6, intro, tor-dirauth security
 sybil


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs