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

Adds text about server padding handshake to PMTU #1013

Closed
wants to merge 1 commit into from

Conversation

janaiyengar
Copy link
Contributor

Closes #1010.

@janaiyengar
Copy link
Contributor Author

I understand that there's a PMTU requirement of > 1280 in the draft at the moment, but I have #1014 to track that. I'm happy to rewrite this PR depending on how that issue is resolved.


Servers MUST ignore an initial plaintext packet from a client if its total size
is less than 1200 octets.

Similarly, servers MUST ensure that the first handshake packet they send to
Copy link
Member

Choose a reason for hiding this comment

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

Use capital H here and maybe be more explicit. A Version Negotiation or Retry packet shouldn't be larger, and in fact should be kept small. (We can't pad Version Negotiation anyway, except with greasing values, which isn't much use.)

Similarly, servers MUST ensure that the first handshake packet they send to
clients, and any retransmissions of those octets, has a QUIC packet size that is
the same as the received initial client packet, unless the server knows the PMTU
to the client to be smaller. Sending a packet of this size ensures that the
Copy link
Member

Choose a reason for hiding this comment

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

This "smaller" clause here means that the server-to-client path could end up with a PMTU less than 1200. I think that you need to set a minimum of 1200 here explicitly.

Copy link
Contributor

Choose a reason for hiding this comment

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

The client side "at least 1200 byte" is justified by a security concern, but that concern does not apply to server responses. I think we need some better text, and that the justification for the padding should come after the requirement of minimum 1280 MTU. Setting a minimum size for the first handshake packet ensures that the handshake will fail if the network does not support the recommended MTU, and is only justified if we want enforce the "minimum 1280" requirement.

But if we do that, maybe we should develop the idea in a full paragraph. For example, is it OK to meet the requirement in IPv4 by setting the DF bit to "fragmentation authorized"?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am a bit skeptical about the requirement for "any retransmissions of those octets". It will be yet another special case in the handling of re-transmission. The net effect will be that servers pad all handshake message re-transmissions to 1200 bytes.

Copy link
Member

Choose a reason for hiding this comment

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

I agree that this would make it much more difficult to implement. You need special handling for this case and I can see how you would end up with a fixed size for Handshake packets. That would be unfortunate for several reasons.

Copy link
Contributor

@huitema huitema left a comment

Choose a reason for hiding this comment

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

The current text on the "minimum enforced MTU" is contorted and spread over several paragraphs. I think the paragraph explaining that "QUIC depends on the network path supporting a MTU of at least 1280 octets" should be moved up, just after the introduction of path MTU, and before making statements such as "In the absence of these mechanisms, QUIC endpoints SHOULD NOT send IP packets larger than 1280 octets."

Once this is said, we can have the additional text on server handshake messages. But we need to work some more on that text.

Similarly, servers MUST ensure that the first handshake packet they send to
clients, and any retransmissions of those octets, has a QUIC packet size that is
the same as the received initial client packet, unless the server knows the PMTU
to the client to be smaller. Sending a packet of this size ensures that the
Copy link
Contributor

Choose a reason for hiding this comment

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

The client side "at least 1200 byte" is justified by a security concern, but that concern does not apply to server responses. I think we need some better text, and that the justification for the padding should come after the requirement of minimum 1280 MTU. Setting a minimum size for the first handshake packet ensures that the handshake will fail if the network does not support the recommended MTU, and is only justified if we want enforce the "minimum 1280" requirement.

But if we do that, maybe we should develop the idea in a full paragraph. For example, is it OK to meet the requirement in IPv4 by setting the DF bit to "fragmentation authorized"?

Similarly, servers MUST ensure that the first handshake packet they send to
clients, and any retransmissions of those octets, has a QUIC packet size that is
the same as the received initial client packet, unless the server knows the PMTU
to the client to be smaller. Sending a packet of this size ensures that the
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a bit skeptical about the requirement for "any retransmissions of those octets". It will be yet another special case in the handling of re-transmission. The net effect will be that servers pad all handshake message re-transmissions to 1200 bytes.

@janaiyengar janaiyengar self-assigned this Mar 14, 2018
@martinthomson
Copy link
Member

martinthomson commented Sep 24, 2018

Next steps: suggestion was to suggest that a server might choose to pad its datagrams to the same size as the incoming datagrams prior to getting path validation. A suggestion only, because the server might know something else about the MTU that suggests otherwise, and this only really provides minimum MTU validation, it's not really PMTUD. We like the fate-sharing thing that the client Initial has, and replicating that for the server is nice, but it's not obligatory.

We will wait for the resolution to the deadlocking issue we discussed in NYC before moving on this one.

@janaiyengar
Copy link
Contributor Author

Death by aging.

@MikeBishop MikeBishop deleted the server-pmtu branch February 6, 2019 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants