Opened 5 months ago

Closed 5 months ago

#24894 closed defect (fixed)

v3 onion services don't respect MAX_REND_FAILURES

Reported by: arma Owned by: arma
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version: Tor: 0.3.2.1-alpha
Severity: Normal Keywords: 032-backport, review-group-30
Cc: Actual Points:
Parent ID: Points: 0.5
Reviewer: asn Sponsor:

Description

In can_relaunch_service_rendezvous_point() we check

  if (circ->build_state->failure_count > MAX_REND_FAILURES ||
      circ->build_state->expiry_time <= time(NULL)) {

for whether to abort the relaunch.

But in retry_service_rendezvous_point(), we do this:

  /* Transfer build state information to the new circuit state in part to
   * catch any other failures. */
  new_circ->build_state->failure_count = bstate->failure_count++;

That ++ increments the failure_count for the *old* circuit, which means the new circuit gets a failure_count of 0. No new circuits ever have a failure count of anything other than 0.

The legacy onion services handle it better, by doing

  newstate->failure_count = oldstate->failure_count+1;

Child Tickets

Change History (9)

comment:1 Changed 5 months ago by arma

Owner: set to dgoulet
Status: newassigned

Assigning to dgoulet since I bet he'll be excited to fix it.

I set it to milestone 0.3.3.x at first, but we'll probably want to backport.

comment:2 Changed 5 months ago by teor

Keywords: 032-backport added
Points: 0.5
Version: Tor: 0.3.2.1-alpha

We have a tag for backports.

comment:3 Changed 5 months ago by arma

Owner: changed from dgoulet to arma

My bug24894 branch implements a fix. It's based on maint-0.3.2.

comment:4 Changed 5 months ago by arma

Status: assignedneeds_review

comment:5 Changed 5 months ago by nickm

Keywords: review-group-30 added

comment:6 Changed 5 months ago by dgoulet

Reviewer: dgoulet

comment:7 Changed 5 months ago by asn

Status: needs_reviewmerge_ready

LGTM!

comment:8 Changed 5 months ago by asn

Reviewer: dgouletasn

comment:9 Changed 5 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

Okay; merging to 0.3.2 and forward.

Note: See TracTickets for help on using tickets.