[tor-bugs] #16849 [Core Tor/Tor]: clear_status_flags_on_sybil might want to clear more flags

[Tor Bug Tracker & Wiki]

#16849: clear_status_flags_on_sybil might want to clear more flags
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:
                                                 |  ffmancera
     Type:  defect                               |         Status:
                                                 |  needs_review
 Priority:  High                                 |      Milestone:  Tor:
                                                 |  0.3.4.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  easy, SponsorS-deferred, technical-  |  Actual Points:
  debt tor-dirauth pending-disaster              |
Parent ID:                                       |         Points:  small
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by teor):

 Replying to [ticket:16849 teor]:
 > clear_status_flags_on_sybil contains a comment saying "it's easy to add
 a new flag but forget to add it to this clause."
 >
 > It looks like we may have forgot the following flags:
 > * is_hs_dir
 > * version_known?
 > * version_supports_extend2_cells?
 > * has_bandwidth
 > * has_exitsummary?
 > * bw_is_unmeasured? (set to 1?)
 > * bandwidth_kb
 > * has_guardfraction
 > * guardfraction_percentage

 It's been 2 years since this ticket was opened, are there's any new flags
 we should also clear?

 > To deal with the root cause, should we instead zero out the entire
 `routerstatus_t`, then copy the fields we need back in?
 > (This would zero new fields on sybils by default.)

 Do you think we should zero the struct, then copy across the fields we
 want to keep?
 Or do you think that touching the entire struct is a bad idea?

 We could make it easier to clear the flags by putting them in their own
 struct. But that's a pretty extreme refactor.

 > We could also implement a unit test for clear_status_flags_on_sybil that
 checks that certain (important?) flags are cleared, or that all flags are
 cleared (?).

 Would you like to write a unit test to make sure we clear important status
 flags?
 (It's important that we clear consensus flags like Guard, Exit, V2Dir, …)

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