Subject: [quicwg/base-drafts] Gratuitous change of QHKDF-Expand in draft 10 (#1255)

From: Christian Huitema <[email-masked]>
Date: Wed, 28 Mar 2018 18:22:56 +0000 (UTC)

Draft 10 introduces a change in the definition of the QHKDF-Expand function.

It used to be
~~~
QHKDF-Expand(Secret, Label, Length) =             
                HKDF-Expand(Secret, QuicHkdfLabel, Length)

struct {
   uint16 length = Length;
   opaque label<6..255> = "QUIC " + Label;
   uint8 hashLength = 0;
} QuicHkdfLabel;
~~~
Now it's:
~~~
QHKDF-Expand(Secret, Label, Length) =    
                   HKDF-Expand(Secret, QhkdfExpandInfo, Length)

struct {
  uint16 length = Length;
  opaque label<6..255> = "QUIC " + Label;
} QhkdfExpandInfo;
~~~
Notice that the "hashlength" parameter has disappeared. The effect is to make the implementation incompatible with the TLS implementations of HKDF-Expand that assume the presence of an HKDF label. Now QUIC implementations need specific code to implement key derivation, and cannot easily reuse the TLS implementations. And that's a pain point.

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

---

From: Mike Bishop <[email-masked]>
Date: Wed, 28 Mar 2018 18:38:54 +0000 (UTC)

This happened in #1077.

-- 
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/1255#issuecomment-376992125

---

From: Christian Huitema <[email-masked]>
Date: Wed, 28 Mar 2018 18:44:35 +0000 (UTC)

Yes, I guess it happened some time ago. But it only made its way into draft 10, we are just starting interop tests on draft-10, and of course we are finding out the issue now rather than when reviewing #1077.

-- 
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/1255#issuecomment-376993911

---

From: Martin Thomson <[email-masked]>
Date: Wed, 28 Mar 2018 17:22:19 -0700

#1077 also changed the label.  No complaint about that, I see.

-- 
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/1255#issuecomment-377081006

---

From: Lars Eggert <[email-masked]>
Date: Thu, 29 Mar 2018 07:07:54 +0000 (UTC)

Agree with Christian on these TLS changes being a pain.

-- 
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/1255#issuecomment-377143180

---

From: ekr <[email-masked]>
Date: Thu, 29 Mar 2018 15:38:21 +0000 (UTC)

As do I. Actually the prefix change is also a pain...

-- 
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/1255#issuecomment-377276565

---

From: Nick Banks <[email-masked]>
Date: Thu, 29 Mar 2018 15:46:45 +0000 (UTC)

Just to provide a counter side of things; the label change wasn't a big issue for my implementation. The logic was already QUIC specific. Just a two line change:

![image](https://user-images.githubusercontent.com/20663557/38098669-87b21796-332d-11e8-9a8e-61d2d28691f2.png)

-- 
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/1255#issuecomment-377279250

---

From: Christian Huitema <[email-masked]>
Date: Thu, 29 Mar 2018 18:39:12 +0000 (UTC)

Well, of course that's only a one line change. The problem is that it is a one line change in a function deep in the TLS implementation. I ended up having to duplicate three functions from the TLS stack, i.e. 60 some lines of cut-and-paste code + a dozen references, all so that I can change one line. I hate having to do that kind of stuff.  

-- 
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/1255#issuecomment-377332575

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 30 Mar 2018 14:16:09 +0000 (UTC)

I also feel the pain.

I think it is beneficial for most of us to be able to reuse the AEAD construction function that we have in our TLS stacks.

It might make sense to change BaseLabel  to something other than `tls13 `.

But I would prefer to keep all other parameters the same, so that the only change we need to make to our TLS stacks would be to provide a way of passing the BaseLabel as the argument.

-- 
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/1255#issuecomment-377533452

---

From: Alexis La Goutte <[email-masked]>
Date: Fri, 30 Mar 2018 16:10:33 +0000 (UTC)

+1 with kazuho

-- 
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/1255#issuecomment-377559251

---

From: Christian Huitema <[email-masked]>
Date: Fri, 30 Mar 2018 17:09:47 +0000 (UTC)

Proposing PR #1256 to restore compatibility and resolve 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/issues/1255#issuecomment-377572601