Opened 5 months ago

Closed 4 months ago

Last modified 4 months 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: 0.3.0.1-alpha
Severity: Normal Keywords: 029-backport
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

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 5 months ago by nickm

  • Milestone set to Tor: 0.3.0.x-final

comment:2 Changed 5 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 4 months ago by dgoulet

  • Status changed from new to needs_information

comment:4 Changed 4 months ago by arma

AuthDirListBadExits 1 is set, yes.

comment:5 Changed 4 months ago by dgoulet

  • Status changed from needs_information to new

comment:6 Changed 4 months ago by dgoulet

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

comment:7 Changed 4 months 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 months ago by arma

please to be pushing the branch?

comment:9 Changed 4 months ago by nickm

whoops; pushed it

comment:10 Changed 4 months 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 months 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 4 months ago by nickm

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

Merged to 0.2.9

comment:13 Changed 4 months ago by nickm

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

Note: See TracTickets for help on using tickets.