Clarify that trailing data in extensions is forbidden.

[davidben]

This was already a compliance requirement, but spell it out more explicitly.

Closes #1219.

[martinduke]

LGTM

[kaduk]

Why is this specific to extension_data? Why not require this for every length-prefixed construct that contains fields that might not extend to the full length?

AFAICT it isn't.

https://tools.ietf.org/html/draft-ietf-quic-http-34#section-10.8 also comes to mind

[martinduke]

@martinthomson Good catch.

[davidben]

Why is this specific to extension_data? Why not require this for every length-prefixed construct that contains fields that might not extend to the full length?

It's already required. Length prefixes typically come from the T field<floor..ceiling> syntax in the presentation language. Not only would a parser that allows trailing data be non-compliant, but it'd be totally incoherent. Vectors don't tell you the number of elements, so the only way to parse it is to loop until empty, peeling off Ts. That means, if there is any trailing data, it'll be immediately interpreted as another instance of T, not trailing data.

Whereas extension_data is a little different because it's an opaque extension_data<0..2^16-1> byte string whose contents happen to be another structure. TBH, I don't think this case needed a clarification either (it's pretty obvious, IMO), but a clarification doesn't hurt so whatever.

[martinthomson]

@davidben, I think that you are right when it comes to lists of things. I was thinking about structs in general. However, in QUIC, we have transport parameters, which would share the need for this exact treatment; that we decided to manage that within QUIC rather than use the TLS grammar means that we don't need to worry about it, but it suggests that nested extension points will hit this problem too.

I was trying to find other such instances, but I can't find any offhand. I was looking at server_name, and that uses a switch statement, which suggests that it is simply impossible to include a non-zero type value (another reason that server name extension is irredeemably busted; and of course, that's not how NSS implements it; from memory, we assume every type has a 1 byte length prefix...).

I thought that handshake messages might need this, but the grammar seems to prevent that in the same way as server_name. An unknown handshake message hits a switch statement and falls out, presumably into an error condition. So even though the format might allow an unknown handshake message to be ignored the grammar suggests that it is always an error (which, to be clear, is a good thing from a protocol perspective).

The encapsulation of handshake messages in records is another place that is interesting. In TLS, that doesn't matter much because the extra bytes roll over and we are back in your "next instance of T" case, but it is a problem in DTLS where each record needs to contain discrete fragments.

So I do think that it would be better to have this be a general statement, even if it there is precisely one instance where we know it definitely applies.

[tomato42]

the only place where trailing data is allowed is after ClientHello for implementations that don't support extensions

[briansmith]

It's already required. Length prefixes typically come from the T field<floor..ceiling> syntax in the presentation language. Not only would a parser that allows trailing data be non-compliant, but it'd be totally incoherent. Vectors don't tell you the number of elements, so the only way to parse it is to loop until empty, peeling off Ts. That means, if there is any trailing data, it'll be immediately interpreted as another instance of T, not trailing data.

A receiver will not necessarily parse every entry in the vector, if it finds what it's looking for before it gets through the whole vector.

[davidben]

So I do think that it would be better to have this be a general statement, even if it there is precisely one instance where we know it definitely applies.

Hmm. Where were you envisioning putting this generally? The definition of various length prefixes are kind of scattered all over the place.

[martinthomson]

Section 3.9?