search

sframe-wg / sframe

Internet draft for SFrame
Other
7 stars 10 forks source link

derive_key_salt bug #156

Closed chris-wood closed 11 months ago

chris-wood commented 12 months ago

It might be helpful to add an example or test vectors to cover derive_key_salt. I suspect some people may trip up on the KID and cipher_suite encoding details, even though they're straightforward.

bifurcation commented 12 months ago

With respect to examples, I think the test vectors suffice here, since they show all the internal values for derive_key_salt. However, looking at this, we actually have a spec bug, where the reference implementation and the spec differ on derive_key_salt behavior:

Spec:

def derive_key_salt(KID, base_key):
  sframe_secret = HKDF-Extract("", base_key)
  info = "SFrame 1.0 Secret key " + KID + cipher_suite
  sframe_key = HKDF-Expand(sframe_secret, info, AEAD.Nk)
  sframe_salt = HKDF-Expand(sframe_secret, info, AEAD.Nn)
  return sframe_key, sframe_salt

Reference implementation:

        static KEY_LABEL: &[u8] = b"SFrame 1.0 Secret key ";
        static SALT_LABEL: &[u8] = b"SFrame 1.0 Secret salt ";
        // ...
        let sframe_key_label = [KEY_LABEL, &kid_bytes, &cipher_suite_bytes].concat();
        let sframe_salt_label = [SALT_LABEL, &kid_bytes, &cipher_suite_bytes].concat();

The spec is wrong, since the key and salt use the same label with HKDF. So we need to change the spec to align with the implementation:

 def derive_key_salt(KID, base_key):
   sframe_secret = HKDF-Extract("", base_key)
-  info = "SFrame 1.0 Secret key " + KID + cipher_suite
-  sframe_key = HKDF-Expand(sframe_secret, info, AEAD.Nk)
-  sframe_salt = HKDF-Expand(sframe_secret, info, AEAD.Nn)
+  key_label = "SFrame 1.0 Secret key " + KID + cipher_suite
+  salt_label = "SFrame 1.0 Secret salt " + KID + cipher_suite
+  sframe_key = HKDF-Expand(sframe_secret, key_label, AEAD.Nk)
+  sframe_salt = HKDF-Expand(sframe_secret, salt_label, AEAD.Nn)
   return sframe_key, sframe_salt
bifurcation commented 12 months ago

CC @mulmarta who commented on HKDF stuff in the past

mulmarta commented 12 months ago

Agreed, the spec is wrong. The fix has a typo though, making the labels the same again :-)

-  salt_label = "SFrame 1.0 Secret key " + KID + cipher_suite
+  salt_label = "SFrame 1.0 Secret salt " + KID + cipher_suite
bifurcation commented 12 months ago

@mulmarta I don't know what you're talking about, looks right to me! ;)