nvme: Add length field to Hkdf-Expand-Label computation

[prashanth-nayak]

Fix to add the 2 byte length field to the HKDF-Expand-Label computation for retained and TLS PSK.
#735

[hreinecke]

Can you give a pointer to the HKDF specification where this is mandated?
And does it have an implication to the kernel implementation if we change it?

[prashanth-nayak]

Can you give a pointer to the HKDF specification where this is mandated?

RFC 8446 section 7.1 defines HKDF-Expand-Label function which is used to derive the retained-PSK and TLS-PSK: https://datatracker.ietf.org/doc/html/rfc8446#section-7.1

As per RFC8446:
HKDF-Expand-Label(Secret, Label, Context, Length) = HKDF-Expand(Secret, HkdfLabel, Length)
Where HkdfLabel is specified as:

   struct {
       uint16 length = Length;
       opaque label<7..255> = "tls13 " + Label;
       opaque context<0..255> = Context;
   } HkdfLabel;

And does it have an implication to the kernel implementation if we change it?

I don't see any implication to the current NVMe/TCP-TLS kernel implementation.

[igaw]

Looks reasonable to me. I wonder if we couldn't add some unit test for this.

[prashanth-nayak]

Looks reasonable to me. I wonder if we couldn't add some unit test for this.

Do you want me to add unit tests for this workflow ?
I believe the computation for Retained-PSK and TLS-PSK will change again when support for TP8018 is added.
Should we wait for that before adding the unit tests?

[igaw]

If it is feasible to add a unit test I'd rather have one now than later. When support for TP8018 is added, we just update the unit test.

[prashanth-nayak]

Currently, nvme_insert_tls_key() is the only global function that is exposed by the library. This function destroys the local copy of the keys once they are inserted into the key ring and returns only a key-serial-number to the caller. Calling this function from the test code will not let us validate the computed keys.

derive_nvme_keys(), derive_retained_key() and derive_tls_key() are the static functions where all the real computations happen. So, I don't think it is feasible to writing tests that validate the Retained and TLS PSK that is computed from a configured PSK input.

[hreinecke]

Indeed, that change looks good.