Opened 3 years ago

Closed 3 years ago

#20974 closed defect (fixed)

Call no directory guard happy until its directory headers are received

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords:
Cc: Actual Points: .1
Parent ID: #20822 Points: .5
Reviewer: Sponsor:

Description

In my prop271 code as it is, we call entry_guard_succeeded() from connection_dir_process_inbuf(). But apparently that can get called without data actually having been read. We should instead call the guard successful only when we get good-looking headers from it.

Child Tickets

Change History (10)

comment:1 Changed 3 years ago by teor

If we are going to make this change, maybe we should make it in the bootstrapping code, too?
It calls a hard-coded directory connection successful when it connects.
But maybe we want to wait until we start receiving sensible info from it?
(This could be a separate but related ticket.)

comment:2 Changed 3 years ago by nickm

I think that would be good idea; but let's make a separate ticket for it?

comment:3 Changed 3 years ago by nickm

Owner: set to nickm
Status: newassigned

comment:4 Changed 3 years ago by nickm

Points: .5

comment:5 Changed 3 years ago by nickm

Status: assignedneeds_review

My branch bug20974 seems possible here.

comment:6 Changed 3 years ago by nickm

Actual Points: .1

comment:7 Changed 3 years ago by dgoulet

Status: needs_reviewmerge_ready

lgtm; I'm unsure how to test this but the change seems reasonable. make test is happy.

comment:8 Changed 3 years ago by asn

I can verify that this fix works, as Tor used to mark many guards as successful just because we reached connection_dir_process_inbuf() (which apparently can happen when the socket closes).

The new code does not seem to false-positive mark guards as up. merge_ready from me as well.

I'll be monitoring the new code.

comment:9 Changed 3 years ago by nickm

Merging this. Thanks for looking at it!

comment:10 Changed 3 years ago by nickm

Resolution: fixed
Status: merge_readyclosed
Note: See TracTickets for help on using tickets.