Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

One packet type in invariants #963

Closed
MikeBishop opened this issue Nov 28, 2017 · 13 comments
Closed

One packet type in invariants #963

MikeBishop opened this issue Nov 28, 2017 · 13 comments

Comments

@MikeBishop
Copy link
Contributor

The set of packet types is version-dependent, except that Version Negotiation has a fixed packet type across all versions. We could make packet types entirely version-dependent by a simple change: Version Negotiation packets always indicate a version of '0', and the type field is randomized on send / ignored on receipt.

@MikeBishop
Copy link
Contributor Author

If we land #939 first, we have the option of saying that version 0 packets don't have a packet number and saving a few bytes, but the question is open whether we want to have what becomes a third packet header.

@martinthomson
Copy link
Member

I think that it makes sense to continue to mark Version Negotiation as long. Otherwise there is some amount of guessing required to find the Version field. That suggests that it continues to use that bit, but that doesn't necessarily constrain us to the use of a particular type beyond that.

The expected order of operations for receipt of a packet is to check the long header bit, then to check the version. Once that is done, we could say that the type is ignored. That means we could make it entirely randomized (which effectively destroys our future ability to use those bits), a specific value (which would allow us to generate other signals for clients that support future versions if we specified that packets are ignore if other values are found there), or define a more specific scheme.

I don't think that there is any value in versioning version negotiation. That way leads to madness. I'm ambivalent about random vs. specific value.

@marten-seemann
Copy link
Contributor

Defining a fixed value for the type byte of the version negotiation packet has the nice property that for the client, the only thing you need to do when parsing a packet header is look at the type byte (it's the same principle I'm arguing for in #426).

@MikeBishop
Copy link
Contributor Author

MikeBishop commented Nov 30, 2017

That's a desire I'm sympathetic to, but a packet inherently cannot be self-describing from the first byte unless you already know what version you're interpreting the first byte against. I'm willing to take a slightly slower path in the cases where that's not true.

On a client, that's less of a problem (I think), because at any given time, there are at most two versions you could get -- the one you sent, or version negotiation. That means that the type value could be quasi-invariant -- it's version-dependent, except that some value has to be reserved for version negotiation, but since the server obviously didn't speak your version that value has to be fixed across versions. I don't like something being kinda-sorta-invariant, but it would work.

On a server, where you come into an initial packet with no advance knowledge of the connection's version, the space is huge, so the process must be to find the long header bit, find the version field, and then either start version negotiation (if you don't recognize it) or begin version-specific parsing (if you do). Servers will have to look at more than the first byte to establish the expected packet layout.

And if we're willing to say that you have to look at the version field to figure out the packet layout, then I think the requirement becomes:

  • From the first byte, you can identify the location of the Version field
  • If you already know the version, you can identify the packet layout from the first byte (but please verify the version)

@mikkelfj
Copy link
Contributor

I'd argue for testing long header bit, then check for version 0 and for other known versions.

I'd argue against randommizing the type byte for version 0. The reason being that it makes it difficult to multiplex other protocols over the same connection. QUIC version numbers may also be chosen very carefully to avoid conflicts with other protocols where there might be conflicts beyond the first byte when the high bit is set.

@janaiyengar
Copy link
Contributor

I agree with @MikeBishop. I don't think there's a need to specify the order of processing things, but thank you for laying it out clearly. That said, randomizing it right now will conflict with the same types that #956 seeks to avoid. Greasing the packet type will run into that, so we'll need to specify carefully how to grease this byte.

@huitema
Copy link
Contributor

huitema commented Dec 1, 2017

I would rather use a constant value, and actually keep the version negotiation type that we have already defined. Yes, this will constrain further versions to not redefine that type. But then, so what?

I am also very lukewarm about the idea of making packet types version dependent, e.g. having sometimes the client initial be 17 and some times 29. That will require implementations to insert some level of indirection between the abstract types used internally and the type in the wire format. This looks like gratuitous complexity.

I like the idea of removing the dependency on the sequence number. That's a clear benefit, since implementations cannot guarantee that they can parse sequence numbers from unsupported versions.

@janaiyengar
Copy link
Contributor

@huitema: are you suggesting that we make packet types version-independent? That seems unnecessary rigidity, since a later version of QUIC shouldn't have to deal with legacy types it's not using. For instance, we currently use different short types for different packet number sizes; it's seems quite likely that we'll change that in future versions.

I think it's a good idea to be clear about what won't change and what will across versions. Our general premise has been so far to assume packet type will change across versions, which is why VN packets stick out, and using the version field instead of type for them makes all types version independent. There's value in this that we can unequivocally that packet type must not be interpreted by stateless middleboxes, otherwise we have to equivocate.

Agreed on removing packet number from VN packets.

@janaiyengar
Copy link
Contributor

@marten-seemann : I agree that it's simpler with simply a type byte, but honestly I don't think it's much more complex with a version field in the picture as well. It's a bit of getting used to, but the value is in the clarity about the type field being entirely version-dependent, for both long and short header packets (which it almost was, but this makes it entirely so.)

Importantly, making the type field version dependent and stating it as such hopefully forces middleboxes to actually pay attention to the version field, and middlebox implementers to read the spec. This is a big part of greasing -- to ensure that implementers have to go read the spec to figure out how to parse packets instead of looking for patterns on the wire -- and having types be different across versions helps, IMO.

@ianswett
Copy link
Contributor

ianswett commented Dec 1, 2017

I'm a pretty big fan of this change on a high level. In particular, it forces implementations to actually implement version specific parsing, and the removal of the packet number reinforces the idea that everything after the version is not an invariant.

I'll note: If we have a version, and then there's only one defined type byte value, but it's valid to grease the type byte for that version, we end up with the type byte being random :) So I favor randomizing the type byte, but could live without it.

@huitema
Copy link
Contributor

huitema commented Dec 2, 2017

OK, so I actually did an implementation of the version dependent header parsing. I agree that once this version dependent parsing is done, using version=0 to define renegotiation packets makes sense.

@huitema
Copy link
Contributor

huitema commented Dec 3, 2017

Some implementation experience. In the current state, greasing the version ID is a bit hard. The server will correctly handle a version ID that it does not understand, and will copy the incoming version ID into the version field of the negotiation packet's header. But the client that does the greasing has to make a wild guess -- it cannot really parse the type field, because this is version dependent, and it cannot really predict what kind of encoding the server used. Testing V==0 would be cleaner.

@janaiyengar
Copy link
Contributor

@huitema: Thank you for trying out an implementation and for sharing what you learned, that is very helpful!

Based on the discussion here, and in the interest of moving forward, I will merge #968 tomorrow, which will close this issue. (As always, please re-open this issue if you think further discussion is needed.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants