Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#4607 closed defect (fixed)

Fix possibly goofy behaviour of intro-circ-creation ratelimiting

Reported by: rransom Owned by: rransom
Priority: Medium Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-hs
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

See commit e46d56a9b4458370cb8de0c92e10688402749845. The behaviour described by the old version of that comment is probably correct; the behaviour implemented by that chunk of code (and described in the new version of that comment) is daft.

Child Tickets

Change History (10)

comment:1 Changed 8 years ago by nickm

rransom: I agree; is this something you can throw together a patch for in time for 0.2.3.x?

comment:2 Changed 8 years ago by rransom

Status: newneeds_review

The behaviour described by the old version of that comment is also silly -- if Tor has established all the intro points that it wants for a service, it won't need to establish more for at least INTRO_CIRC_RETRY_PERIOD seconds (currently five minutes), so there's no point in resetting service->intro_period_started.

See my bug4607 branch for a patch to remove that paragraph of code.

comment:3 Changed 8 years ago by arma

It seems unlikely this is a bugfix on 0.0.7rc1. Were you thinking it was git commit 011ccbbf8 that introduced it? Tor 0.0.7pre1 has the more likely looking changelog entry (git commit a782b83c).

comment:4 Changed 8 years ago by nickm

Should we retain the "continue", or was that erroneous even before a782b83c ?

comment:5 in reply to:  3 ; Changed 8 years ago by rransom

Replying to arma:

It seems unlikely this is a bugfix on 0.0.7rc1. Were you thinking it was git commit 011ccbbf8 that introduced it? Tor 0.0.7pre1 has the more likely looking changelog entry (git commit a782b83c).

This is a bugfix on commit a782b83c2877b3beebf114d602f006fa6781da86. The first Git tag containing that commit is “tor-0.0.7rc1”.

comment:6 in reply to:  4 Changed 8 years ago by rransom

Replying to nickm:

Should we retain the "continue", or was that erroneous even before a782b83c ?

We should retain it. It appears to be unnecessary (i.e. not change the behaviour of the function), but I didn't realize that I was ripping it out, and it shouldn't be removed by this commit.

Fixup pushed.

comment:7 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Squashed and merged; thanks!

comment:8 in reply to:  5 Changed 8 years ago by arma

Replying to rransom:

Replying to arma:

It seems unlikely this is a bugfix on 0.0.7rc1. Were you thinking it was git commit 011ccbbf8 that introduced it? Tor 0.0.7pre1 has the more likely looking changelog entry (git commit a782b83c).

This is a bugfix on commit a782b83c2877b3beebf114d602f006fa6781da86. The first Git tag containing that commit is “tor-0.0.7rc1”.

Looks like the git tag is mixed up then. I just checked the 0.0.7pre1 tarball and it has that commit in it. (Archeologists would cry.)

comment:9 Changed 7 years ago by nickm

Keywords: tor-hs added

comment:10 Changed 7 years ago by nickm

Component: Tor Hidden ServicesTor
Note: See TracTickets for help on using tickets.