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

ClientHello needs to fit in a single packet #338

Merged
merged 4 commits into from Mar 8, 2017
Merged

Conversation

martinthomson
Copy link
Member

I'm marking this as editorial. It touches both TLS and transport. Anyone who thinks that this is OK can merge it.

@martinthomson martinthomson added editorial An issue that does not affect the design of the protocol; does not require consensus. -tls -transport labels Feb 23, 2017
Implementations are reminded that large session tickets or HelloRetryRequest
cookies, multiple or large key shares, and long lists of supported ciphers,
signature algorithms, versions, and other negotiable parameters and extensions
could cause this message to grow.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also needs to inclde the transport params, right?

@@ -1223,11 +1246,11 @@ by an attacker.
Certificate caching {{?RFC7924}} can reduce the size of the server's handshake
messages significantly.

A client SHOULD also pad {{!RFC7685}} its ClientHello to at least 1024 octets.
QUIC requires that the packet containing a ClientHello be padded to 1280 octets.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"at least 1280 octets"

The initial cryptographic handshake message needs to be sent in a single packet.
This avoids having to reassemble a message from multiple packets. This would
require that servers maintain state prior to establishing a connection, exposing
servers to a denial of service risk.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phrasing unclear. How about "This avoids having to reassemble a message from multiple packets since doing so would require that a server maintain state prior to establishing a connection, exposing the server to a denial of service risk."

servers to a denial of service risk.

The first client packet of the cryptographic handshake protocol MUST fit within
a 1280 octet QUIC packet. this includes overheads that reduce the space
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we're requiring 1280 bytes, just that it fit within a packet, right? We talk about MTU elsewhere, how about just saying that this packet MUST fit within a single QUIC packet, assuming that the packet is MTU-sized or smaller?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, we can't exclude implementations that choose to only send 1280 octet packets. That would be unwise, I think. This is MORE strict, I agree, but I think that we want to specifically exclude handshake packets that exceed this or we will find that they aren't properly applicable across multiple networks and use cases.

If a handshake packet grows too much, then we limit this to networks that can handle a larger MTU. Given that we don't need the space right now, this seems like a constraint that won't be hard to meet.

## ClientHello Size

QUIC requires that the initial handshake packet from a client fit within a
single packet of 1280 octets. With framing and packet overheads this value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"at least 1280 octets."

risk.

The first client packet of the cryptographic handshake protocol MUST fit within
a 1280 octet QUIC packet. This includes overheads that reduce the space
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change "1280 octet" to "MTU-sized"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-tls -transport editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants