Subject: Re: [quicwg/base-drafts] Clarify Actions on nonzero Reserved Bits (#2280)

From: janaiyengar <[email-masked]>
Date: Fri, 04 Jan 2019 17:31:16 -0800

janaiyengar commented on this pull request.

> @@ -3402,9 +3402,10 @@ Reserved Bits (R):
 
 : The next two bits (those with a mask of 0x0c) of byte 0 are reserved.  These
   bits are protected using header protection (see Section 5.4 of {{QUIC-TLS}}).
-  The value included prior to protection MUST be set to 0.  An endpoint MUST
-  treat receipt of a packet that has a non-zero value for these bits after
-  removing protection as a connection error of type PROTOCOL_VIOLATION.
+  The value included prior to protection MUST be set to 0.  An endpoint MUST treat
+  receipt of a packet that has a non-zero value for these bits after removing
+  packet (not just header) protection as a connection error of type

Isn't "packet protection" enough? I don't think you need to say "not just header"

-- 
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/2280#discussion_r245460883

---

From: janaiyengar <[email-masked]>
Date: Fri, 04 Jan 2019 17:32:21 -0800

Not relevant to this PR, but @martinthomson : there are other side-channels in QUIC sadly. One could simply (at a high level) observe whether an ACK is sent or not, for instance.

-- 
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/2280#issuecomment-451616064

---

From: martinduke <[email-masked]>
Date: Sat, 05 Jan 2019 15:36:41 -0800

I think my change makes it clearer. If people think this is a bad change, I'll kill the 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/pull/2280#issuecomment-451701620

---

From: Martin Thomson <[email-masked]>
Date: Tue, 08 Jan 2019 16:10:35 -0800

martinthomson commented on this pull request.

I think that mentioning both types of protection is fine, but the parenthetical was a little strange.

It's both.  My suggestion would be:

```suggestion
  packet and header protection as a connection error of type
```

> @@ -3529,8 +3530,8 @@ Reserved Bits (R):
   bits are protected using header protection (see Section 5.4 of
   {{QUIC-TLS}}).  The value included prior to protection MUST be set to 0.  An
   endpoint MUST treat receipt of a packet that has a non-zero value for these
-  bits after removing protection as a connection error of type
-  PROTOCOL_VIOLATION.
+  bits after removing packet (not just header) protection as a connection error

```suggestion
  bits after removing packet and header protection as a connection error
```

-- 
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/2280#pullrequestreview-190515153

---

From: ianswett <[email-masked]>
Date: Wed, 09 Jan 2019 06:06:44 -0800

ianswett commented on this pull request.

@martinthomson 's suggestion is good, but I'm confused why you need to remove packet protection to know the bits are not set to 0.  Isn't removing header protection sufficient?

-- 
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/2280#discussion_r246392825

---

From: martinduke <[email-masked]>
Date: Wed, 09 Jan 2019 09:09:15 -0800

@martinduke pushed 1 commit.

f780ef402954ac5a9efabb23a556769f121dbc1d  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2280/files/f668691287e3eccf8e330761fdb79f44e4d4edb8..f780ef402954ac5a9efabb23a556769f121dbc1d

---

From: martinduke <[email-masked]>
Date: Wed, 09 Jan 2019 09:11:56 -0800

3b36352247f614b80c4710256547915f151d3e82  Merge branch 'master' into patch-32

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2280/files/f780ef402954ac5a9efabb23a556769f121dbc1d..3b36352247f614b80c4710256547915f151d3e82

---

From: martinduke <[email-masked]>
Date: Wed, 09 Jan 2019 09:12:53 -0800

See MT's first comment. We need consistent-time crypto results.

On Wed, Jan 9, 2019 at 6:06 AM ianswett <[email-masked]> wrote:

> *@ianswett* commented on this pull request.
> ------------------------------
>
> In draft-ietf-quic-transport.md
> <https://github.com/quicwg/base-drafts/pull/2280#discussion_r246392825>:
>
> @martinthomson <https://github.com/martinthomson> 's suggestion is good,
> but I'm confused why you need to remove packet protection to know the bits
> are not set to 0. Isn't removing header protection sufficient?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/quicwg/base-drafts/pull/2280#discussion_r246392825>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AXRMEdPDZinhGkRgBaT3EYniSzAqQ9oAks5vBfd1gaJpZM4ZlOXd>
> .
>

-- 
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/2280#issuecomment-452767293

---

From: ianswett <[email-masked]>
Date: Wed, 09 Jan 2019 17:35:44 +0000 (UTC)

> @@ -3420,7 +3420,19 @@ Long Packet Type (T):
 
 Type-Specific Bits (X):
 
-: The lower four bits (those with a mask of 0x0f) of byte 0 are type-specific.
+: The next two bits (those with a mask of 0x0c) of byte 0 are reserved.  These

Is this diff not against head?

-- 
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/2280#pullrequestreview-190841189

---

From: ianswett <[email-masked]>
Date: Wed, 09 Jan 2019 17:36:37 +0000 (UTC)

Thanks @martinduke I missed his comment.  That is totally reasonable, but also not obvious.  But it's in the TLS text, so I hopefully everyone reads that?

-- 
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/2280#issuecomment-452780927

---

From: martinduke <[email-masked]>
Date: Wed, 09 Jan 2019 09:49:58 -0800

@ianswett yeah, this PR is trying to make it clear, because I also find it non-obvious (and buried in the security considerations section of the TLS draft).

While this PR makes it crystal clear what is supposed to happen, I can easily see a developer thinking they're smarter than the spec and discarding after header protection. I wouldn't mind some explanatory text, but one thing at a time.

-- 
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/2280#issuecomment-452786741

---

From: David Schinazi <[email-masked]>
Date: Wed, 09 Jan 2019 21:08:29 +0000 (UTC)

Out of curiosity, why are we validating these bits in the first place? Can't we just say receivers MUST ignore them? That removes timing attacks and also allows for future extensibility. I get that these are not inside the full encryption boundary but the associated data + header protection boundary should be good enough to prevent middleboxes from looking at or messing with these bits.

-- 
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/2280#issuecomment-452835167

---

From: Marten Seemann <[email-masked]>
Date: Thu, 10 Jan 2019 01:07:06 +0000 (UTC)

@DavidSchinazi We had this discussion before in #2022, and we decided to require a fixed value. 

-- 
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/2280#issuecomment-452931870

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 09 Jan 2019 18:19:31 -0800

@DavidSchinazi I think it is a good practice to forbid the endpoints using the reserved bits without negotiation, because the bits could mean different things between them. They should negotiate the use of bits (that would allow then to accept non-zero values), then use them.

-- 
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/2280#issuecomment-452945636

---

From: ianswett <[email-masked]>
Date: Thu, 10 Jan 2019 03:49:47 -0800

@kazuho Given these are within the encrypted envelope, if one side thinks they mean something and the other doesn't or vice versa, there are sure to be problems, so any usage would have to be negotiated as you say.

@marten-seemann I couldn't find that in #2022.  My reading of that discussion was header protection is better than just doing packet number encryption.

I don't have a strong opinion here, but I'm also not seeing much reason to specify what the values need to be and saying they can be anything seems simpler.  Saying they have to be 0 invites people to short-cut the full decryption and drop packets that have other values immediately.  To be clear, I probably would have done that if I hadn't read this 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/pull/2280#issuecomment-453069012

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 10 Jan 2019 12:58:45 +0000 (UTC)

@ianswett 
> Saying they have to be 0 invites people to short-cut the full decryption and drop packets that have other values immediately.

Honestly speaking, I am not sure short-cutting in case the zero check fails would lead to side channel vulnerability, considering the fact that a failed check immediately ends up in connection termination. The only information an attacker can obtain is that the sender failed to set the bits correctly.

Anyways, I do not think that removing the zero-check dissolves the vulnerability to the side-channel attack, because any use of the bits protected by header protection prior to AEAD-decrypting the payload is forbidden. The real attack exists on the PN field, where an attacker can flip the bits of the encrypted PN and use the timing difference of the receiver that short-cuts AEAD decryption based on the value of the PN to infer what the value of the unprotected PN was.

Therefore, I think that we should spend our effort to be clear about when the unprotected header bits can be used, rather than trying to fixing a use-case that does not have a high impact.

-- 
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/2280#issuecomment-453087410

---

From: ianswett <[email-masked]>
Date: Thu, 10 Jan 2019 07:50:08 -0800

In that case, let's land this and move onto more important 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/pull/2280#issuecomment-453144406

---

From: martinduke <[email-masked]>
Date: Thu, 10 Jan 2019 16:11:52 +0000 (UTC)

@ianswett My previous reply was too glib. One problem with applying the check right after removing header protection is that the consequences are a connection error -- so a garbage packet has a 3 in 4 chance of taking down the connection.

There are three equilbria here:
1) Simply discard the packet after header protection. As @marten-seemann pointed out, it's not clear why this would provide any sort of intelligence to an attacker, as there is no response.
2) After packet protection, it's a PROTOCOL_VIOLATION. This is the current state of the draft with this PR serving to clarify a bit.
3) Not having any requirement for the reserved bits. There is a difference in opinion over whether this is a side channel issue or not, or if this should be done only with some sort of negotiation. I'm not the one to articulate this positon.

Which is all to say that I should have taken @martinthomson 's advice a week ago and filed and 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/pull/2280#issuecomment-453152955