Opened 10 months ago

Last modified 5 months ago

#32830 accepted defect

Relay_extended - hash and padding - specs are wrong or unclear

Reported by: Aymeric Owned by: nickm
Priority: Medium Milestone: Tor: 0.4.4.x-final
Component: Core Tor Version:
Severity: Normal Keywords: 044-can, postfreeze-ok
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


I noticed recently plenty of 'unrecognized' relay_extended messages for node-Tor project while everything was working fine in the past after I updated the code and made it modular (and some abnormal delays to establish circuits), see also this is temporary but I had to put a lot of debug stuff to find out what was going on

Finally I figured out why: unlike what is writen in the main Tor specs the hash is calculated not only with the real payload of the relay_extended messages but includes also the padding (apparently starting with 00000000, not sure where it comes from neither where it is specified)

This looks quite strange, what is the rationale for this, is it a bug, why is it not documented and does it impact other types of messages?

Child Tickets

Change History (9)

comment:1 Changed 10 months ago by Aymeric

You can close it, apparently it was more a problem on my side and the whole stream content must be processed (but still not clear in the specs where the payload is mentionned)

comment:2 Changed 10 months ago by nickm

Do you have a suggested change to the specs? I'd like to take a patch to make this more clear.

comment:3 Changed 10 months ago by Aymeric

Maybe it's me because rereading it I find it clear now but I misinterpreted "cell's entire payload" with "stream payload"

So, maybe, it does not harm to add something like:

"the 'digest' field is computed as the first four bytes of the running digest of all the bytes that have been destined for this hop of the circuit or originated from this hop of the circuit, seeded from Df or Db respectively (obtained in section 5.2 above), and including this RELAY cell's entire payload (including padding bytes and taken with the digest field set to zero)."

Why is there this 4 zero bytes separator before padding bytes? (is it somewhere in the specs?)

comment:4 Changed 10 months ago by nickm

Milestone: Tor: 0.4.3.x-final
Owner: set to nickm
Status: newaccepted

comment:5 Changed 9 months ago by nickm

Keywords: 043-deferred added

All 0.4.3.x tickets without 043-must, 043-should, or 043-can are about to be deferred.

comment:6 Changed 9 months ago by nickm

Keywords: 043-can added; 043-deferred removed

comment:7 Changed 5 months ago by nickm

Milestone: Tor: 0.4.3.x-finalTor: 0.4.4.x-final

0.4.3 was released: Move non merge-ready 0.4.3 tickets to 044.

comment:8 Changed 5 months ago by nickm

Keywords: 044-can added; 043-can removed

comment:9 Changed 5 months ago by nickm

Keywords: postfreeze-ok added

Mark tickets which are important or safe enough to look at post-freeze for 0.4.4.

Note: See TracTickets for help on using tickets.