Subject: Re: [quicwg/base-drafts] Add retry integrity tag (#3120)

From: Jana Iyengar <[email-masked]>
Date: Tue, 05 Nov 2019 15:20:36 -0800

janaiyengar approved this pull request.

-- 
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/3120#pullrequestreview-312108966

---

From: Christian Huitema <[email-masked]>
Date: Thu, 14 Nov 2019 21:54:00 -0800

huitema commented on this pull request.

> +The Retry Pseudo-Packet is not sent over the wire. It is computed by taking
+the transmitted Retry packet, removing the Retry Integrity Tag and prepending
+the two following fields:
+
+ODCID Len:
+
+: The ODCID Len contains the length in bytes of the Original Destination
+  Connection ID field that follows it, encoded as an 8-bit unsigned integer.
+
+Original Destination Connection ID:
+
+: The Original Destination Connection ID contains the value of the Destination
+  Connection ID from the Initial packet that this Retry is in response to. The
+  length of this field is given in ODCID Len. The presence of this field
+  mitigates an off-path attacker's ability to inject a Retry packet.
+

Overselling! If the off-path attacker has a copy of the original Initial packet sent by the client, then it knows the ODCID. I don't see how appending a checksum will change 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/3120#pullrequestreview-317419613

---

From: David Schinazi <[email-masked]>
Date: Thu, 14 Nov 2019 22:01:48 -0800

DavidSchinazi commented on this pull request.

By "off-path attacker" I meant an attacker who does not have a copy of any packets previously sent on-path. Can you suggest better wording?

-- 
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/3120#discussion_r346672099

---

From: David Schinazi <[email-masked]>
Date: Thu, 14 Nov 2019 22:03:33 -0800

-- 
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/3120#discussion_r346672446

---

From: David Schinazi <[email-masked]>
Date: Thu, 14 Nov 2019 22:03:58 -0800

-- 
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/3120#discussion_r346672528

---

From: Christian Huitema <[email-masked]>
Date: Thu, 14 Nov 2019 22:43:48 -0800

If the off-path attacker has not seen the client's initial packet, it cannot set the DCID of the Retry to the SCID of the client's Initial, which means the Retry would be rejected by the client already.

The additional protection is only effective if the client uses predictable CID, e.g. zero length -- but that's a truly bad idea for a variety of reasons. Such clients would be exposed to other attacks, e.g. injecting random bytes in the crypto-stream.

So in practice, the retry attack is only effective if the attacker is "in the middle" or "on the side". And the checksum will not protect against 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/3120#discussion_r346680010

---

From: Christian Huitema <[email-masked]>
Date: Thu, 14 Nov 2019 22:50:33 -0800

But I would shut up if you replaced "mitigates" by "diminishes".

-- 
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/3120#discussion_r346681472

---

From: David Schinazi <[email-masked]>
Date: Thu, 14 Nov 2019 23:20:06 -0800

@DavidSchinazi pushed 2 commits.

08077e12f9f35647b59ef1b293780fce1b4cb14d  Merge branch 'master' into prot_retry
a61c2b36de62fc10694951fcaf47dcf01633f2ae  Comment from Christian Huitema

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3120/files/739d272564a791476b75cff0a6ea258f0e4d17a8..a61c2b36de62fc10694951fcaf47dcf01633f2ae

---

From: David Schinazi <[email-masked]>
Date: Thu, 14 Nov 2019 23:20:37 -0800

In my mind mitigate and diminish have very similar meanings in this context, so I've switched the to text to use the word diminish

-- 
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/3120#discussion_r346688540

---

From: Christian Huitema <[email-masked]>
Date: Fri, 15 Nov 2019 08:05:35 +0000 (UTC)

huitema approved this pull request.

I may have sounded negative, but the protection against network errors alone justifies this PR. As long as we don't make foolhardy statements about mitigating attacks, I am fine.

-- 
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/3120#pullrequestreview-317457632

---

From: ekr <[email-masked]>
Date: Fri, 15 Nov 2019 00:57:26 -0800

I would suggest two revisions to this PR.

1. Let's encrypt the Retry Token.
2. Instead of having a zero key, let's use a per-version key (analagous to the per-version salt). This has the same anti-ossification benefits.

-- 
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/3120#issuecomment-554271932

---

From: David Schinazi <[email-masked]>
Date: Fri, 15 Nov 2019 02:08:12 -0800

@ekr
1. encrypting the token has been discussed in this PR and does not have consensus
2. the zero key can be per-version (by which I mean that QUICv1 uses a zero key, and QUICv2 can decide to use a different key)

The discussion on this PR and related issue #3014 indicates that there is interest in the Retry Integrity Tag. On top of that, the PR as-is is currently covered by the chairs' latest consensus call. Therefore I'd suggest you please file a separate issue to discuss encrypting the token, and we can discuss that once this PR has landed. You can also file a second issue to discuss zero key vs non-zero.

-- 
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/3120#issuecomment-554298004

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 15 Nov 2019 02:16:04 -0800

@DavidSchinazi 
> the zero key can be per-version (by which I mean that QUICv1 uses a zero key, and QUICv2 can decide to use a different key)

I think we should change the Retry key for every draft version (or every time we change the Initial salt), as that would prevent ossification assuming that we would be running certain number of draft versions concurrently.

I also prefer having a non-zero salt for the upcoming draft version, as it would be an indication to the implementors that the key is a "constant", rather than zeros that endpoints might synthetically generate (e.g., by using `bzero`).

-- 
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/3120#issuecomment-554300804

---

From: ekr <[email-masked]>
Date: Fri, 15 Nov 2019 02:32:11 -0800

> The discussion on this PR and related issue #3014 indicates that there is interest in the Retry Integrity Tag. On top of that, the PR as-is is currently covered by the chairs' latest consensus call.

Yes,  and I'm responding to that saying we ought to encrypt.

-- 
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/3120#issuecomment-554306346

---

From: Christian Huitema <[email-masked]>
Date: Fri, 15 Nov 2019 16:19:14 -0800

Can you elaborate why? Is this about security, or ossification?

-- Christian Huitema 

> On Nov 15, 2019, at 6:32 PM, ekr <[email-masked]> wrote:
> 
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or unsubscribe.

-- 
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/3120#issuecomment-554579886

---

From: ekr <[email-masked]>
Date: Fri, 15 Nov 2019 17:31:15 -0800

Ossification. Same reason we encrypt Initial

On Fri, Nov 15, 2019 at 4:19 PM Christian Huitema <[email-masked]>
wrote:

> > The discussion on this PR and related issue #3014 indicates that there
> is interest in the Retry Integrity Tag. On top of that, the PR as-is is
> currently covered by the chairs' latest consensus call.
> >
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/quicwg/base-drafts/pull/3120?email_source=notifications&email_token=AAIPLIKXYMQMDUA52WE6GMDQT4373A5CNFSM4JCAOODKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHDPLQ#issuecomment-554579886>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAIPLIJPCTOVFIYEGVOBUB3QT4373ANCNFSM4JCAOODA>
> .
>

-- 
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/3120#issuecomment-554589952

---

From: David Schinazi <[email-masked]>
Date: Mon, 18 Nov 2019 03:16:45 -0800

@DavidSchinazi pushed 1 commit.

98c7e87650c7e4934dbe667334335233e3f8a415  Use initial_salt instead of zero key

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3120/files/a61c2b36de62fc10694951fcaf47dcf01633f2ae..98c7e87650c7e4934dbe667334335233e3f8a415

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 19 Nov 2019 18:28:49 -0800

Let's get this in.

-- 
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/3120#pullrequestreview-319476705

---

From: ekr <[email-masked]>
Date: Tue, 19 Nov 2019 18:55:54 -0800

To recap my proposal from this morning.

1. We should have a separate fixed key derived by HKDF-ing the salt. This is just good key separation practice and we can publish the fixed value in the spec if we want. I think there was consensus on this and I can send text for this. @DavidSchinazi ?

2. We should encrypt the token using that fixed key and use the top min(12,dcid_len) bytes of the DCID. This will provide some obfuscation at the cost of just the AES operation but not the KDF.

-- 
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/3120#issuecomment-555813446

---

From: David Schinazi <[email-masked]>
Date: Tue, 19 Nov 2019 19:14:23 -0800

bdde08d8ac2d62954dc3fb0b49322b339e5a4a5c  make new key

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3120/files/98c7e87650c7e4934dbe667334335233e3f8a415..bdde08d8ac2d62954dc3fb0b49322b339e5a4a5c

---

From: David Schinazi <[email-masked]>
Date: Tue, 19 Nov 2019 19:17:11 -0800

I've made this a separate random key to avoid the HKDF. @ekr I hope this addresses point (1).

Regarding point (2) I don't care either way but if we were to use a connection ID as the nonce I would use the original destination connection ID since that's not sent over the wire but required to be known by endpoints.

-- 
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/3120#issuecomment-555818194

---

From: ekr <[email-masked]>
Date: Tue, 19 Nov 2019 19:30:27 -0800

Yes a random key is fine

On Tue, Nov 19, 2019 at 7:17 PM David Schinazi <[email-masked]>
wrote:

> I've made this a separate random key to avoid the HKDF. @ekr
> <https://github.com/ekr> I hope this addresses point (1).
>
> Regarding point (2) I don't care either way but if we were to use a
> connection ID as the nonce I would use the original destination connection
> ID since that's not sent over the wire but required to be known by
> endpoints.
>
> —
> <https://github.com/quicwg/base-drafts/pull/3120?email_source=notifications&email_token=AAIPLINGDZEISEC6PG2WPX3QUST3DA5CNFSM4JCAOODKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEQRZUQ#issuecomment-555818194>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAIPLIJY5KGC4N4CNYOBOTDQUST3DANCNFSM4JCAOODA>
> .
>

-- 
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/3120#issuecomment-555821004

---

From: Marten Seemann <[email-masked]>
Date: Fri, 22 Nov 2019 00:45:46 -0800

Can we add a test vector to the appendix of the TLS document?

-- 
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/3120#issuecomment-557445269

---

From: Martin Thomson <[email-masked]>
Date: Sun, 24 Nov 2019 22:32:18 -0800

> Can we add a test vector to the appendix of the TLS document?

A pull request would be appreciated.  One that creates a .js file - so that the value can be automatically generated - would be ideal.

-- 
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/3120#issuecomment-558011834

---

From: Christian Huitema <[email-masked]>
Date: Sat, 30 Nov 2019 18:39:35 -0800

In Singapore, we were debating whether to just do checksum, versus encrypt and use an initial vector based on the CID. I wanted to compare the two variants using a benchmark designed for Picotls, but I ran into a little bit of trouble because of the structure of the picotls encryption API, which looks like:

1) Initialize an encryption context with the key context, the IV, and the authenticated data
2) Encrypt the message
3) Compute the AEAD checksum

With that API, there is no option to not use an IV,. Using a constant IV would have exactly the same cost as using a variable IV. I went on to compare the time to encrypt 64 bytes versus the time to just authenticate 64 bytes. Results are below for two implementations of AES128 GCM, one from OpenSSL crypto library and another from Windows Bcrypt library -- both measured using 64 bit code on my Windows laptop:

   | encrypt | decrypt | authenticate | verify
--  | -- | -- | -- | --
bcrypt  | 0.35751 | 0.22602 | 0.24687 | 0.20189
openssl 1.1.1c | 0.30211 | 0.23956 | 0.24296 | 0.23926

The times are measured here in microseconds -- these are the average on 100,000 operations. We see that the times are fractions of microseconds in both cases. The encryption operation is more expensive than a simple authentication, 45% more with brcrypt, 24% more with openssl. The decrypt operation is 12% more expensive than the simple authenticate with brcypt, almost the same with openssl.

-- 
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/3120#issuecomment-560043120