Opened 11 months ago

Closed 10 months ago

Last modified 10 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 11 months ago by nickm

Milestone: Tor: 0.3.0.x-final

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

Status: newneeds_information

comment:4 Changed 10 months ago by arma

AuthDirListBadExits 1 is set, yes.

comment:5 Changed 10 months ago by dgoulet

Status: needs_informationnew

comment:6 Changed 10 months ago by dgoulet

Owner: set to nickm
Status: newassigned

comment:7 Changed 10 months ago by nickm

Keywords: 029-backport added
Status: assignedneeds_review

bug21108_029 fixes a bug that could cause this.

comment:8 Changed 10 months ago by arma

please to be pushing the branch?

comment:9 Changed 10 months ago by nickm

whoops; pushed it

comment:10 Changed 10 months ago by arma

Status: needs_reviewmerge_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 10 months ago by nickm

Milestone: Tor: 0.3.0.x-finalTor: 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 10 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

Merged to 0.2.9

comment:13 Changed 10 months ago by nickm

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

Note: See TracTickets for help on using tickets.