Opened 4 months ago

Closed 2 months ago

Last modified 2 months ago

#22779 closed defect (fixed)

choose_good_entry_server() is no longer used to choose entry guards

Reported by: teor Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version: Tor: 0.3.0.1-alpha
Severity: Normal Keywords: maybe-030-backport, review-group-22
Cc: Actual Points:
Parent ID: Points: 0.5
Reviewer: asn Sponsor: SponsorV

Description

choose_good_entry_server() says:

 * If <b>state</b> is NULL, we're choosing a router to serve as an entry
 * guard, not for any particular circuit.

But in 0.3.0 and later, we use guardsets to choose entry guards.
(And we use this function to choose individual entries.)

We should:

  • fix the comment,
  • check for NULL state (and do what?), and
  • make sure that guard selection and entry selection doesn't get out of sync. (I think we're ok here, but I don't understand the new guard code very well.)

Child Tickets

Change History (7)

comment:1 Changed 3 months ago by nickm

Owner: set to nickm
Status: newaccepted

comment:2 in reply to:  description Changed 3 months ago by nickm

Sponsor: SponsorV
Status: acceptedneeds_review

Replying to teor:

I have a patch here as bug22779_031.

  • fix the comment,

done

  • check for NULL state (and do what?)

In this case, we treat it as a nonfatal assertion. I've audited the code that calls this to make sure it can't happen.

  • make sure that guard selection and entry selection doesn't get out of sync. (I think we're ok here, but I don't understand the new guard code very well.)

I think we're okay here.

comment:3 Changed 2 months ago by nickm

Keywords: review-group-22 added

comment:4 Changed 2 months ago by asn

Reviewer: asn

comment:5 Changed 2 months ago by dgoulet

Status: needs_reviewmerge_ready

ack.

comment:6 Changed 2 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

merged to 0.3.2; I think we don't need to backport.

comment:7 Changed 2 months ago by asn

ACKing this as well.

Note: See TracTickets for help on using tickets.