search

martinthomson / train-protocol

Choo Choo!
Other
0 stars 0 forks source link

shotgun thumpers #6

Closed huitema closed 3 months ago

huitema commented 4 months ago

I am a bit concerned that the simplistic way for intermediaries to implement the "tumping" is to write something like:

    if(*(uint32_t*)&packet[1] == TRAIN_VERSION) {
        *(uint32_t*)&packet[1] = PACING_RATE_VERSION;
    }

This will work well 99.9999% of the time, but if will also catch the 1RTT packets in which the CID bytes happen to match the chosen version, or one of the reserved versions that match rates. There are two dozen such versions, so the probability is about 1/2^28. Not large, but it makes for a silly heisenbug. Pick one of the wrong CIDs and your packets get mangled in traffic.

Of course, this does not happen if the test is written as:

    if((packet[0]&0xc0) == 0xc0 && *(uint32_t*)&packet[1] == TRAIN_VERSION) {
        *(uint32_t*)&packet[1] = PACING_RATE_VERSION;
    }

... but that means we have to fight laziness, because the wrong code is shorter and also "almost right".

So maybe we should be very explicit about that in the text.

huitema commented 3 months ago

I agree that the new design will fix the issue. Messing with the top byte of a V1 packet will cause the checksum to fail.