named-data-iot/ndn-lite

ndn_hkdf: does not conform to RFC 5869

Opened this issue · 5 comments

5aea33b attempts to fix #56, but it is not a valid fix. It seems that I cannot reopen that issue, so I'm opening a new issue.

RFC5869 does not limit length of IKM. Thus, the implementation should support arbitrary key length.
The underlying TinyCrypt library does not have any limitation on HMAC-SHA256 key length. The limitation comes from a flawed design here:

memset(hmac_key->key_value, 0, NDN_SEC_HMAC_MAX_KEY_SIZE);
memcpy(hmac_key->key_value, key_value, key_size);
hmac_key->key_size = key_size;

To avoid this problem:

  1. Store tc_hmac_state_struct as part of abstract_hmac_key.
  2. In ndn_lite_default_hmac_load_key, load the key with tc_hmac_set_key, and don't copy the key.
  3. In ndn_lite_default_hmac_sha256_init, memcpy the tc_hmac_state_struct from abstract_hmac_key to abstract_hmac_sha256_state_t.

Also, #54 was accidentally reverted and brought back some warnings.

@Zhiyi-Zhang How do you see this HMAC change?

Personally I agree with "don't copy the key". But in default AES and ECC, they do copy keys as well. If we change them all, numbers of existing tests and examples would require modification.

I wonder is there a balance here: when key length below NDN_SEC_HMAC_MAX_KEY_SIZE, do normal key copy, and when above it, load the key with tc_hmac_set_key. No modification to existing tests and examples would be required.

numbers of existing tests and examples would require modification

No. tc_hmac_state_struct should have all the information from the key. It does not require the caller to retain memory of the key itself.

when key length below NDN_SEC_HMAC_MAX_KEY_SIZE, do normal key copy, and when above it, load the key with tc_hmac_set_key.

This would be inconsistent behavior and very surprising to users.

I agree with Junxiao's comments. Let's follow his suggestion on this issue.

Clear, while still a little question: should a method be reserved for users retrieving the key bytes from the ndn_hmac_key? This job is currently done by copying keys in ndn_lite_hmac_load_key.

Also, #54 was accidentally reverted and brought back some warnings.

It was a mistake for my carelessness, sorry for that. I've already changed it back.

retrieving the key bytes from the ndn_hmac_key

No. In general, secret keys (HMAC, EC, etc) are not extractable. It may as well reside in a hardware TPM (e.g. ECC508 chip).
Caller can see secret key only when the key pair is being generated. Caller can also import secret key into a key instance. From a key instance, there's no way to get the secret key back.