Subject: [quicwg/base-drafts] Martin Duke Transport Comment 1 (#4450)

From: Lucas Pardue <[email-masked]>
Date: Tue, 22 Dec 2020 10:00:31 -0800

@martinduke said:

> 17.2.1 I believe it is correct that there will be no negative consequences from
not having Retry-like integrity protection on VN packets. But I ask the editors
to take one more careful look at it, as the VN format is one of those things we
really cannot fix later.

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 22 Dec 2020 13:32:08 -0800

I wouldn't be so confident in saying that there will be no negative consequences.  Indeed, I think that integrity protection for Version Negotiation would go a long way to improving the protocol.  However, I also don't feel confident in defining an integrity protection mechanism.

One way or other, we're going to pay for this.  We can only hope that the cost isn't too high in the end.

-- 
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/4450#issuecomment-749785328

---

From: martinduke <[email-masked]>
Date: Tue, 22 Dec 2020 13:42:33 -0800

My reading of this, together with the Version Negotiation Draft, is that the specific case that drove the Retry Integrity check was that a flipped bit drove the client to a timeout because it couldn't accept a second Retry. A flipped bit in VN would simply result in a second VN, or a CONN_CLOSE early in the handshake -- no timeout necessary.

For both Retry and VN, actual malicious injection requires the TP mechanisms to discover -- integrity checks won't help. Therefore, the risk of defining a integrity check with a soon-to-be-deprecated mechanism is not so high.

Anyway, that's my reasoning, FWIW, but I am not the expert. As my original comment implies, I'm happy to go with whatever action you guys decide after you think it through.

-- 
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/4450#issuecomment-749789271

---

From: Martin Thomson <[email-masked]>
Date: Tue, 22 Dec 2020 14:06:59 -0800

Yes, the bitflip is exactly the sort of problem I think we'll need to deal with.  A flipped bit (or attack) will be detected, but at the cost of time. Possibly quite a lot of time.  Good point about the timeout with Retry; as you say, this is maybe less prone to ending up with a timeout, though not entirely.  A bitflip that is limited to the connection ID fields, or that causes the version to change to the attempted version, will cause the packet to be dropped and that will lead to a timeout.

Thankfully, with things like Alt-Svc and SVCB, we might not depend on the version negotiation mechanism very much, if at all.

In other words, yeah, this design is suboptimal, but I think that it's the best balance.  (I have some ideas for how we might fix this post-hoc if it comes to that, but they have their own issues.)

-- 
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/4450#issuecomment-749799936

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 22 Dec 2020 14:17:36 -0800

I agree that having integrity protection would be a good thing for VN packets -- we've recently seen the IP/UDP checksum fail badly in deployed settings. That said, I'm wary of adding this now primarily because:
- agreeing on something for all time is likely going to be difficult
- as @martinduke and @martinthomson note, the reliance on and cost of corrupt VN is potentially much lower than of Retry 
- if we decide this is important enough to do in the future, all is not lost -- we have ways to introduce an integrity check later (by sacrificing VN space)

I think I'm happy with what we have now, especially given that we _can_, if needed, bring an integrity check in later.

-- 
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/4450#issuecomment-749804245

---

From: martinduke <[email-masked]>
Date: Tue, 22 Dec 2020 14:19:44 -0800

I don't know what you mean by "sacrificing VN space". How would a v1 client interpret this new format?

-- 
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/4450#issuecomment-749805167

---

From: Martin Thomson <[email-masked]>
Date: Tue, 22 Dec 2020 14:21:19 -0800

Well, the good thing is that a v1 client doesn't read the version negotiation packet past the connection ID fields.  So we have some options available to us.

-- 
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/4450#issuecomment-749805861

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 22 Dec 2020 14:36:17 -0800

There are various things possible here. One is change the payload format, as @martinthomson suggests. Another strawman is that we reserve a 16-bit version space (say, 0x1111****) for carrying a 16-bit hash. So we have ways.

-- 
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/4450#issuecomment-749812522

---

From: Marten Seemann <[email-masked]>
Date: Wed, 23 Dec 2020 02:14:10 -0800

I don't see any way of making this change without breaking interop, in the strict sense. That said, I'm still in favor of making this change, as I believe there's an easy way that we can include a hash here that will not break interop in practice. 

Let me make a straw-man proposal here: If we choose a 16-bit hash, we could place a 16-bit magic value, followed by the hash right [after the SCID](https://tools.ietf.org/html/draft-ietf-quic-invariants-07#section-5):
```
0                   1                   2                   3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
|         magic value           |            hash             |
````

The magic value would have to be chosen such that it doesn't collide with a prefix of an already deployed QUIC version. Looking at the [version registry](https://github.com/quicwg/base-drafts/wiki/QUIC-Versions) (and assuming it's up-to-date), there should be plenty of options. 
A client that is aware of the change here would only run the checksum verification if the magic value matches (if clients want to get fancy, they could also consider the Levenshtein distance, in order to also catch flips in the magic value). Once draft versions have disappeared from the internet, this magic-value-matching would become unnecessary, and the respective code could be removed from clients.

As the magic value prefix doesn't collide with any versions that were ever deployed (by construction), clients that are using the old VN format would ignore the 32 bits we're adding, as they'd parse them as a QUIC version number that they don't support.

-- 
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/4450#issuecomment-750082737

---

From: Mike Bishop <[email-masked]>
Date: Mon, 28 Dec 2020 12:45:00 -0800

It's a cute scheme, analogous to Jana's comment.  But it equally demonstrates how this could be added as an extension later, if we decide it's worth doing; it would be just as transparent to un-extended clients as your proposal would be to draft clients.  The VN draft or some future draft could reserve that version range in the registry and define how to do an integrity check on the payload of a VN packet.

My feeling right now is that this isn't required to ship; we have a VN draft adopted, and if as part of our fuller consideration of how to do version negotiation we decide it's warranted, let's do it 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/4450#issuecomment-751858627

---

From: Marten Seemann <[email-masked]>
Date: Tue, 29 Dec 2020 03:40:11 -0800

> But it equally demonstrates how this could be added as an extension later, if we decide it's worth doing; it would be just as transparent to un-extended clients as your proposal would be to draft clients. 

I agree, with one caveat: We'd have to reserve a block of version numbers for the magic value. If we want to do this, we should probably do this earlier rather than later, otherwise these version numbers might get used by people running private versions / experiments.

-- 
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/4450#issuecomment-752045997

---

From: Mike Bishop <[email-masked]>
Date: Tue, 29 Dec 2020 09:40:00 -0800

It would take 65k versions registered / in unregistered use before that's the case, and that only if they're perfectly distributed across the space.  I hear the caution, but I'm not sure I share the urgency.

-- 
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/4450#issuecomment-752177632