Proposal 340 clarifications
Notably:
- end-of-message marker is omitted when message data ends at exactly a relay cell boundary.
- packing is more for hiding control messages from network observers than for network efficiency.
- Specify the new subprotocol RelayCell.
- Explicitly state that cells with unrecognized relay commands should result in immediate circuit closure, even if the rest of the message hasn't arrived yet due to fragmentation.
- Add note that initial packing algorithm needs to be updated.
Merge request reports
Activity
assigned to @dgoulet
requested review from @dgoulet
118 123 119 An unrecognized `command` is considered invalid and thus MUST result in the 120 circuit being immediately destroyed. 124 A message header with an unrecognized `command` is considered invalid and thus 125 MUST result in the circuit being immediately destroyed (without waiting for the 126 rest of the relay message to arrive, in the case of a fragmented message). 121 127 122 ## Negotiation (OBSOLETE) 128 ## New subprotocol `RelayCell` 123 129 124 > We do not want to do it this way. Instead, we want to use the system 125 > described in the forthcoming proposal, "xxx-protovers-again.md". 130 We introduce a new subprotocol `RelayCell` to specify the relay cell ABI. The 131 new format specified in this proposal, supporting packing and fragmentation, 132 corresponds to `RelayCell` version 1. The ABI prior to this proposal is 133 `RelayCell` version 0. - Comment on lines +130 to +133
We don't actually need to define a new subprotocol for this if we don't want to, now that we accepted proposal 346, which makes subprotocol versions count as flags rather than as strictly increasing version numbers.
IOW, we could use
Relayfor this if we want. That is, we could say "Relay=6 (or whatever) declares support for this message format." 
We did though agreed some months ago to try to expand our protover to be more specific. Which would also avoid having 1 protover handle all protocol change for instance.
So
RelayCellfor me is still logical in that sense and allows us to negotiate the relay cell ABI on the wire with the end point.
125 > described in the forthcoming proposal, "xxx-protovers-again.md". 130 We introduce a new subprotocol `RelayCell` to specify the relay cell ABI. The 131 new format specified in this proposal, supporting packing and fragmentation, 132 corresponds to `RelayCell` version 1. The ABI prior to this proposal is 133 `RelayCell` version 0. 126 134 127 This message format requires a new `Relay` subprotocol version to indicate 128 support. If a client wants to indicate support for this format, it sends the 129 following extension as part of its `ntor3` handshake: 135 All clients and relays implicitly support `RelayCell` version 0. 130 136 131 EXT_FIELD_TYPE: 137 > XXX: Do we want to consider some migration path for eventually removing 138 > support for `RelayCell` version 0? e.g. maybe this should be something like 139 > "Support for any of `Relay` versions 1-5 imply support for `RelayCell` 140 > version 0"? - Comment on lines +138 to +140
What exactly does this look like? I suppose something like:
- Once the protover (e.g. RelayCell=1 assuming we go with that) is in
required-relay-protocols, relays no longer need to explicitly advertise it. - Once the protover is in
required-client-protocols, relays could start rejecting clients that don't negotiate it. (maybe a separate consensus variable would control whether they actually do?) - Do we ever stop requiring clients to explicitly negotiate it?
- Once the protover (e.g. RelayCell=1 assuming we go with that) is in
-
Almost! I was thinking:
- At some point well after the protover is in
required-relay-protocols, we can make clients assume that all relays have it. - At some point well after the protover is in
required-client-protocols, we can have relays rejecting clients that don't negotiate it. - We never drop it from the negotiation...
- ... though someday we might add an omnibus "assume all options that were required as of 202x" flag.
- At some point well after the protover is in
126 rest of the relay message to arrive, in the case of a fragmented message). 121 127 122 ## Negotiation (OBSOLETE) 128 ## New subprotocol `RelayCell` 123 129 124 > We do not want to do it this way. Instead, we want to use the system 125 > described in the forthcoming proposal, "xxx-protovers-again.md". 130 We introduce a new subprotocol `RelayCell` to specify the relay cell ABI. The 131 new format specified in this proposal, supporting packing and fragmentation, 132 corresponds to `RelayCell` version 1. The ABI prior to this proposal is 133 `RelayCell` version 0. 126 134 127 This message format requires a new `Relay` subprotocol version to indicate 128 support. If a client wants to indicate support for this format, it sends the 129 following extension as part of its `ntor3` handshake: 135 All clients and relays implicitly support `RelayCell` version 0. 139 140 EXT_FIELD_TYPE: 141 142 [04] -- Packed and Fragmented Cell Response 143 144 Again, the extension presence indicates to the client that the Exit has 145 acknowledged the feature and is ready to use it. If the extension is not 146 present, the client MUST not use the packed and fragmented feature even though 147 the Exit has advertised the correct protover. 148 149 The client MUST reject the handshake and thus close the circuit if: 150 151 - The response extension is seen for a non-ntorv3 handshake. 152 - The response extension is seen but no request was made initially. 145 To use a `RelayCell` version other than 0, that version must be negotiated with 146 every relay on the circuit using an `ntor_v3` extension, as per [proposal -
Yeah, I was thinking it would be a problem if intermediate relays were looking in the wrong place for the recognized and digest fields. I suppose that's not a problem though since looking in the wrong place should result in the relay correctly deciding that it's not the final destination of that cell.
changed this line in version 2 of the diff
181 179 with as much data as possible. Otherwise, the client sends the cell 182 180 with no packed data. 183 181 182 > XXX: This isn't quite right. What was actually implemented in tor, and 183 > what we want in arti, is to defer sending some "control" messages like 184 > xon, xoff, and confluence switch, until they can be invisibly packed 185 > into a cell for a DATA message. 186 > dgoulet: Could you update this section? -
For this MR I mainly just wanted to get in a note in that the first draft algorithm above isn't what is actually getting implemented, along with a TODO to flesh out the details of the actual algorithm.
But yeah I think there are a lot of details that would be helpful to specify. e.g.
- If we have data to send, but the corresponding DATA messages don't leave enough room to pack in the deferred control message(s), what do we do? If we continue deferring could we end up deferring forever if the application always writes in chunks that happen to align this way?
- Since cells containing any part of a DATA message is subject to congestion windows, does that mean if our congestion window is empty we can't send these control messages either (until the window becomes non-empty)?
changed this line in version 2 of the diff
93 93 | Padding | 10 | 94 94 | FlowCtrl | 11 | 95 95 | Conflux | 12 | 96 | Datagram | 13 | 96 | RelayCell| 13 | marked this merge request as draft from jnewsome/torspec@b07437e5
mentioned in commit b749595a