Subject: [quicwg/base-drafts] Let Endpoints Ignore invalid Initial Packets (#1819)

From: martinduke <[email-masked]>
Date: Mon, 01 Oct 2018 15:38:43 -0700

A little more flexibility to defeat injection attacks. This is the other half of #1786.
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/1819

-- Commit Summary --

  * Let Endpoints Ignore invalid Initial Packets

-- File Changes --

    M draft-ietf-quic-transport.md (16)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/1819.patch
https://github.com/quicwg/base-drafts/pull/1819.diff

-- 
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/pull/1819

---

From: martinduke <[email-masked]>
Date: Mon, 01 Oct 2018 15:39:17 -0700

@martinduke pushed 1 commit.

14bdb20  fix typo

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1819/files/b6a17f4d685b3f34baa37b98db3b42e7eb2e60f2..14bdb20066fdd5577b7ab6eac545a383260fc9cf

---

From: ianswett <[email-masked]>
Date: Mon, 01 Oct 2018 18:12:02 -0700

ianswett commented on this pull request.

Looks good, one comment about the text you're modifying.

> @@ -720,9 +720,7 @@ require multiple round trips or retransmissions of this data.
 
 The payload of an Initial packet includes a CRYPTO frame (or frames) containing

I think "containing a cryptographic handshake message" section makes this sentence more confusing.  Can you remove it?

-- 
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/pull/1819#pullrequestreview-160539210

---

From: martinduke <[email-masked]>
Date: Wed, 03 Oct 2018 12:46:33 -0700

efc1876  Resolved Ian's issue

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1819/files/14bdb20066fdd5577b7ab6eac545a383260fc9cf..efc18760eb7a70db0bb4265639cc906bdc67be2f

---

From: martinduke <[email-masked]>
Date: Wed, 03 Oct 2018 12:46:59 -0700

9dad6cb  argh typos

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1819/files/efc18760eb7a70db0bb4265639cc906bdc67be2f..9dad6cb2db0b2585af2bc27b2af52d5fda7d128b

---

From: Mike Bishop <[email-masked]>
Date: Wed, 03 Oct 2018 13:36:51 -0700

MikeBishop commented on this pull request.

> @@ -736,6 +733,18 @@ and will contain a CRYPTO frame with an offset matching the size of the CRYPTO
 frame sent in the first Initial packet.  Cryptographic handshake messages
 subsequent to the first do not need to fit within a single UDP datagram.
 
+### Handling of Fatal Initial Packets
+
+The contents of some Initial packets may, according to this specification, force

I'm not a big fan of a specification saying you can ignore the specification; I'd prefer consistency.  You removed the language above indicating that things should be fatal, so you're part-way there.

Perhaps something like "Receipt of an invalid Initial packet could indicate an injection attack rather than a peer failure.  Endpoints abort a connection attempt if no valid Initial packets are received after an appropriate timeout."  That gives enough latitude that the timeout could be immediate or a long observation for multiple possibilities.

-- 
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/pull/1819#pullrequestreview-161350765

---

From: martinduke <[email-masked]>
Date: Fri, 05 Oct 2018 16:09:00 -0700

martinduke commented on this pull request.

I really don't want to go everywhere it says we MUST terminate the connection and then add "unless it's an Initial and ..." Inevitably it's a bit clunky.

Your language is fine, but I'd like to preserve the option to just drop the packet without running any timer. It's a simpler course of action for the endpoint and I don't see a significant downside.

-- 
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/pull/1819#discussion_r223158298

---

From: martinduke <[email-masked]>
Date: Mon, 08 Oct 2018 14:55:55 -0700

bf5162c  Respond to @MikeBishop

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1819/files/9dad6cb2db0b2585af2bc27b2af52d5fda7d128b..bf5162c4bedf714b334734210326ce634eae2c01

---

From: ianswett <[email-masked]>
Date: Thu, 18 Oct 2018 21:08:17 +0000 (UTC)

+### Handling of Fatal Initial Packets
+
+The contents of some Initial packets may, according to this specification, force
+connection termination. For example, they might contain forbidden frame types
+or a CONNECTION_CLOSE frame. As Initial packets are not protected, these could
+indicate injection attacks to terminate the connection.
+
+Endpoints MAY treat the receipt of such packets as a connection error, drop them

The more I think about this, I think this is basically an intractable problem unless one is willing to keep copies of the QUIC handshake state machinery.

1) If you start processing a packet, and process one frame, then the next frame is invalid, you'd have to rewind/discard the state machinery.  Is that what you're recommending?
2) You have text that indicates you should process Initial packets, but not treat them as errors once you have received a Handshake packet, but similar to my first concern, I think that's fairly impractical unless you keep copies of the handshake.
3) One could ACK initial packets, but not process them, but then if the client ACKs a packet that was never sent, the server can close the connection, so you're still in a bad place.

To be clear, I sort of hate implicit ACKs of Initial, but we keep making decisions, such as that only Handshake packets count as address confirmation, that make most frames sent in Initial useless.  I have https://github.com/quicwg/base-drafts/pull/1862 to fix the rest of the deadlock case, but that would be fixed if we didn't send ACKs in Initial and relied upon implicit ACKs instead.

-- 
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/pull/1819#pullrequestreview-166294304

---

From: MikkelFJ <[email-masked]>
Date: Thu, 18 Oct 2018 21:17:42 +0000 (UTC)

mikkelfj commented on this pull request.

So why is there such resistance against implicit ACKs in handshake, when the solve a concrete problem, as opposed to a being a whacky optimization?

-- 
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/pull/1819#discussion_r226467403

---

From: ianswett <[email-masked]>
Date: Thu, 18 Oct 2018 14:55:33 -0700

I believe we've also said previously we don't care about man on the side attacks, but I think it's clear we do, but we don't want to do a full analysis of them, which leaves us in a weird place.

-- 
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/pull/1819#discussion_r226478559

---

From: martinduke <[email-masked]>
Date: Thu, 18 Oct 2018 16:43:20 -0700

There is nothing you can really do about injection attacks during the Initial phase of the handshake. This PR is more about the 3RTT (or, for lazy implementations, infinite) time that the initial keys remain lying around for use. This radically increases the time where a connection is vulnerable (especially if the server uses the client-generated CID, so that Initial keys are easy to generate).

In that context, I'm not sure how you would meaningfully process a late-arriving Initial Packet. Say the attacker sends the server Initial Packet #2 well after the handshake, with ACK, CRYPTO, and CONNECTION_CLOSE frames. Since Initial exchange is actually done, there are no outstanding packets to ACK. Since I've already started handshake CRYPTO exchange, it's too late to deliver any new CRYPTO data. So it is basically impossible to use frames to alter connection state except to bring down the connection by generating an error.

If your code sends off an ACK before processing the frames, that is a problem, but that seems like a bad thing to do for other reasons.

-- 
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/pull/1819#discussion_r226498538

---

From: Martin Thomson <[email-masked]>
Date: Thu, 18 Oct 2018 22:20:04 -0700

As I said in #1890, I don't think that we want to do this.  Not only because it requires a lot of words that are very hard to get right.

Yes, an implementation can attempt to manage state in a way that is resilient to interference in the handshake.  Every packet that can be successfully processed relative to any pre-existing checkpoint in the state of the connection adds another dependent checkpoint to that tree.  For instance, should I have received N packets, I would have potentially 2^N copies of the nascent connection state as a result, though likely fewer as each packet is likely only valid when interpreted from a certain subset of those states.  When a packet causes the connection to be successfully established, other states can then be discarded.

I've heard that there are partial approaches that are less onerous than the fully exhaustive option and this advocates for something in that space.

I don't think that we need to specify any of that.  On the positive side, I don't think that anything we write into the draft would prohibit someone from implementing these techniques.  They might be complex, but it would be very nice if we could do away with packet injection attacks being used to interfere with connection establishment.  

In the end, this runs directly counter to what we already agreed was the guidelines: we would not put in measures that prevented an attacker from interfering with connection establishment if they were able to observe and inject packets.  QUICv2 seems like the right place for this, if anything.

-- 
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/pull/1819#issuecomment-431247799

---

From: martinduke <[email-masked]>
Date: Fri, 19 Oct 2018 15:38:32 -0700

I would argue that this PR doesn't specify anything, but merely authorizes those advanced techniques should someone elect to implement them. As indicated above, this is also about reducing the vulnerability when keeping Initial keys around for a conservatively long time.

But if this is the consensus, we can close the PR. Especially if I'm the only one interested in using this MAY in v1.

-- 
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/pull/1819#issuecomment-431518850

---

From: Martin Thomson <[email-masked]>
Date: Thu, 25 Oct 2018 22:33:30 -0700

Closed #1819.

-- 
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/pull/1819#event-1927944488

---

From: Martin Thomson <[email-masked]>
Date: Thu, 25 Oct 2018 22:33:30 -0700

Closing as agreed.

-- 
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/pull/1819#issuecomment-433291544