[PATCH 06/13] nvme-keyring: restrict match length for version '1' identifiers

[Hannes Reinecke]

On 3/7/24 13:08, Sagi Grimberg wrote:
> 
> 
> On 07/03/2024 13:35, Hannes Reinecke wrote:
>> On 3/7/24 11:49, Sagi Grimberg wrote:
>>>
>>>
>>> On 27/01/2024 11:30, hare at kernel.org wrote:
>>>> From: Hannes Reinecke <hare at suse.de>
>>>>
>>>> TP8018 changed the TLS PSK identifiers to append a PSK hash value,
>>>> so to lookup any version '1' identifiers we need to restrict the
>>>> match length to exclude the PSK hash value (which we don't have
>>>> when looking up keys).
>>>>
>>>> Signed-off-by: Hannes Reinecke <hare at suse.de>
>>>> ---
>>>>   drivers/nvme/common/keyring.c | 21 +++++++++++++++++++++
>>>>   1 file changed, 21 insertions(+)
>>>>
>>>> diff --git a/drivers/nvme/common/keyring.c 
>>>> b/drivers/nvme/common/keyring.c
>>>> index c16f9496643f..51b99b34e100 100644
>>>> --- a/drivers/nvme/common/keyring.c
>>>> +++ b/drivers/nvme/common/keyring.c
>>>> @@ -44,6 +44,27 @@ static bool nvme_tls_psk_match(const struct key 
>>>> *key,
>>>>           return false;
>>>>       }
>>>>       match_id = match_data->raw_data;
>>>> +    if (memcmp(match_id, "NVMe1", 5)) {
>>>> +        char *e = (char *)match_id;
>>>> +        size_t offset = 0;
>>>> +        int n = 0;
>>>> +
>>>> +        while (*e != ' ' && offset < match_len) {
>>>> +            if (*e == ' ') {
>>>> +                n++;
>>>> +                if (n == 3)
>>>> +                    break;
>>>> +            }
>>>> +            e++;
>>>> +            offset++;
>>>> +        }
>>>> +        if (n != 3) {
>>>> +            pr_debug("%s: error parsing '%s'\n",
>>>> +                 __func__, match_id);
>>>> +            return false;
>>>> +        }
>>>> +        match_len = offset;
>>>> +    }
>>>
>>> My eyes hurt... I am failing to understand what this even does...
>>>
>>> You have a concrete example?
>>
>> Yeah, it's ugly. What I'm trying to do here is to lookup TLS PSK 
>> identities which are either in the original format:
>>
>> NVMe0[RG]0[12] <hostnqn> <subsysnqn>
>>
>> _or_ the TP8018 TLS PSK format:
>>
>> NVMe1[RG]0[12] <hostnqn> <subsysnqn> <digest>
>>
>> given that I only have <hostnqn> and <subsysnqn> as lookup keys.
>> So for the memcmp() I'll have to exclude the ' <digest>' bit, which
>> happens to the after the third blank space in the identity.
>> I'll have a slightly better version queued, but parsing remains ugly.
> 
> Shouldn't strtok/strsep simplify what you are trying to do here?

Yeah. I'll be reworking that.

Cheers,

Hannes