crypto: Support referencing the Hubble key by a PSA key id - #329
Conversation
Return more appropriated error codes for PSA crypto. Signed-off-by: Flavio Ceolin <flavio@hubble.com>
| static int _kbkdf_counter(const uint8_t *key, const char *label, | ||
| size_t label_len, const uint8_t *context, | ||
| static int _kbkdf_counter(const uint8_t *key, const char *label, size_t label_len, | ||
| bool hubble_key, const uint8_t *context, |
There was a problem hiding this comment.
I usually don't like passing bool as parameters because it isn't clear in the function call what it means.
err = _kbkdf_counter(master_key, "DeviceKey",
strlen("DeviceKey"), true, context,
strlen((const char *)context), output_key,
CONFIG_HUBBLE_KEY_SIZE);
I like creating an enum and passing in an enum value to ease readability. Something like
typedef enum {
KEY_USE_ID,
KEY_USE_MATERIAL,
KEY_USE_COUNT
} key_use_t;
Then passing in KEY_USE_ID or KEY_USE_MATERIAL to the function.
Thoughts on this? Since this function already takes a handful of parameters, this could be helpful in a reader understanding what the function call is doing
There was a problem hiding this comment.
I agree in general, specially for public APIs. The thing here is creating a new alias for such small scope and a binary option. Would make it clear doing 's/hubble_key/use_key_id/' ?
There was a problem hiding this comment.
Yeah that seems clear to me. The one thought I'd have with strings, is that this could start to increase our flash space by comparing to strings that have to be stored in memory. I suppose the same concern could be raised about enums (likely compiled to uint32_t)
Just some thoughts. As long as we consider the tradeoffs, happy to go any direction
There was a problem hiding this comment.
Compilers are smart and use smaller types, the thing for me is really the scope, having an alias for something that is self content and binary option does not bring much benefits, that is said if we get more type of keys or it needs to be exposed I think your suggestion is the right direction.
|
LGTM, will let someone else approve |
HongNguyen635
left a comment
HongNguyen635
left a comment
There was a problem hiding this comment.
sorry for being pedantic, just 2 minor points, everything else is looking good.
Add CONFIG_HUBBLE_NETWORK_CRYPTO_PSA_USE_KEY_ID so applications can hand the SDK a PSA key identifier instead of raw key material. Signed-off-by: Flavio Ceolin <flavio@hubble.com>
Add a helper to generate the PSA attributes of the Hubble key held in a CRACEN KMU slot, so the SDK can reference it by key id only. Signed-off-by: Flavio Ceolin <flavio@hubble.com>
Allow the sample to reference the Hubble key by a PSA key identifier instead of embedding the key material. Signed-off-by: Flavio Ceolin <flavio@hubble.com>
No worries :) It is better this way |
3 participants
Add CONFIG_HUBBLE_NETWORK_CRYPTO_PSA_USE_KEY_ID, so the SDK signs through a key
the PSA implementation already holds and never sees the key material.
Includes tools/ncs-generate-key.py to build the nrfutil keyslot JSON, and a ble-beacon
sample setup provisioning the key into CRACEN KMU on nRF54L15