Subject: [quicwg/base-drafts] For disconnect with Initial Injection (#1951)

From: Christian Huitema <[email-masked]>
Date: Fri, 02 Nov 2018 23:14:05 -0700

Suppose that a connection has progressed and the server has sent an Initial packet with a Server Hello to the client. At that point, we expect that all following packets will be protected by the handshake key or by the application key, preventing 3rd parties from interfering with the connection. 

Suppose now that a third party sends to the server a spoofed initial packet, containing for example a malformed acknowledgement. The server will process the packet, find the malformed frame, generate a protocol violation error and close the connection. Which means that the 3rd party is able to interfere with the connection after the handshake keys have already been exchanged.

Is that an attack we should worry about?

-- 
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/1951

---

From: Christian Huitema <[email-masked]>
Date: Fri, 02 Nov 2018 23:25:14 -0700

Proposed protection:

On the client side, Initial packets should not be processed after the handshake decryption key has been installed. On the server side, initial packets should not be processed after the first handshake packet from the client has been received. There is a gray area in the server between sending the first handshake packet and receiving the client handshake message, especially if the server hello required transmission of several initial packets.

-- 
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/1951#issuecomment-435564424

---

From: ianswett <[email-masked]>
Date: Fri, 02 Nov 2018 23:54:45 -0700

Fixing this basically requires implicit ack of Initial packets, which I expect all implementations to do anyway.  I'll try to create a PR.

-- 
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/1951#issuecomment-435565607

---

From: ianswett <[email-masked]>
Date: Sat, 03 Nov 2018 00:06:08 -0700

Martin points out that HRR makes this rather odd, because the client can't ACK the HRR, so the server has to send both the HRR and ServerHello in the second flight.  Which is ok, but ugly.

-- 
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/1951#issuecomment-435566101

---

From: Christian Huitema <[email-masked]>
Date: Sat, 03 Nov 2018 01:54:50 -0700

I think there are two issues that are interacting. We want to limit what the server sends without some continuity test, to prevent amplification attacks. And then we want to prevent injection of initial packets after the handshake key is set, in order to prevent DOS on the connection. The HRR/ServerHello issue is tied to amplification attack prevention. 

-- 
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/1951#issuecomment-435571859

---

From: Christian Huitema <[email-masked]>
Date: Sat, 03 Nov 2018 03:20:34 -0700

By the way, why do you say that clients cannot ACK the HRR? I just did a trial with the picoquic client and server,  using a client option to elicit an HRR by not sending any key share in the Client Hello. I see the following:
```
Client Initial, seq=0, contains Crypto Stream bytes 0..246 (Client Hello)
    Server Initial, seq=0, contains ACK(0), CS bytes 0..87 (HRR)
Client Initial, seq=1, contains ACK(0), CS bytes 247..562 (New Client Hello)
   Server Initial, seq=1, contains ACK(0..1), CS bytes 88..242 (Server Hello)
   etc.
```

-- 
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/1951#issuecomment-435576477

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 12 Nov 2018 18:58:49 -0800

My understanding is that what @huitema is proposing in
https://github.com/quicwg/base-drafts/issues/1951#issuecomment-435564424 is just defining when the endpoints should drop Initial keys (and stop sending / receiving Initial packets).

Until that happens, endpoints are expected to ACK Initial packets. So in case of HRR, the exchanges of packets will be exactly the same as we have now up to the point when the server receives the 2nd Client Hello.

I think that the approach would work fine, and that it is trivial to implement.

The only annoyance I feel about the proposal is the fact that it is an attempt to overturn our previous conclusion to use explicit ACKs. But I agree that this is an improvement in terms of security and also that it is trivial to implement. Hence my +1 to adopting the proposal.

-- 
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/1951#issuecomment-438112817

---

From: Subodh Iyengar <[email-masked]>
Date: Sun, 25 Nov 2018 21:13:09 -0800

I think we shouldn’t do this until we resolve the congestion controller exemptions for this type of vulnerable data. 

I’m not sure whether that is finished yet. Given that I would be fine with this

-- 
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/1951#issuecomment-441521157

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 29 Nov 2018 18:13:43 -0800

FWIW, I am starting to wonder if there could be an alternative approach than dropping Initial keys.

Specifically, I am wondering if just having the rules for dealing with potentially attacking frames (instead of packets) would be either equivalent to (or even better than) trying to drop the keys.

To me it seems that declaring the following three rules (quoted from https://github.com/quicwg/base-drafts/pull/2053#discussion_r237723449) is sufficient, or even better considering invalid ACK injection attacks during the exchange of Initial packets. The other benefit is that the loss recovery / congestion control logic can remain unchanged.

> How about stating that, for Initial packets:
>
> * an endpoint MUST ignore CRYPTO frames once it obtains Handshake keys
> * an endpoint MUST ignore CONNECTION_CLOSE frames once it receives a Handshake packet
> * an endpoint MUST ignore unexpected packet numbers found in an ACK frame

-- 
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/1951#issuecomment-443065734

---

From: Marten Seemann <[email-masked]>
Date: Thu, 29 Nov 2018 18:34:19 -0800

That seems like a very fragile solution to me. There are a lot more error conditions that than you describe (e.g. frames that are invalid in an Initial packet, incomplete frames, unknown frame types etc.).

It also feels wrong to me to act on some frames, and ignore other frames sent in the same packet. Either we have reason to believe that a packet was sent by an attacker, then we should drop it on the floor as a whole, or we believe it was sent by the peer, then we should process it entirely.

Dropping the keys as soon as possible has the beautiful property that we don’t have to worry about complex rules which packets are valid are which ones aren’t any more - since we know for sure that either way we don’t need to read these packets. 

-- 
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/1951#issuecomment-443069551

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 29 Nov 2018 18:57:29 -0800

I agree that dropping at per-frame basis is more fragile than dropping keys. However, it has the following benefits:
* The rule to evade attacks before the Handshake keys are exchanged. Specifically, ignoring unexpected PNs in ACK is a nice-to-have for #2076.
* The rules are easier to implement than what we have now in #2053, because the rules can be applied in one-path, rather than in two-pass (i.e. first check that the frames are valid then apply all of them).

To put it another way, per-frame rules is something easy to implement, and also is beneficial to evade attacks during the exchange of Initials. And if we use the framework also for ignoring CRYTO frames once the Handshake key is obtained, dropping the Initial keys become unnecessary.

-- 
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/1951#issuecomment-443073733

---

From: Marten Seemann <[email-masked]>
Date: Thu, 29 Nov 2018 19:02:39 -0800

> The rules are easier to implement than what we have now in #2053, because the rules can be applied in one-path, rather than in two-pass (i.e. first check that the frames are valid then apply all of them).

I don't think that's true. Consider the case where a packet contains two ACK frames, the first of which is valid, the second one acknowledges an unsent packet. When processing the frames, you'd first happily process (and clean up your history of sent packets), only to encounter the second, invalid frame. What do you do now? You *could* unwind the effects of the first ACK frame, which at least in my implementation would a huge amount of complexity, or you could ignore the second ACK frame, accepting the fact that you acted upon a packet that was probably sent by an attacker.

-- 
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/1951#issuecomment-443074692

---

From: ianswett <[email-masked]>
Date: Thu, 29 Nov 2018 19:02:49 -0800

In my experience, per-frame rules are very problematic.  Particularly any that drop some frames and process others without closing the connection.

That's a complex road I'm unwilling to go down.

-- 
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/1951#issuecomment-443074727

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 29 Nov 2018 19:09:53 -0800

@marten-seemann In this specific case, that is not an issue.

Before the Handshake keys are exchanged, you need to rely on ACKs. At that moment, having rules to reject certain ACKs never hurts.
After the Handshake keys are known, endpoints can ignore ACK frames entirely.

So two-pass is not required if you drop frames at per-frame level.

-- 
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/1951#issuecomment-443075895

---

From: Marten Seemann <[email-masked]>
Date: Thu, 29 Nov 2018 19:14:59 -0800

> After the Handshake keys are known, endpoints can ignore ACK frames entirely.

Which requires implicit acking, along the lines of #2045, which I think is the best fix we have for the injection attack.

> Before the Handshake keys are exchanged, you need to rely on ACKs. At that moment, having rules to reject certain ACKs never hurts.

True, but it doesn't fix this issue. There are various ways to defend against injections during the 3-way handshake, and implementations may do something fancy here, but I don't think we have the tools to completely prevent this attack yet.

-- 
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/1951#issuecomment-443076675

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 29 Nov 2018 22:40:12 -0800

> Which requires implicit acking, along the lines of #2045

It does not require implicit ACKing, because ACKs are always sent.

-- 
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/1951#issuecomment-443107233

---

From: Marten Seemann <[email-masked]>
Date: Thu, 29 Nov 2018 22:44:10 -0800

If we ignore ACK frames for Initial packets after handshake keys become available, why would we still send them? And if we don't ignore them, we're vulnerable to injection attacks.

-- 
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/1951#issuecomment-443108006

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 30 Nov 2018 07:30:54 +0000 (UTC)

> If we ignore ACK frames for Initial packets after handshake keys become available, why would we still send them?

I am saying that ignoring suspicious ACK (either partially or entirely) does not disrupt a handshake once an endpoint has obtained the handshake keys, because the effect is the same as using implicit ACKs.

The benefit of exchanging ACKs (that can be ignored) rather than doing implicit ACKs is that they provide valuable information to loss recovery and congestion control.

>  And if we don't ignore them, we're vulnerable to injection attacks.

I disagree. Assuming that we adopt the rules in 
https://github.com/quicwg/base-drafts/issues/1951#issuecomment-443065734, I think that we would be as safe as #2054 after handshake keys are obtained. Also, the proposed approach is less vulnerable to injection attacks before the handshake keys are obtained.

-- 
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/1951#issuecomment-443116632

---

From: MikkelFJ <[email-masked]>
Date: Fri, 30 Nov 2018 12:21:00 -0800

@marten-seemann  wrote:

> I'm not sure if this is true. Consider the case where a packet contains two ACK frames, the first of which is valid, the second one acknowledges an unsent packet. When processing the frames, you'd first happily process (and clean up your history of sent packets), only to encounter the second, invalid frame. What do you do now? You could unwind the effects of the first ACK frame, which at least in my implementation would a huge amount of complexity, or you could ignore the second ACK frame, accepting the fact that you acted upon a packet that was probably sent by an attacker.

Whatever we do, we need to recognise that this particular case can also be an optimistic ACK attack by a valid endpoint that misbehaves. It is more likely to be a misbehaving client though. Therefore, special casing around this needs to close the connection once handshake encryption is online, but not before. That is a fair bit of complexity right there.

-- 
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/1951#issuecomment-443327768