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
Clarify that trailing data in extensions is forbidden. #1220
Conversation
This was already a compliance requirement, but spell it out more explicitly. Closes tlswg#1219.
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
@martinthomson Good catch. |
It's already required. Length prefixes typically come from the Whereas |
@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 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 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. |
draft-ietf-tls-rfc8446bis.md
Outdated
otherwise specified, trailing data is forbidden. That is, senders MUST NOT | ||
include data after the structure in the "extension_data" field, and receivers | ||
MUST abort the handshake with a "decode_error" alert if there is data left | ||
over after parsing the structure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please clarify this so that it is clear that if the receiver doesn't parse the extension data, then it doesn't have to check it? The way it is worded now seems to imply that receivers must parse every extension every time, just to comply with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. Does the new text work?
the only place where trailing data is allowed is after ClientHello for implementations that don't support extensions |
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. |
Hmm. Where were you envisioning putting this generally? The definition of various length prefixes are kind of scattered all over the place. |
Section 3.9? |
This was already a compliance requirement, but spell it out more explicitly.
Closes #1219.