[tor-bugs] #26228 [Core Tor/Tor]: Clarify/determine specification for padding bytes, PADDING cell

[Tor Bug Tracker & Wiki]

#26228: Clarify/determine specification for padding bytes, PADDING cell
--------------------------+------------------------
 Reporter:  dmr           |          Owner:  (none)
     Type:  defect        |         Status:  new
 Priority:  Medium        |      Milestone:
Component:  Core Tor/Tor  |        Version:
 Severity:  Normal        |     Resolution:
 Keywords:  tor-spec      |  Actual Points:
Parent ID:                |         Points:
 Reviewer:                |        Sponsor:
--------------------------+------------------------

Comment (by teor):

 Replying to [ticket:26228 dmr]:
 > ==== Background
 > I was trying to interpret the tor-spec for padding bytes, and ending up
 asking nickm for some clarification over IRC.
 > …
 > ==== Discussion: padding bytes
 > For the padding bytes that are not part of `PADDING` cells, nickm
 offered the following as a non-exhaustive set of possible forward-
 compatible options:
 > * "the [padding] bytes SHOULD be zero, and that implementations MUST
 ignore them"
 > * "The first 8 padding bytes MUST be zero; all subsequent padding bytes
 SHOULD be randomized. Implementations MUST ignore padding bytes"

 I think this is a good option, because it balances the advantages of
 randomised payloads vs forwards compatibility,

 Here's how adding a new field would work in practice:
 * the first N bytes of padding become the new "number of items" field (N
 <= 8)
 * the number of items determines the length of the new payload fields
 * the rest of the cell remains padding, with 8 bytes of zeroes followed by
 random bytes

 > * "All padding bytes should be randomized; implemenations MUST ignore
 unrecognized padding bytes"
 > ... and mentioned that "[he doesn't] know enough of the argument in
 favor of randomization to have a very strong preference"

 Neither do I, but I remember Isis saying that it was ok to just have zero
 bytes. But I can't remember why.

 > ==== Inconsistency: `PADDING` cell payload
 > (see bullet above)
 >
 > These references highlight the inconsistency:
 >
 > ^^[1^^] `PADDING: Payload is unused.` per
 [[https://gitweb.torproject.org/torspec.git/tree/tor-
 spec.txt?id=f6e93d9751002d970614662c8173ff2fa5b7c193#n469|sec 3 "Cell
 Packet format"]].
 >   implies 0 bytes of payload, so the rest should be padded per that
 section
 > ^^[2^^] `The contents of a PADDING, VPADDING, or DROP cell SHOULD be
 chosen randomly, and MUST be ignored.` per
 [[https://gitweb.torproject.org/torspec.git/tree/tor-
 spec.txt?id=f6e93d9751002d970614662c8173ff2fa5b7c193#n1723|sec 7.2 "Link
 padding"]].
 >   implies the payload of a `PADDING` cell actually is the rest of the
 size of the cell, and that it SHOULD be chosen randomly

 Unless the contents of a cell include payload and padding.

 > The `PADDING` cells were mentioned in IRC but not discussed.
 > I think a simple change to make the spec consistent between the two
 sections would be this:
 > {{{
 > PADDING: Payload contains random data. (See Sec 7.2)
 > }}}
 >
 > However, given the other points here, is that correct?

 No, the payload of a cell is the data that an implementation SHOULD parse.
 Calling the [V]PADDING and DROP content a "payload" adds to the confusion,
 because implementations SHOULD NOT parse the content of these cells. (It's
 meaningless.)

 Instead, let's say that these cells have zero-length payloads, and then
 define padding bytes consistently.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/26228#comment:1>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online