Opened 2 months ago

Closed 3 weeks ago

Last modified 3 weeks ago

#21108 closed defect (fixed)

Dir auths should vote BadExit even if they don't vote Running

Reported by: arma Owned by: nickm
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version: Tor:
Severity: Normal Keywords: 029-backport
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


While debugging #21107, I found that moria1 decided that the atlassky relay was not Running -- and as a result, it voted only the V2Dir flag for it, even though it thinks it's a BadExit.

Since BadExit is based on a threshold of badexit-voting authorities, I think that means moria1 is effectively voting *against* the badexit flag for this relay, right?

It seems that wherever we decide to withhold most of the flags for relays that we don't think are running, we should stop withholding this one.

Child Tickets

Change History (13)

comment:1 Changed 2 months ago by nickm

  • Milestone set to Tor: 0.3.0.x-final

comment:2 Changed 2 months ago by nickm

Hm. Have you got AuthDirListBadExits set?

Could this maybe be getting omitted as a sybil too? It appears that clear_status_flags_on_sybil erroneously removes is_bad_exit. Could that be the problem?

comment:3 Changed 6 weeks ago by dgoulet

  • Status changed from new to needs_information

comment:4 Changed 6 weeks ago by arma

AuthDirListBadExits 1 is set, yes.

comment:5 Changed 5 weeks ago by dgoulet

  • Status changed from needs_information to new

comment:6 Changed 4 weeks ago by dgoulet

  • Owner set to nickm
  • Status changed from new to assigned

comment:7 Changed 4 weeks ago by nickm

  • Keywords 029-backport added
  • Status changed from assigned to needs_review

bug21108_029 fixes a bug that could cause this.

comment:8 Changed 4 weeks ago by arma

please to be pushing the branch?

comment:9 Changed 4 weeks ago by nickm

whoops; pushed it

comment:10 Changed 4 weeks ago by arma

  • Status changed from needs_review to merge_ready

bug21108_029 looks fine to merge.

I think it could go into 030, since it isn't super critical to get it into 029.

I also think it isn't obviously the fix for this bug, since in my description I said moria1 was voting for V2Dir, and the code in your patch sets rs->is_v2_dir to 0, right? Oh wait, no, the code you're fixing used to skip setting is_v2_dir to 0, and your patch now does set it to 0. Perfect -- it sounds like this could indeed be the bug!

In that case, can we add a comment in there, about how we are explicitly choosing not to reset is_bad_exit to 0? Especially with the /* FFFF */ right below it about how we should be careful not to forget new flags. Actually, even better, I'll do it: bug21108 in my arma. (I cherry-picked your commit onto master first, so now it's in 030-land. Feel free to move around as you think best.)

comment:11 Changed 4 weeks ago by nickm

  • Milestone changed from Tor: 0.3.0.x-final to Tor: 0.2.9.x-final

I cherry-picked that back to my bug21108_029, and merged it to master. Leaving it for consideration in an 029 backport.

comment:12 Changed 3 weeks ago by nickm

  • Resolution set to fixed
  • Status changed from merge_ready to closed

Merged to 0.2.9

comment:13 Changed 3 weeks ago by nickm

(not backporting any further, since this is an authority-only bug)

Note: See TracTickets for help on using tickets.