Opened 3 months ago

Closed 3 months ago

Last modified 2 months ago

#31098 closed defect (fixed)

transition when we send our first padding packet, not on received

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: 0.4.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords:
Cc: asn, mikeperry, gaba Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The RP circpad machine, on the relay side, should transition as soon as a padding cell is sent to the client according to the code comment. The code currently transitions on receiving a padding cell, not sending.

Copied from:
https://github.com/torproject/tor/pull/1167

Gaba, I think this is Sponsor 2, because it's a circuit padding bug.

Mike or asn, please add tags.

Child Tickets

Change History (11)

comment:1 Changed 3 months ago by teor

Status: newneeds_review

comment:2 Changed 3 months ago by teor

Keywords: 041-backport-maybe added

comment:3 Changed 3 months ago by asn

Status: needs_reviewmerge_ready

LGTM! Rend machines send just a single DROP (padding) cell and then they go into END state, so this should be good since the behavior does not change (it just goes into END earlier (when the padding is sent and not when it's received)).

Last edited 3 months ago by asn (previous) (diff)

comment:4 Changed 3 months ago by nickm

Keywords: 041-should added
Status: merge_readyneeds_review

So, how tested is this, and how certain are we that it is correct?

Also, could somebody please add a changes file here?

comment:5 Changed 3 months ago by nickm

Status: needs_reviewneeds_revision

comment:6 Changed 3 months ago by asn

Similarly to #31112 this is tested by running the padanalyzer script and making sure that it pads properly. This should not change the behavior of wtf-pad, and hence it's not really a bugfix so I think it can go in master.

You can find it along with #31112 and a changes file in ​https://github.com/torproject/tor/pull/1185 .

comment:7 Changed 3 months ago by asn

Status: needs_revisionmerge_ready

comment:8 Changed 3 months ago by asn

Milestone: Tor: 0.4.1.x-finalTor: 0.4.2.x-final

comment:9 Changed 3 months ago by asn

Keywords: 041-should removed

comment:10 Changed 3 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

Thanks for the explanation! Merged to master; not backporting.

comment:11 Changed 2 months ago by teor

Keywords: 041-backport-maybe removed
Note: See TracTickets for help on using tickets.