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

PMTUD (ICMP variant) #105

Closed
wants to merge 1 commit into from
Closed

PMTUD (ICMP variant) #105

wants to merge 1 commit into from

Conversation

martinduke
Copy link
Contributor

Here is an alternate solution to he PMTUD section as discussed in Issue #64
#64

I think it is much cleaner and clearer about what to do. It assumes that ICMP black holes are not a huge problem in the internet, and that no one is using a UDP socket API that prevents them from stopping various noncompliant behaviors. I much prefer this version, although some may find it too restrictive.

Here is an alternate solution to he PMTUD section as discussed in Issue #64
#64

I think it is much cleaner and clearer about what to do. It assumes that ICMP black holes are not a huge problem in the internet, and that no one is using a UDP socket API that prevents them from stopping various noncompliant behaviors. I much prefer this version, although some may find it too restrictive.
@martinduke martinduke mentioned this pull request Dec 28, 2016
determinations.

(TODO: Should there be a high minimum MTU for QUIC to avoid ICMP attacks? If so,
does the endpoint fail over to TCP or simply allow fragmentation?)
Copy link
Member

Choose a reason for hiding this comment

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

Rather than log this here, open a separate issue.

combination of local and remote IP addresses (as each pairing may have a
different minimum MTU in the path).

## Packet MTU Determination
Copy link
Member

Choose a reason for hiding this comment

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

Path MTU Discovery?

QUIC endpoints MUST maintain a separate PMTU estimate for each IP address the
peer is using in the connection. Endpoints SHOULD maintain an estimate for each
combination of local and remote IP addresses (as each pairing may have a
different minimum MTU in the path).
Copy link
Member

Choose a reason for hiding this comment

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

I think that we're going to need a definition for "path", which leads to this being simply: Endpoints MUST maintain a separate PMTU estimate for each path.

The implication of your construction is that different remote IP addresses get a different PMTU estimate, but different local ones might not. That's probably fine if the endpoint knows that outgoing packets from different local addresses go to the same remote address over the same path, but I'd rather assume nothing in any mandate we make.

That said, if an endpoint has really good information that paths are the "same", then estimates can be easily copied from one path to another.


### DF Marking {#dfmarking}

QUIC endpoints set the Don't Fragment (DF) bit in the IP header of selected
Copy link
Member

Choose a reason for hiding this comment

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

You need to point out that this only applies to IPv4.

QUIC datagrams. These packets MUST use PADDING frames, as necessary, to raise the
overall packet size to the expected maximum.

The first packet from the client MUST be maximum-size and set the DF bit.
Copy link
Member

Choose a reason for hiding this comment

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

Why? What is the maximum size? Can a client reduce the size if it discovers that the packet doesn't fit? How far can it reduce the size?

Is that an RFC6919 "MUST (but we know that you won't)"? I say that because clients will have to balance the probability of packets arriving with the desire to avoid amplification attack. I appreciate that fragmentation here is hazardous for numerous reasons, so I agree with the DF marking, but the actual value we choose here is going to be better if it is fixed somehow so that clients can be compliant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that an RFC6919 "MUST (but we know that you won't)"?

It may very well be. I thought there was consensus that QUIC endpoints MUST pad handshake packets, but if not we can fight it out in #106.


The first packet from the client MUST be maximum-size and set the DF bit.

The last server-generated packet in the Transport Handshake MUST be padded to
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what message you are referring to here. The transport handshake doesn't really have concrete messages.

If you are referring to the TLS handshake, then that only has one message from the server in many cases. If you pad that message, that could be an issue. We're already pushing the limits of what is reasonable given that it usually contains a server certificate.

What problem are you attempting to solve by requiring DF on the server messages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Padding the server message verifies that the MTU is the same on the reverse path.

recorded PMTU to probe for a change in the PMTU. In a change from {{!RFC1191}}
and {{!RFC1981}}, these packets MUST NOT be sent on a path more often than
every 20 RTTs (implying that the chance of a blind attack succeeding is less than
5% without a storm of ICMP packets, given the mechanisms below).
Copy link
Member

Choose a reason for hiding this comment

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

Is the assumption that an attacker won't be able to a) observe the initiation of the connection, and b) guess the RTT? Because both of those are not true in the general case. Randomizing the interval and recommending a minimum idle period might work, but only if you also levy some sort of penalty on an attacker. For instance, spurious ICMP/PTB messages can cause future messages to be ignored.

Given that you have the rationale for this in the next section, I think that you should move this requirement down there.

context except the IP/port 4-tuple. The following requirements (in addition to
limits on DF marking above) mitigate these problems:

The endpoint MUST maintain a list of all unacknowledged packet numbers with 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 list needs bullets.

DF flag set.

The endpoint MUST remove packet numbers from the list when the loss detection
algorithm declares the packet lost, or the packet is acknowledged.
Copy link
Member

Choose a reason for hiding this comment

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

Would it be reasonable to trim this list more aggressively than otherwise? Several reasons:

  1. It only results in lost PMTUD feedback
  2. It reduces exposure to those off-path attacks
  3. Path elements are able to respond in less than 1RTT


Incoming Packet Too Big messages SHOULD be applied to all QUIC connections that
share the same local and remote IP addresses, assuming they are valid for one
connection.
Copy link
Member

Choose a reason for hiding this comment

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

We should refer to a path here. How we identify a path locally is probably something we need to include in the definition.


Valid Packet Too Big messages MUST trigger immediate retransmission of
retransmittable data from packet numbers in the list, with no adjustment in
congestion control parameters consistent with a congestion-induced loss.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not an expert but this seems wrong: if the signal originates from the remote side of a congested link, isn't this a great way to add to that congestion?

@martinthomson martinthomson changed the title Update draft-ietf-quic-transport.md PMTUD (proper variant) Jan 3, 2017
@martinthomson martinthomson changed the title PMTUD (proper variant) PMTUD (ICMP variant) Jan 4, 2017
@martinduke martinduke mentioned this pull request Jan 10, 2017
@martinduke
Copy link
Contributor Author

Thanks for the comments. I'm closing this PR in favor of a significantly modified #106 that incorporates the comments that are still relevant.

@martinduke martinduke closed this Jan 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants