Subject: [quicwg/base-drafts] Defense against version number corruption (#3532)

From: Christian Huitema <[email-masked]>
Date: Wed, 18 Mar 2020 11:23:19 -0700

The "packet corruption" test that @marten-seemann added to the interop-runner has surfaced an issue about version number corruption. Suppose that a long header packet is corrupted in transit, and that the corruption changes the value of the version number. Per spec, the server will respond with a version number negotiation packet. In the test, we observe that when clients receive such packets, they abandon the connection attempt.

Obviously, this is a rare event. We don't know how frequent packet corruption is today. The [1997 paper by Vern Paxson ](https://cseweb.ucsd.edu/classes/wi01/cse222/papers/paxson-e2e-packets-sigcomm97.pdf) measured "1 packet in 5000". In theory, only one in 2^16 of these corruptions would not be detected by the UDP checksum, leading to undetected corruption in 1 in 300 million packets.

That's a very small number, but I don't know whether the 1997 measurements are still valid. They could be higher. I also don't know whether the loss patterns would be detected by the UDP checksum, since some middleboxes appear to rewrite these checksums. Faced with exactly the same issue, we decided recently to add a checksum to the Retry packets.

Clients can protect against such corruptions by checking the incoming VN packets for sanity. If the server issues a VN in response to a corrupted packet, the list of supported versions will include the version proposed by the client. That should normally never happen. Ignoring such bogus VN does protect against most version number correction issues. Should we point that out in the spec?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3532

---

From: Martin Thomson <[email-masked]>
Date: Wed, 18 Mar 2020 21:16:51 +0000 (UTC)

We used to require that a client ignore Version Negotiation packets that include the version the client attempted.

@DavidSchinazi edited that out.  Maybe we should restore it.  That pushes the odds of failure to 1 in 90 quadrillion.  Would that help?

I want to say that this is a design issue.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3532#issuecomment-600862789

---

From: Christian Huitema <[email-masked]>
Date: Wed, 18 Mar 2020 14:34:51 -0700

The problem with the "one in 300 million" figure is that it relies on the UDP checksum. If you cannot trust the UDP checksum, the figure is "1 in 5000", which is more concerning. And there are scenarios in which uncontrolled middle-boxes do rewrite that checksum.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3532#issuecomment-600870295

---

From: ianswett <[email-masked]>
Date: Wed, 18 Mar 2020 14:37:46 -0700

I'd support restoring that check that @DavidSchinazi edited out.

Anecdotally, we've found packet corruption to be very rare and basically a non-issue.  Or maybe something in the network or the host is just dropping them, but we don't receive them in userspace.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3532#issuecomment-600871402

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 18 Mar 2020 14:50:04 -0700

I'm fine with adding this check back in. It's not onerous to implementers. This is a design issue.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3532#issuecomment-600876289

---

From: Christian Huitema <[email-masked]>
Date: Wed, 18 Mar 2020 14:58:14 -0700

Now if we were really paranoid, we would consider the possibility that the VN packet itself is corrupted. 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3532#issuecomment-600879441

---

From: Martin Thomson <[email-masked]>
Date: Wed, 18 Mar 2020 15:04:25 -0700

> [..] we would consider the possibility that the VN packet itself is corrupted.

I did.  If we don't trust the UDP checksum on either, the error rates are still pretty high, but either checksum bumps it up to 1.5 trillion and both is 90 quadrillion.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3532#issuecomment-600881690

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 18 Mar 2020 15:05:00 -0700

I was thinking about that -- it doesn't need paranoia, it's just a question of how often -- and in this case the chances are vanishingly small.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3532#issuecomment-600881892

---

From: David Schinazi <[email-masked]>
Date: Wed, 18 Mar 2020 16:48:50 -0700

To clarify the `@DavidSchinazi edited that out` statement, that happened when we decided to move version negotiation to an extension. The [current text](https://tools.ietf.org/html/draft-ietf-quic-transport-27#section-6.2) states that `When a client receives a Version Negotiation packet, it MUST abandon the current connection attempt.` - I'd be supportive of adding a sentence such as `Clients MUST ignore received Version Negotiation packets that contain the version they attempted to use.`

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3532#issuecomment-600913066

---

From: martinduke <[email-masked]>
Date: Thu, 19 Mar 2020 12:26:00 -0700

I support the change.

Does the general concern about security proofs that is driving us to authenticate the CIDs also apply to the version field?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3532#issuecomment-601374157

---

From: David Schinazi <[email-masked]>
Date: Thu, 19 Mar 2020 13:16:46 -0700

@martinduke The security concerns do apply to the version field, our current plan to address that is [draft-ietf-quic-version-negotiation](https://tools.ietf.org/html/draft-ietf-quic-version-negotiation).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3532#issuecomment-601395553

---

From: martinduke <[email-masked]>
Date: Thu, 19 Mar 2020 14:21:59 -0700

@DavidSchinazi Thanks, but is it sufficient for this to be an extension? What if the base spec isn't provably secure?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3532#issuecomment-601422959

---

From: David Schinazi <[email-masked]>
Date: Thu, 19 Mar 2020 16:01:40 -0700

@martinduke there's still time for us to make the use of that extension mandatory before we ship, but I'd suggest having that discussion on a separate issue.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3532#issuecomment-601455493