Trac issueshttps://gitlab.torproject.org/legacy/trac/-/issues2020-06-13T15:28:29Zhttps://gitlab.torproject.org/legacy/trac/-/issues/26894spec: say CREATE/CREATE2 and EXTEND/EXTEND2 when we mean to2020-06-13T15:28:29ZTaylor Yuspec: say CREATE/CREATE2 and EXTEND/EXTEND2 when we mean toIn several places in tor-spec.txt, it looks like we say only `CREATE` or `EXTEND` when we really mean to also include `CREATE2` or `EXTEND2`. We should fix those.In several places in tor-spec.txt, it looks like we say only `CREATE` or `EXTEND` when we really mean to also include `CREATE2` or `EXTEND2`. We should fix those.Tor: 0.3.5.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/26893spec: all-zeroes special case is for relay IDs, not cell digests2020-06-13T15:28:28ZTaylor Yuspec: all-zeroes special case is for relay IDs, not cell digeststor-spec.txt Section 5.3 (Creating circuits) says:
```
As special cases, if the extend cell includes a digest of
all zeroes, or asks to extend back to the relay that sent the extend
```
The implementation uses "digest" internally t...tor-spec.txt Section 5.3 (Creating circuits) says:
```
As special cases, if the extend cell includes a digest of
all zeroes, or asks to extend back to the relay that sent the extend
```
The implementation uses "digest" internally to refer to relay ID hashes, while tor-spec.txt primarily uses it to refer to the truncated running SHA-1 hash that relays use to "recognize" a cell as targeting them.
arma says this probably intends to refer to relay ID hashes, and pointed to commit e34727a65c411a6260f3960ff8a753088e287565.
We should reword this text to make it clear it refers to relay ID hashes.Tor: 0.3.5.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/26885tor-spec: Generalise "exit" to "end"2020-06-13T15:28:26Zteortor-spec: Generalise "exit" to "end"In a Tor circuit, the end nodes can be an exit or a directory mirror or an onion service directory or an intro or a rend node.
Perhaps we should note this somewhere in torspec?In a Tor circuit, the end nodes can be an exit or a directory mirror or an onion service directory or an intro or a rend node.
Perhaps we should note this somewhere in torspec?Tor: 0.3.5.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/26872tor-spec: Fix error in recognized, and explain better2020-06-13T15:28:23Zteortor-spec: Fix error in recognized, and explain betterRecognized is 2 bytes, so it is zero with probability `1 in 2^16`, not `1 in 2^32`.Recognized is 2 bytes, so it is zero with probability `1 in 2^16`, not `1 in 2^32`.Tor: 0.3.5.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/26228Clarify/determine specification for padding bytes, (formerly also PADDING cell)2020-06-13T15:28:22ZdmrClarify/determine specification for padding bytes, (formerly also PADDING cell)**EDIT: strikethrough content below is now covered in #26870 instead**
==== Background
I was trying to interpret the tor-spec for padding bytes, and ending up asking nickm for some clarification over IRC.
nickm suggested most of the cc'...**EDIT: strikethrough content below is now covered in #26870 instead**
==== Background
I was trying to interpret the tor-spec for padding bytes, and ending up asking nickm for some clarification over IRC.
nickm suggested most of the cc'd for the ticket - I added atagar, too.
==== Unclear areas
Here are the points that need clarification / specification:
* spec for padding bytes does not clearly say what senders `MUST` or `SHOULD` do, [[mentioning that padding is with 0 bytes](https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt?id=f6e93d9751002d970614662c8173ff2fa5b7c193#n412|only)] or [[NUL bytes](https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt?id=f6e93d9751002d970614662c8173ff2fa5b7c193#n1480|(elsewhere))]
* spec for padding bytes does not say what receivers `MUST` or `SHOULD` do, when receiving non-zero bytes in the Cell (e.g. warn? ignore?)
* ~~spec is a bit inconsistent with `PADDING` cells ^^[1^^]^^[2^^]~~
==== 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"
* "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"
==== ~~Inconsistency: `PADDING` cell payload~~
~~(see bullet above)~~
~~These references highlight the inconsistency:~~
~~^^[1^^] `PADDING: Payload is unused.` per [[3 "Cell Packet format"](https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt?id=f6e93d9751002d970614662c8173ff2fa5b7c193#n469|sec)].~~
~~ 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 [[7.2 "Link padding"](https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt?id=f6e93d9751002d970614662c8173ff2fa5b7c193#n1723|sec)].~~
~~ implies the payload of a `PADDING` cell actually is the rest of the size of the cell, and that it SHOULD be chosen randomly~~
~~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?~~Tor: 0.3.5.x-finaldmrdmrhttps://gitlab.torproject.org/legacy/trac/-/issues/26870Spec: clarify inconsistency for [V]PADDING/DROP cell content vs. padding bytes2020-06-13T15:28:22ZdmrSpec: clarify inconsistency for [V]PADDING/DROP cell content vs. padding bytesThis ticket is split off from #26228.
Here's the relevant description from that ticket:
Replying to [dmr](#26228):
> [...]
> ==== Unclear areas
> Here are the points that need clarification / specification:
> * [...]
> * spec is a bit i...This ticket is split off from #26228.
Here's the relevant description from that ticket:
Replying to [dmr](#26228):
> [...]
> ==== Unclear areas
> Here are the points that need clarification / specification:
> * [...]
> * spec is a bit inconsistent with `PADDING` cells ^^[1^^]^^[2^^]
> [...]
>
> ==== Inconsistency: `PADDING` cell payload
> (see bullet above)
>
> These references highlight the inconsistency:
>
> ^^[1^^] `PADDING: Payload is unused.` per [[3 "Cell Packet format"](https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt?id=f6e93d9751002d970614662c8173ff2fa5b7c193#n469|sec)].
> 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 [[7.2 "Link padding"](https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt?id=f6e93d9751002d970614662c8173ff2fa5b7c193#n1723|sec)].
> implies the payload of a `PADDING` cell actually is the rest of the size of the cell, and that it SHOULD be chosen randomly
>
> 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?Tor: 0.3.5.x-finaldmrdmrhttps://gitlab.torproject.org/legacy/trac/-/issues/26860Spec: decryption order appears to be wrongly specified2020-06-13T15:28:19ZdmrSpec: decryption order appears to be wrongly specifiedcatalyst noted in `#tor-dev` that the decryption algorithm in 5.5:
* `won't work unless decryption is commutative (which it is for plain AES-CTR, but not for the proposal)`
* `doesn't look like it'll work to handle a relay cell arriving ...catalyst noted in `#tor-dev` that the decryption algorithm in 5.5:
* `won't work unless decryption is commutative (which it is for plain AES-CTR, but not for the proposal)`
* `doesn't look like it'll work to handle a relay cell arriving at the OP that originates from an intermediate node`
I had also noted the latter, so I postulated `is the order of that line backwards? that is... should it be "For I=1...N?"` and asked `what does little-t tor do?`
And asn noted:
> in relay_decrypt_cell() it does:
> {{{
> if (CIRCUIT_IS_ORIGIN(circ)) { /* We're at the beginning of the circuit.
> * We'll want to do layered decrypts. */
> crypt_path_t *thishop, *cpath = TO_ORIGIN_CIRCUIT(circ)->cpath;
> thishop = cpath;
> }}}
> i think this is I=1...N like you say
> (for 1 being the hop closest to the OP)
So it looks like this is an error in the spec, rather than a problem with `tor`.Tor: 0.3.5.x-finaldmrdmr