Re: [PATCH 02/18] nvme-keyring: define a 'psk' keytype

[Hannes Reinecke <hare@suse.de>]

On 3/22/23 09:29, Sagi Grimberg wrote:
> 
> 
> On 3/21/23 14:43, Hannes Reinecke wrote:
>> Define a 'psk' keytype to hold the NVMe TLS PSKs.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   drivers/nvme/common/keyring.c | 96 +++++++++++++++++++++++++++++++++++
>>   include/linux/nvme-keyring.h  |  8 +++
>>   2 files changed, 104 insertions(+)
>>
>> diff --git a/drivers/nvme/common/keyring.c 
>> b/drivers/nvme/common/keyring.c
>> index 3a6e8a0b38e2..6cbb9d66e0f6 100644
>> --- a/drivers/nvme/common/keyring.c
>> +++ b/drivers/nvme/common/keyring.c
>> @@ -11,6 +11,96 @@
>>   static struct key *nvme_keyring;
>> +key_serial_t nvme_keyring_id(void)
>> +{
>> +    return nvme_keyring->serial;
>> +}
>> +EXPORT_SYMBOL_GPL(nvme_keyring_id);
>> +
>> +static void nvme_tls_psk_describe(const struct key *key, struct 
>> seq_file *m)
>> +{
>> +    seq_puts(m, key->description);
>> +    seq_printf(m, ": %u", key->datalen);
>> +}
>> +
>> +static bool nvme_tls_psk_match(const struct key *key,
>> +                   const struct key_match_data *match_data)
>> +{
>> +    const char *match_id;
>> +    size_t match_len;
>> +
>> +    if (!key->description) {
>> +        pr_debug("%s: no key description\n", __func__);
>> +        return false;
>> +    }
>> +    match_len = strlen(key->description);
>> +    pr_debug("%s: id %s len %zd\n", __func__, key->description, 
>> match_len);
>> +
>> +    if (!match_data->raw_data) {
>> +        pr_debug("%s: no match data\n", __func__);
>> +        return false;
>> +    }
>> +    match_id = match_data->raw_data;
>> +    pr_debug("%s: match '%s' '%s' len %lu\n",
>> +         __func__, match_id, key->description, match_len);
>> +    return !memcmp(key->description, match_id, match_len);
>> +}
>> +
>> +static int nvme_tls_psk_match_preparse(struct key_match_data 
>> *match_data)
>> +{
>> +    match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
>> +    match_data->cmp = nvme_tls_psk_match;
>> +    return 0;
>> +}
>> +
>> +static struct key_type nvme_tls_psk_key_type = {
>> +    .name           = "psk",
>> +    .flags          = KEY_TYPE_NET_DOMAIN,
>> +    .preparse       = user_preparse,
>> +    .free_preparse  = user_free_preparse,
>> +    .match_preparse = nvme_tls_psk_match_preparse,
>> +    .instantiate    = generic_key_instantiate,
>> +    .revoke         = user_revoke,
>> +    .destroy        = user_destroy,
>> +    .describe       = nvme_tls_psk_describe,
>> +    .read           = user_read,
>> +};
>> +
> 
> Hannes, can you please provide a documentation section
> to this function? most importantly 'generated' argument.
> 
Sure. There are two types of PSK types specified in the NVMe TCP spec;
a 'retained' one which has to be provided by the admin, and a 
'generated' one which is derived from the shared key material from 
DH-HMAC-CHAP when doing a secure channel concatenation.
And each type has two possible hash algorithms (SHA-256 and SHA-384),
resulting in 4 possible PSKs.

And indeed, the secure channel concatenation bits are missing as we're 
still hashing out details at the fmds group.
I do have a patchset for that, though, but decided not to include it in 
this submission as it'll increase the patchset even more.
Can do if you like ...

But will be updating the documentation.

>> +struct key *nvme_tls_psk_lookup(key_ref_t keyring,
>> +        const char *hostnqn, const char *subnqn,
>> +        int hmac, bool generated)
>> +{
>> +    char *identity;
>> +    size_t identity_len = (NVMF_NQN_SIZE) * 2 + 11;
>> +    key_ref_t keyref;
>> +    key_serial_t keyring_id;
>> +
>> +    identity = kzalloc(identity_len, GFP_KERNEL);
>> +    if (!identity)
>> +        return ERR_PTR(-ENOMEM);
>> +
>> +    snprintf(identity, identity_len, "NVMe0%c%02d %s %s",
>> +         generated ? 'G' : 'R', hmac, hostnqn, subnqn);
> 
> Is that a format that is expected from userspace to produce?
> 
Yes. See
NVMe-TCP 1.0a section 3.6.1.3 "TLS PSK and PSK Identity Derivation"
for details.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman