On Thu, May 14, 2020 at 3:15 PM David Goulet dgoulet@torproject.org wrote:
On 11 May (16:47:24), Nick Mathewson wrote:
[...]
# Onion service concerns.
We allocate a new extension for use in the ESTABLISH_INTRO by onion services, to indicate that they can receive a wide INTRODUCE2 cell. This extension contains:
struct wide_intro2_ok { u16 max_len; }We allocate a new extension for use in the `ESTABLISH_RENDEZVOUS` cell, to indicate acceptance of wide `RENDEZVOUS2` cells. This extension contains:
struct wide_rend2_ok { u16 max_len; }(Note that `ESTABLISH_RENDEZVOUS` cells do not currently have a an extension mechanism. They should be extended to use the same extension format as `ESTABLISH_INTRO` cells, with extensions placed after the rendezvous cookie.)
Why would a client need to announce wide cells in the ESTABLISH phase as opposed to using protover "Relay=N" ?
This is not for announcing support of wide cells -- this is for reporting a setting for how wide fragmented cells should be.
The maximum length of a fragmented cell is capped to 2^16 (u16) so we don't really need the establish process to inform us of the maximum expected length but rather use the max_len in the first fragment?
This all comes back to an earlier part of the proposal:
Not all lengths up to 65535 are valid lengths for a fragmented cell. Any length under 499 bytes SHOULD cause the circuit to close, since that could fit into a non-fragmented RELAY cell. Parties SHOULD enforce maximum lengths for cell types that they understand.
In other words, I'm imagining that there is a maximum length for each cell type that is much shorter than 65535, even though we're using two bytes for the length field.
The extension in the establish_intro cell is to tell the intro point the longest introduce1 cell that it should accept; this extension in the establish_rend cell is to tell the rendezvous point the longest rendezvous1 cell that it should accept.
Another way we could do this would be with a set of network parameters to describe the maximum length of each fragmented cell. Do you think that would be simpler?
(I can't quite remember why I specified it this way in the first place.)
Furthermore, ESTABLISH_INTRO has extensions (only 1 as of today) so they could also be fragments themselves and thus I'm not sure I see the point of having two different ways of "expecting" fragments for the ESTABLISH_* cells and the INTRO/RENDEZVOUS cells?
The difference thing here is that everybody can tell which protocols that a relay supports, but there is no automatic way to tell which protocols an onion service or client supports. Since INTRODUCE2/RENDEZVOUS2 cells are handled by these clients, they need to get opted into by the relays.
(I'm not sure I understood the question completely.)
# Compatibility
This proposal will require the allocation of a new 'Relay' protocol version, to indicate understanding of the RELAY_FRAGMENTED command.
Here is a thought about a DoS vector. Here goes:
As an upper limit of 65KB total fragment size, it represents ~126 cells in total so I could basically send *125* cells and then stop which will put in memory a bit more than 64KB and it will stay there until the last fragment is received.
And then I do that on 1000 different circuits bringing the total count in memory to 64GB. All stuck there, all "waiting" for the last fragment.
Our OOM would kick in killing circuits but it just seems to me a very easy way to continously kick the OOM of a _service_ which is pretty bad side channel.
A few responses here:
First, we shouldn't allow 65535-byte fragmented cells. The actual maximum length should be something more like 1024 or 4096 bytes.
Second, we should make sure that when we are reassembling cells, we use the same buf_t buffers that we use for other stuff. Our buffers are timestamped, so we can tell which buffer has had data stalling for the longest, and we should use that to make sure we're killing off the right circuits preferentially.
Third, fragments should only be allowed at an onion service for INTRODUCE2, and those should only come one at a time from each introduction point, so the number that it's reassembling at the time will be limited by the number of intro circuits it has open. It'll be the the intro points that have to be keeping a bunch of cells in assembly at once, and be ready to kill off circuits that dawdle too long.
Does this make more sense? If so I'll try to clarify it in the proposal.